fix USE_GCPCACHE conditional#187
Conversation
it evaluated to true even when `USE_GCPCACHE=false` because I believe it was just testing whether the variable was empty or not. this should fix local builds as they don't have the GCPCACHE key.
|
oh lol fair, considering I screwed this up to begin with @giordano can you check this properly |
giordano
left a comment
giordano
left a comment
There was a problem hiding this comment.
Looks good, but doesn't need to trigger registration. Please add [skip ci] to the merge commit message
|
But cache isn't being used? |
|
ReactantBuilder/R/Reactant/build_tarballs.jl Line 812 in e6d063e |
|
semi relatedly @giordano can we add info on skip ci to the readme, I also personally keep forgetting the specific things we need to do for that |
|
Yeah, USE_GCPCACHE is wrong: https://github.com/EnzymeAD/ReactantBuilder/actions/runs/28508321396/job/84502440684#step:10:104. I'll fix it. |
giordano
force-pushed
the
ss/fix-gcpcache-if
branch
from
a11c74b to
e1435ec
Compare
giordano
force-pushed
the
ss/fix-gcpcache-if
branch
from
e1435ec to
8d40ba5
Compare
| # Don't use ccache on Yggdrasil, doesn't seem to work. | ||
| USE_CCACHE=$(!BinaryBuilder.is_yggdrasil()) | ||
| USE_GCPCACHE=$(get(ENV, "GITHUB_ACTIONS", "false") == "true" && get(ENV, "GITHUB_ACTION_REPOSITORY", "") in ("EnzymeAD/ReactantBuilder", "EnzymeAD/Enzyme-JAX")) | ||
| USE_GCPCACHE=$(get(ENV, "GITHUB_ACTIONS", "false") == "true" && get(ENV, "GITHUB_REPOSITORY", "") in ("EnzymeAD/ReactantBuilder", "EnzymeAD/Enzyme-JAX")) |
There was a problem hiding this comment.
@giordano do we actually need to check if its in those two ?
There was a problem hiding this comment.
can we instead change this to check os == 'jll' ?
There was a problem hiding this comment.
In 56ec692 I changed it to check the name of the runner
giordano
force-pushed
the
ss/fix-gcpcache-if
branch
from
703835c to
55498b3
Compare
giordano
force-pushed
the
ss/fix-gcpcache-if
branch
from
55498b3 to
56ec692
Compare
| # Don't use ccache on Yggdrasil, doesn't seem to work. | ||
| USE_CCACHE=$(!BinaryBuilder.is_yggdrasil()) | ||
| USE_GCPCACHE=$(get(ENV, "GITHUB_ACTIONS", "false") == "true" && get(ENV, "GITHUB_ACTION_REPOSITORY", "") in ("EnzymeAD/ReactantBuilder", "EnzymeAD/Enzyme-JAX")) | ||
| USE_GCPCACHE=$(get(ENV, "GITHUB_ACTIONS", "false") == "true" && get(ENV, "GITHUB_REPOSITORY", "") in ("EnzymeAD/ReactantBuilder", "EnzymeAD/Enzyme-JAX")) |
There was a problem hiding this comment.
can we instead change this to check os == 'jll' ?
| # Don't use ccache on Yggdrasil, doesn't seem to work. | ||
| USE_CCACHE=$(!BinaryBuilder.is_yggdrasil()) | ||
| USE_GCPCACHE=$(BinaryBuilder.is_yggdrasil()) | ||
| USE_GCPCACHE=$(get(ENV, "GITHUB_ACTIONS", "false") == "true" && startswith(get(ENV, "RUNNER_NAME", ""), "jll")) |
There was a problem hiding this comment.
can we check if the runner has the jll tag rather than checking the runner name?
We may have other named runners with the jll group
There was a problem hiding this comment.
That's not accessible from https://docs.github.com/en/actions/reference/workflows-and-actions/variables
3 participants
it evaluated to true even when
USE_GCPCACHE=falsebecause I believe it was just testing whether the variable was empty or not.this should fix local builds as they don't have the GCPCACHE key.