diff --git a/.github/workflows/release-script-check.yml b/.github/workflows/release-script-check.yml index e4052240f..4a0fbf593 100644 --- a/.github/workflows/release-script-check.yml +++ b/.github/workflows/release-script-check.yml @@ -546,3 +546,76 @@ jobs: if ($message -ne '') { throw "a complete pack with node present was refused: '$message'" } Write-Host 'knowledge preflight: refuses without node, names a missing tool, exempts a pack-less tree, passes a healthy one.' + + - name: A failed git mutation stops the cut + shell: pwsh + run: | + # PowerShell does not stop on a native non-zero exit. On the 2.4.0 cut a stale + # .git/index.lock failed the release add and commit, the script reported the + # commit anyway, and Step 7 tagged the commit before it. Invoke-Git is lifted + # by AST, so the code under test is the code that ships, and run against a + # throwaway repository holding that same lock. + $path = (Resolve-Path scripts/cut-release.ps1).Path + $tokens = $null; $errors = $null + $ast = [System.Management.Automation.Language.Parser]::ParseFile($path, [ref]$tokens, [ref]$errors) + $fn = $ast.FindAll({ + param($n) + $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $n.Name -eq 'Invoke-Git' + }, $true) | Select-Object -First 1 + if (-not $fn) { throw 'cut-release.ps1 no longer defines Invoke-Git' } + Invoke-Expression $fn.Extent.Text + + function Get-Refusal([scriptblock]$block) { + try { & $block 2>$null; return '' } catch { return $_.Exception.Message } + } + + $repo = Join-Path ([IO.Path]::GetTempPath()) ('git-exit-' + [guid]::NewGuid().ToString()) + New-Item -ItemType Directory -Path $repo | Out-Null + Push-Location $repo + try { + git init -q + git config user.email ci@example.invalid + git config user.name ci + Set-Content -Path 'a.txt' -Value 'a' + Set-Content -Path 'b c.txt' -Value 'b' + $lock = Join-Path $repo '.git/index.lock' + [IO.File]::WriteAllText($lock, '') + + # 1. The control: a bare git call under the lock does not throw. Without + # this the checks below could be passing on a harness that throws anyway. + $bare = Get-Refusal { git add a.txt } + if ($bare -ne '') { throw "a bare git add threw on its own — the harness proves nothing: '$bare'" } + + # 2. The same calls through Invoke-Git stop, and say which call failed. + foreach ($case in @( + @{ label = 'add'; block = { Invoke-Git add a.txt } }, + @{ label = 'commit'; block = { Invoke-Git commit -m 'Release v9.9.9' } })) { + $message = Get-Refusal $case.block + if ($message -notmatch "^git $($case.label) .*failed \(exit \d+\)") { + throw "Invoke-Git $($case.label) under a held index.lock did not stop: '$message'" + } + } + + # 3. Without the lock the helper passes its arguments through unchanged — a + # path with a space and a message with a space arrive as one argument each. + [IO.File]::Delete($lock) + $files = @('a.txt', 'b c.txt') + Invoke-Git add @files + Invoke-Git commit -q -m 'Release v9.9.9' + $tag = 'v9.9.9' + Invoke-Git tag -a $tag -m "Release $tag" + if ((git log -1 --format=%s) -ne 'Release v9.9.9') { throw 'the commit message did not arrive intact' } + if (((git show --name-only --format= HEAD) -join ',') -ne 'a.txt,b c.txt') { throw 'the staged paths did not arrive intact' } + if ((git tag -l --format='%(contents:subject)' $tag) -ne 'Release v9.9.9') { throw 'the tag message did not arrive intact' } + + # 4. A tag that already exists and a push with no remote stop too. + if ((Get-Refusal { Invoke-Git tag -a $tag -m 'again' }) -notmatch '^git tag .*failed') { throw 'an existing tag did not stop the cut' } + if ((Get-Refusal { Invoke-Git push origin $tag }) -notmatch '^git push .*failed') { throw 'a failed push did not stop the cut' } + } finally { + Pop-Location + } + Write-Host 'Invoke-Git: stops on a held lock, an existing tag and a failed push; passes arguments intact.' + # The last git call above failed on purpose, and the Actions pwsh wrapper ends + # every step with `exit $LASTEXITCODE` — without this the step would pass every + # check and still report exit 128. + $global:LASTEXITCODE = 0 diff --git a/core/src/test/java/com/demcha/documentation/ReleaseScriptGitExitCodeGuardTest.java b/core/src/test/java/com/demcha/documentation/ReleaseScriptGitExitCodeGuardTest.java new file mode 100644 index 000000000..075b1746e --- /dev/null +++ b/core/src/test/java/com/demcha/documentation/ReleaseScriptGitExitCodeGuardTest.java @@ -0,0 +1,87 @@ +package com.demcha.documentation; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Guards that {@code cut-release.ps1} stops when a git call that changes the + * repository fails. + * + *

