From f91d393cf99c3cd7d97fe2f6d8e0435cd5f71563 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 1/3] 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 | 14 ++++++++++++++ 3 files changed, 51 insertions(+), 5 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 8efa9a5..d5f3b1d 100644 --- a/action.yml +++ b/action.yml @@ -73,6 +73,18 @@ runs: run: | . "$GITHUB_ACTION_PATH/scripts/utils.sh" + # If we were triggered by a PR then this is easy + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then + # shellcheck disable=SC2016 + # expanded by the runner + pr_number='${{ github.event.number }}' + log "Triggered by a pull request with index: $pr_number" + log "Expected PR URL: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/pulls/$pr_number" + log "Writing output to: $GITHUB_OUTPUT" + echo "pr-number=$pr_number" >> "$GIHUB_OUTPUT" + exit 0 + fi + 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} @@ -83,6 +95,8 @@ runs: -H "Authorization: token $GITHUB_TOKEN" \ -H 'Accept: application/json') + log "Open PRs: $(echo "$prs" | jq --compact output 'map(.number)')" + pr_number=$(echo "$prs" | jq --arg head_ref "$head_ref" '.[] | select(.head.ref == $head_ref) | .number') 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 2/3] 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 9ae615fc0042d37553d9e9ef958b9792d1d57306 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 3/3] 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 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 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