Skip to content
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

nixpkgs-check-by-name: make all github-driven checks locally reproducible #266937

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 38 additions & 133 deletions .github/workflows/check-by-name.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,9 @@ jobs:
env:
GH_TOKEN: ${{ github.token }}
run: |
# This checks for mergeability of a pull request as recommended in
# https://docs.github.com/en/rest/guides/using-the-rest-api-to-interact-with-your-git-database?apiVersion=2022-11-28#checking-mergeability-of-pull-requests
while true; do
echo "Checking whether the pull request can be merged"
prInfo=$(gh api \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
/repos/"$GITHUB_REPOSITORY"/pulls/${{ github.event.pull_request.number }})
mergeable=$(jq -r .mergeable <<< "$prInfo")
mergedSha=$(jq -r .merge_commit_sha <<< "$prInfo")

if [[ "$mergeable" == "null" ]]; then
# null indicates that GitHub is still computing whether it's mergeable
# Wait a couple seconds before trying again
echo "GitHub is still computing whether this PR can be merged, waiting 5 seconds before trying again"
sleep 5
else
break
fi
done

if [[ "$mergeable" == "true" ]]; then
echo "The PR can be merged, checking the merge commit $mergedSha"
else
echo "The PR cannot be merged, it has a merge conflict"
exit 1
fi
echo "mergedSha=$mergedSha" >> "$GITHUB_ENV"
env
set -x
exec pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/check-mergeability.sh
Copy link
Member

Choose a reason for hiding this comment

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

This is very GitHub Actions specific. Locally you'd want to test your current working tree, not the PR's HEAD, so this should stay here.

- uses: actions/checkout@v4
with:
# pull_request_target checks out the base branch by default
Expand All @@ -57,117 +32,47 @@ jobs:
fetch-depth: 2
- name: Determining PR git hashes
run: |
# For pull_request_target this is the same as $GITHUB_SHA
echo "baseSha=$(git rev-parse HEAD^1)" >> "$GITHUB_ENV"

echo "headSha=$(git rev-parse HEAD^2)" >> "$GITHUB_ENV"
# Please don't put any logic here. To ensure local
# reproducibility, this file should only print diagnostics
# (`env`, `set -x`) and execute scripts found elsewhere in
# nixpkgs.
env
set -x
exec pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/determine-pr-hashes.sh
Copy link
Member

Choose a reason for hiding this comment

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

This should also stay, it's GitHub-specific. Locally it's rarely the case to have a merge commit with the right parent commits.

- uses: cachix/install-nix-action@v23
- name: Determining channel to use for dependencies
run: |
echo "Determining which channel to use for PR base branch $GITHUB_BASE_REF"
if [[ "$GITHUB_BASE_REF" =~ ^(release|staging|staging-next)-([0-9][0-9]\.[0-9][0-9])$ ]]; then
# Use the release channel for all PRs to release-XX.YY, staging-XX.YY and staging-next-XX.YY
channel=nixos-${BASH_REMATCH[2]}
echo "PR is for a release branch, using release channel $channel"
else
# Use the nixos-unstable channel for all other PRs
channel=nixos-unstable
echo "PR is for a non-release branch, using unstable channel $channel"
fi
echo "channel=$channel" >> "$GITHUB_ENV"
# Please don't put any logic here. To ensure local
# reproducibility, this file should only print diagnostics
# (`env`, `set -x`) and execute scripts found elsewhere in
# nixpkgs.
env
set -x
exec pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/determine-channel-for-dependencies.sh
Copy link
Member

Choose a reason for hiding this comment

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

It's not great how there's all of these separate files. The main reason these steps are split up so that it's easier to see where time is spent (e.g. see https://github.com/NixOS/nixpkgs/actions/runs/6844035438/job/18607346212). But that's not very important, I'd rather have this and all steps after as a single script.

Also we shouldn't make env necessary. If we have a script to reproduce it locally, the script shouldn't depend on GitHub-specific variables.

I also don't think set -x is necessary, the code is very verbose and already prints everything useful.

