Skip to content

Bug fix: Validate cached julia binary exists and check PATH resolution - #389

Merged
DilumAluthge merged 3 commits into
julia-actions:masterfrom
krynju:kr/path-fixes
May 11, 2026
Merged

Bug fix: Validate cached julia binary exists and check PATH resolution#389
DilumAluthge merged 3 commits into
julia-actions:masterfrom
krynju:kr/path-fixes

Conversation

@krynju

@krynju krynju commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

I noticed this in CI. It's related to some problems with juliaup being installed on the workers and being messed with by install-juliaup (but that's a different issue)

/home/actions-runner-12/.juliaup/bin/julia --version
Error: The Julia launcher failed to figure out which juliaup channel to use.
Error: The process '/home/actions-runner-12/.juliaup/bin/julia' failed with exit code 1

setup-julia failed here at the julia --version check, because path resolved to a faulty local juliaup installation. This makes me think the cached binary might not have been fully there (just a .complete marker present).

Changes

1. Added bin/julia exists check which causes a cache miss on failure prompting a new download

.complete bin/julia exists Before After
absent miss miss
present absent hit miss
present present hit hit

2. Added a PATH resolution check (warning)

We're basically checking if the selected Julia version/binary is the one that would resolve from PATH. I'm not sure if this should be fatal, so leaving it as a warning in case such an issue appears again.

@krynju
krynju requested a review from a team as a code owner April 30, 2026 16:16
@krynju krynju changed the title fix: make sure the binary is ok and path resolves to it fix: make sure the binary is ok on cache hit and PATH resolves to it Apr 30, 2026
@DilumAluthge
DilumAluthge requested review from DilumAluthge and removed request for a team April 30, 2026 23:56
@DilumAluthge

Copy link
Copy Markdown
Member

Can you put the changes that modify lib/* and dist/* into a separate commit?

Comment thread src/setup-julia.ts Outdated
// Verify that PATH lookup of `julia` resolves to the binary we just installed.
// On self-hosted runners other Julia entries (e.g. juliaup launcher in
// ~/.juliaup/bin) can shadow the toolcache binary depending on PATH state.
try {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to implement this without try-catch?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@DilumAluthge

Copy link
Copy Markdown
Member

To clarify, in this PR, you're not actually running the Julia binary to make sure it's runnable, you're just verifying that the Julia binary exists as a file on disk in the expected place?

So, if I understand correctly:

Before this PR:

  1. .complete is absent - we correctly conclude "cache miss".
  2. .complete is present but bin/julia is absent - we incorrectly conclude "cache hit".
  3. .complete is present and bin/julia is present, but if you tried to run bin/julia it would error - we conclude "cache hit".

After this PR:

  1. .complete is absent - we correctly conclude "cache miss".
  2. .complete is present but bin/julia is absent - we now correctly conclude "cache miss".
  3. .complete is present and bin/julia is present, but if you tried to run bin/julia it would error - we conclude "cache hit".

Have I described things correctly?

I'm totally fine if that's the case1, I just would like to make the PR description (and the code comments) a little more precise, to better describe what we're actually checking.

Footnotes

  1. Actually running the Julia binary would eat up time that we probably shouldn't be spending.

@krynju krynju changed the title fix: make sure the binary is ok on cache hit and PATH resolves to it fix: validate cached julia binary exists and check PATH resolution May 5, 2026
@krynju

krynju commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

To clarify, in this PR, you're not actually running the Julia binary to make sure it's runnable, you're just verifying that the Julia binary exists as a file on disk in the expected place?

So, if I understand correctly:

Before this PR:

  1. .complete is absent - we correctly conclude "cache miss".
  2. .complete is present but bin/julia is absent - we incorrectly conclude "cache hit".
  3. .complete is present and bin/julia is present, but if you tried to run bin/julia it would error - we conclude "cache hit".

After this PR:

  1. .complete is absent - we correctly conclude "cache miss".
  2. .complete is present but bin/julia is absent - we now correctly conclude "cache miss".
  3. .complete is present and bin/julia is present, but if you tried to run bin/julia it would error - we conclude "cache hit".

Have I described things correctly?

I'm totally fine if that's the case1, I just would like to make the PR description (and the code comments) a little more precise, to better describe what we're actually checking.

Footnotes

  1. Actually running the Julia binary would eat up time that we probably shouldn't be spending.

I updated the PR description.

Also removed the part which checks if the julia binary is runnable.
Now the bin/julia check causes a cache miss if the binary doesn't exist. There's an additional PATH resolution check that ends up in a warning.

Also split the commit

@krynju
krynju requested a review from DilumAluthge May 5, 2026 17:01
@DilumAluthge DilumAluthge changed the title fix: validate cached julia binary exists and check PATH resolution bugfix: validate cached julia binary exists and check PATH resolution May 11, 2026
@DilumAluthge DilumAluthge changed the title bugfix: validate cached julia binary exists and check PATH resolution Bug fix: validate cached julia binary exists and check PATH resolution May 11, 2026
@DilumAluthge DilumAluthge changed the title Bug fix: validate cached julia binary exists and check PATH resolution Bug fix: Validate cached julia binary exists and check PATH resolution May 11, 2026
@DilumAluthge
DilumAluthge merged commit fa02766 into julia-actions:master May 11, 2026
81 checks passed
@DilumAluthge

Copy link
Copy Markdown
Member

Thank you @krynju!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants