-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
extract julia directly to tool path to maintain mtimes #196
Conversation
672e081
to
1293a89
Compare
f869840
to
f8eb0f4
Compare
fae5794
to
a34a04a
Compare
This reverts commit a34a04a.
7ef79c7
to
b677f38
Compare
Can we add a test for this? E.g. do |
Added mtime test which fails on master #200 |
Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
e7870f5
to
b789ee5
Compare
b789ee5
to
dcc55a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes are reasonable to avoid having the tool-cache
module from modifying the mtimes. My only concern is that the tc.cacheDir
function is described as "Caches a directory and installs it to the tool cacheDir" so it is possible that a non-breaking change to tc.cacheDir
such as making the tool path immutable would break this code. However, since the version of @actions/tool-cache
is pinned until we update it we should be able to avoid such a scenario.
Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the tests from here, and keep the tests in #200, and merge #200 separately? Reason: I am hoping that this PR will be a short-term solution; I'm hoping that the long-term solution will be for me to get a patch into upstream |
If we're changing |
Ok, #200 is updated. Probably best to merge that as failing first then rebase this |
This is great! Is this the last change needed to make |
Yeah, should be. |
I made a test release for 1.9.5 using with
And all platforms avoid re-precompilation once cached |
Upstream |
A bit of a hack to get julia installed with original mtimes to help JuliaLang/julia#50667
The comments explain the reasoning.
Also should be a bit faster as it avoids the
cp
action.You can see here that 1.10.0 has the release mtimes
The added mtime test fails on master to illustrate the issue. #200
Closes #100
Closes #165