- name: Fetching latest version of channel
run: |
echo "Fetching latest version of channel $channel"
# This is probably the easiest way to get Nix to output the path to a downloaded channel!
nixpkgs=$(nix-instantiate --find-file nixpkgs -I nixpkgs=channel:"$channel")
# This file only exists in channels
rev=$(<"$nixpkgs"/.git-revision)
echo "Channel $channel is at revision $rev"
echo "nixpkgs=$nixpkgs" >> "$GITHUB_ENV"
echo "rev=$rev" >> "$GITHUB_ENV"
# Please don't put any logic here. To ensure local
# reproducibility, this file should only print diagnostics
# (`env`, `set -x`) and execute scripts found elsewhere in
# nixpkgs.
env
set -x
exec pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/fetch-latest-version-of-channel.sh
- name: Fetching pre-built nixpkgs-check-by-name from the channel
run: |
echo "Fetching pre-built nixpkgs-check-by-name from channel $channel at revision $rev"
# Passing --max-jobs 0 makes sure that we won't build anything
nix-build "$nixpkgs" -A tests.nixpkgs-check-by-name --max-jobs 0
# Please don't put any logic here. To ensure local
# reproducibility, this file should only print diagnostics
# (`env`, `set -x`) and execute scripts found elsewhere in
# nixpkgs.
env
set -x
exec pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/fetch-pre-built-by-name-from-channel.sh
- name: Running nixpkgs-check-by-name
run: |
echo "Checking whether the check succeeds on the base branch $GITHUB_BASE_REF"
git checkout -q "$baseSha"
if baseOutput=$(result/bin/nixpkgs-check-by-name . 2>&1); then
baseSuccess=1
else
baseSuccess=
fi
printf "%s\n" "$baseOutput"

echo "Checking whether the check would succeed after merging this pull request"
git checkout -q "$mergedSha"
if mergedOutput=$(result/bin/nixpkgs-check-by-name . 2>&1); then
mergedSuccess=1
exitCode=0
else
mergedSuccess=
exitCode=1
fi
printf "%s\n" "$mergedOutput"

resultToEmoji() {
if [[ -n "$1" ]]; then
echo ":heavy_check_mark:"
else
echo ":x:"
fi
}

# Print a markdown summary in GitHub actions
{
echo "| Nixpkgs version | Check result |"
echo "| --- | --- |"
echo "| Latest base commit | $(resultToEmoji "$baseSuccess") |"
echo "| After merging this PR | $(resultToEmoji "$mergedSuccess") |"
echo ""

if [[ -n "$baseSuccess" ]]; then
if [[ -n "$mergedSuccess" ]]; then
echo "The check succeeds on both the base branch and after merging this PR"
else
echo "The check succeeds on the base branch, but would fail after merging this PR:"
echo "\`\`\`"
echo "$mergedOutput"
echo "\`\`\`"
echo ""
fi
else
if [[ -n "$mergedSuccess" ]]; then
echo "The check fails on the base branch, but this PR fixes it, nicely done!"
else
echo "The check fails on both the base branch and after merging this PR, unknown if only this PRs changes would satisfy the check, the base branch needs to be fixed first."
echo ""
echo "Failure on the base branch:"
echo "\`\`\`"
echo "$baseOutput"
echo "\`\`\`"
echo ""
echo "Failure after merging this PR:"
echo "\`\`\`"
echo "$mergedOutput"
echo "\`\`\`"
echo ""
fi
fi

echo "### Details"
echo "- nixpkgs-check-by-name tool:"
echo " - Channel: $channel"
echo " - Nixpkgs commit: [$rev](https://github.com/${GITHUB_REPOSITORY}/commit/$rev)"
echo " - Store path: \`$(realpath result)\`"
echo "- Tested Nixpkgs:"
echo " - Base branch: $GITHUB_BASE_REF"
echo " - Latest base branch commit: [$baseSha](https://github.com/${GITHUB_REPOSITORY}/commit/$baseSha)"
echo " - Latest PR commit: [$headSha](https://github.com/${GITHUB_REPOSITORY}/commit/$headSha)"
echo " - Merge commit: [$mergedSha](https://github.com/${GITHUB_REPOSITORY}/commit/$mergedSha)"
} >> "$GITHUB_STEP_SUMMARY"

exit "$exitCode"

