From ca72db2a3b1cb4f2957d1e8981112be1db11645e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?= Date: Fri, 11 Jul 2025 18:23:06 +0200 Subject: [PATCH 1/4] refactor: try to condense the action in fewer steps This should make it easier to debug/maintain. --- action.yml | 64 ++++++++++-------------------------------------- scripts/run.sh | 61 +++++++++++++++++++++++++++++++++++++++++++++ scripts/utils.sh | 5 ++++ 3 files changed, 79 insertions(+), 51 deletions(-) create mode 100755 scripts/run.sh diff --git a/action.yml b/action.yml index 8efa9a5..f27253b 100644 --- a/action.yml +++ b/action.yml @@ -67,42 +67,18 @@ outputs: runs: using: 'composite' steps: - - name: Find PR (if it exists) + - name: Run id: pr-number - if: inputs.comment-on-pr == 'true' + env: + BASE_BRANCH: ${{ inputs.base-branch }} + COMMENT: ${{ inputs.comment-on-pr }} + DO_COMPARISON: ${{ inputs.do-comparison }} + GENERATE_ARTIFACT: ${{ inputs.generate-artifact }} + JOB_NAME: ${{ inputs.job-name }} run: | - . "$GITHUB_ACTION_PATH/scripts/utils.sh" - - log 'Determine head_ref' - # For push & tag events it'll bet GITHUB_REF_NAME, for pull_request events it'll be GITHUB_HEAD_REF - head_ref=${GITHUB_REF_NAME:-$GITHUB_HEAD_REF} - - log "Get PR number for $head_ref" - prs=$(curl -X 'GET' \ - "$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/pulls?state=open&sort=recentupdate" \ - -H "Authorization: token $GITHUB_TOKEN" \ - -H 'Accept: application/json') - - pr_number=$(echo "$prs" | - jq --arg head_ref "$head_ref" '.[] | select(.head.ref == $head_ref) | .number') - - # This seems to create the file??? - log "GITHUB_OUTPUT=$GITHUB_OUTPUT" - log "$(ls -l "$GITHUB_OUTPUT")" - - # Protect against running before a PR is made or if it is triggered on the main branch - if [ -z "$pr_number" ]; then - warn "No PR created for this commit" - echo "pr-number=" >> "$GIHUB_OUTPUT" - exit 0 - fi - - log "Retrieved index: $pr_number" - log "Expected PR URL: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/pulls/$pr_number" - - echo "pr-number=$pr_number" >> "$GITHUB_OUTPUT" + "$GITHUB_ACTION_PATH/scripts/run.sh" - name: Find previous comment (if present) - # We want to generate a comment, and we we able to fin the PR number + # We want to generate a comment and we we able to find the PR number if: inputs.comment-on-pr == 'true' && steps.pr-number.outputs.pr-number != '' id: find-comment uses: https://github.com/peter-evans/find-comment@3eae4d37986fb5a8592848f6a574fdf654e61f9e # v3 @@ -110,35 +86,21 @@ runs: issue-number: ${{ steps.pr-number.outputs.pr-number }} direction: first body-includes: "" - - name: Create report - if: inputs.comment-on-pr == 'true' || inputs.generate-artifact == 'true' + - name: Create report and comment on PR + # We want to generate a comment and we we able to find the PR number + if: inputs.comment-on-pr == 'true' && steps.pr-number.outputs.pr-number != '' env: PR_ID: ${{ steps.pr-number.outputs.pr-number }} - COMMENT: ${{ inputs.comment-on-pr }} COMMENT_ID: ${{ steps.find-comment.outputs.comment-id }} - ARTIFACT_NAME: ${{ inputs.artifact-name }} DO_COMPARISON: ${{ inputs.do-comparison }} BASE_BRANCH: ${{ inputs.base-branch }} - JOB_NAME: ${{ inputs.job-name }} run: | . "$GITHUB_ACTION_PATH/scripts/utils.sh" - # Input validation - if [ "$DO_COMPARISON" = 'true' ] && [ -z "$JOB_NAME" ]; then - error 'job-name should be set if you want to generate a comparison report' - exit 1 - fi - - # Create Size Report - "$GITHUB_ACTION_PATH/scripts/create-report.sh" report.json - - # Nothing else to do - if [ "$COMMENT" != 'true' ]; then exit 0; fi - # Try to do a comparison report if [ "$DO_COMPARISON" = 'true' ]; then if "$GITHUB_ACTION_PATH/scripts/retrieve-old-report.sh" && [ -f old-report.json ]; then - log "Reporting on sizes and comparing to sizes in $HEAD_BRANCH" + log "Reporting on sizes and comparing to sizes in $BASE_BRANCH" "$GITHUB_ACTION_PATH/scripts/comment_on_pr.sh" report.json old-report.json exit 0 diff --git a/scripts/run.sh b/scripts/run.sh new file mode 100755 index 0000000..33e3068 --- /dev/null +++ b/scripts/run.sh @@ -0,0 +1,61 @@ +#!/bin/sh + +. "$GITHUB_ACTION_PATH/scripts/utils.sh" + +# Input validation +if [ "$COMMENT" != "true" ] && [ "$GENERATE_ARTIFACT" != "true" ]; then + panic 'Neither comment-on-pr nor generate-artifact is set + note: this looks like an error; if it isn'"'"'t disable this action with "step.if"' +fi + +if [ "$DO_COMPARISON" = 'true' ] && [ -z "$JOB_NAME" ]; then + panic 'Requested a comparison report but job-name wasn'"'"'t set' +fi + +# Create Size Report (will be uploaded by the upload-artifact action) +"$GITHUB_ACTION_PATH/scripts/create-report.sh" report.json + +# Nothing else to do +if [ "$COMMENT" != "true" ]; then exit 0; fi + +# Find the PR for this commit so we can post a comment on it +pr_number= +case "$GITHUB_EVENT_NAME" in +"pull_request") + pr_number=$(jq .number "$GITHUB_EVENT_PATH") + log "Triggered by a pull request with index: $pr_number" + ;; +"push") + log "Triggered by a push to $GITHUB_REF_NAME autodetecting PR number" + + log "Get PR number for $GITHUB_REF_NAME" + prs=$(curl -X 'GET' \ + "$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/pulls?state=open&sort=recentupdate" \ + -H "Authorization: token $GITHUB_TOKEN" \ + -H 'Accept: application/json') + + log "Found these open PRs: $(echo "$prs" | jq '[.[] | .number]')" + + pr_number=$(echo "$prs" | + jq --arg ref "$GITHUB_REF_NAME" '.[] | select(.head.ref == $ref) | .number') + + # Protect against running before a PR is made or if it is triggered on the main branch + if [ -z "$pr_number" ]; then + warn "No PR created for this commit" + exit 0 + fi + + log "The PR we found for $GITHUB_REF_NAME is $pr_number" + ;; +*) + panic "Unexpected event $GITHUB_EVENT_NAME for commenting on a PR, expected push or pull_request" + ;; +esac + +log "Expected PR URL: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/pulls/$pr_number" + +# This seems to create the file??? +log "GITHUB_OUTPUT=$GITHUB_OUTPUT" +log "$(ls -l "$GITHUB_OUTPUT")" + +echo "pr-number=$pr_number" >>"$GITHUB_OUTPUT" diff --git a/scripts/utils.sh b/scripts/utils.sh index 74c31a3..a46bc1f 100755 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -12,6 +12,11 @@ error() { log "\e[0;31m[WARN]:" "$@" "\e[0m" } +panic() { + error "$@" + exit 1 +} + group() { echo "::group::$1" } From 7d7a576f501aac897f0c3d8ea74791a4c7001cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?= Date: Thu, 10 Jul 2025 19:39:36 +0200 Subject: [PATCH 2/4] ci: test both push and pull_request events I think we have a bug with pull_request events so... --- .forgejo/workflows/check.yml | 9 ++++----- .forgejo/workflows/test-pr.yml | 33 +++++++++++++++++++++++++++++++++ action.yml | 6 ++++-- scripts/retrieve-old-report.sh | 15 +++++++++------ 4 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 .forgejo/workflows/test-pr.yml diff --git a/.forgejo/workflows/check.yml b/.forgejo/workflows/check.yml index 14a13b1..1c90cbe 100644 --- a/.forgejo/workflows/check.yml +++ b/.forgejo/workflows/check.yml @@ -24,18 +24,17 @@ jobs: uses: ./ with: # Create a comment on the associated PR - comment-on-pr: ${{ github.ref_name != 'main' }} + comment-on-pr: 'false' # Generate artifacts on main (to speed up comparisons) # generate-artifact: ${{ github.ref_name == 'main' }} # Always generate artifacts for testing purposes generate-artifact: 'true' - # Generate comparisons to main - do-comparison: 'true' - # This job's name (so we can find the artifacts) + # This job's name (so we can find the previous artifacts) job-name: report-size report-download-check: runs-on: nixos - needs: report-size + needs: report-size-push + if: github.event_name == 'push' steps: - name: Download previous report uses: https://git.salame.cl/actions/download-artifact@d8d0a99033603453ad2255e58720b460a0555e1e # v4 diff --git a/.forgejo/workflows/test-pr.yml b/.forgejo/workflows/test-pr.yml new file mode 100644 index 0000000..43fb880 --- /dev/null +++ b/.forgejo/workflows/test-pr.yml @@ -0,0 +1,33 @@ +on: + pull_request: +jobs: + report-size-pr: + runs-on: nixos + steps: + - uses: "https://git.salame.cl/actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683" # v4 + - run: nix --version + - name: Create Size Report + uses: ./ + with: + # Create a comment on the associated PR + comment-on-pr: 'true' + # Generate artifacts on main (to speed up comparisons) + # generate-artifact: ${{ github.ref_name == 'main' }} + # Always generate artifacts for testing purposes + generate-artifact: 'true' + # Generate comparisons to main + do-comparison: 'true' + # Get the previous artifacts from report-size-push (since those run on main) + job-name: report-size + artifact-name: report.json + report-download-check-pr: + runs-on: nixos + needs: report-size-pr + steps: + - name: Download previous report + uses: https://git.salame.cl/actions/download-artifact@d8d0a99033603453ad2255e58720b460a0555e1e # v4 + with: + name: report.json + - name: Verify report exists + run: | + cat report.json diff --git a/action.yml b/action.yml index f27253b..db929d2 100644 --- a/action.yml +++ b/action.yml @@ -90,10 +90,12 @@ runs: # We want to generate a comment and we we able to find the PR number if: inputs.comment-on-pr == 'true' && steps.pr-number.outputs.pr-number != '' env: - PR_ID: ${{ steps.pr-number.outputs.pr-number }} + ARTIFACT_NAME: ${{ inputs.artifact-name }} + BASE_BRANCH: ${{ inputs.base-branch }} COMMENT_ID: ${{ steps.find-comment.outputs.comment-id }} DO_COMPARISON: ${{ inputs.do-comparison }} - BASE_BRANCH: ${{ inputs.base-branch }} + JOB_NAME: ${{ inputs.job-name }} + PR_ID: ${{ steps.pr-number.outputs.pr-number }} run: | . "$GITHUB_ACTION_PATH/scripts/utils.sh" diff --git a/scripts/retrieve-old-report.sh b/scripts/retrieve-old-report.sh index ee83f08..06b13bf 100755 --- a/scripts/retrieve-old-report.sh +++ b/scripts/retrieve-old-report.sh @@ -40,7 +40,9 @@ has_report() { # If a base branch is not provided, use the default branch base_branch=${BASE_BRANCH:-$(default_branch)} -if [ "$(in_private_repo)" != 'true' ] && [ "$JOB_NAME" ]; then +if in_private_repo; then + warn 'Detected that this is a private repo cannot retrieve old report' +elif [ "$JOB_NAME" ]; then url=$(base_report_url "$base_branch") log "Found previous run at: $url" @@ -56,13 +58,14 @@ if [ "$(in_private_repo)" != 'true' ] && [ "$JOB_NAME" ]; then exit 0 fi error "Failed to find previous report, expected at: $report_url" +else + panic 'job-name is missing, therefore we cannot find the previous report' fi -warn "Couldn't retrieve old report:" -warn ' This usuially happens when running on private repos' -warn ' or when job-name is not set.' -warn -warn ' See the README for more details' +warn "Couldn't retrieve old report: + note: This usually happens when running on private repos or when job-name is not set. + + See the README for more details" error "Falling back to slow method (checkout $base_branch and generate the report)" From 6b0b78e4962d7aae6243b4601ba8257be99e3a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?= Date: Sun, 13 Jul 2025 15:02:37 +0200 Subject: [PATCH 3/4] fix(report): building the wrong derivation for home-manager Small typo that was breaking this script T-T: ```bash # Wrong: nix build --print-out-paths ".#homeConfigurations.$config.activationPackages" # Correct: nix build --print-out-paths ".#homeConfigurations.$config.config.home.activationPackage" ``` --- scripts/create-report.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-report.sh b/scripts/create-report.sh index cfe4209..cab2166 100755 --- a/scripts/create-report.sh +++ b/scripts/create-report.sh @@ -51,7 +51,7 @@ pkgs_json() { hm_configs_json() { for config in $hmConfigs; do log "Building $config" - path=$(nix build --print-out-paths ".#homeConfigurations.$config.activationPackages") + path=$(nix build --print-out-paths ".#homeConfigurations.$config.config.home.activationPackage") closure_size "$config" "$path" done } From e5d1a0751adb4963c0a4982503806ae5f19f52da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?= Date: Sun, 13 Jul 2025 18:39:23 +0200 Subject: [PATCH 4/4] fix(report): use nix eval to list the components This is less error prone than parsing the information from `nix flake show`. --- scripts/create-report.sh | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/scripts/create-report.sh b/scripts/create-report.sh index cab2166..6e04b9f 100755 --- a/scripts/create-report.sh +++ b/scripts/create-report.sh @@ -7,24 +7,25 @@ util_path="${GITHUB_ACTION_PATH:-.}/scripts/utils.sh" # shellcheck source=scripts/utils.sh . "${util_path}" -group 'Retrieving Flake information' -flake_info=$(nix flake show --json --quiet --quiet) -endgroup - system=$(nix eval --impure --json --expr 'builtins.currentSystem') -group 'Show Packages' -packages=$(echo "$flake_info" | jq --raw-output --argjson system "$system" 'getpath(["packages", $system]) | select(. != null) | keys[]') +# Extract the names of a flake attrset +get_names() { + nix eval --json --apply builtins.attrNames "$1" 2>/dev/null | jq --raw-output '.[]' +} + +group "Show Packages for $system" +packages=$(get_names .#packages."$system") [ -z "$packages" ] || log "$packages" endgroup group 'Show Home Manager Configurations' -hmConfigs=$(echo "$flake_info" | jq --raw-output '.homeConfigurations | select(. != null) | keys[]') +hmConfigs=$(get_names .#homeConfigurations) [ -z "$hmConfigs" ] || log "$hmConfigs" endgroup group 'Show NixOS Configurations' -nixosConfigs=$(echo "$flake_info" | jq --raw-output '.nixosConfigurations | select(. != null) | keys[]') +nixosConfigs=$(get_names .#nixosConfigurations) [ -z "$nixosConfigs" ] || log "$nixosConfigs" endgroup