PowerShell does not stop on a native command's non-zero exit. A bare + * {@code git commit} that loses to a stale {@code .git/index.lock} prints + * {@code fatal:} and the script carries on. On the 2.4.0 cut both the release + * {@code add} and {@code commit} failed that way, the script reported the commit, and + * Step 7 tagged the commit before it. The push was skipped, so nothing left the + * machine, but without {@code -SkipPush} the wrong commit would have been tagged and + * published.

+ * + *

Every {@code add}, {@code commit}, {@code tag} and {@code push} therefore goes + * through {@code Invoke-Git}, which throws on a non-zero exit. This test holds both + * halves: no bare repository-changing call is left — the four the script makes, and + * {@code reset}, {@code checkout}, {@code merge} and {@code rm} should one be added — + * and the helper still checks the exit code. That the + * helper actually throws under a held lock is executed by the A failed git + * mutation stops the cut step in {@code release-script-check.yml}.

+ */ +class ReleaseScriptGitExitCodeGuardTest { + + private static final Path SCRIPT = RepoRoot.get().resolve("scripts/cut-release.ps1"); + + /** A statement that starts with a repository-changing git command. */ + private static final Pattern BARE_MUTATION = Pattern.compile( + "^\\s*(?:&\\s*)?git\\s+(add|commit|tag|push|reset|checkout|merge|rm)\\b"); + + /** The helper's body, from its declaration to the first column-0 closing brace. */ + private static final Pattern HELPER = Pattern.compile( + "(?ms)^function Invoke-Git \\{\\r?\\n(.*?)^}"); + + @Test + void everyGitMutationGoesThroughTheCheckedHelper() throws IOException { + List lines = Files.readAllLines(SCRIPT); + List bare = new ArrayList<>(); + int routed = 0; + + for (int i = 0; i < lines.size(); i++) { + String line = lines.get(i); + if (BARE_MUTATION.matcher(line).find()) { + bare.add("line " + (i + 1) + ": " + line.strip()); + } + if (line.strip().startsWith("Invoke-Git ")) { + routed++; + } + } + + assertThat(routed) + .describedAs("sanity: the script must commit, tag and push through Invoke-Git — " + + "no routed call means this guard is reading a script that changed shape") + .isGreaterThanOrEqualTo(4); + assertThat(bare) + .describedAs("a bare git mutation does not stop the script when it fails: PowerShell " + + "ignores a native non-zero exit, so the cut reports the step and carries on — " + + "on 2.4.0 it tagged the commit before the release. Call Invoke-Git instead") + .isEmpty(); + } + + @Test + void theHelperThrowsOnANonZeroExit() throws IOException { + Matcher helper = HELPER.matcher(Files.readString(SCRIPT)); + + assertThat(helper.find()) + .describedAs("cut-release.ps1 no longer defines Invoke-Git at column 0") + .isTrue(); + assertThat(helper.group(1)) + .describedAs("Invoke-Git must run git and throw when $LASTEXITCODE is non-zero — " + + "a helper that only forwards the call restores the silent failure") + .contains("git @args") + .containsPattern("if \\(\\$LASTEXITCODE -ne 0\\)\\s*\\{\\s*\\r?\\n\\s*throw "); + } +} diff --git a/docs/contributing/release-process.md b/docs/contributing/release-process.md index ae15a7e20..ec41c68d6 100644 --- a/docs/contributing/release-process.md +++ b/docs/contributing/release-process.md @@ -100,6 +100,8 @@ Running `pwsh ./scripts/cut-release.ps1 -Version ` performs: 7. **Annotated tag** `v` (`git tag -a -m "Release v"`). 8. **Push** `develop` and the tag to `origin` (skip with `-SkipPush`). + Steps 6–8 run every `git add`, `commit`, `tag` and `push` through `Invoke-Git`, which stops the cut on a non-zero exit. PowerShell does not stop on a native command's failure by itself: on the 2.4.0 cut a stale `.git/index.lock` failed the release commit, the script reported it anyway, and Step 7 tagged the commit before it. `ReleaseScriptGitExitCodeGuardTest` holds every mutation to the helper, and the **A failed git mutation stops the cut** step in [`release-script-check.yml`](../../.github/workflows/release-script-check.yml) runs the helper against a held lock. + The script supports `-DryRun` (preview every step), `-SkipPush` (commit + tag locally only), `-SkipVerify` (skip the verify + japicmp gates; the knowledge regen still runs), and `-PostReleaseOnly`. The latter skips release work entirely and instead **opens the next development line**: it bumps every train pom to the next patch `-SNAPSHOT`, moves the `graph-compose-templates` japicmp previous-release pin (`japicmp.baseline.previous`) onto the release just published, flips GH_BASE back to `/blob/develop`, and regenerates the knowledge pack surfaces at the new `-SNAPSHOT` (same commands and gates as Step 5c — the surfaces embed the reactor version, so a bump committed without them turns develop's "Knowledge pack — API surface is current" CI job red until a follow-up regen lands), then commits — staging `knowledge/` beside the poms — and pushes. A pre-bump probe (`extract-api --from-reactor --check`) refuses before any pom moves when the tree cannot regenerate the surfaces — e.g. no compiled classes after a `clean` — so the failure lands on a clean tree, not mid-bump. It deliberately leaves the README/showcase **install snippets on the just-published release** — during a `-SNAPSHOT` cycle they must advertise the version actually on Central, which `VersionConsistencyGuardTest` enforces; `cut-release.ps1` rewrites them to the new version at the next release commit. `-PostReleaseOnly` is idempotent: if the poms already carry a `-SNAPSHOT`, the bump is skipped (only the showcase flip runs, if needed). **Knowledge tooling is a pre-flight condition.** Step 5c is the release's own run of a gate that also runs on the tag, and the tag is the problem: [`release.yml`](../../.github/workflows/release.yml) triggers `on: push: tags:`, so by the time it re-verifies, the tag is already on origin — and a tag Maven Central has validated cannot be moved. So `Assert-KnowledgeToolingAvailable` runs in **Step 0 of both mutating modes**, beside the branch and roadmap checks and before the first pom moves: if this tree ships a knowledge pack and `node` is not on `PATH`, or any of the four tools is missing, the script throws — naming what is missing — and nothing is written, committed, tagged or pushed. `Update-KnowledgeSurfaces` asserts the same condition again before it runs a command, so calling it directly cannot fail open either. **A tree with no knowledge pack is valid and skips the requirement entirely**: the 1.x line ships none, so a 1.9.x cut needs no Node.js and the preflight passes with a note. Pushing the tag still re-verifies independently in `release.yml` — the local gate makes that re-verification predictable, it does not replace it. `ReleaseKnowledgeGateGuardTest` holds the ordering, the refusal and the local/tag parity; that the refusal actually fires when Node is missing is *executed* by the **Missing Node aborts both release paths** step in [`release-script-check.yml`](../../.github/workflows/release-script-check.yml). @@ -319,6 +321,7 @@ The published jar is final. **Never force-move a tag** that Maven Central has al | `incompatible types: possible lossy conversion from double to float` on `.margin(...)` | `DocumentInsets` accessor returns `double`, the `float` overload narrows | Switch the call to `.margin(layout.margin())` (the `DocumentInsets` overload) | | `GenerateAllExamples` dies mid-run on a specific PDF | Windows file lock from an open viewer | Ask the user to close the viewer; do not retry blindly | | `cut-release.ps1` aborts at "Working tree has uncommitted changes" | Untracked junk (zero-byte `{,`, `0)` etc.) or unstaged pre-release fix | Verify each is 0 bytes, delete by exact name; never `git clean -fd` blindly | +| `cut-release.ps1` stops at Step 6 or 7 with `git commit … failed` or `git tag … failed` and `Unable to create '.git/index.lock': File exists` | Another git process — an IDE, another agent working in the same tree — held the index, or left a stale lock behind | Nothing was pushed. A Step 6 stop committed nothing; a Step 7 stop left the `Release v` commit without a tag. Make sure no git process is running and delete `.git/index.lock`, then finish the remaining steps by hand with the file list and messages the script prints under `-DryRun`, or reset to `origin/` and re-run the cut | ### Release-publication failure recovery diff --git a/scripts/cut-release.ps1 b/scripts/cut-release.ps1 index 963bcf293..cda2b523f 100644 --- a/scripts/cut-release.ps1 +++ b/scripts/cut-release.ps1 @@ -124,6 +124,20 @@ function Run($command) { } } +function Invoke-Git { + # Every git call that changes the repository — add, commit, tag, push — goes + # through here. PowerShell does not stop on a native command's non-zero exit, so a + # bare `git commit` that loses to a stale .git/index.lock prints "fatal:" and the + # script carries on. On the 2.4.0 cut that is what happened: the add and the commit + # both failed, the script reported "commit: Release v2.4.0", and Step 7 tagged the + # commit before it. Throwing here stops the release at the step that failed, before + # a tag can name the wrong commit or a push can publish it. + git @args + if ($LASTEXITCODE -ne 0) { + throw "git $($args -join ' ') failed (exit $LASTEXITCODE); nothing after it ran." + } +} + function Assert-BranchPreflight($branch) { # Shared safety gate for BOTH a full release cut and -PostReleaseOnly: the current # branch is the target branch, the working tree is clean, and the local branch is @@ -1440,15 +1454,15 @@ if ($PostReleaseOnly) { Write-Host " [DRY RUN] git add $($filesToCommit -join ' ')" -ForegroundColor Yellow Write-Host " [DRY RUN] git commit -m `"$msg`"" -ForegroundColor Yellow } else { - git add @filesToCommit - git commit -m $msg + Invoke-Git add @filesToCommit + Invoke-Git commit -m $msg Note "commit: $msg" } Step 5 "Push $Branch" if ($DryRun) { Write-Host " [DRY RUN] git push origin $Branch" -ForegroundColor Yellow } else { - git push origin $Branch + Invoke-Git push origin $Branch } } else { Note "Nothing to do (showcase already on /blob/$Branch and version already a SNAPSHOT)." @@ -1879,8 +1893,8 @@ try { Write-Host " [DRY RUN] git add $($commitFiles -join ' ')" -ForegroundColor Yellow Write-Host " [DRY RUN] git commit -m `"$commitMsg`"" -ForegroundColor Yellow } else { - git add @commitFiles - git commit -m $commitMsg + Invoke-Git add @commitFiles + Invoke-Git commit -m $commitMsg Note "commit: $commitMsg" } @@ -1888,7 +1902,7 @@ try { if ($DryRun) { Write-Host " [DRY RUN] git tag -a $tag -m `"Release $tag`"" -ForegroundColor Yellow } else { - git tag -a $tag -m "Release $tag" + Invoke-Git tag -a $tag -m "Release $tag" Note "tag: $tag" } @@ -1902,8 +1916,8 @@ try { Write-Host " [DRY RUN] git push origin $Branch" -ForegroundColor Yellow Write-Host " [DRY RUN] git push origin $tag" -ForegroundColor Yellow } else { - git push origin $Branch - git push origin $tag + Invoke-Git push origin $Branch + Invoke-Git push origin $tag Note "pushed: $Branch + $tag" } }