# Please don't put any logic here. To ensure local
# reproducibility, this file should only print diagnostics
# (`env`, `set -x`) and execute scripts found elsewhere in
# nixpkgs.
env
set -x
exec pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/reproduce.sh "$baseSha" "$toolingSha" "$headSha"
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/usr/bin/env bash
# This checks for mergeability of a pull request as recommended in
# https://docs.github.com/en/rest/guides/using-the-rest-api-to-interact-with-your-git-database?apiVersion=2022-11-28#checking-mergeability-of-pull-requests

while true; do
echo "Checking whether the pull request can be merged"
prInfo=$(gh api \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
/repos/"$GITHUB_REPOSITORY"/pulls/${{ github.event.pull_request.number }})
mergeable=$(jq -r .mergeable <<< "$prInfo")
mergedSha=$(jq -r .merge_commit_sha <<< "$prInfo")

if [[ "$mergeable" == "null" ]]; then
# null indicates that GitHub is still computing whether it's mergeable
# Wait a couple seconds before trying again
echo "GitHub is still computing whether this PR can be merged, waiting 5 seconds before trying again"
sleep 5
else
break
fi
done

if [[ "$mergeable" == "true" ]]; then
echo "The PR can be merged, checking the merge commit $mergedSha"
else
echo "The PR cannot be merged, it has a merge conflict"
exit 1
fi
echo "mergedSha=$mergedSha" >> "$GITHUB_ENV"
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/env bash
echo "Determining which channel to use for PR base branch $GITHUB_BASE_REF"
if [[ "$GITHUB_BASE_REF" =~ ^(release|staging|staging-next)-([0-9][0-9]\.[0-9][0-9])$ ]]; then
# Use the release channel for all PRs to release-XX.YY, staging-XX.YY and staging-next-XX.YY
channel=nixos-${BASH_REMATCH[2]}
echo "PR is for a release branch, using release channel $channel"
else
# Use the nixos-unstable channel for all other PRs
channel=nixos-unstable
echo "PR is for a non-release branch, using unstable channel $channel"
fi
echo "channel=$channel" >> "$GITHUB_ENV"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash
# For pull_request_target this is the same as $GITHUB_SHA
echo "baseSha=$(git rev-parse HEAD^1)" >> "$GITHUB_ENV"

echo "headSha=$(git rev-parse HEAD^2)" >> "$GITHUB_ENV"
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/env bash
echo "Fetching latest version of channel $channel"
# This is probably the easiest way to get Nix to output the path to a downloaded channel!
nixpkgs=$(nix-instantiate --find-file nixpkgs -I nixpkgs=channel:"$channel")
# This file only exists in channels
toolingBinariesSha=$(<"$nixpkgs"/.git-revision)
echo "Channel $channel is at revision $toolingBinariesSha"
echo "nixpkgs=$nixpkgs" >> "$GITHUB_ENV"
echo "toolingBinariesSha=$toolingbinariesSha" >> "$GITHUB_ENV"
Comment on lines +8 to +9
Copy link
Member

Choose a reason for hiding this comment

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

You can remove all GITHUB_ENV assignments if we only have a single step and script.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/usr/bin/env bash
echo "Fetching pre-built nixpkgs-check-by-name from channel $channel at revision $toolingBinariesSha"
# Passing --max-jobs 0 makes sure that we won't build anything
nix-build "$nixpkgs" -A tests.nixpkgs-check-by-name --max-jobs 0
103 changes: 103 additions & 0 deletions pkgs/test/nixpkgs-check-by-name/workflows/check-by-name/reproduce.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#!/usr/bin/env bash

set -e
# Usage: pkgs/test/nixpkgs-check-by-name/reproduce.sh <base SHA> <tooling SHA> <merged SHA>

# TODO(amjoseph): allow omitting the final argument, since it is
# often a commit which exists only in github (and is difficult to
# `git fetch` from github before the PR is merged).
Comment on lines +4 to +8
Copy link
Member

@infinisil infinisil Nov 13, 2023

Choose a reason for hiding this comment

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

