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
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. |
0b83653 to
0f17925
Compare
Done |
| core.info(`${env_name} already set to: ${existing}`) | ||
| return | ||
| } | ||
| const tool_cache = process.env['RUNNER_TOOL_CACHE'] |
There was a problem hiding this comment.
Is there an API in any of the @actions/ packages for getting this value? It's not the end of the world to read it from the environment, but an API might be better.
E.g. does @actions/tool-cache have any public API that we can use?
There was a problem hiding this comment.
_getCacheDirectory is private in @actions/tool-cache and it does the same thing as we do here (just getting it from the env directly)
It doesn't look like there are any non hacky public entrypoints for this
Searched github and it seems other actions are also using the ENV directly
There was a problem hiding this comment.
Hmmm. What about doing something like this?
let juliaup_depot_path: string
juliaup_depot_path = tc.find("juliaup_depot_path", "all_versions", "all_arches")
if (!juliaup_depot_path) {
const my_empty_tempdir = [code to create a new empty temp directory]
juliaup_depot_path = await tc.cacheDir(my_empty_tempdir, tool_name, "all_versions", "all_arches")
}That way, we're only relying on public API, and we don't need the environment variable.
There was a problem hiding this comment.
I added this in 16d5746
Seems to work. Can't use all_versions though, needs to be semver, so I set it to 3.0.0 (same major as install-juliaup)
It doesn't follow the pattern a bit, because the tools are apparently meant to be immutable once set, but I guess it doesn't matter too much.
I found that the runner tool cache env is public too, so either solution for me is ok. https://docs.github.com/en/actions/reference/workflows-and-actions/variables#:~:text=D%3A%5Ca%5C_temp-,RUNNER_TOOL_CACHE,-The%20path%20to
There was a problem hiding this comment.
because the tools are apparently meant to be immutable once set
Interesting. Does that suggest that we shouldn't be using the toolcache at all?
Is there prior art here? What do other tools do on GitHub Actions? E.g. Node/Rust/Python/UV?
@MichaelHatherly What do you think about this? Some options are:
- Read the
RUNNER_TOOL_CACHEenvironment variable, and use that.- Pros:
- It's public API (per the link Krystian posted above).
- Prevents clashing with
~/.julia/juliaup
- Cons:
- Does GitHub intend for the entire toolcache to be immutable once written?
- Pros:
- Use the
tc.cacheDirfunction.- Pros:
tc.cacheDiris public API.- Prevents clashing with
~/.julia/juliaup
- Cons:
- We have to specify a semver version number (we can't use something like
ALL_VERSIONS). - Does GitHub intend for this kind of toolcache entry to be immutable once written?
- We have to specify a semver version number (we can't use something like
- Pros:
- Something else?
There was a problem hiding this comment.
The common usage pattern seems to be mostly around immutable binaries, but I haven't found any requirement for that in source code or elsewhere, so I think we're good to go here
For me either toolcache usage or just putting it directly in runner tool cache seem like the same deal and both are ok. Toolcache has more logic built in, but we're skipping all of that in this pattern.
As for direct storage in RUNNER_TOOL_CACHE two major actions are doing it: setup-java and setup-python
I also looked for other actions similiar to juliaup in functionality and found this example of a mutable depot in the cache:
volta-cli/action
// installer.ts:220-241
let voltaHome = tc.find('volta', version);
if (voltaHome === '') {
const toolRoot = await acquireVolta(version, options);
voltaHome = await tc.cacheDir(toolRoot, 'volta', version);
await setupVolta(version, voltaHome); // runs `volta setup` — populates dir
}
core.addPath(path.join(voltaHome, 'bin'));
core.exportVariable('VOLTA_HOME', voltaHome); // ← depot env var = tool-cache path
What Volta does:
- Caches Volta binary at $RUNNER_TOOL_CACHE/volta/// via tc.cacheDir (normal usage).
- Sets VOLTA_HOME to that same tool-cache path.
- Volta then installs Node versions into $VOLTA_HOME/bin/, $VOLTA_HOME/tools/, etc. — the tool-cache directory is the depot.
There was a problem hiding this comment.
I have no strong preference for any of those options. Just following along with what the java and python ones are doing seems reasonable in my mind.
There was a problem hiding this comment.
@DilumAluthge want me to revert the latest toolcache commit or are we leaving it as is?
There was a problem hiding this comment.
Ah, thanks for the bump. Let me follow up later today.
|
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. |
JULIAUP_DEPOT_PATH to a location in the runner tool cache
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.