feat: Set JULIAUP_DEPOT_PATH to a location in the runner tool cache#205
feat: Set JULIAUP_DEPOT_PATH to a location in the runner tool cache#205krynju wants to merge 5 commits into
JULIAUP_DEPOT_PATH to a location in the runner tool cache#205Conversation
krynju
force-pushed
the
kr/depot-runner-cache
branch
from
5c5902d to
0b83653
Compare
|
Would you be able to put the changes that touch |
|
At first glance, at a high-level, this seems reasonable to me, but I haven't done a line-by-line review yet. |
krynju
force-pushed
the
kr/depot-runner-cache
branch
from
0b83653 to
0f17925
Compare
Done |
|
I left one comment, but overall this makes sense and we should do it. Users can recover the old behavior by manually setting the One question though, is whether this is breaking. My initial instinct is that it is breaking. @MichaelHatherly what do you think? We could work around it by making it opt-in in the current major version of |
I'm inclined to agree with that. Opt-in during this current major release and then switching it in the next might be the most reasonable. |
Assuming people use the action as intended it shouldn't be breaking. The only side effect I think would be that the cache is not reused on next the first run after the upgrade. |
DilumAluthge
changed the title
JULIAUP_DEPOT_PATH to a location in the runner tool cache
krynju
force-pushed
the
kr/depot-runner-cache
branch
from
2fa90cf to
86c213d
Compare
|
I've been giving this some more thought, and I wonder if we can call this a bugfix, with the idea being that GitHub Actions are expected to use the toolcache generally, so the fact that we haven't been using the toolcache up until now could be considered a bug. So then we'd say this is a bugfix, but users that need to recover the old behavior can manually set the By calling it a bugfix, we also don't need to create an opt-in mechanism, so the code surface becomes simpler. @MichaelHatherly What do you think? |
|
BTW, before merging, @krynju could you rebase on the latest |
| // https://docs.github.com/en/actions/reference/workflows-and-actions/variables | ||
| const tool_cache = process.env['RUNNER_TOOL_CACHE'] | ||
| if (!tool_cache || tool_cache.length === 0) { | ||
| core.warning(`RUNNER_TOOL_CACHE is not set; leaving ${env_name} unset (Juliaup will use its default location)`) |
There was a problem hiding this comment.
Escalate this to a core.error? (Non-fatal, but the log message looks scarier). I think it would be pretty unusual for RUNNER_TOOL_CACHE to not be defined.
| inputs.get_juliaup_channel_input() | ||
| inputs.get_juliaup_version_input() | ||
|
|
||
| // Step 1b: Pin the Juliaup depot under RUNNER_TOOL_CACHE so it does |
There was a problem hiding this comment.
Probably want to edit line 16 to say "Step 1a", for consistency.
3 participants
Found because a worker had juliaup preinstalled and we've hit
Error: The Julia launcher failed to figure out which juliaup channel to use.I think it makes more sense to put it in the runner cache, so that it completely ignores what's currently on the runner.