diff --git a/.github/workflows/ingest.yml b/.github/workflows/ingest.yml index 749f5bb6786..4fc2f5776b6 100644 --- a/.github/workflows/ingest.yml +++ b/.github/workflows/ingest.yml @@ -107,7 +107,7 @@ jobs: fi git commit -m "Matched advisory candidates (snapshot)" { - echo "base=${GITHUB_SHA}" + echo "diff_base=${GITHUB_SHA}" echo "commit=$(git rev-parse HEAD)" echo "created=true" } >> "${GITHUB_OUTPUT}" @@ -115,7 +115,7 @@ jobs: - name: Partition matched advisories if: steps.snapshot.outputs.created == 'true' env: - BASE_SHA: ${{ steps.snapshot.outputs.base }} + DIFF_BASE_SHA: ${{ steps.snapshot.outputs.diff_base }} SNAPSHOT_SHA: ${{ steps.snapshot.outputs.commit }} SHARD_OUTPUT: ${{ runner.temp }}/advisory-shards # Deltas at or below this many files publish as one standing PR; @@ -126,7 +126,7 @@ jobs: mkdir -p "${SHARD_OUTPUT}" changed="${RUNNER_TEMP}/changed-advisories" git diff --name-only -z --no-renames --diff-filter=AM \ - "${BASE_SHA}" "${SNAPSHOT_SHA}" -- advisories/ > "${changed}" + "${DIFF_BASE_SHA}" "${SNAPSHOT_SHA}" -- advisories/ > "${changed}" count="$(tr -dc '\0' < "${changed}" | wc -c)" if (( count <= SINGLE_PR_LIMIT )); then echo "Publishing ${count} paths as a single pull request." @@ -149,7 +149,7 @@ jobs: if: steps.snapshot.outputs.created == 'true' env: GITHUB_TOKEN: ${{ secrets.HOMEBREW_GITHUB_PUBLIC_REPO_TOKEN || github.token }} - BASE_SHA: ${{ steps.snapshot.outputs.base }} + DIFF_BASE_SHA: ${{ steps.snapshot.outputs.diff_base }} SNAPSHOT_SHA: ${{ steps.snapshot.outputs.commit }} SHARD_OUTPUT: ${{ runner.temp }}/advisory-shards run: | @@ -161,6 +161,25 @@ jobs: exit 1 fi + # The matcher runs for over an hour, so main may advance after checkout. + # Base every published branch on main as it exists at publication time. + git fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main + PUBLISH_BASE_SHA="$(git rev-parse origin/main)" + merge_status=0 + PUBLISH_SNAPSHOT_TREE="$( + git merge-tree --write-tree --no-messages --merge-base="${DIFF_BASE_SHA}" \ + "${PUBLISH_BASE_SHA}" "${SNAPSHOT_SHA}" + )" || merge_status=$? + if (( merge_status != 0 )); then + if (( merge_status == 1 )); then + echo "::error::Matched advisory changes conflict with main at publication time." + else + echo "::error::git merge-tree failed with exit status ${merge_status}." + fi + echo "${PUBLISH_SNAPSHOT_TREE}" + exit "${merge_status}" + fi + # Commands must use `|| return` because this function runs in an `if`. publish_shard() { local manifest="$1" @@ -177,14 +196,18 @@ jobs: body="Automated candidates via \`brew advisory-match --all\`, sharded by formula (SHA-256 bucket \`${shard}\`). See [CONTRIBUTING.md](https://github.com/${GITHUB_REPOSITORY}/blob/HEAD/CONTRIBUTING.md#reviewing-matched-candidates) for the review checklist." fi - git switch --discard-changes --force-create "${branch}" "${BASE_SHA}" || return + git switch --discard-changes --force-create "${branch}" "${PUBLISH_BASE_SHA}" || return git clean --force -d -- advisories/ || return - git restore --source="${SNAPSHOT_SHA}" --pathspec-from-file="${manifest}" \ + git restore --source="${PUBLISH_SNAPSHOT_TREE}" --pathspec-from-file="${manifest}" \ --pathspec-file-nul || return # Regenerate rebuilds data/advisories.json separately. git add advisories/ || return + if git diff --cached --quiet; then + echo "Shard ${shard} is already on main; nothing to publish." + return 0 + fi git commit -m "${title}" \ - -m "Base: ${BASE_SHA}" || return + -m "Base: ${PUBLISH_BASE_SHA}" || return # No lease: every branch is rebuilt from main on each run. git push --force origin "HEAD:refs/heads/${branch}" || return @@ -219,6 +242,7 @@ jobs: # A mode switch (standing <-> sharded) leaves the other mode's PRs # open with superseded content; close them and delete their branches. + # Every successful manifest counts as published, including those already on main. published=" " for manifest in "${manifests[@]}"; do shard="${manifest##*/}" diff --git a/spec/workflows_spec.rb b/spec/workflows_spec.rb index 2d6f8a84a24..bec65d2f580 100644 --- a/spec/workflows_spec.rb +++ b/spec/workflows_spec.rb @@ -100,6 +100,13 @@ publish = steps.fetch(publish_index).fetch("run") expect(snapshot).to include('git switch --detach "${GITHUB_SHA}"') + expect(snapshot).to include('echo "diff_base=${GITHUB_SHA}"') + expect(snapshot).not_to include('echo "base=${GITHUB_SHA}"') + expect(steps.fetch(partition_index).dig("env", "DIFF_BASE_SHA")).to eq( + "${{ steps.snapshot.outputs.diff_base }}", + ) + expect(steps.fetch(partition_index).fetch("env")).not_to have_key("BASE_SHA") + expect(partition).to include('"${DIFF_BASE_SHA}" "${SNAPSHOT_SHA}"') expect(partition).to include("bundle exec rake advisories:shard") expect(partition).to include("--no-renames") expect(partition).to include("--diff-filter=AM") @@ -117,17 +124,44 @@ ) expect(publish).not_to include("gh auth setup-git") expect(publish).to include('manifests=("${SHARD_OUTPUT}"/*)') - expect(publish).to include('git switch --discard-changes --force-create "${branch}" "${BASE_SHA}"') + fetch_main = "git fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main" + merge_snapshot = + 'git merge-tree --write-tree --no-messages --merge-base="${DIFF_BASE_SHA}"' + switch_to_publish_base = + 'git switch --discard-changes --force-create "${branch}" "${PUBLISH_BASE_SHA}"' + expect(publish).to include(fetch_main) + expect(publish).to include('PUBLISH_BASE_SHA="$(git rev-parse origin/main)"') + expect(publish).to include(merge_snapshot) + expect(publish).to include(')" || merge_status=$?') + expect(publish).to include("if (( merge_status == 1 ))") + expect(publish).to include("Matched advisory changes conflict with main") + expect(publish).to include("git merge-tree failed with exit status ${merge_status}") + expect(publish).to include('echo "${PUBLISH_SNAPSHOT_TREE}"') + expect(publish).to include('exit "${merge_status}"') + expect(publish).to include(switch_to_publish_base) + expect(publish.index(fetch_main)).to be < publish.index(merge_snapshot) + expect(publish.index(merge_snapshot)).to be < publish.index(switch_to_publish_base) + expect(steps.fetch(publish_index).dig("env", "DIFF_BASE_SHA")).to eq( + "${{ steps.snapshot.outputs.diff_base }}", + ) + expect(steps.fetch(publish_index).fetch("env")).not_to have_key("BASE_SHA") expect(publish).to include("git clean --force -d -- advisories/") - expect(publish).to include('git restore --source="${SNAPSHOT_SHA}" --pathspec-from-file="${manifest}"') + expect(publish).to include( + 'git restore --source="${PUBLISH_SNAPSHOT_TREE}" --pathspec-from-file="${manifest}"', + ) expect(publish).to include("--pathspec-file-nul") expect(publish).not_to include("bundle exec rake advisories:concat") expect(publish).to include("git add advisories/") expect(publish).not_to include("git add advisories/ data/") + empty_guard = "if git diff --cached --quiet; then" + expect(publish).to include(empty_guard) + expect(publish).to include('echo "Shard ${shard} is already on main; nothing to publish."') + expect(publish.index("git add advisories/")).to be < publish.index(empty_guard) + expect(publish.index(empty_guard)).to be < publish.index('git commit -m "${title}"') expect(publish).to include('branch="matched-advisories"') expect(publish).to include('branch="matched-advisories-${shard}"') expect(publish).to include('git push --force origin "HEAD:refs/heads/${branch}"') - expect(publish).to include("Base: ${BASE_SHA}") + expect(publish).to include("Base: ${PUBLISH_BASE_SHA}") expect(publish).to include("No advisory shards were produced") expect(publish).to include("failed=0") expect(publish).to include('if publish_shard "${manifest}"') @@ -135,6 +169,7 @@ expect(publish).to include("--delete-branch") expect(publish).to include("matched-advisories(-[0-9a-f]{2})?$") expect(publish).to include('exit "${failed}"') + expect(publish).to include("including those already on main") # Reconciliation only runs after full publication, enumerates open PRs # with a checked paginated command, and propagates close failures.