Ideally we'd have two scripts:

  • ./reproduce.sh <tooling SHA> <base SHA> <merged SHA>: Reproducible from the args as here (I'd order the args this way).
  • ./reproduce-latest.sh <base branch name>: Tests the current HEAD against the latest commit of the base branch name, effectively replicating what a new CI run should do if you committed and pushed the current code. This should first determine the arguments for ./reproduce.sh and then call it.

Can you implement something like that?


baseSha="$1"
toolingBinariesSha="$2"
mergedSha="$3"

nixpkgs_for_tooling_binaries=$(nix-instantiate --eval --expr "builtins.fetchTarball \"https://github.com/nixos/nixpkgs/archive/$toolingBinariesSha.tar.gz\"" | tr -d '"')
Copy link
Member

Choose a reason for hiding this comment

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

This duplicates the tooling Nixpkgs download, it should re-use the one it already fetched from the channels. Otherwise this adds another ~15 seconds to the CI time.


nix-store --realise --add-root nixpkgs-for-tooling-binaries $nixpkgs_for_tooling_binaries

echo "Fetching pre-built nixpkgs-check-by-name from channel $channel at revision $toolingBinariesSha"
nix-build \
--option extra-substituters https://cache.nixos.org \
--option trusted-public-keys 'cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=' \
nixpkgs-for-tooling-binaries \
-A tests.nixpkgs-check-by-name \
--max-jobs 0

echo "Checking whether the check succeeds on the base branch $GITHUB_BASE_REF"
git checkout -q "$baseSha"
if baseOutput=$(result/bin/nixpkgs-check-by-name . 2>&1); then
baseSuccess=1
else
baseSuccess=
fi
printf "%s\n" "$baseOutput"

echo "Checking whether the check would succeed after merging this pull request"
git checkout -q "$mergedSha"
if mergedOutput=$(result/bin/nixpkgs-check-by-name . 2>&1); then
mergedSuccess=1
exitCode=0
else
mergedSuccess=
exitCode=1
fi
printf "%s\n" "$mergedOutput"

resultToEmoji() {
if [[ -n "$1" ]]; then
echo ":heavy_check_mark:"
else
echo ":x:"
fi
}

# Print a markdown summary in GitHub actions
{
echo "| Nixpkgs version | Check result |"
echo "| --- | --- |"
echo "| Latest base commit | $(resultToEmoji "$baseSuccess") |"
echo "| After merging this PR | $(resultToEmoji "$mergedSuccess") |"
echo ""

if [[ -n "$baseSuccess" ]]; then
if [[ -n "$mergedSuccess" ]]; then
echo "The check succeeds on both the base branch and after merging this PR"
else
echo "The check succeeds on the base branch, but would fail after merging this PR:"
echo "\`\`\`"
echo "$mergedOutput"
echo "\`\`\`"
echo ""
fi
else
if [[ -n "$mergedSuccess" ]]; then
echo "The check fails on the base branch, but this PR fixes it, nicely done!"
else
echo "The check fails on both the base branch and after merging this PR, unknown if only this PRs changes would satisfy the check, the base branch needs to be fixed first."
echo ""
echo "Failure on the base branch:"
echo "\`\`\`"
echo "$baseOutput"
echo "\`\`\`"
echo ""
echo "Failure after merging this PR:"
echo "\`\`\`"
echo "$mergedOutput"
echo "\`\`\`"
echo ""
fi
fi

echo "### Details"
echo "- nixpkgs-check-by-name tool binaries:"
echo " - Channel: $channel"
echo " - Tooling binaries built at nixpkgs commit: [$toolingBinariesSha](https://github.com/${GITHUB_REPOSITORY}/commit/$toolingBinariesSha)"
echo " - Store path: \`$(realpath result)\`"
echo "- Tested Nixpkgs:"
echo " - Base branch: $BASE_SHA"
echo " - Latest base branch commit: [$baseSha](https://github.com/${GITHUB_REPOSITORY}/commit/$baseSha)"
echo " - Latest PR commit: [$headSha](https://github.com/${GITHUB_REPOSITORY}/commit/$headSha)"
echo " - Merge commit: [$mergedSha](https://github.com/${GITHUB_REPOSITORY}/commit/$mergedSha)"
} | tee -a "${GITHUB_STEP_SUMMARY:-/dev/null}"
Comment on lines +94 to +101
Copy link
Member

Choose a reason for hiding this comment

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

We should avoid all GitHub-specific variables from such a script, such that you can run it locally without knowing these variables.


exit "$exitCode"