-
Notifications
You must be signed in to change notification settings - Fork 41
Bump nix binary to 2.32.0 without caching #2563
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
Changes from all commits
d013e04
607ecbd
a3f0a03
5f160a3
d423b6b
925e123
20fddda
eb97931
f299366
6728a67
4b56865
6aafc5c
a02c1bb
dcbf412
bbecb2d
60248a2
0f94215
10e8b5c
f81368a
b74683b
bcb3d44
8b83e26
6814e44
a8aac68
9545649
78280cd
b168596
072a87f
54979b4
daf9b99
6fc6702
8f58bad
ec92b34
df9ba84
dcc1db0
8920f86
fd6a63a
e75ba96
faeb743
285dc2e
a5115a4
39c6b7e
180665b
f39ce61
a500bc5
c9decd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,9 +38,14 @@ runs: | |
shell: bash | ||
run: | | ||
set -euxo pipefail | ||
NIX_BINARY_VERSION=2.32.0 | ||
echo "NIX_BINARY_VERSION=$NIX_BINARY_VERSION" >> $GITHUB_ENV | ||
cat nix/canton-sources.json | ||
git ls-files nix/ | grep -v '[.]md$' | LC_ALL=C sort | xargs sha256sum -b > /tmp/nix-cache-key | ||
uname -m >> /tmp/nix-cache-key # Add architecture to the cache key | ||
echo "gh_cache_version: ${{ inputs.cache_version }}" >> /tmp/nix-cache-key # Add cache version to the cache key | ||
echo "home: $HOME" >> /tmp/nix-cache-key # important when restoring simlinks from cache, apparently | ||
echo "nix binary version: $NIX_BINARY_VERSION" >> /tmp/nix-cache-key # different nix versions might behave differently and corrupt the caches | ||
if [ "${{ inputs.oss_only }}" == true ]; then | ||
echo "Using OSS only dependencies" | ||
echo "oss_only: ${{ inputs.oss_only }}" >> /tmp/nix-cache-key | ||
|
@@ -86,8 +91,9 @@ runs: | |
# we use rsync here because it's simply faster to install | ||
rsync -avi /cache/nix/$cache_key/.nix-* $HOME/ | ||
rsync -avi "/cache/nix/$cache_key/nix" $HOME/.config/ | ||
rsync -avi "/cache/nix/$cache_key/nix_store/var/" /nix/var | ||
sudo mount --bind /cache/nix/$cache_key/nix_store/store /nix/store | ||
# TODO (#2663): fix & uncomment these two lines | ||
# rsync -avi "/cache/nix/$cache_key/nix_store/var/" /nix/var | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we do uncomment that let's create an issue for it documenting how it fails and what you tried. @martinflorian-da @nicu-da wdyt? I'm leaning towards saying getting an upgrade in with the slowdown and then moving the fix for this to the top of our prio list is better than to keep pushing out that upgrade for another few months (the pressure from slow CI also felt more so chances are higher we'll actually tackle it). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (With my limited context:) Agreed to merging this without this caching part, a bit worried about "just moving the fix to the top of our prio list". Can we give assign it to a concrete person already that will realistically get to it soon? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @martinflorian-da those 2 minutes were really annoying in the past, though I guess with the current CI timing it might be less so. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #2663 adding as TODOs |
||
# sudo mount --bind /cache/nix/$cache_key/nix_store/store /nix/store | ||
else | ||
sudo mkdir -p "/cache/nix/$cache_key" | ||
sudo chown $(whoami):$(whoami) "/cache/nix/$cache_key" | ||
|
@@ -126,7 +132,7 @@ runs: | |
max-jobs = 16 | ||
EOF | ||
fi | ||
sh <(curl -fsSL --retry 8 https://releases.nixos.org/nix/nix-2.13.3/install) --no-daemon | ||
sh <(curl -fsSL --retry 8 "https://releases.nixos.org/nix/nix-$NIX_BINARY_VERSION/install") --no-daemon | ||
sudo mkdir -p /etc/nix | ||
sudo chmod a+rw /etc/nix | ||
if [[ "${{ inputs.oss_only }}" == true ]]; then | ||
|
@@ -147,8 +153,6 @@ runs: | |
target="default" | ||
fi | ||
nix develop path:nix#${target} -v --profile "$HOME/.nix-shell" --command echo "Done loading packages" | ||
echo "Garbage collecting to reduce cache size" | ||
nix-store --gc | ||
Comment on lines
-150
to
-151
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why it was done above, as opposed to before saving |
||
fi | ||
- name: Invoke nix before saving cache | ||
|
@@ -176,6 +180,9 @@ runs: | |
export USER=$(whoami) | ||
. ~/.nix-profile/etc/profile.d/nix.sh | ||
echo "Garbage collecting to reduce cache size" | ||
nix-store --gc | ||
nix copy --all --to 'file:///cache/nix/binary_cache?trusted=1' -v | ||
CLONE_COMMAND="rclone --no-update-dir-modtime --no-update-modtime --size-only --multi-thread-streams=32 --transfers=32 --ignore-existing --links --create-empty-src-dirs --fast-list --metadata --order-by name,mixed --retries 10 copy" | ||
|
@@ -187,7 +194,8 @@ runs: | |
#requires to preserve read only during clone | ||
sudo ${CLONE_COMMAND} /nix/store/ /cache/nix/$cache_key/nix_store/store | ||
sudo ${CLONE_COMMAND} /nix/var/ "/cache/nix/$cache_key/nix_store/var" | ||
# TODO (#2663): fix & uncomment this line | ||
# sudo ${CLONE_COMMAND} /nix/var/ "/cache/nix/$cache_key/nix_store/var" | ||
echo "done" > "/cache/nix/$cache_key/cached" | ||
fi | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,18 +20,18 @@ jobs: | |
# Checkout the PR head commit to get the commit message first | ||
ref: ${{ github.event.pull_request.head.sha }} | ||
|
||
- name: Check out repository code | ||
uses: actions/checkout@v4 | ||
if: inputs.commit_sha != '' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this and below are the only ones that had this... what for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @isegall-da is that leftover? I could see it only being set when you run from a fork but then it should be used for every checkout and it's definitely not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping @isegall-da , don't want to merge without knowing this is 100% fine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed with @OriolMunoz-da now, just copying the conclusion here as well: In tests that run split_tests first, we check the static label in split_tests which is why we don't have these separate steps there, and we just checkout commit_sha there. (in ts_cli we flipped the order between (2) and (3) which is wrong, we'll fix that) |
||
with: | ||
ref: ${{ inputs.commit_sha }} | ||
|
||
- name: Check if static only | ||
uses: ./.github/actions/tests/skip_on_static | ||
id: skip | ||
with: | ||
gh_token: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Check out repository code | ||
uses: actions/checkout@v4 | ||
if: inputs.commit_sha != '' | ||
with: | ||
ref: ${{ inputs.commit_sha }} | ||
|
||
- name: Setup | ||
id: setup | ||
if: steps.skip.outputs.skip != 'true' | ||
|
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.
don't see the harm in keeping these