From b56dd9f310255c9057f537ed85b5132fd2c74245 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:55:00 +0200 Subject: [PATCH 01/14] Implement the apply goal as a Maven-native port of prevent-overwrites.sh Adds com.jardoapps:pao-maven-plugin:0.1.0 with a single "apply" goal that manages branch-specific SNAPSHOT versions: on a feature branch it appends the branch name to the project version, and on a core branch it strips that suffix back off, from the project version and from dependency versions alike. The goal runs as its own invocation rather than binding into the build, because Maven reads and interpolates every POM before any mojo executes, so a version written during a build would not change what that build deploys. ApplyMojo is parameter plumbing only; the behaviour lives in PreventOverwritesRunner, which works on plain types so it can be driven directly from tests. Every environment variable of the shell version maps to a pao.* property, and the .prevent-overwrites.conf format is supported unchanged. Rather than depending on versions-maven-plugin internals, PomDocument scans a pom.xml into elements carrying their source offsets and splices edits into the original text, leaving formatting, comments and attribute quoting untouched. Comments, CDATA and quoted attributes are skipped, so a inside a comment is never rewritten. Working from Maven's model rather than grep and sed fixes three defects in the shell implementation: - Multi-module reactors broke, because only a outside was rewritten and module references were left pointing at a version that no longer existed. Parent references across the reactor now move with the project version. - A two-digit patch number was truncated: the prefix pattern matched a single patch digit, so 1.2.10-feature-x-SNAPSHOT stripped to 1.2.1-SNAPSHOT. - Pinned values needed a restricted character set purely because they flowed into a sed replacement; with structured editing the check is now that a pin is revertible, which is the actual requirement. Two behaviours differ from the shell version on purpose, both documented: versions written as ${property} are followed to the property that defines them rather than having the reference overwritten, and every configuration row is validated even when it targets another branch, so typos surface on the first run. Covered by 71 unit tests, including the twelve cases ported from the shell test suite, which compare output to the original fixtures byte for byte. Mojo wiring, reactor collection and the git operations are covered by it/run-integration-tests.sh. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 6 + README.md | 197 +++++++++++- it/run-integration-tests.sh | 207 ++++++++++++ pom.xml | 77 +++++ .../java/com/jardoapps/pao/ApplyMojo.java | 145 +++++++++ .../com/jardoapps/pao/BranchDetector.java | 56 ++++ .../com/jardoapps/pao/BranchVersions.java | 96 ++++++ .../java/com/jardoapps/pao/GlobMatcher.java | 51 +++ .../java/com/jardoapps/pao/PaoException.java | 15 + .../pao/PreventOverwritesRunner.java | 286 +++++++++++++++++ .../java/com/jardoapps/pao/ProjectModel.java | 26 ++ .../com/jardoapps/pao/RunnerSettings.java | 108 +++++++ .../com/jardoapps/pao/config/PinConfig.java | 43 +++ .../jardoapps/pao/config/PinConfigParser.java | 148 +++++++++ .../pao/git/CommandLineGitClient.java | 115 +++++++ .../java/com/jardoapps/pao/git/GitClient.java | 22 ++ .../com/jardoapps/pao/pom/PomDocument.java | 301 ++++++++++++++++++ .../com/jardoapps/pao/pom/PomEditSession.java | 114 +++++++ .../java/com/jardoapps/pao/pom/PomReader.java | 78 +++++ .../com/jardoapps/pao/pom/XmlElement.java | 77 +++++ .../com/jardoapps/pao/BranchVersionsTest.java | 92 ++++++ .../com/jardoapps/pao/GlobMatcherTest.java | 42 +++ .../com/jardoapps/pao/MultiModuleTest.java | 67 ++++ .../pao/PreventOverwritesRunnerTest.java | 256 +++++++++++++++ .../jardoapps/pao/PropertyVersionTest.java | 89 ++++++ .../com/jardoapps/pao/RunnerTestSupport.java | 90 ++++++ .../pao/config/PinConfigParserTest.java | 113 +++++++ .../com/jardoapps/pao/git/FakeGitClient.java | 56 ++++ .../jardoapps/pao/pom/PomDocumentTest.java | 230 +++++++++++++ ...ected-config-exclusive-suffix-rederive.xml | 16 + ...xpected-config-pin-dependency-versions.xml | 21 ++ .../expected-config-pin-project-version.xml | 16 + .../poms/expected-enforce-branch-version.xml | 16 + .../poms/expected-remove-branch-version.xml | 16 + ...cted-remove-dependency-branch-versions.xml | 26 ++ .../poms/expected-revision-enforced.xml | 21 ++ .../poms/multimodule-expected/core/pom.xml | 20 ++ .../poms/multimodule-expected/pom.xml | 13 + .../resources/poms/multimodule/core/pom.xml | 20 ++ src/test/resources/poms/multimodule/pom.xml | 13 + .../resources/poms/sample-pom-revision.xml | 21 ++ .../resources/poms/sample-pom-two-deps.xml | 21 ++ .../poms/sample-pom-with-branch-deps.xml | 26 ++ .../poms/sample-pom-with-branch-version.xml | 16 + src/test/resources/poms/sample-pom.xml | 16 + 45 files changed, 3500 insertions(+), 1 deletion(-) create mode 100644 .gitignore create mode 100755 it/run-integration-tests.sh create mode 100644 pom.xml create mode 100644 src/main/java/com/jardoapps/pao/ApplyMojo.java create mode 100644 src/main/java/com/jardoapps/pao/BranchDetector.java create mode 100644 src/main/java/com/jardoapps/pao/BranchVersions.java create mode 100644 src/main/java/com/jardoapps/pao/GlobMatcher.java create mode 100644 src/main/java/com/jardoapps/pao/PaoException.java create mode 100644 src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java create mode 100644 src/main/java/com/jardoapps/pao/ProjectModel.java create mode 100644 src/main/java/com/jardoapps/pao/RunnerSettings.java create mode 100644 src/main/java/com/jardoapps/pao/config/PinConfig.java create mode 100644 src/main/java/com/jardoapps/pao/config/PinConfigParser.java create mode 100644 src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java create mode 100644 src/main/java/com/jardoapps/pao/git/GitClient.java create mode 100644 src/main/java/com/jardoapps/pao/pom/PomDocument.java create mode 100644 src/main/java/com/jardoapps/pao/pom/PomEditSession.java create mode 100644 src/main/java/com/jardoapps/pao/pom/PomReader.java create mode 100644 src/main/java/com/jardoapps/pao/pom/XmlElement.java create mode 100644 src/test/java/com/jardoapps/pao/BranchVersionsTest.java create mode 100644 src/test/java/com/jardoapps/pao/GlobMatcherTest.java create mode 100644 src/test/java/com/jardoapps/pao/MultiModuleTest.java create mode 100644 src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java create mode 100644 src/test/java/com/jardoapps/pao/PropertyVersionTest.java create mode 100644 src/test/java/com/jardoapps/pao/RunnerTestSupport.java create mode 100644 src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java create mode 100644 src/test/java/com/jardoapps/pao/git/FakeGitClient.java create mode 100644 src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java create mode 100644 src/test/resources/poms/expected-config-exclusive-suffix-rederive.xml create mode 100644 src/test/resources/poms/expected-config-pin-dependency-versions.xml create mode 100644 src/test/resources/poms/expected-config-pin-project-version.xml create mode 100644 src/test/resources/poms/expected-enforce-branch-version.xml create mode 100644 src/test/resources/poms/expected-remove-branch-version.xml create mode 100644 src/test/resources/poms/expected-remove-dependency-branch-versions.xml create mode 100644 src/test/resources/poms/expected-revision-enforced.xml create mode 100644 src/test/resources/poms/multimodule-expected/core/pom.xml create mode 100644 src/test/resources/poms/multimodule-expected/pom.xml create mode 100644 src/test/resources/poms/multimodule/core/pom.xml create mode 100644 src/test/resources/poms/multimodule/pom.xml create mode 100644 src/test/resources/poms/sample-pom-revision.xml create mode 100644 src/test/resources/poms/sample-pom-two-deps.xml create mode 100644 src/test/resources/poms/sample-pom-with-branch-deps.xml create mode 100644 src/test/resources/poms/sample-pom-with-branch-version.xml create mode 100644 src/test/resources/poms/sample-pom.xml diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..8fa4946 --- /dev/null +++ b/.gitignore @@ -0,0 +1,6 @@ +target/ +*.iml +.idea/ +.classpath +.project +.settings/ diff --git a/README.md b/README.md index b6121c7..76fd2ef 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,197 @@ # pao-maven-plugin -Maven plugin for preventing artifact version overwrites from different branches + +Maven plugin that prevents SNAPSHOT artifacts from different Git branches from overwriting each other in a Maven repository. + +When several branches of a library publish `1.1.0-SNAPSHOT`, whichever pipeline finished last wins, and downstream builds start failing in ways that look random. The fix is to give each branch its own version — `1.1.0-feature-FEA-123-SNAPSHOT` — and to strip that suffix again when the branch is merged. This plugin does both, automatically, in CI. + +This is a Maven-native port of the [prevent-artifact-overwrites](https://github.com/maven-flow/prevent-artifact-overwrites) CI script. + +## How it works + +The `apply` goal looks at the branch being built: + +- **On a feature branch** it appends the branch name to the project version, with slashes replaced by hyphens: `1.1.0-SNAPSHOT` becomes `1.1.0-feature-FEA-123-SNAPSHOT`. +- **On a core branch** (`main`, `master`, `develop`, `release*` by default) it strips the suffix back off — from the project version and from any dependency versions that carry one. You can merge a feature branch without hand-editing versions first. + +The change is written to `pom.xml`, committed, and pushed. Because the version lives in the POM rather than being computed at build time, IDEs and plain `mvn` on a developer machine see exactly what CI sees, with no local setup. + +## Usage + +Run the goal as its own invocation, **before** the build that publishes the artifacts: + +```bash +mvn -B com.jardoapps:pao-maven-plugin:0.1.0:apply +mvn -B deploy +``` + +Two invocations are required, not a stylistic choice: Maven reads and interpolates every POM before any mojo runs, so a version written during a build does not change what that same build deploys. + +No POM changes are needed — the fully-qualified form above works on any project. + +### GitHub Actions + +```yaml +name: Java CI with Maven + +on: push + +jobs: + build: + runs-on: ubuntu-latest + permissions: + contents: write # needed to push the version commit + + steps: + - uses: actions/checkout@v4 + with: + token: ${{ github.token }} # needed to push the version commit + + - uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + cache: maven + + - name: Prevent artifact overwrites + run: > + mvn -B com.jardoapps:pao-maven-plugin:0.1.0:apply + -Dpao.enforceBranchVersion=true + -Dpao.commitMessageSuffix='[skip ci]' + + - name: Build + run: mvn -B deploy +``` + +The goal appends `changes-made=true|false` to `$GITHUB_OUTPUT`, so a later step can react to whether anything was rewritten. + +### GitLab CI/CD + +```yaml +prevent-overwrites: + stage: prepare + image: maven:3.9-eclipse-temurin-17 + script: + - mvn -B com.jardoapps:pao-maven-plugin:0.1.0:apply -Dpao.outputFile=build.env + artifacts: + reports: + dotenv: build.env +``` + +### Declaring it in the POM + +Optional. Putting the plugin in `` keeps the configuration with the project instead of in the CI file: + +```xml + + com.jardoapps + pao-maven-plugin + 0.1.0 + + true + main master develop release* + + +``` + +The CI call stays fully qualified. The short `mvn pao:apply` form additionally needs `com.jardoapps` in `` in `settings.xml`, which is per-developer setup and best avoided. + +## Libraries vs applications + +- **Libraries** — the project whose version must change. Set `enforceBranchVersion` to `true` (the default). +- **Applications** — a project that consumes branch-versioned libraries. Set `enforceBranchVersion` to `false`. The project keeps its own version, but branch-specific dependency versions are still reset on core branches, so merging does not carry a feature branch's dependency versions into `develop`. + +## Configuration + +| Parameter | Property | Default | Description | +|---|---|---|---| +| `branchName` | `pao.branchName` | auto-detected | The branch being built. | +| `enforceBranchVersion` | `pao.enforceBranchVersion` | `true` | Whether the project itself gets a branch-specific version. | +| `pushChanges` | `pao.pushChanges` | `true` | Whether to push the resulting commits to `origin`. | +| `commitMessageSuffix` | `pao.commitMessageSuffix` | *(empty)* | Appended to every commit message, e.g. `[skip ci]`. | +| `gitUserName` | `pao.gitUserName` | `ci-bot` | Git user name for the commits. | +| `gitUserEmail` | `pao.gitUserEmail` | `ci-bot@example.com` | Git email for the commits. | +| `coreBranches` | `pao.coreBranches` | `main master develop release*` | Branch patterns that keep the plain version. Globs allowed; space- or comma-separated. | +| `configFile` | `pao.configFile` | `.prevent-overwrites.conf` | Optional per-branch pinning file, relative to the top-level project. | +| `outputFile` | `pao.outputFile` | *(none)* | File to append `changes-made=` to. | +| `skip` | `pao.skip` | `false` | Skips execution entirely. | + +The branch name is taken from `branchName` if set, otherwise from the first of `GITHUB_REF_NAME`, `CI_COMMIT_REF_NAME`, `BITBUCKET_BRANCH`, `CIRCLE_BRANCH` or `TRAVIS_BRANCH` that is present, otherwise from `git rev-parse --abbrev-ref HEAD`. + +## Version format + +A branch version is `--SNAPSHOT`, where the base is a numeric version with an optional `-rc` qualifier: + +| Version | Base | Branch suffix | +|---|---|---| +| `1.2.3-feature-abc-SNAPSHOT` | `1.2.3` | `feature-abc` | +| `1.2.10-feature-abc-SNAPSHOT` | `1.2.10` | `feature-abc` | +| `1.2.3-rc.4-feature-abc-SNAPSHOT` | `1.2.3-rc.4` | `feature-abc` | +| `1.2.3-SNAPSHOT` | — | *(not a branch version)* | +| `1.2.3-rc.4-SNAPSHOT` | — | *(not a branch version)* | + +## Multi-module projects + +The whole reactor is handled in one run. When the project version changes, `` in every module that points at a reactor project moves with it — otherwise the modules would reference a parent version that no longer exists and the build would stop resolving. + +## Versions defined by properties + +A version written as `${some.version}` is followed to the `` entry that defines it, anywhere in the reactor, and the property is updated instead of the reference. This covers the CI-friendly `${revision}` style: + +```xml +${revision} + + 1.2.3-SNAPSHOT + +``` + +If the property is not defined anywhere in the reactor, the reference is left alone and a warning is logged. + +## Custom per-branch version pinning + +By default the branch version is derived from the branch name. To pin explicit values for specific branches, add a configuration file (default `.prevent-overwrites.conf`). If the file is absent, or has no row matching the current branch, behaviour is unchanged. + +``` +# branch-pattern target value +feature/f1 project-version 1.2.3-f1-SNAPSHOT +feature/f1 dependency:com.example:d1 2.0.0-f1-SNAPSHOT +feature/f2 project-version 1.2.3-f2-SNAPSHOT +* exclusive-version-suffix feature-abc +``` + +- **`branch-pattern`** — glob-matched against the branch name, so `feature/*` works. As in bash, `*` already spans slashes. +- **`target`** — `project-version`, `dependency::`, or `exclusive-version-suffix`. +- **`value`** — the version to pin to, or for `exclusive-version-suffix` the suffix to protect. + +Blank lines are ignored, and everything from a `#` to the end of a line is a comment. + +### Rules + +- **Pinned values must be branch versions** (`1.2.3-f1-SNAPSHOT`, not `vf1`). That is what lets them be reverted to `-SNAPSHOT` on a core branch. An invalid value fails the build — including on rows for other branches, so typos surface on the first run rather than whenever that branch is next built. +- **`project-version`** pins apply only when `enforceBranchVersion` is `true`. A pin wins even over a version that already carries a branch suffix. +- **`dependency:*`** pins apply on non-core branches regardless of `enforceBranchVersion`, so applications can pin what they build against. A pin matches every `` with those coordinates, including entries under `` and inside plugin ``. + +### Exclusive version suffixes + +When a POM already carries a branch version it is normally left alone. That is a problem for long-lived feature branches: branching off `feature/abc` (whose POM says `1.2.3-feature-abc-SNAPSHOT`) means inheriting that version and publishing over `feature/abc`'s artifacts. + +Marking a suffix exclusive says it belongs to one branch: + +``` +# branch-pattern target value +* exclusive-version-suffix feature-abc +``` + +On any other branch the version is then re-derived instead of inherited. On the owning branch it is left untouched, and re-runs change nothing. The value is a version *suffix*, not a branch name — slashes are already replaced with hyphens (`feature/abc` → `feature-abc`). + +## Building and testing + +```bash +mvn test # unit tests +bash it/run-integration-tests.sh # integration tests (real Maven invocations and git repositories) +``` + +The integration tests install the plugin into the local repository first. + +## License + +MIT — see [LICENSE](LICENSE). diff --git a/it/run-integration-tests.sh b/it/run-integration-tests.sh new file mode 100755 index 0000000..d93b7e5 --- /dev/null +++ b/it/run-integration-tests.sh @@ -0,0 +1,207 @@ +#!/usr/bin/env bash +set -euo pipefail + +# ============================================================================ +# Integration tests for pao-maven-plugin. +# +# The unit tests cover the version logic; these cover the parts only a real +# Maven invocation exercises: parameter binding, reactor collection from the +# session, and the git operations against an actual repository. +# +# Usage: bash it/run-integration-tests.sh +# ============================================================================ + +PLUGIN_DIR="$(cd "$(dirname "$0")/.." && pwd)" +PLUGIN_VERSION=$(cd "$PLUGIN_DIR" && mvn -q -B help:evaluate -Dexpression=project.version -DforceStdout) +GOAL="com.jardoapps:pao-maven-plugin:${PLUGIN_VERSION}:apply" + +PASSED=0 +FAILED=0 + +log() { + echo "[IT] $*" +} + +fail() { + echo "[IT] FAILED: $*" >&2 + FAILED=$((FAILED + 1)) +} + +assert_version() { + local file="$1" expected="$2" description="$3" + local actual + actual=$(grep -o '[^<]*' "$file" | head -1 | sed 's|||g') + if [[ "$actual" == "$expected" ]]; then + log " ok: $description" + else + fail "$description (expected '$expected', got '$actual')" + fi +} + +# Creates a two-module project in a fresh git repository. +setup_repo() { + local dir="$1" version="$2" + mkdir -p "$dir/core" + cat > "$dir/pom.xml" < + + 4.0.0 + com.example + my-parent + ${version} + pom + + core + + +EOF + cat > "$dir/core/pom.xml" < + + 4.0.0 + + com.example + my-parent + ${version} + + core + +EOF + git -C "$dir" init -q . + git -C "$dir" config user.email "it@example.com" + git -C "$dir" config user.name "it" + git -C "$dir" add -A + git -C "$dir" commit -qm "Initial commit" +} + +run_goal() { + local dir="$1" + shift + (cd "$dir" && mvn -B -q "$GOAL" -Dpao.pushChanges=false "$@") +} + +# --- Test: the whole feature-branch / core-branch round trip ---------------- + +test_round_trip() { + log "round trip across a multi-module reactor" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.10-SNAPSHOT" + + run_goal "$dir" -Dpao.branchName=feature/FEA-123 + assert_version "$dir/pom.xml" "1.2.10-feature-FEA-123-SNAPSHOT" "aggregator gained the branch version" + assert_version "$dir/core/pom.xml" "1.2.10-feature-FEA-123-SNAPSHOT" "module parent reference followed" + + # The reactor must still resolve, which is what breaks if parent references + # are left behind. + if (cd "$dir" && mvn -B -q validate > /dev/null 2>&1); then + log " ok: reactor still resolves" + else + fail "reactor does not resolve after enforcing the branch version" + fi + + run_goal "$dir" -Dpao.branchName=main + assert_version "$dir/pom.xml" "1.2.10-SNAPSHOT" "aggregator version restored on a core branch" + assert_version "$dir/core/pom.xml" "1.2.10-SNAPSHOT" "module parent reference restored" + + local commits + commits=$(git -C "$dir" log --oneline | wc -l | tr -d ' ') + if [[ "$commits" == "3" ]]; then + log " ok: one commit per change" + else + fail "expected 3 commits, found $commits" + fi + + PASSED=$((PASSED + 1)) +} + +# --- Test: a second run on a core branch changes nothing -------------------- + +test_idempotent() { + log "re-running on a core branch makes no further commits" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.3-SNAPSHOT" + run_goal "$dir" -Dpao.branchName=main + + local commits + commits=$(git -C "$dir" log --oneline | wc -l | tr -d ' ') + if [[ "$commits" == "1" ]]; then + log " ok: nothing committed" + else + fail "expected no new commit, found $((commits - 1))" + fi + + PASSED=$((PASSED + 1)) +} + +# --- Test: the branch name is taken from the CI environment ----------------- + +test_branch_from_environment() { + log "branch name detected from the CI environment" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.3-SNAPSHOT" + (cd "$dir" && GITHUB_REF_NAME=feature/from-env mvn -B -q "$GOAL" -Dpao.pushChanges=false) + assert_version "$dir/pom.xml" "1.2.3-feature-from-env-SNAPSHOT" "branch read from GITHUB_REF_NAME" + + PASSED=$((PASSED + 1)) +} + +# --- Test: an invalid pin fails the build ----------------------------------- + +test_invalid_pin_fails() { + log "an invalid pin fails the build" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.3-SNAPSHOT" + echo "feature/f1 project-version vf1" > "$dir/.prevent-overwrites.conf" + + if run_goal "$dir" -Dpao.branchName=feature/f1 > /dev/null 2>&1; then + fail "expected a non-zero exit for an invalid pin" + else + log " ok: build failed as expected" + fi + + PASSED=$((PASSED + 1)) +} + +# --- Test: pao.skip short-circuits ------------------------------------------ + +test_skip() { + log "pao.skip leaves the project alone" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.3-SNAPSHOT" + run_goal "$dir" -Dpao.branchName=feature/FEA-123 -Dpao.skip=true + assert_version "$dir/pom.xml" "1.2.3-SNAPSHOT" "version untouched" + + PASSED=$((PASSED + 1)) +} + +log "Using goal: $GOAL" +log "Installing the plugin into the local repository..." +(cd "$PLUGIN_DIR" && mvn -q -B install -DskipTests) + +test_round_trip +test_idempotent +test_branch_from_environment +test_invalid_pin_fails +test_skip + +echo "" +echo "================================================================" +echo "Integration tests: $PASSED passed, $FAILED failed" +echo "================================================================" + +[[ "$FAILED" -eq 0 ]] diff --git a/pom.xml b/pom.xml new file mode 100644 index 0000000..677d03e --- /dev/null +++ b/pom.xml @@ -0,0 +1,77 @@ + + + + 4.0.0 + + com.jardoapps + pao-maven-plugin + 0.1.0 + maven-plugin + + Prevent Artifact Overwrites Maven Plugin + Prevents Maven SNAPSHOT artifacts from different Git branches from overwriting each other by managing branch-specific version suffixes in pom.xml files. + + + + MIT License + https://opensource.org/licenses/MIT + + + + + 17 + UTF-8 + 3.9.6 + 3.13.1 + 5.10.2 + + + + + org.apache.maven + maven-plugin-api + ${maven.version} + provided + + + org.apache.maven + maven-core + ${maven.version} + provided + + + org.apache.maven.plugin-tools + maven-plugin-annotations + ${maven-plugin-tools.version} + provided + + + + org.junit.jupiter + junit-jupiter + ${junit.version} + test + + + + + + + org.apache.maven.plugins + maven-plugin-plugin + ${maven-plugin-tools.version} + + pao + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + + + + + diff --git a/src/main/java/com/jardoapps/pao/ApplyMojo.java b/src/main/java/com/jardoapps/pao/ApplyMojo.java new file mode 100644 index 0000000..df328be --- /dev/null +++ b/src/main/java/com/jardoapps/pao/ApplyMojo.java @@ -0,0 +1,145 @@ +package com.jardoapps.pao; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import org.apache.maven.execution.MavenSession; +import org.apache.maven.model.Parent; +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.MojoFailureException; +import org.apache.maven.plugins.annotations.LifecyclePhase; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.project.MavenProject; + +import com.jardoapps.pao.git.CommandLineGitClient; +import com.jardoapps.pao.git.GitClient; + +/** + * Applies branch-specific versions to the reactor's pom.xml files, commits the + * result and optionally pushes it. + * + *

On a feature branch the project version gains a suffix derived from the branch + * name ({@code 1.1.0-SNAPSHOT} becomes {@code 1.1.0-feature-foo-SNAPSHOT}); on a core + * branch that suffix is stripped again, from the project version and from any + * dependency versions that carry one. + * + *

Run this as its own invocation, before the build that publishes the artifacts: + * Maven has already read the POMs by the time any mojo executes, so a version written + * during a build does not change what that same build deploys. + */ +@Mojo(name = "apply", + defaultPhase = LifecyclePhase.VALIDATE, + aggregator = true, + requiresProject = true, + threadSafe = true) +public class ApplyMojo extends AbstractMojo { + + @Parameter(defaultValue = "${session}", readonly = true, required = true) + private MavenSession session; + + /** The branch being built. Auto-detected from the CI environment or git when unset. */ + @Parameter(property = "pao.branchName") + private String branchName; + + /** + * Whether to give the project itself a branch-specific version. Set this to true + * for libraries, and false for applications that only need their dependency + * versions reset on core branches. + */ + @Parameter(property = "pao.enforceBranchVersion", defaultValue = "true") + private boolean enforceBranchVersion; + + /** Whether to push the resulting commits to {@code origin}. */ + @Parameter(property = "pao.pushChanges", defaultValue = "true") + private boolean pushChanges; + + /** Appended to every commit message, e.g. {@code [skip ci]}. */ + @Parameter(property = "pao.commitMessageSuffix", defaultValue = "") + private String commitMessageSuffix; + + @Parameter(property = "pao.gitUserName", defaultValue = "ci-bot") + private String gitUserName; + + @Parameter(property = "pao.gitUserEmail", defaultValue = "ci-bot@example.com") + private String gitUserEmail; + + /** + * Branch name patterns that must keep the plain version. Glob patterns are + * supported, and the list may be separated by spaces or commas. + */ + @Parameter(property = "pao.coreBranches", defaultValue = "main master develop release*") + private String coreBranches; + + /** Optional per-branch version pinning file, relative to the top-level project. */ + @Parameter(property = "pao.configFile", defaultValue = ".prevent-overwrites.conf") + private String configFile; + + /** Optional file to append {@code changes-made=} to, for CI to pick up. */ + @Parameter(property = "pao.outputFile") + private String outputFile; + + /** Skips execution entirely. */ + @Parameter(property = "pao.skip", defaultValue = "false") + private boolean skip; + + @Override + public void execute() throws MojoExecutionException, MojoFailureException { + if (skip) { + getLog().info("Skipping (pao.skip=true)."); + return; + } + + MavenProject topLevelProject = session.getTopLevelProject(); + Path baseDirectory = topLevelProject.getBasedir().toPath(); + + RunnerSettings settings = new RunnerSettings() + .setBranchName(branchName) + .setEnforceBranchVersion(enforceBranchVersion) + .setPushChanges(pushChanges) + .setCommitMessageSuffix(commitMessageSuffix) + .setGitUserName(gitUserName) + .setGitUserEmail(gitUserEmail) + .setCoreBranches(coreBranches) + .setConfigFile(configFile == null ? null : Path.of(configFile)) + .setOutputFile(outputFile == null || outputFile.isBlank() ? null : Path.of(outputFile)); + + GitClient git = new CommandLineGitClient(baseDirectory, getLog()); + PreventOverwritesRunner runner = + new PreventOverwritesRunner(settings, git, baseDirectory, System::getenv, getLog()); + + try { + runner.run(collectReactor(topLevelProject)); + } catch (PaoException e) { + throw new MojoFailureException(e.getMessage(), e); + } catch (RuntimeException e) { + throw new MojoExecutionException("Failed to apply branch-specific versions: " + e.getMessage(), e); + } + } + + /** The reactor as plain coordinates, with the top-level project first. */ + private List collectReactor(MavenProject topLevelProject) { + List reactor = new ArrayList<>(); + reactor.add(toModel(topLevelProject)); + for (MavenProject project : session.getAllProjects()) { + if (project != topLevelProject) { + reactor.add(toModel(project)); + } + } + return reactor; + } + + private ProjectModel toModel(MavenProject project) { + Parent parent = project.getModel().getParent(); + return new ProjectModel( + project.getFile().toPath(), + project.getGroupId(), + project.getArtifactId(), + project.getVersion(), + parent == null ? null : parent.getGroupId(), + parent == null ? null : parent.getArtifactId(), + parent == null ? null : parent.getVersion()); + } +} diff --git a/src/main/java/com/jardoapps/pao/BranchDetector.java b/src/main/java/com/jardoapps/pao/BranchDetector.java new file mode 100644 index 0000000..5ae8798 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/BranchDetector.java @@ -0,0 +1,56 @@ +package com.jardoapps.pao; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.function.UnaryOperator; + +import org.apache.maven.plugin.logging.Log; + +import com.jardoapps.pao.git.GitClient; + +/** Works out which branch the build is running on. */ +public final class BranchDetector { + + /** CI environment variables that carry the branch name, in priority order. */ + private static final Map CI_VARIABLES = new LinkedHashMap<>(); + + static { + CI_VARIABLES.put("GITHUB_REF_NAME", "GitHub Actions"); + CI_VARIABLES.put("CI_COMMIT_REF_NAME", "GitLab CI/CD"); + CI_VARIABLES.put("BITBUCKET_BRANCH", "Bitbucket Pipelines"); + CI_VARIABLES.put("CIRCLE_BRANCH", "CircleCI"); + CI_VARIABLES.put("TRAVIS_BRANCH", "Travis CI"); + } + + private BranchDetector() { + } + + /** + * Returns the configured branch name, else the first CI variable that is set, + * else the branch git reports. + * + * @throws PaoException if the branch cannot be determined + */ + public static String detect(String configured, UnaryOperator environment, GitClient git, Log log) { + if (configured != null && !configured.isBlank()) { + log.info("Branch name provided: " + configured); + return configured; + } + + for (Map.Entry variable : CI_VARIABLES.entrySet()) { + String value = environment.apply(variable.getKey()); + if (value != null && !value.isBlank()) { + log.info("Detected " + variable.getValue() + ", branch: " + value); + return value; + } + } + + return git.currentBranch() + .map(branch -> { + log.info("Detected local git, branch: " + branch); + return branch; + }) + .orElseThrow(() -> new PaoException( + "Could not detect the branch name. Set it with -Dpao.branchName=.")); + } +} diff --git a/src/main/java/com/jardoapps/pao/BranchVersions.java b/src/main/java/com/jardoapps/pao/BranchVersions.java new file mode 100644 index 0000000..17680ab --- /dev/null +++ b/src/main/java/com/jardoapps/pao/BranchVersions.java @@ -0,0 +1,96 @@ +package com.jardoapps.pao; + +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Rules for deriving, recognising and stripping branch-specific versions. + * + *

A branch version is {@code --SNAPSHOT}, for example + * {@code 1.2.3-feature-abc-SNAPSHOT} or {@code 1.2.3-rc.4-feature-abc-SNAPSHOT}. + * The base may carry any number of numeric segments and an optional {@code -rc} + * qualifier; everything between the base and {@code -SNAPSHOT} is the suffix. + */ +public final class BranchVersions { + + private static final String SNAPSHOT = "-SNAPSHOT"; + + /** + * The numeric part plus an optional {@code -rc} qualifier. + * + *

Both quantifiers are possessive so the base can never be re-read as + * something shorter: without that, {@code 1.2.3-rc.4-SNAPSHOT} would match with a + * base of {@code 1.2.3} and a "branch suffix" of {@code rc.4}, and stripping it + * would silently drop the release candidate qualifier. + */ + private static final String BASE = "\\d++(?:\\.\\d++)++(?:-rc(?:\\.\\d++)?)?+"; + + private static final Pattern BRANCH_VERSION = Pattern.compile("^(" + BASE + ")-(.+)-SNAPSHOT$"); + + private static final Pattern PLAIN_SNAPSHOT = Pattern.compile("^(" + BASE + ")-SNAPSHOT$"); + + /** A {@code ${...}} property reference used in place of a literal version. */ + private static final Pattern PROPERTY_REFERENCE = Pattern.compile("^\\$\\{([^}]+)}$"); + + private BranchVersions() { + } + + public static boolean isBranchVersion(String version) { + return version != null && BRANCH_VERSION.matcher(version).matches(); + } + + /** The base of a branch version, e.g. {@code 1.2.3-rc.4-feature-abc-SNAPSHOT} -> {@code 1.2.3-rc.4}. */ + public static Optional baseOf(String version) { + Matcher matcher = BRANCH_VERSION.matcher(version); + return matcher.matches() ? Optional.of(matcher.group(1)) : Optional.empty(); + } + + /** The suffix of a branch version, e.g. {@code 1.2.3-feature-abc-SNAPSHOT} -> {@code feature-abc}. */ + public static Optional suffixOf(String version) { + Matcher matcher = BRANCH_VERSION.matcher(version); + return matcher.matches() ? Optional.of(matcher.group(2)) : Optional.empty(); + } + + /** Strips the branch suffix, e.g. {@code 1.2.3-feature-abc-SNAPSHOT} -> {@code 1.2.3-SNAPSHOT}. */ + public static Optional withoutBranch(String version) { + return baseOf(version).map(base -> base + "-SNAPSHOT"); + } + + /** + * Builds the branch version for the given suffix. An existing branch suffix is + * replaced, otherwise the suffix is inserted before {@code -SNAPSHOT}. + */ + public static String withBranch(String version, String branchSuffix) { + Optional base = baseOf(version); + if (base.isPresent()) { + return base.get() + "-" + branchSuffix + "-SNAPSHOT"; + } + Matcher plain = PLAIN_SNAPSHOT.matcher(version); + if (plain.matches()) { + return plain.group(1) + "-" + branchSuffix + "-SNAPSHOT"; + } + String stripped = version.endsWith(SNAPSHOT) ? version.substring(0, version.length() - SNAPSHOT.length()) + : version; + return stripped + "-" + branchSuffix + SNAPSHOT; + } + + /** Turns a branch name into a version suffix: {@code feature/abc} -> {@code feature-abc}. */ + public static String branchSuffix(String branchName) { + return branchName.replace('/', '-'); + } + + /** + * A pinned version must itself be a branch version, otherwise it could not be + * reverted to {@code -SNAPSHOT} when the branch is merged to a core branch. + */ + public static boolean isValidPin(String version) { + return isBranchVersion(version); + } + + /** The property name referenced by {@code ${name}}, if the value is exactly such a reference. */ + public static Optional propertyReference(String value) { + Matcher matcher = PROPERTY_REFERENCE.matcher(value); + return matcher.matches() ? Optional.of(matcher.group(1)) : Optional.empty(); + } +} diff --git a/src/main/java/com/jardoapps/pao/GlobMatcher.java b/src/main/java/com/jardoapps/pao/GlobMatcher.java new file mode 100644 index 0000000..e6782e7 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/GlobMatcher.java @@ -0,0 +1,51 @@ +package com.jardoapps.pao; + +import java.util.regex.Pattern; + +/** + * Glob matching for branch patterns, mirroring bash {@code [[ $branch == $pattern ]]}: + * {@code *} matches any run of characters including {@code /}, {@code ?} matches a + * single character, and {@code [...]} is a character class. + */ +public final class GlobMatcher { + + private GlobMatcher() { + } + + public static boolean matches(String pattern, String value) { + return toRegex(pattern).matcher(value).matches(); + } + + static Pattern toRegex(String glob) { + StringBuilder regex = new StringBuilder(glob.length() + 16); + int i = 0; + while (i < glob.length()) { + char c = glob.charAt(i); + switch (c) { + case '*' -> regex.append(".*"); + case '?' -> regex.append('.'); + case '[' -> { + int close = glob.indexOf(']', i + 1); + if (close < 0) { + regex.append("\\["); + } else { + String body = glob.substring(i + 1, close); + if (body.startsWith("!")) { + body = "^" + body.substring(1); + } + regex.append('[').append(body).append(']'); + i = close; + } + } + default -> { + if ("\\.^$+{}|()".indexOf(c) >= 0) { + regex.append('\\'); + } + regex.append(c); + } + } + i++; + } + return Pattern.compile(regex.toString(), Pattern.DOTALL); + } +} diff --git a/src/main/java/com/jardoapps/pao/PaoException.java b/src/main/java/com/jardoapps/pao/PaoException.java new file mode 100644 index 0000000..3eba813 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/PaoException.java @@ -0,0 +1,15 @@ +package com.jardoapps.pao; + +/** Signals a configuration or repository problem that should fail the build. */ +public class PaoException extends RuntimeException { + + private static final long serialVersionUID = 1L; + + public PaoException(String message) { + super(message); + } + + public PaoException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java new file mode 100644 index 0000000..1b1faf8 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java @@ -0,0 +1,286 @@ +package com.jardoapps.pao; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.UnaryOperator; + +import org.apache.maven.plugin.logging.Log; + +import com.jardoapps.pao.config.PinConfig; +import com.jardoapps.pao.config.PinConfigParser; +import com.jardoapps.pao.git.GitClient; +import com.jardoapps.pao.pom.PomDocument; +import com.jardoapps.pao.pom.PomEditSession; +import com.jardoapps.pao.pom.XmlElement; + +/** + * The whole behaviour of the plugin, expressed against plain types so it can be + * driven either by the mojo or directly by tests. + */ +public class PreventOverwritesRunner { + + /** What a run did, for reporting and for CI outputs. */ + public record RunResult( + boolean changesMade, + String branchName, + boolean coreBranch, + String projectVersion, + List commitMessages) { + } + + private static final String COMMIT_ENFORCE = "Switched to branch-specific version."; + private static final String COMMIT_PIN_DEPENDENCIES = "Pinned branch-specific dependency versions."; + private static final String COMMIT_REMOVE_VERSION = "Switched to non branch-specific version."; + private static final String COMMIT_REMOVE_DEPENDENCIES = "Switched to non branch dependency versions."; + + private final RunnerSettings settings; + private final GitClient git; + private final Path baseDirectory; + private final UnaryOperator environment; + private final Log log; + + public PreventOverwritesRunner(RunnerSettings settings, GitClient git, Path baseDirectory, + UnaryOperator environment, Log log) { + this.settings = settings; + this.git = git; + this.baseDirectory = baseDirectory; + this.environment = environment; + this.log = log; + } + + /** Runs against a reactor whose first entry is the top-level project. */ + public RunResult run(List reactor) { + if (reactor.isEmpty()) { + throw new PaoException("No Maven projects to process."); + } + + String branchName = BranchDetector.detect(settings.getBranchName(), environment, git, log); + boolean coreBranch = isCoreBranch(branchName); + log.info("Current branch: '" + branchName + "'"); + log.info("Needs branch version: " + !coreBranch); + + ProjectModel root = reactor.get(0); + log.info("Project version: " + root.version()); + if (reactor.size() > 1) { + log.info("Reactor contains " + reactor.size() + " projects."); + } + + PinConfig pins = new PinConfigParser(log).parse(resolveConfigFile(), branchName); + + git.configureUser(settings.getGitUserName(), settings.getGitUserEmail()); + + List commits = new ArrayList<>(); + if (coreBranch) { + removeBranchVersion(reactor, root, commits); + removeDependencyBranchVersions(reactor, commits); + } else { + enforceBranchVersion(reactor, root, branchName, pins, commits); + applyDependencyPins(reactor, pins, commits); + } + + boolean changesMade = !commits.isEmpty(); + log.info(changesMade ? "Changes have been made." : "No changes have been made."); + writeOutput("changes-made", String.valueOf(changesMade)); + + if (changesMade) { + if (settings.isPushChanges()) { + log.info("Pushing changes to branch '" + branchName + "'..."); + git.push(branchName); + } else { + log.info("Push changes disabled. Skipping push."); + } + } + + return new RunResult(changesMade, branchName, coreBranch, root.version(), List.copyOf(commits)); + } + + // --- Feature branches ------------------------------------------------- + + private void enforceBranchVersion(List reactor, ProjectModel root, String branchName, PinConfig pins, + List commits) { + if (!settings.isEnforceBranchVersion()) { + log.info("Project version enforcement is turned off."); + return; + } + + String currentVersion = root.version(); + String branchSuffix = BranchVersions.branchSuffix(branchName); + String newVersion; + + Optional pinned = pins.getProjectVersion(); + if (pinned.isPresent()) { + // An explicit pin always wins, even over an inherited branch suffix. + newVersion = pinned.get(); + if (newVersion.equals(currentVersion)) { + log.info("Project already at pinned version."); + return; + } + log.info("Using pinned project version: " + newVersion); + } else if (BranchVersions.isBranchVersion(currentVersion)) { + String currentSuffix = BranchVersions.suffixOf(currentVersion).orElseThrow(); + if (!currentSuffix.equals(branchSuffix) && pins.isExclusiveSuffix(currentSuffix)) { + // The suffix belongs to another branch, so an inherited version would + // publish under - and overwrite - that branch's artifacts. + newVersion = BranchVersions.withBranch(currentVersion, branchSuffix); + log.info("Suffix '" + currentSuffix + "' is exclusive to another branch. Re-deriving to: " + + newVersion); + } else { + log.info("Project already has a branch version."); + return; + } + } else { + newVersion = BranchVersions.withBranch(currentVersion, branchSuffix); + log.info("Project does not have a branch version. Changing to: " + newVersion); + } + + if (!BranchVersions.isBranchVersion(newVersion)) { + log.warn("Version '" + newVersion + "' does not match '--SNAPSHOT', so it will not be" + + " stripped automatically when this branch is merged into a core branch."); + } + + changeProjectVersion(reactor, currentVersion, newVersion, COMMIT_ENFORCE, commits); + } + + private void applyDependencyPins(List reactor, PinConfig pins, List commits) { + Map pinnedVersions = pins.getDependencyVersions(); + if (pinnedVersions.isEmpty()) { + return; + } + + PomEditSession session = new PomEditSession(reactor, log); + for (PomDocument document : session.documents()) { + for (PomDocument.DependencyEntry dependency : document.dependencies()) { + String pinnedVersion = pinnedVersions.get(dependency.key()); + if (pinnedVersion == null) { + continue; + } + if (session.setVersion(document, dependency.versionElement(), null, pinnedVersion)) { + log.info("Pinning dependency " + dependency.key() + " to " + pinnedVersion + " in " + + document.getPath()); + } + } + } + commit(session, COMMIT_PIN_DEPENDENCIES, commits); + } + + // --- Core branches ---------------------------------------------------- + + private void removeBranchVersion(List reactor, ProjectModel root, List commits) { + String currentVersion = root.version(); + Optional stripped = BranchVersions.withoutBranch(currentVersion); + if (stripped.isEmpty()) { + return; + } + log.info("Project has a branch version. Removing it, since we are on a core branch."); + log.info("New version: " + stripped.get()); + changeProjectVersion(reactor, currentVersion, stripped.get(), COMMIT_REMOVE_VERSION, commits); + } + + private void removeDependencyBranchVersions(List reactor, List commits) { + PomEditSession session = new PomEditSession(reactor, log); + for (PomDocument document : session.documents()) { + for (XmlElement version : document.allVersionElements()) { + stripBranchVersion(document, version, "version", commits); + } + for (Map.Entry property : document.properties().entrySet()) { + stripBranchVersion(document, property.getValue(), "property " + property.getKey(), commits); + } + } + commit(session, COMMIT_REMOVE_DEPENDENCIES, commits); + } + + private void stripBranchVersion(PomDocument document, XmlElement element, String description, + List commits) { + String value = document.valueOf(element); + BranchVersions.withoutBranch(value).ifPresent(stripped -> { + log.info("Replacing " + description + " " + value + " with " + stripped + " in " + document.getPath()); + document.setValue(element, stripped); + }); + } + + // --- Shared ----------------------------------------------------------- + + /** + * Rewrites the project version across the reactor. Modules that inherit the + * version carry it in {@code }, which must move in step or the + * build breaks - so parent references to reactor projects are updated too. + */ + private void changeProjectVersion(List reactor, String oldVersion, String newVersion, String message, + List commits) { + Set reactorKeys = new HashSet<>(); + reactor.forEach(project -> reactorKeys.add(project.key())); + + PomEditSession session = new PomEditSession(reactor, log); + for (ProjectModel project : reactor) { + PomDocument document = session.document(project.pomFile()); + + if (oldVersion.equals(project.version())) { + document.projectVersion() + .ifPresent(element -> session.setVersion(document, element, oldVersion, newVersion)); + } + + if (project.hasParent() && oldVersion.equals(project.parentVersion()) + && reactorKeys.contains(project.parentKey())) { + document.parentVersion() + .ifPresent(element -> session.setVersion(document, element, oldVersion, newVersion)); + } + } + commit(session, message, commits); + } + + private void commit(PomEditSession session, String message, List commits) { + List written = session.save(); + if (written.isEmpty()) { + log.debug("No pom file required a change for: " + message); + return; + } + written.forEach(path -> log.info("Updated " + path)); + + String fullMessage = message + settings.getCommitMessageSuffix(); + git.commitAll(fullMessage); + commits.add(fullMessage); + } + + private boolean isCoreBranch(String branchName) { + return settings.getCoreBranches().stream().anyMatch(pattern -> GlobMatcher.matches(pattern, branchName)); + } + + private Path resolveConfigFile() { + Path configured = settings.getConfigFile(); + if (configured == null) { + configured = Path.of(".prevent-overwrites.conf"); + } + return configured.isAbsolute() ? configured : baseDirectory.resolve(configured); + } + + /** Mirrors the outputs the shell version exposed to GitHub Actions and GitLab. */ + private void writeOutput(String name, String value) { + log.info("Output: " + name + "=" + value); + appendOutput(environment.apply("GITHUB_OUTPUT"), name, value); + if (settings.getOutputFile() != null) { + appendOutput(settings.getOutputFile().toString(), name, value); + } + } + + private void appendOutput(String file, String name, String value) { + if (file == null || file.isBlank()) { + return; + } + try { + Files.writeString(Path.of(file), name + "=" + value + System.lineSeparator(), StandardCharsets.UTF_8, + StandardOpenOption.CREATE, StandardOpenOption.APPEND); + } catch (IOException e) { + throw new UncheckedIOException("Cannot write output file " + file, e); + } + } +} diff --git a/src/main/java/com/jardoapps/pao/ProjectModel.java b/src/main/java/com/jardoapps/pao/ProjectModel.java new file mode 100644 index 0000000..dbd51a0 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/ProjectModel.java @@ -0,0 +1,26 @@ +package com.jardoapps.pao; + +import java.nio.file.Path; + +/** The coordinates of one reactor project, together with the pom.xml they came from. */ +public record ProjectModel( + Path pomFile, + String groupId, + String artifactId, + String version, + String parentGroupId, + String parentArtifactId, + String parentVersion) { + + public String key() { + return groupId + ":" + artifactId; + } + + public boolean hasParent() { + return parentArtifactId != null; + } + + public String parentKey() { + return hasParent() ? parentGroupId + ":" + parentArtifactId : null; + } +} diff --git a/src/main/java/com/jardoapps/pao/RunnerSettings.java b/src/main/java/com/jardoapps/pao/RunnerSettings.java new file mode 100644 index 0000000..944306c --- /dev/null +++ b/src/main/java/com/jardoapps/pao/RunnerSettings.java @@ -0,0 +1,108 @@ +package com.jardoapps.pao; + +import java.nio.file.Path; +import java.util.Arrays; +import java.util.List; + +/** Everything the runner needs to know, independent of how it was configured. */ +public class RunnerSettings { + + private String branchName; + private boolean enforceBranchVersion = true; + private boolean pushChanges = true; + private String commitMessageSuffix = ""; + private String gitUserName = "ci-bot"; + private String gitUserEmail = "ci-bot@example.com"; + private List coreBranches = List.of("main", "master", "develop", "release*"); + private Path configFile; + private Path outputFile; + + public String getBranchName() { + return branchName; + } + + public RunnerSettings setBranchName(String branchName) { + this.branchName = branchName; + return this; + } + + public boolean isEnforceBranchVersion() { + return enforceBranchVersion; + } + + public RunnerSettings setEnforceBranchVersion(boolean enforceBranchVersion) { + this.enforceBranchVersion = enforceBranchVersion; + return this; + } + + public boolean isPushChanges() { + return pushChanges; + } + + public RunnerSettings setPushChanges(boolean pushChanges) { + this.pushChanges = pushChanges; + return this; + } + + public String getCommitMessageSuffix() { + return commitMessageSuffix == null ? "" : commitMessageSuffix; + } + + public RunnerSettings setCommitMessageSuffix(String commitMessageSuffix) { + this.commitMessageSuffix = commitMessageSuffix; + return this; + } + + public String getGitUserName() { + return gitUserName; + } + + public RunnerSettings setGitUserName(String gitUserName) { + this.gitUserName = gitUserName; + return this; + } + + public String getGitUserEmail() { + return gitUserEmail; + } + + public RunnerSettings setGitUserEmail(String gitUserEmail) { + this.gitUserEmail = gitUserEmail; + return this; + } + + public List getCoreBranches() { + return coreBranches; + } + + public RunnerSettings setCoreBranches(List coreBranches) { + this.coreBranches = coreBranches; + return this; + } + + /** Accepts the space- or comma-separated form used by the CI inputs. */ + public RunnerSettings setCoreBranches(String coreBranches) { + this.coreBranches = coreBranches == null || coreBranches.isBlank() + ? List.of() + : Arrays.stream(coreBranches.split("[,\\s]+")).filter(s -> !s.isBlank()).toList(); + return this; + } + + public Path getConfigFile() { + return configFile; + } + + public RunnerSettings setConfigFile(Path configFile) { + this.configFile = configFile; + return this; + } + + public Path getOutputFile() { + return outputFile; + } + + public RunnerSettings setOutputFile(Path outputFile) { + this.outputFile = outputFile; + return this; + } +} diff --git a/src/main/java/com/jardoapps/pao/config/PinConfig.java b/src/main/java/com/jardoapps/pao/config/PinConfig.java new file mode 100644 index 0000000..f39064d --- /dev/null +++ b/src/main/java/com/jardoapps/pao/config/PinConfig.java @@ -0,0 +1,43 @@ +package com.jardoapps.pao.config; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +/** The entries of the per-branch configuration file that apply to the current branch. */ +public final class PinConfig { + + private final String projectVersion; + private final Map dependencyVersions; + private final Set exclusiveSuffixes; + + public PinConfig(String projectVersion, Map dependencyVersions, Set exclusiveSuffixes) { + this.projectVersion = projectVersion; + this.dependencyVersions = new LinkedHashMap<>(dependencyVersions); + this.exclusiveSuffixes = Set.copyOf(exclusiveSuffixes); + } + + public static PinConfig empty() { + return new PinConfig(null, Map.of(), Set.of()); + } + + /** The pinned project version for this branch, if one was configured. */ + public Optional getProjectVersion() { + return Optional.ofNullable(projectVersion); + } + + /** Pinned dependency versions for this branch, keyed by {@code groupId:artifactId}. */ + public Map getDependencyVersions() { + return Map.copyOf(dependencyVersions); + } + + /** Suffixes declared as belonging to a single branch. */ + public boolean isExclusiveSuffix(String suffix) { + return exclusiveSuffixes.contains(suffix); + } + + public boolean isEmpty() { + return projectVersion == null && dependencyVersions.isEmpty() && exclusiveSuffixes.isEmpty(); + } +} diff --git a/src/main/java/com/jardoapps/pao/config/PinConfigParser.java b/src/main/java/com/jardoapps/pao/config/PinConfigParser.java new file mode 100644 index 0000000..55d3296 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/config/PinConfigParser.java @@ -0,0 +1,148 @@ +package com.jardoapps.pao.config; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.maven.plugin.logging.Log; + +import com.jardoapps.pao.BranchVersions; +import com.jardoapps.pao.GlobMatcher; +import com.jardoapps.pao.PaoException; + +/** + * Reads the optional per-branch pinning file. + * + *

Each non-empty line is three whitespace-separated columns; everything from a + * {@code #} to the end of the line is a comment. + * + * 

+ * <branch-pattern>  project-version                     <pinned-version>
+ * <branch-pattern>  dependency:<groupId>:<artifactId>   <pinned-version>
+ * <branch-pattern>  exclusive-version-suffix            <suffix>
+ *
+ */ +public final class PinConfigParser { + + private static final String PROJECT_VERSION = "project-version"; + private static final String EXCLUSIVE_SUFFIX = "exclusive-version-suffix"; + private static final String DEPENDENCY_PREFIX = "dependency:"; + + private final Log log; + + public PinConfigParser(Log log) { + this.log = log; + } + + /** Parses the file if it exists, keeping only the rows matching {@code branchName}. */ + public PinConfig parse(Path file, String branchName) { + if (file == null || !Files.isRegularFile(file)) { + log.info("No config file at '" + file + "'. Using default behaviour."); + return PinConfig.empty(); + } + + log.info("Loading config from '" + file + "' for branch '" + branchName + "'..."); + + List lines; + try { + lines = Files.readAllLines(file, StandardCharsets.UTF_8); + } catch (IOException e) { + throw new UncheckedIOException("Cannot read " + file, e); + } + + String projectVersion = null; + Map dependencyVersions = new LinkedHashMap<>(); + Set exclusiveSuffixes = new LinkedHashSet<>(); + + for (int i = 0; i < lines.size(); i++) { + String line = stripComment(lines.get(i)); + if (line.isBlank()) { + continue; + } + + String[] columns = line.trim().split("\\s+"); + if (columns.length < 3) { + throw new PaoException(file + ":" + (i + 1) + + ": malformed line (expected 3 columns: ): " + line.trim()); + } + + String pattern = columns[0]; + String target = columns[1]; + String value = columns[2]; + + // Every row is validated, not just the ones for this branch: a typo in + // another branch's row would otherwise stay silent until that branch builds. + String dependencyKey = validateRow(file, i + 1, target, value); + + if (!GlobMatcher.matches(pattern, branchName)) { + continue; + } + + if (PROJECT_VERSION.equals(target)) { + if (projectVersion == null) { + projectVersion = value; + log.info("Pin: project-version -> " + value); + } else { + log.warn("Multiple project-version pins match branch '" + branchName + "' in " + file + + "; keeping '" + projectVersion + "' and ignoring '" + value + "'."); + } + } else if (EXCLUSIVE_SUFFIX.equals(target)) { + exclusiveSuffixes.add(value); + log.info("Exclusive version suffix: " + value); + } else { + String previous = dependencyVersions.put(dependencyKey, value); + if (previous != null && !previous.equals(value)) { + log.warn("Multiple pins for dependency " + dependencyKey + " match branch '" + branchName + "' in " + + file + "; using '" + value + "'."); + } + log.info("Pin: dependency " + dependencyKey + " -> " + value); + } + } + + return new PinConfig(projectVersion, dependencyVersions, exclusiveSuffixes); + } + + /** + * Checks that a row names a known target and carries a usable value. + * + * @return the {@code groupId:artifactId} for a dependency row, otherwise null + */ + private String validateRow(Path file, int lineNumber, String target, String value) { + if (EXCLUSIVE_SUFFIX.equals(target)) { + return null; + } + + String dependencyKey = null; + if (target.startsWith(DEPENDENCY_PREFIX)) { + dependencyKey = target.substring(DEPENDENCY_PREFIX.length()); + if (dependencyKey.chars().filter(c -> c == ':').count() != 1 || dependencyKey.startsWith(":") + || dependencyKey.endsWith(":")) { + throw new PaoException(file + ":" + lineNumber + ": malformed dependency target '" + target + + "' (expected 'dependency::')."); + } + } else if (!PROJECT_VERSION.equals(target)) { + throw new PaoException(file + ":" + lineNumber + ": unknown target '" + target + + "' (expected 'project-version', 'dependency::' or '" + EXCLUSIVE_SUFFIX + + "')."); + } + + if (!BranchVersions.isValidPin(value)) { + throw new PaoException(file + ":" + lineNumber + ": invalid pinned version '" + value + "' for target '" + + target + "'. Pinned versions must match '--SNAPSHOT' (e.g. 1.2.3-f1-SNAPSHOT)" + + " so they can be reverted to '-SNAPSHOT' on core branches."); + } + return dependencyKey; + } + + private static String stripComment(String line) { + int hash = line.indexOf('#'); + return hash < 0 ? line : line.substring(0, hash); + } +} diff --git a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java new file mode 100644 index 0000000..1356cc3 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java @@ -0,0 +1,115 @@ +package com.jardoapps.pao.git; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +import org.apache.maven.plugin.logging.Log; + +import com.jardoapps.pao.PaoException; + +/** + * Runs the {@code git} executable in the repository working directory. + * + *

Shelling out rather than embedding JGit is deliberate: CI runners already have + * credentials configured for the {@code git} binary (token helpers, SSH agents, + * {@code insteadOf} rewrites), and reproducing that setup in-process is a common + * source of authentication failures. + */ +public class CommandLineGitClient implements GitClient { + + private static final long TIMEOUT_SECONDS = 120; + + private final Path workingDirectory; + private final Log log; + + public CommandLineGitClient(Path workingDirectory, Log log) { + this.workingDirectory = workingDirectory; + this.log = log; + } + + @Override + public void configureUser(String name, String email) { + log.info("Setting up git configuration..."); + run(true, "config", "--local", "user.name", name); + run(true, "config", "--local", "user.email", email); + } + + @Override + public boolean hasUncommittedChanges() { + return !run(true, "status", "--porcelain").output().isBlank(); + } + + @Override + public void commitAll(String message) { + if (!hasUncommittedChanges()) { + log.debug("Nothing to commit."); + return; + } + run(true, "commit", "-a", "-m", message); + } + + @Override + public void push(String branch) { + // HEAD: works in the detached HEAD state that CI checkouts often use. + run(true, "push", "origin", "HEAD:" + branch); + } + + @Override + public Optional currentBranch() { + Result result = run(false, "rev-parse", "--abbrev-ref", "HEAD"); + if (result.exitCode() != 0) { + return Optional.empty(); + } + String branch = result.output().trim(); + return branch.isEmpty() || "HEAD".equals(branch) ? Optional.empty() : Optional.of(branch); + } + + private record Result(int exitCode, String output) { + } + + private Result run(boolean failOnError, String... arguments) { + List command = new ArrayList<>(); + command.add("git"); + command.addAll(Arrays.asList(arguments)); + + log.debug("Running: " + String.join(" ", command)); + + Process process; + try { + process = new ProcessBuilder(command) + .directory(workingDirectory.toFile()) + .redirectErrorStream(true) + .start(); + } catch (IOException e) { + throw new PaoException("Cannot run: " + String.join(" ", command), e); + } + + String output; + int exitCode; + try { + output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + if (!process.waitFor(TIMEOUT_SECONDS, TimeUnit.SECONDS)) { + process.destroyForcibly(); + throw new PaoException("Timed out after " + TIMEOUT_SECONDS + "s: " + String.join(" ", command)); + } + exitCode = process.exitValue(); + } catch (IOException e) { + throw new PaoException("Cannot read output of: " + String.join(" ", command), e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new PaoException("Interrupted while running: " + String.join(" ", command), e); + } + + if (exitCode != 0 && failOnError) { + throw new PaoException("Command failed (exit " + exitCode + "): " + String.join(" ", command) + + System.lineSeparator() + output.strip()); + } + return new Result(exitCode, output); + } +} diff --git a/src/main/java/com/jardoapps/pao/git/GitClient.java b/src/main/java/com/jardoapps/pao/git/GitClient.java new file mode 100644 index 0000000..b517fd2 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/git/GitClient.java @@ -0,0 +1,22 @@ +package com.jardoapps.pao.git; + +import java.util.Optional; + +/** The git operations the plugin needs, kept behind an interface so runs can be faked in tests. */ +public interface GitClient { + + /** Sets the local user identity used for commits. */ + void configureUser(String name, String email); + + /** True if the working tree has uncommitted changes. */ + boolean hasUncommittedChanges(); + + /** Commits all tracked modifications. Does nothing if the tree is clean. */ + void commitAll(String message); + + /** Pushes HEAD to the given branch on {@code origin}. */ + void push(String branch); + + /** The branch currently checked out, empty in detached HEAD state or outside a repository. */ + Optional currentBranch(); +} diff --git a/src/main/java/com/jardoapps/pao/pom/PomDocument.java b/src/main/java/com/jardoapps/pao/pom/PomDocument.java new file mode 100644 index 0000000..c60ce41 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/pom/PomDocument.java @@ -0,0 +1,301 @@ +package com.jardoapps.pao.pom; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.Deque; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +/** + * A pom.xml held as raw text plus an index of its elements. + * + *

Edits are applied as targeted splices into the original source, so + * formatting, comments, entities and attribute quoting outside the edited + * values are preserved byte for byte. This is deliberately not a DOM: marshalling + * a parsed model back out would reformat the whole file. + */ +public final class PomDocument { + + /** A dependency (or dependencyManagement / plugin dependency) entry. */ + public record DependencyEntry(String groupId, String artifactId, XmlElement versionElement) { + + public String key() { + return groupId + ":" + artifactId; + } + } + + private record Edit(int start, int end, String replacement) { + } + + private final Path path; + private final String source; + private final List elements; + private final List edits = new ArrayList<>(); + + private PomDocument(Path path, String source, List elements) { + this.path = path; + this.source = source; + this.elements = elements; + } + + public static PomDocument load(Path path) { + try { + return parse(path, Files.readString(path, StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new UncheckedIOException("Cannot read " + path, e); + } + } + + public static PomDocument parse(Path path, String source) { + return new PomDocument(path, source, scan(source, path)); + } + + // --- Scanning --------------------------------------------------------- + + private static List scan(String s, Path path) { + List found = new ArrayList<>(); + Deque open = new ArrayDeque<>(); + int i = 0; + + while ((i = s.indexOf('<', i)) >= 0) { + if (s.startsWith("", i), path, "unterminated comment") + 3; + } else if (s.startsWith("", i), path, "unterminated CDATA section") + 3; + } else if (s.startsWith("", i), path, "unterminated processing instruction") + 2; + } else if (s.startsWith("', i), path, "unterminated declaration") + 1; + } else if (s.startsWith("', i), path, "unterminated end tag"); + if (open.isEmpty()) { + throw new IllegalArgumentException(path + ": end tag without matching start tag at offset " + i); + } + found.get(open.pop()).closeAt(i, s); + i = gt + 1; + } else { + int gt = findTagEnd(s, i, path); + int nameEnd = i + 1; + while (nameEnd < gt && !isNameEnd(s.charAt(nameEnd))) { + nameEnd++; + } + String name = s.substring(i + 1, nameEnd); + XmlElement element = new XmlElement(name, found.size(), open.isEmpty() ? -1 : open.peek(), gt + 1); + found.add(element); + if (s.charAt(gt - 1) == '/') { + element.closeAt(gt + 1, s); + } else { + open.push(element.getIndex()); + } + i = gt + 1; + } + } + + if (!open.isEmpty()) { + throw new IllegalArgumentException(path + ": unclosed element <" + found.get(open.peek()).getName() + ">"); + } + return found; + } + + private static int requireEnd(int position, Path path, String what) { + if (position < 0) { + throw new IllegalArgumentException(path + ": " + what); + } + return position; + } + + private static boolean isNameEnd(char c) { + return Character.isWhitespace(c) || c == '/' || c == '>'; + } + + /** Finds the '>' closing a start tag, ignoring any inside quoted attribute values. */ + private static int findTagEnd(String s, int start, Path path) { + char quote = 0; + for (int j = start + 1; j < s.length(); j++) { + char c = s.charAt(j); + if (quote != 0) { + if (c == quote) { + quote = 0; + } + } else if (c == '"' || c == '\'') { + quote = c; + } else if (c == '>') { + return j; + } + } + throw new IllegalArgumentException(path + ": unterminated start tag at offset " + start); + } + + // --- Queries ---------------------------------------------------------- + + public Path getPath() { + return path; + } + + public String valueOf(XmlElement element) { + return source.substring(element.getValueStart(), element.getValueEnd()); + } + + private boolean hasPath(XmlElement element, String... namesFromRoot) { + XmlElement current = element; + for (int i = namesFromRoot.length - 1; i >= 0; i--) { + if (current == null || !current.getName().equals(namesFromRoot[i])) { + return false; + } + current = current.getParentIndex() < 0 ? null : elements.get(current.getParentIndex()); + } + return current == null; + } + + private Optional child(XmlElement parent, String name) { + return elements.stream() + .filter(e -> e.getParentIndex() == parent.getIndex() && e.getName().equals(name)) + .findFirst(); + } + + private Optional firstWithPath(String... namesFromRoot) { + return elements.stream().filter(e -> hasPath(e, namesFromRoot)).findFirst(); + } + + /** The {@code } that is a direct child of {@code }, if the project declares one. */ + public Optional projectVersion() { + return firstWithPath("project", "version"); + } + + /** The {@code } inside {@code }, if there is a parent. */ + public Optional parentVersion() { + return firstWithPath("project", "parent", "version"); + } + + public Optional projectArtifactId() { + return firstWithPath("project", "artifactId").map(this::valueOf); + } + + public Optional projectGroupId() { + return firstWithPath("project", "groupId").map(this::valueOf); + } + + public Optional parentGroupId() { + return firstWithPath("project", "parent", "groupId").map(this::valueOf); + } + + public Optional parentArtifactId() { + return firstWithPath("project", "parent", "artifactId").map(this::valueOf); + } + + /** Module names declared in {@code }. */ + public List modules() { + return elements.stream() + .filter(e -> hasPath(e, "project", "modules", "module")) + .map(this::valueOf) + .toList(); + } + + /** + * Every {@code } in the file that declares a version, wherever it + * sits: plain dependencies, dependencyManagement, profiles and plugin + * dependencies are all included. + */ + public List dependencies() { + List result = new ArrayList<>(); + for (XmlElement element : elements) { + if (!element.getName().equals("dependency")) { + continue; + } + Optional version = child(element, "version"); + if (version.isEmpty()) { + continue; + } + String groupId = child(element, "groupId").map(this::valueOf).orElse(""); + String artifactId = child(element, "artifactId").map(this::valueOf).orElse(""); + result.add(new DependencyEntry(groupId, artifactId, version.get())); + } + return result; + } + + /** Properties declared in {@code }, in document order. */ + public Map properties() { + Map result = new LinkedHashMap<>(); + for (XmlElement element : elements) { + if (hasPath(element, "project", "properties", element.getName())) { + result.putIfAbsent(element.getName(), element); + } + } + return result; + } + + /** Every {@code } element in the file, in document order. */ + public List allVersionElements() { + return elements.stream().filter(e -> e.getName().equals("version")).toList(); + } + + // --- Editing ---------------------------------------------------------- + + /** + * Queues a replacement of an element's text value. Repeating the same replacement + * is a no-op, which happens when several coordinates resolve to one shared + * property; asking for two different values for one element is a bug and fails. + */ + public void setValue(XmlElement element, String newValue) { + Edit edit = new Edit(element.getValueStart(), element.getValueEnd(), newValue); + for (Edit existing : edits) { + if (existing.start() == edit.start() && existing.end() == edit.end()) { + if (!existing.replacement().equals(newValue)) { + throw new IllegalStateException(path + ": conflicting replacements for <" + element.getName() + + ">: '" + existing.replacement() + "' and '" + newValue + "'"); + } + return; + } + } + edits.add(edit); + } + + public boolean isModified() { + return !edits.isEmpty(); + } + + /** Renders the document with all queued edits applied. */ + public String render() { + if (edits.isEmpty()) { + return source; + } + List ordered = new ArrayList<>(edits); + ordered.sort(Comparator.comparingInt(Edit::start)); + StringBuilder out = new StringBuilder(source.length() + 64); + int cursor = 0; + for (Edit edit : ordered) { + if (edit.start() < cursor) { + throw new IllegalStateException(path + ": overlapping edits at offset " + edit.start()); + } + out.append(source, cursor, edit.start()).append(edit.replacement()); + cursor = edit.end(); + } + out.append(source, cursor, source.length()); + return out.toString(); + } + + /** Writes the edited document back to disk. Returns true if anything was written. */ + public boolean save() { + if (edits.isEmpty()) { + return false; + } + String rendered = render(); + if (rendered.equals(source)) { + return false; + } + try { + Files.writeString(path, rendered, StandardCharsets.UTF_8); + } catch (IOException e) { + throw new UncheckedIOException("Cannot write " + path, e); + } + return true; + } +} diff --git a/src/main/java/com/jardoapps/pao/pom/PomEditSession.java b/src/main/java/com/jardoapps/pao/pom/PomEditSession.java new file mode 100644 index 0000000..91669ef --- /dev/null +++ b/src/main/java/com/jardoapps/pao/pom/PomEditSession.java @@ -0,0 +1,114 @@ +package com.jardoapps.pao.pom; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.apache.maven.plugin.logging.Log; + +import com.jardoapps.pao.BranchVersions; +import com.jardoapps.pao.ProjectModel; + +/** + * A batch of pom.xml edits that are written out together. + * + *

Version elements are frequently written as {@code ${some.version}} rather than a + * literal. This session resolves such references to the {@code } entry + * that defines them - anywhere in the reactor - and edits that instead, so the + * indirection survives the rewrite. + */ +public final class PomEditSession { + + private final Map documents = new LinkedHashMap<>(); + private final Log log; + + public PomEditSession(List reactor, Log log) { + this.log = log; + for (ProjectModel project : reactor) { + documents.computeIfAbsent(project.pomFile(), PomDocument::load); + } + } + + public PomDocument document(Path pomFile) { + PomDocument document = documents.get(pomFile); + if (document == null) { + throw new IllegalArgumentException("Not part of this session: " + pomFile); + } + return document; + } + + public List documents() { + return List.copyOf(documents.values()); + } + + /** + * Sets an element's version, following a {@code ${...}} reference to the property + * that defines it. + * + * @param expectedCurrent the value the element must currently resolve to, or null to overwrite regardless + * @return true if an edit was queued + */ + public boolean setVersion(PomDocument document, XmlElement element, String expectedCurrent, String newValue) { + String raw = document.valueOf(element); + + Optional property = BranchVersions.propertyReference(raw); + if (property.isPresent()) { + return setProperty(document, property.get(), expectedCurrent, newValue, element); + } + + if (expectedCurrent != null && !raw.equals(expectedCurrent)) { + return false; + } + if (raw.equals(newValue)) { + return false; + } + document.setValue(element, newValue); + return true; + } + + private boolean setProperty(PomDocument origin, String propertyName, String expectedCurrent, String newValue, + XmlElement reference) { + // Look in the pom that made the reference first, then anywhere else in the + // reactor, since parent poms commonly hold the shared property. + List searchOrder = new ArrayList<>(); + searchOrder.add(origin); + documents.values().stream().filter(d -> d != origin).forEach(searchOrder::add); + + for (PomDocument candidate : searchOrder) { + XmlElement propertyElement = candidate.properties().get(propertyName); + if (propertyElement == null) { + continue; + } + String current = candidate.valueOf(propertyElement); + if (expectedCurrent != null && !current.equals(expectedCurrent)) { + continue; + } + if (current.equals(newValue)) { + return false; + } + log.info(" " + origin.getPath().getFileName() + ": <" + reference.getName() + "> is ${" + propertyName + + "}, updating the property in " + candidate.getPath()); + candidate.setValue(propertyElement, newValue); + return true; + } + + log.warn("Cannot update <" + reference.getName() + ">${" + propertyName + "} in " + + origin.getPath() + ": property '" + propertyName + + "' is not defined in the reactor. Leaving it unchanged."); + return false; + } + + /** Writes every modified document. Returns the files that changed on disk. */ + public List save() { + List written = new ArrayList<>(); + for (PomDocument document : documents.values()) { + if (document.save()) { + written.add(document.getPath()); + } + } + return written; + } +} diff --git a/src/main/java/com/jardoapps/pao/pom/PomReader.java b/src/main/java/com/jardoapps/pao/pom/PomReader.java new file mode 100644 index 0000000..fc466f6 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/pom/PomReader.java @@ -0,0 +1,78 @@ +package com.jardoapps.pao.pom; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import com.jardoapps.pao.BranchVersions; +import com.jardoapps.pao.PaoException; +import com.jardoapps.pao.ProjectModel; + +/** + * Builds a reactor model straight from pom.xml files by following {@code }. + * + *

The plugin itself uses Maven's own resolved reactor; this reader exists so the + * same logic can be exercised against plain files in tests. + */ +public final class PomReader { + + private PomReader() { + } + + /** Reads the project at {@code pomFile} and, recursively, all of its modules. */ + public static List readReactor(Path pomFile) { + List reactor = new ArrayList<>(); + collect(pomFile.toAbsolutePath().normalize(), reactor); + return reactor; + } + + private static void collect(Path pomFile, List reactor) { + if (!Files.isRegularFile(pomFile)) { + throw new PaoException("POM file not found: " + pomFile); + } + PomDocument document = PomDocument.load(pomFile); + reactor.add(toModel(pomFile, document)); + + Path baseDir = pomFile.getParent(); + for (String module : document.modules()) { + Path modulePath = baseDir.resolve(module).normalize(); + if (Files.isDirectory(modulePath)) { + modulePath = modulePath.resolve("pom.xml"); + } + collect(modulePath, reactor); + } + } + + private static ProjectModel toModel(Path pomFile, PomDocument document) { + String parentGroupId = document.parentGroupId().orElse(null); + String parentArtifactId = document.parentArtifactId().orElse(null); + String parentVersion = document.parentVersion().map(document::valueOf).orElse(null); + + String groupId = document.projectGroupId().orElse(parentGroupId); + String artifactId = document.projectArtifactId() + .orElseThrow(() -> new PaoException(pomFile + ": no found")); + String version = document.projectVersion().map(document::valueOf).orElse(parentVersion); + + if (version == null) { + throw new PaoException(pomFile + ": no project version and no parent version found"); + } + + return new ProjectModel(pomFile, groupId, artifactId, interpolate(document, version), parentGroupId, + parentArtifactId, interpolate(document, parentVersion)); + } + + /** + * Resolves a {@code ${...}} version against the pom's own properties, so the model + * carries the same effective version Maven would report. + */ + private static String interpolate(PomDocument document, String value) { + if (value == null) { + return null; + } + return BranchVersions.propertyReference(value) + .map(name -> document.properties().get(name)) + .map(document::valueOf) + .orElse(value); + } +} diff --git a/src/main/java/com/jardoapps/pao/pom/XmlElement.java b/src/main/java/com/jardoapps/pao/pom/XmlElement.java new file mode 100644 index 0000000..46f3e37 --- /dev/null +++ b/src/main/java/com/jardoapps/pao/pom/XmlElement.java @@ -0,0 +1,77 @@ +package com.jardoapps.pao.pom; + +/** + * A single element occurrence in a scanned POM, remembering where its text + * content lives in the original source so it can be rewritten in place. + */ +public final class XmlElement { + + private final String name; + private final int index; + private final int parentIndex; + + /** Bounds of the raw content between the start and end tag. */ + private int contentStart; + private int contentEnd; + + /** Bounds of the content with surrounding whitespace trimmed off. */ + private int valueStart; + private int valueEnd; + + XmlElement(String name, int index, int parentIndex, int contentStart) { + this.name = name; + this.index = index; + this.parentIndex = parentIndex; + this.contentStart = contentStart; + this.contentEnd = contentStart; + this.valueStart = contentStart; + this.valueEnd = contentStart; + } + + void closeAt(int contentEnd, String source) { + this.contentEnd = contentEnd; + int start = contentStart; + int end = contentEnd; + while (start < end && Character.isWhitespace(source.charAt(start))) { + start++; + } + while (end > start && Character.isWhitespace(source.charAt(end - 1))) { + end--; + } + this.valueStart = start; + this.valueEnd = end; + } + + public String getName() { + return name; + } + + public int getIndex() { + return index; + } + + public int getParentIndex() { + return parentIndex; + } + + public int getContentStart() { + return contentStart; + } + + public int getContentEnd() { + return contentEnd; + } + + public int getValueStart() { + return valueStart; + } + + public int getValueEnd() { + return valueEnd; + } + + @Override + public String toString() { + return "<" + name + "> @" + contentStart; + } +} diff --git a/src/test/java/com/jardoapps/pao/BranchVersionsTest.java b/src/test/java/com/jardoapps/pao/BranchVersionsTest.java new file mode 100644 index 0000000..73e3a21 --- /dev/null +++ b/src/test/java/com/jardoapps/pao/BranchVersionsTest.java @@ -0,0 +1,92 @@ +package com.jardoapps.pao; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +class BranchVersionsTest { + + @ParameterizedTest + @ValueSource(strings = { + "1.2.3-feature-abc-SNAPSHOT", + "1.2.3-rc.4-feature-abc-SNAPSHOT", + "1.2.10-feature-abc-SNAPSHOT", + "1.2.3.4-feature-abc-SNAPSHOT", + "1.2-feature-abc-SNAPSHOT" }) + void recognisesBranchVersions(String version) { + assertTrue(BranchVersions.isBranchVersion(version)); + } + + @ParameterizedTest + @ValueSource(strings = { + "1.2.3-SNAPSHOT", + "1.2.3", + "1.2.3-rc.4-SNAPSHOT", + "${revision}", + "not-a-version" }) + void rejectsNonBranchVersions(String version) { + assertFalse(BranchVersions.isBranchVersion(version)); + } + + @ParameterizedTest + @CsvSource({ + "1.2.3-feature-abc-SNAPSHOT, 1.2.3-SNAPSHOT", + "1.2.3-rc.4-feature-abc-SNAPSHOT, 1.2.3-rc.4-SNAPSHOT", + "1.2.3.4-feature-abc-SNAPSHOT, 1.2.3.4-SNAPSHOT" }) + void stripsBranchSuffix(String version, String expected) { + assertEquals(Optional.of(expected), BranchVersions.withoutBranch(version)); + } + + @Test + @DisplayName("a two-digit patch number survives the round trip") + void keepsMultiDigitPatchNumbers() { + String branchVersion = BranchVersions.withBranch("1.2.10-SNAPSHOT", "feature-abc"); + + assertEquals("1.2.10-feature-abc-SNAPSHOT", branchVersion); + assertEquals(Optional.of("1.2.10-SNAPSHOT"), BranchVersions.withoutBranch(branchVersion)); + } + + @ParameterizedTest + @CsvSource({ + "1.2.3-SNAPSHOT, feature-abc, 1.2.3-feature-abc-SNAPSHOT", + "1.2.3-rc.4-SNAPSHOT, feature-abc, 1.2.3-rc.4-feature-abc-SNAPSHOT", + "1.2.3-feature-old-SNAPSHOT, feature-abc, 1.2.3-feature-abc-SNAPSHOT" }) + void addsOrReplacesBranchSuffix(String version, String suffix, String expected) { + assertEquals(expected, BranchVersions.withBranch(version, suffix)); + } + + @Test + void extractsSuffix() { + assertEquals(Optional.of("feature-abc"), BranchVersions.suffixOf("1.2.3-feature-abc-SNAPSHOT")); + assertEquals(Optional.of("feature-abc"), BranchVersions.suffixOf("1.2.3-rc.4-feature-abc-SNAPSHOT")); + assertEquals(Optional.empty(), BranchVersions.suffixOf("1.2.3-SNAPSHOT")); + } + + @Test + void convertsBranchNameToSuffix() { + assertEquals("feature-FEA-123-comments", BranchVersions.branchSuffix("feature/FEA-123-comments")); + assertEquals("main", BranchVersions.branchSuffix("main")); + } + + @Test + void acceptsOnlyRevertiblePins() { + assertTrue(BranchVersions.isValidPin("1.2.3-f1-SNAPSHOT")); + assertFalse(BranchVersions.isValidPin("vf1")); + assertFalse(BranchVersions.isValidPin("1.2.3-SNAPSHOT")); + } + + @Test + void detectsPropertyReferences() { + assertEquals(Optional.of("revision"), BranchVersions.propertyReference("${revision}")); + assertEquals(Optional.empty(), BranchVersions.propertyReference("1.2.3-SNAPSHOT")); + assertEquals(Optional.empty(), BranchVersions.propertyReference("prefix-${revision}")); + } +} diff --git a/src/test/java/com/jardoapps/pao/GlobMatcherTest.java b/src/test/java/com/jardoapps/pao/GlobMatcherTest.java new file mode 100644 index 0000000..42c22af --- /dev/null +++ b/src/test/java/com/jardoapps/pao/GlobMatcherTest.java @@ -0,0 +1,42 @@ +package com.jardoapps.pao; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class GlobMatcherTest { + + @Test + void matchesLiterals() { + assertTrue(GlobMatcher.matches("main", "main")); + assertFalse(GlobMatcher.matches("main", "master")); + } + + @Test + void matchesStarAcrossSlashes() { + assertTrue(GlobMatcher.matches("release*", "release/2.0")); + assertTrue(GlobMatcher.matches("*", "feature/abc")); + assertTrue(GlobMatcher.matches("feature/*", "feature/abc")); + } + + @Test + @DisplayName("** behaves like bash, where * already spans slashes") + void treatsDoubleStarLikeBash() { + assertTrue(GlobMatcher.matches("**/*", "feature/abc")); + assertFalse(GlobMatcher.matches("**/*", "main")); + } + + @Test + void matchesSingleCharacterWildcard() { + assertTrue(GlobMatcher.matches("v?", "v1")); + assertFalse(GlobMatcher.matches("v?", "v10")); + } + + @Test + void treatsRegexCharactersAsLiterals() { + assertTrue(GlobMatcher.matches("release-1.0", "release-1.0")); + assertFalse(GlobMatcher.matches("release-1.0", "release-1x0")); + } +} diff --git a/src/test/java/com/jardoapps/pao/MultiModuleTest.java b/src/test/java/com/jardoapps/pao/MultiModuleTest.java new file mode 100644 index 0000000..13c7c1d --- /dev/null +++ b/src/test/java/com/jardoapps/pao/MultiModuleTest.java @@ -0,0 +1,67 @@ +package com.jardoapps.pao; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Path; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Multi-module projects carry the shared version in each module's + * {@code }. If those references are not moved along with the + * aggregator's version, every module points at a parent that no longer exists. + */ +class MultiModuleTest extends RunnerTestSupport { + + @TempDir + Path project; + + private void writeReactor() { + writeFixture(project, "multimodule/pom.xml", "pom.xml"); + writeFixture(project.resolve("core"), "multimodule/core/pom.xml", "pom.xml"); + } + + @Test + @DisplayName("enforcing a branch version updates module parent references too") + void updatesParentReferencesOnEnforce() { + writeReactor(); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertMatchesFixture(project.resolve("pom.xml"), "multimodule-expected/pom.xml"); + assertMatchesFixture(project.resolve("core/pom.xml"), "multimodule-expected/core/pom.xml"); + } + + @Test + @DisplayName("stripping a branch version on a core branch updates module parent references too") + void updatesParentReferencesOnRemoval() { + writeFixture(project, "multimodule-expected/pom.xml", "pom.xml"); + writeFixture(project.resolve("core"), "multimodule-expected/core/pom.xml", "pom.xml"); + + run(project, settings -> settings.setBranchName("main")); + + assertMatchesFixture(project.resolve("pom.xml"), "multimodule/pom.xml"); + assertMatchesFixture(project.resolve("core/pom.xml"), "multimodule/core/pom.xml"); + } + + @Test + @DisplayName("a module's unrelated dependency version is left alone") + void leavesUnrelatedDependencyAlone() { + writeReactor(); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertTrue(read(project.resolve("core/pom.xml")).contains("9.9.9-SNAPSHOT")); + } + + @Test + @DisplayName("the reactor is discovered through ") + void readsWholeReactor() { + writeReactor(); + + assertEquals(2, com.jardoapps.pao.pom.PomReader.readReactor(project.resolve("pom.xml")).size()); + } +} diff --git a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java new file mode 100644 index 0000000..6d2f441 --- /dev/null +++ b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java @@ -0,0 +1,256 @@ +package com.jardoapps.pao; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.jardoapps.pao.PreventOverwritesRunner.RunResult; + +/** Ports the behaviour covered by the shell implementation's test suite. */ +class PreventOverwritesRunnerTest extends RunnerTestSupport { + + @TempDir + Path project; + + @Test + @DisplayName("feature branch adds the branch suffix and leaves dependencies alone") + void enforcesBranchVersion() { + Path pom = writePom(project, "sample-pom.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertMatchesFixture(pom, "expected-enforce-branch-version.xml"); + assertTrue(result.changesMade()); + assertEquals(List.of("Switched to branch-specific version."), git.getCommits()); + } + + @Test + @DisplayName("a project that already carries a branch version is left untouched") + void skipsWhenAlreadyBranchVersioned() { + Path pom = writePom(project, "sample-pom-with-branch-version.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertMatchesFixture(pom, "sample-pom-with-branch-version.xml"); + assertFalse(result.changesMade()); + assertTrue(git.getCommits().isEmpty()); + } + + @Test + @DisplayName("enforceBranchVersion=false leaves the project version alone") + void skipsWhenEnforcementDisabled() { + Path pom = writePom(project, "sample-pom.xml"); + + RunResult result = run(project, settings -> settings + .setBranchName("feature/my-feature") + .setEnforceBranchVersion(false)); + + assertMatchesFixture(pom, "sample-pom.xml"); + assertFalse(result.changesMade()); + } + + @Test + @DisplayName("core branch strips the branch suffix from the project version") + void removesBranchVersionOnCoreBranch() { + Path pom = writePom(project, "sample-pom-with-branch-version.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("main")); + + assertMatchesFixture(pom, "expected-remove-branch-version.xml"); + assertTrue(result.changesMade()); + assertTrue(result.coreBranch()); + assertEquals(List.of("Switched to non branch-specific version."), git.getCommits()); + } + + @Test + @DisplayName("core branch strips branch suffixes from dependency versions, including rc versions") + void removesDependencyBranchVersionsOnCoreBranch() { + Path pom = writePom(project, "sample-pom-with-branch-deps.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("main")); + + assertMatchesFixture(pom, "expected-remove-dependency-branch-versions.xml"); + assertTrue(result.changesMade()); + assertEquals(List.of("Switched to non branch dependency versions."), git.getCommits()); + } + + @Test + @DisplayName("core branch with nothing to strip makes no changes") + void makesNoChangesOnCleanCoreBranch() { + Path pom = writePom(project, "sample-pom.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("main")); + + assertMatchesFixture(pom, "sample-pom.xml"); + assertFalse(result.changesMade()); + assertTrue(git.getCommits().isEmpty()); + } + + @Test + @DisplayName("release* glob matches core branches") + void treatsGlobMatchedBranchAsCore() { + writePom(project, "sample-pom-with-branch-version.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("release/2.0")); + + assertTrue(result.coreBranch()); + } + + // --- Per-branch configuration ---------------------------------------- + + @Test + @DisplayName("config pins the project version for a matching branch") + void pinsProjectVersion() { + Path pom = writePom(project, "sample-pom.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # branch-pattern target value + feature/f1 project-version 1.2.3-f1-SNAPSHOT + """); + + run(project, settings -> settings.setBranchName("feature/f1")); + + assertMatchesFixture(pom, "expected-config-pin-project-version.xml"); + } + + @Test + @DisplayName("config pins dependency versions even when enforcement is off") + void pinsDependencyVersions() { + Path pom = writePom(project, "sample-pom-two-deps.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # branch-pattern target value + feature/f1 dependency:com.example:d1 1.0.0-f1-SNAPSHOT + feature/f1 dependency:com.example:d2 5.0.0-f1-SNAPSHOT + """); + + run(project, settings -> settings + .setBranchName("feature/f1") + .setEnforceBranchVersion(false)); + + assertMatchesFixture(pom, "expected-config-pin-dependency-versions.xml"); + assertEquals(List.of("Pinned branch-specific dependency versions."), git.getCommits()); + } + + @Test + @DisplayName("inline and indented comments in the config are ignored") + void ignoresComments() { + Path pom = writePom(project, "sample-pom-two-deps.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # full-line comment: branch-pattern target value + feature/f1 dependency:com.example:d1 1.0.0-f1-SNAPSHOT # pin d1 for f1 + feature/f1 dependency:com.example:d2 5.0.0-f1-SNAPSHOT # pin d2 for f1 + # indented full-line comment should be ignored too + """); + + run(project, settings -> settings + .setBranchName("feature/f1") + .setEnforceBranchVersion(false)); + + assertMatchesFixture(pom, "expected-config-pin-dependency-versions.xml"); + } + + @Test + @DisplayName("a config that matches no branch falls back to the derived version") + void fallsBackWhenConfigDoesNotMatch() { + Path pom = writePom(project, "sample-pom.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # Config only covers feature/f1 - this run is on feature/my-feature + feature/f1 project-version 1.2.3-f1-SNAPSHOT + """); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertMatchesFixture(pom, "expected-enforce-branch-version.xml"); + } + + @Test + @DisplayName("a pinned version that is not a branch version fails the build") + void rejectsInvalidPin() { + writePom(project, "sample-pom.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # 'vf1' is not a valid branch version - must be rejected + feature/f1 project-version vf1 + """); + + PaoException failure = assertThrows(PaoException.class, + () -> run(project, settings -> settings.setBranchName("feature/f1"))); + + assertTrue(failure.getMessage().contains("vf1"), failure.getMessage()); + } + + @Test + @DisplayName("an inherited exclusive suffix is re-derived for the current branch") + void reDerivesExclusiveSuffix() { + Path pom = writePom(project, "sample-pom-with-branch-version.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # branch-pattern target value + * exclusive-version-suffix feature-old + """); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertMatchesFixture(pom, "expected-config-exclusive-suffix-rederive.xml"); + } + + @Test + @DisplayName("the branch that owns an exclusive suffix keeps its version") + void leavesExclusiveSuffixOwnerAlone() { + Path pom = writePom(project, "sample-pom-with-branch-version.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + # branch-pattern target value + * exclusive-version-suffix feature-old + """); + + RunResult result = run(project, settings -> settings.setBranchName("feature/old")); + + assertMatchesFixture(pom, "sample-pom-with-branch-version.xml"); + assertFalse(result.changesMade()); + } + + // --- Branch detection and outputs ------------------------------------- + + @Test + @DisplayName("the branch name is taken from the CI environment when not configured") + void detectsBranchFromEnvironment() { + writePom(project, "sample-pom.xml"); + environment = Map.of("GITHUB_REF_NAME", "feature/my-feature"); + + RunResult result = run(project, settings -> { + }); + + assertEquals("feature/my-feature", result.branchName()); + } + + @Test + @DisplayName("the changes-made output is appended to the configured output file") + void writesOutputFile() { + writePom(project, "sample-pom.xml"); + Path output = project.resolve("build.env"); + + run(project, settings -> settings + .setBranchName("feature/my-feature") + .setOutputFile(output)); + + assertEquals("changes-made=true", read(output).strip()); + } + + @Test + @DisplayName("changes are pushed to the detected branch when pushing is enabled") + void pushesToDetectedBranch() { + writePom(project, "sample-pom.xml"); + + run(project, settings -> settings + .setBranchName("feature/my-feature") + .setPushChanges(true)); + + assertEquals(List.of("feature/my-feature"), git.getPushes()); + } +} diff --git a/src/test/java/com/jardoapps/pao/PropertyVersionTest.java b/src/test/java/com/jardoapps/pao/PropertyVersionTest.java new file mode 100644 index 0000000..6f9f8a3 --- /dev/null +++ b/src/test/java/com/jardoapps/pao/PropertyVersionTest.java @@ -0,0 +1,89 @@ +package com.jardoapps.pao; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Path; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Versions written as {@code ${property}} - the CI-friendly {@code ${revision}} style + * among them - must be followed to the property that defines them. + */ +class PropertyVersionTest extends RunnerTestSupport { + + @TempDir + Path project; + + @Test + @DisplayName("a ${revision} project version is applied to the property") + void enforcesThroughRevisionProperty() { + Path pom = writePom(project, "sample-pom-revision.xml"); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertMatchesFixture(pom, "expected-revision-enforced.xml"); + } + + @Test + @DisplayName("a core branch strips the branch suffix out of the property") + void removesThroughRevisionProperty() { + Path pom = writeFixture(project, "expected-revision-enforced.xml", "pom.xml"); + + run(project, settings -> settings.setBranchName("main")); + + assertMatchesFixture(pom, "sample-pom-revision.xml"); + } + + @Test + @DisplayName("a pinned dependency whose version is a property updates the property") + void pinsThroughDependencyProperty() { + Path pom = writePom(project, "sample-pom-revision.xml"); + writeFile(project, ".prevent-overwrites.conf", """ + feature/f1 dependency:com.example:shared-lib 4.5.6-f1-SNAPSHOT + """); + + run(project, settings -> settings + .setBranchName("feature/f1") + .setEnforceBranchVersion(false)); + + String result = read(pom); + assertTrue(result.contains("4.5.6-f1-SNAPSHOT"), result); + assertTrue(result.contains("${shared-lib.version}"), result); + } + + @Test + @DisplayName("a property version that is not defined anywhere is reported and left alone") + void leavesUnknownPropertyAlone() { + Path pom = writeFile(project, "pom.xml", """ + + + 4.0.0 + com.example + my-app + 1.2.3-SNAPSHOT + + + com.example + d1 + ${undefined.version} + + + + """); + String before = read(pom); + writeFile(project, ".prevent-overwrites.conf", """ + feature/f1 dependency:com.example:d1 1.0.0-f1-SNAPSHOT + """); + + run(project, settings -> settings + .setBranchName("feature/f1") + .setEnforceBranchVersion(false)); + + assertEquals(before, read(pom)); + assertTrue(git.getCommits().isEmpty()); + } +} diff --git a/src/test/java/com/jardoapps/pao/RunnerTestSupport.java b/src/test/java/com/jardoapps/pao/RunnerTestSupport.java new file mode 100644 index 0000000..f10afa4 --- /dev/null +++ b/src/test/java/com/jardoapps/pao/RunnerTestSupport.java @@ -0,0 +1,90 @@ +package com.jardoapps.pao; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.function.Consumer; + +import org.apache.maven.plugin.logging.Log; +import org.apache.maven.plugin.logging.SystemStreamLog; + +import com.jardoapps.pao.git.FakeGitClient; +import com.jardoapps.pao.pom.PomReader; + +/** Sets up a throwaway project directory and runs the plugin logic against it. */ +public class RunnerTestSupport { + + protected final Log log = new SystemStreamLog(); + + protected FakeGitClient git = new FakeGitClient(); + + protected Map environment = Map.of(); + + /** Copies a fixture from {@code src/test/resources/poms} to {@code

/}. */ + protected Path writeFixture(Path directory, String fixture, String name) { + try (InputStream in = getClass().getResourceAsStream("/poms/" + fixture)) { + if (in == null) { + throw new IllegalArgumentException("No such fixture: " + fixture); + } + Files.createDirectories(directory); + Path target = directory.resolve(name); + Files.write(target, in.readAllBytes()); + return target; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + protected Path writePom(Path directory, String fixture) { + return writeFixture(directory, fixture, "pom.xml"); + } + + protected Path writeFile(Path directory, String name, String content) { + try { + Files.createDirectories(directory); + Path target = directory.resolve(name); + Files.writeString(target, content, StandardCharsets.UTF_8); + return target; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + protected PreventOverwritesRunner.RunResult run(Path baseDirectory, Consumer configure) { + RunnerSettings settings = new RunnerSettings().setPushChanges(false); + configure.accept(settings); + PreventOverwritesRunner runner = + new PreventOverwritesRunner(settings, git, baseDirectory, environment::get, log); + return runner.run(PomReader.readReactor(baseDirectory.resolve("pom.xml"))); + } + + protected String read(Path file) { + try { + return Files.readString(file, StandardCharsets.UTF_8); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + protected String fixture(String name) { + try (InputStream in = getClass().getResourceAsStream("/poms/" + name)) { + if (in == null) { + throw new IllegalArgumentException("No such fixture: " + name); + } + return new String(in.readAllBytes(), StandardCharsets.UTF_8); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + /** Asserts the file matches a fixture byte for byte, so formatting changes are caught. */ + protected void assertMatchesFixture(Path file, String fixtureName) { + assertEquals(fixture(fixtureName), read(file), file + " does not match " + fixtureName); + } +} diff --git a/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java b/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java new file mode 100644 index 0000000..2b45747 --- /dev/null +++ b/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java @@ -0,0 +1,113 @@ +package com.jardoapps.pao.config; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.Optional; + +import org.apache.maven.plugin.logging.SystemStreamLog; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.jardoapps.pao.PaoException; + +class PinConfigParserTest { + + @TempDir + Path directory; + + private final PinConfigParser parser = new PinConfigParser(new SystemStreamLog()); + + private PinConfig parse(String content, String branch) { + try { + Path file = directory.resolve(".prevent-overwrites.conf"); + Files.writeString(file, content, StandardCharsets.UTF_8); + return parser.parse(file, branch); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Test + void returnsEmptyConfigWhenFileIsMissing() { + assertTrue(parser.parse(directory.resolve("nope.conf"), "feature/f1").isEmpty()); + } + + @Test + void readsAllThreeTargets() { + PinConfig config = parse(""" + feature/f1 project-version 1.2.3-f1-SNAPSHOT + feature/f1 dependency:com.example:d1 2.0.0-f1-SNAPSHOT + feature/f1 exclusive-version-suffix f1 + """, "feature/f1"); + + assertEquals(Optional.of("1.2.3-f1-SNAPSHOT"), config.getProjectVersion()); + assertEquals(Map.of("com.example:d1", "2.0.0-f1-SNAPSHOT"), config.getDependencyVersions()); + assertTrue(config.isExclusiveSuffix("f1")); + assertFalse(config.isExclusiveSuffix("f2")); + } + + @Test + @DisplayName("rows for other branches are ignored") + void keepsOnlyMatchingRows() { + PinConfig config = parse(""" + feature/f1 project-version 1.2.3-f1-SNAPSHOT + feature/f2 project-version 1.2.3-f2-SNAPSHOT + """, "feature/f2"); + + assertEquals(Optional.of("1.2.3-f2-SNAPSHOT"), config.getProjectVersion()); + } + + @Test + @DisplayName("the first matching project-version pin wins") + void keepsFirstProjectVersionPin() { + PinConfig config = parse(""" + feature/* project-version 1.2.3-star-SNAPSHOT + feature/f1 project-version 1.2.3-f1-SNAPSHOT + """, "feature/f1"); + + assertEquals(Optional.of("1.2.3-star-SNAPSHOT"), config.getProjectVersion()); + } + + @Test + void rejectsPinsThatCannotBeReverted() { + PaoException failure = + assertThrows(PaoException.class, () -> parse("feature/f1 project-version vf1\n", "feature/f1")); + + assertTrue(failure.getMessage().contains("vf1"), failure.getMessage()); + } + + @Test + void rejectsUnknownTargets() { + PaoException failure = assertThrows(PaoException.class, + () -> parse("feature/f1 something-else 1.2.3-f1-SNAPSHOT\n", "feature/f1")); + + assertTrue(failure.getMessage().contains("something-else"), failure.getMessage()); + } + + @Test + void rejectsMalformedDependencyTargets() { + assertThrows(PaoException.class, + () -> parse("feature/f1 dependency:justone 1.2.3-f1-SNAPSHOT\n", "feature/f1")); + } + + @Test + void rejectsLinesWithTooFewColumns() { + assertThrows(PaoException.class, () -> parse("feature/f1 project-version\n", "feature/f1")); + } + + @Test + @DisplayName("a malformed row for another branch still fails, so typos surface early") + void validatesRowsForOtherBranchesToo() { + assertThrows(PaoException.class, () -> parse("feature/f9 bogus-target x\n", "feature/f1")); + } +} diff --git a/src/test/java/com/jardoapps/pao/git/FakeGitClient.java b/src/test/java/com/jardoapps/pao/git/FakeGitClient.java new file mode 100644 index 0000000..ada5abf --- /dev/null +++ b/src/test/java/com/jardoapps/pao/git/FakeGitClient.java @@ -0,0 +1,56 @@ +package com.jardoapps.pao.git; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +/** Records what the runner asked git to do, without touching a repository. */ +public class FakeGitClient implements GitClient { + + private final List commits = new ArrayList<>(); + private final List pushes = new ArrayList<>(); + private String configuredUser; + private String currentBranch; + + @Override + public void configureUser(String name, String email) { + configuredUser = name + " <" + email + ">"; + } + + @Override + public boolean hasUncommittedChanges() { + return true; + } + + @Override + public void commitAll(String message) { + commits.add(message); + } + + @Override + public void push(String branch) { + pushes.add(branch); + } + + @Override + public Optional currentBranch() { + return Optional.ofNullable(currentBranch); + } + + public FakeGitClient withCurrentBranch(String branch) { + this.currentBranch = branch; + return this; + } + + public List getCommits() { + return List.copyOf(commits); + } + + public List getPushes() { + return List.copyOf(pushes); + } + + public String getConfiguredUser() { + return configuredUser; + } +} diff --git a/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java b/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java new file mode 100644 index 0000000..45171bf --- /dev/null +++ b/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java @@ -0,0 +1,230 @@ +package com.jardoapps.pao.pom; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class PomDocumentTest { + + private static PomDocument parse(String xml) { + return PomDocument.parse(Path.of("pom.xml"), xml); + } + + @Test + @DisplayName("everything outside the edited value is preserved exactly") + void preservesSurroundingText() { + String xml = """ + + + + my-app + 1.0.0-SNAPSHOT + + """; + PomDocument document = parse(xml); + + document.setValue(document.projectVersion().orElseThrow(), "1.0.0-feature-x-SNAPSHOT"); + + assertEquals(xml.replace("1.0.0-SNAPSHOT", "1.0.0-feature-x-SNAPSHOT"), document.render()); + } + + @Test + @DisplayName("a version inside a comment is not an element and is never rewritten") + void ignoresVersionsInsideComments() { + String xml = """ + + my-app + + 1.0.0-SNAPSHOT + + """; + PomDocument document = parse(xml); + + assertEquals(1, document.allVersionElements().size()); + assertEquals("1.0.0-SNAPSHOT", document.valueOf(document.projectVersion().orElseThrow())); + } + + @Test + void ignoresMarkupInsideCdata() { + String xml = """ + + my-app + 1.0.0-SNAPSHOT + 9.9.9 in your pom]]> + + """; + PomDocument document = parse(xml); + + assertEquals(1, document.allVersionElements().size()); + } + + @Test + @DisplayName("the project version is distinguished from the parent's") + void distinguishesProjectAndParentVersion() { + String xml = """ + + + com.example + my-parent + 2.0.0-SNAPSHOT + + my-app + 1.0.0-SNAPSHOT + + """; + PomDocument document = parse(xml); + + assertEquals("1.0.0-SNAPSHOT", document.valueOf(document.projectVersion().orElseThrow())); + assertEquals("2.0.0-SNAPSHOT", document.valueOf(document.parentVersion().orElseThrow())); + } + + @Test + @DisplayName("a module that inherits its version has no project version element") + void reportsNoProjectVersionWhenInherited() { + String xml = """ + + + com.example + my-parent + 2.0.0-SNAPSHOT + + core + + """; + PomDocument document = parse(xml); + + assertEquals(Optional.empty(), document.projectVersion()); + assertTrue(document.parentVersion().isPresent()); + } + + @Test + @DisplayName("dependencyManagement and plugin dependencies are found as well") + void findsDependenciesEverywhere() { + String xml = """ + + my-app + 1.0.0-SNAPSHOT + + + + com.example + managed + 1.0.0-SNAPSHOT + + + + + + com.example + direct + 2.0.0-SNAPSHOT + + + com.example + no-version + + + + + + some-plugin + + + com.example + plugin-dep + 3.0.0-SNAPSHOT + + + + + + + """; + PomDocument document = parse(xml); + + assertEquals(List.of("com.example:managed", "com.example:direct", "com.example:plugin-dep"), + document.dependencies().stream().map(PomDocument.DependencyEntry::key).toList()); + } + + @Test + void readsProperties() { + String xml = """ + + my-app + ${revision} + + 1.0.0-SNAPSHOT + 17 + + + """; + PomDocument document = parse(xml); + + assertEquals(List.of("revision", "java.version"), List.copyOf(document.properties().keySet())); + assertEquals("1.0.0-SNAPSHOT", document.valueOf(document.properties().get("revision"))); + } + + @Test + void readsModules() { + String xml = """ + + my-parent + 1.0.0-SNAPSHOT + + core + web + + + """; + + assertEquals(List.of("core", "web"), parse(xml).modules()); + } + + @Test + @DisplayName("attributes containing '>' do not confuse the scanner") + void handlesAngleBracketsInAttributes() { + String xml = """ + + my-app + 1.0.0-SNAPSHOT + + """; + PomDocument document = parse(xml); + + assertEquals("my-app", document.projectArtifactId().orElseThrow()); + assertEquals("1.0.0-SNAPSHOT", document.valueOf(document.projectVersion().orElseThrow())); + } + + @Test + @DisplayName("self-closing elements do not unbalance the element stack") + void handlesSelfClosingElements() { + String xml = """ + + my-app + 1.0.0-SNAPSHOT + + + """; + + assertEquals("1.0.0-SNAPSHOT", parse(xml).valueOf(parse(xml).projectVersion().orElseThrow())); + } + + @Test + void reportsNoChangeWhenNothingWasEdited() { + PomDocument document = parse(""" + + my-app + 1.0.0-SNAPSHOT + + """); + + assertFalse(document.isModified()); + } +} diff --git a/src/test/resources/poms/expected-config-exclusive-suffix-rederive.xml b/src/test/resources/poms/expected-config-exclusive-suffix-rederive.xml new file mode 100644 index 0000000..ad5e23e --- /dev/null +++ b/src/test/resources/poms/expected-config-exclusive-suffix-rederive.xml @@ -0,0 +1,16 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-feature-my-feature-SNAPSHOT + + + + com.example + shared-lib + 4.5.6-SNAPSHOT + + + diff --git a/src/test/resources/poms/expected-config-pin-dependency-versions.xml b/src/test/resources/poms/expected-config-pin-dependency-versions.xml new file mode 100644 index 0000000..0ae6325 --- /dev/null +++ b/src/test/resources/poms/expected-config-pin-dependency-versions.xml @@ -0,0 +1,21 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-SNAPSHOT + + + + com.example + d1 + 1.0.0-f1-SNAPSHOT + + + com.example + d2 + 5.0.0-f1-SNAPSHOT + + + diff --git a/src/test/resources/poms/expected-config-pin-project-version.xml b/src/test/resources/poms/expected-config-pin-project-version.xml new file mode 100644 index 0000000..d292318 --- /dev/null +++ b/src/test/resources/poms/expected-config-pin-project-version.xml @@ -0,0 +1,16 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-f1-SNAPSHOT + + + + com.example + shared-lib + 1.2.3-SNAPSHOT + + + diff --git a/src/test/resources/poms/expected-enforce-branch-version.xml b/src/test/resources/poms/expected-enforce-branch-version.xml new file mode 100644 index 0000000..031e4e5 --- /dev/null +++ b/src/test/resources/poms/expected-enforce-branch-version.xml @@ -0,0 +1,16 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-feature-my-feature-SNAPSHOT + + + + com.example + shared-lib + 1.2.3-SNAPSHOT + + + diff --git a/src/test/resources/poms/expected-remove-branch-version.xml b/src/test/resources/poms/expected-remove-branch-version.xml new file mode 100644 index 0000000..44229b1 --- /dev/null +++ b/src/test/resources/poms/expected-remove-branch-version.xml @@ -0,0 +1,16 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-SNAPSHOT + + + + com.example + shared-lib + 4.5.6-SNAPSHOT + + + diff --git a/src/test/resources/poms/expected-remove-dependency-branch-versions.xml b/src/test/resources/poms/expected-remove-dependency-branch-versions.xml new file mode 100644 index 0000000..4b84192 --- /dev/null +++ b/src/test/resources/poms/expected-remove-dependency-branch-versions.xml @@ -0,0 +1,26 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-SNAPSHOT + + + + com.example + lib-a + 2.0.0-SNAPSHOT + + + com.example + lib-b + 3.1.0-rc.1-SNAPSHOT + + + com.example + lib-c + 4.0.0-SNAPSHOT + + + diff --git a/src/test/resources/poms/expected-revision-enforced.xml b/src/test/resources/poms/expected-revision-enforced.xml new file mode 100644 index 0000000..b4dbc20 --- /dev/null +++ b/src/test/resources/poms/expected-revision-enforced.xml @@ -0,0 +1,21 @@ + + + 4.0.0 + + com.example + my-app + ${revision} + + + 1.2.3-feature-my-feature-SNAPSHOT + 4.5.6-SNAPSHOT + + + + + com.example + shared-lib + ${shared-lib.version} + + + diff --git a/src/test/resources/poms/multimodule-expected/core/pom.xml b/src/test/resources/poms/multimodule-expected/core/pom.xml new file mode 100644 index 0000000..2556ef0 --- /dev/null +++ b/src/test/resources/poms/multimodule-expected/core/pom.xml @@ -0,0 +1,20 @@ + + + 4.0.0 + + + com.example + my-parent + 1.2.3-feature-my-feature-SNAPSHOT + + + core + + + + com.example + third-party + 9.9.9-SNAPSHOT + + + diff --git a/src/test/resources/poms/multimodule-expected/pom.xml b/src/test/resources/poms/multimodule-expected/pom.xml new file mode 100644 index 0000000..0de8feb --- /dev/null +++ b/src/test/resources/poms/multimodule-expected/pom.xml @@ -0,0 +1,13 @@ + + + 4.0.0 + + com.example + my-parent + 1.2.3-feature-my-feature-SNAPSHOT + pom + + + core + + diff --git a/src/test/resources/poms/multimodule/core/pom.xml b/src/test/resources/poms/multimodule/core/pom.xml new file mode 100644 index 0000000..83cf0cd --- /dev/null +++ b/src/test/resources/poms/multimodule/core/pom.xml @@ -0,0 +1,20 @@ + + + 4.0.0 + + + com.example + my-parent + 1.2.3-SNAPSHOT + + + core + + + + com.example + third-party + 9.9.9-SNAPSHOT + + + diff --git a/src/test/resources/poms/multimodule/pom.xml b/src/test/resources/poms/multimodule/pom.xml new file mode 100644 index 0000000..b0810e9 --- /dev/null +++ b/src/test/resources/poms/multimodule/pom.xml @@ -0,0 +1,13 @@ + + + 4.0.0 + + com.example + my-parent + 1.2.3-SNAPSHOT + pom + + + core + + diff --git a/src/test/resources/poms/sample-pom-revision.xml b/src/test/resources/poms/sample-pom-revision.xml new file mode 100644 index 0000000..e94f966 --- /dev/null +++ b/src/test/resources/poms/sample-pom-revision.xml @@ -0,0 +1,21 @@ + + + 4.0.0 + + com.example + my-app + ${revision} + + + 1.2.3-SNAPSHOT + 4.5.6-SNAPSHOT + + + + + com.example + shared-lib + ${shared-lib.version} + + + diff --git a/src/test/resources/poms/sample-pom-two-deps.xml b/src/test/resources/poms/sample-pom-two-deps.xml new file mode 100644 index 0000000..7022b30 --- /dev/null +++ b/src/test/resources/poms/sample-pom-two-deps.xml @@ -0,0 +1,21 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-SNAPSHOT + + + + com.example + d1 + 1.0.0-SNAPSHOT + + + com.example + d2 + 5.0.0-SNAPSHOT + + + diff --git a/src/test/resources/poms/sample-pom-with-branch-deps.xml b/src/test/resources/poms/sample-pom-with-branch-deps.xml new file mode 100644 index 0000000..e5edfea --- /dev/null +++ b/src/test/resources/poms/sample-pom-with-branch-deps.xml @@ -0,0 +1,26 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-SNAPSHOT + + + + com.example + lib-a + 2.0.0-feature-xyz-SNAPSHOT + + + com.example + lib-b + 3.1.0-rc.1-bugfix-abc-SNAPSHOT + + + com.example + lib-c + 4.0.0-SNAPSHOT + + + diff --git a/src/test/resources/poms/sample-pom-with-branch-version.xml b/src/test/resources/poms/sample-pom-with-branch-version.xml new file mode 100644 index 0000000..85bf9f5 --- /dev/null +++ b/src/test/resources/poms/sample-pom-with-branch-version.xml @@ -0,0 +1,16 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-feature-old-SNAPSHOT + + + + com.example + shared-lib + 4.5.6-SNAPSHOT + + + diff --git a/src/test/resources/poms/sample-pom.xml b/src/test/resources/poms/sample-pom.xml new file mode 100644 index 0000000..20ee149 --- /dev/null +++ b/src/test/resources/poms/sample-pom.xml @@ -0,0 +1,16 @@ + + + 4.0.0 + + com.example + my-app + 1.2.3-SNAPSHOT + + + + com.example + shared-lib + 1.2.3-SNAPSHOT + + + From c2208f4b3929dd30872b76615b7acac56ef0511b Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:05:15 +0200 Subject: [PATCH 02/14] Move dependencies on sibling reactor modules with the project version changeProjectVersion rewrote only and , so a module depending on a sibling by literal version kept pointing at the old one and resolved it from the repository - the branch-agnostic snapshot the plugin exists to avoid. The reactor fixture gains an "app" module depending on "core", which reproduces it. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java:236. --- .../pao/PreventOverwritesRunner.java | 34 +++++++++++++++++-- .../com/jardoapps/pao/MultiModuleTest.java | 34 ++++++++++++++++--- .../poms/multimodule-expected/app/pom.xml | 25 ++++++++++++++ .../poms/multimodule-expected/pom.xml | 1 + .../resources/poms/multimodule/app/pom.xml | 25 ++++++++++++++ src/test/resources/poms/multimodule/pom.xml | 1 + 6 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 src/test/resources/poms/multimodule-expected/app/pom.xml create mode 100644 src/test/resources/poms/multimodule/app/pom.xml diff --git a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java index 1b1faf8..610371f 100644 --- a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java +++ b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java @@ -211,9 +211,11 @@ private void stripBranchVersion(PomDocument document, XmlElement element, String // --- Shared ----------------------------------------------------------- /** - * Rewrites the project version across the reactor. Modules that inherit the - * version carry it in {@code }, which must move in step or the - * build breaks - so parent references to reactor projects are updated too. + * Rewrites the project version across the reactor. Every reference a module makes + * to another reactor project has to move in step with it, or that module resolves + * against the repository instead of the reactor: {@code } for the + * inherited version, and any {@code } on a sibling that spells the + * version out rather than deriving it. */ private void changeProjectVersion(List reactor, String oldVersion, String newVersion, String message, List commits) { @@ -234,10 +236,36 @@ private void changeProjectVersion(List reactor, String oldVersion, document.parentVersion() .ifPresent(element -> session.setVersion(document, element, oldVersion, newVersion)); } + + updateSiblingDependencies(session, document, reactorKeys, oldVersion, newVersion); } commit(session, message, commits); } + /** Moves dependencies on other reactor modules to the new version. */ + private void updateSiblingDependencies(PomEditSession session, PomDocument document, Set reactorKeys, + String oldVersion, String newVersion) { + for (PomDocument.DependencyEntry dependency : document.dependencies()) { + if (!reactorKeys.contains(dependency.key())) { + continue; + } + if (isMavenExpression(document.valueOf(dependency.versionElement()))) { + // ${project.version} and friends already follow the project version. + continue; + } + if (session.setVersion(document, dependency.versionElement(), oldVersion, newVersion)) { + log.info("Moving dependency " + dependency.key() + " to " + newVersion + " in " + document.getPath()); + } + } + } + + /** True for {@code ${project.*}} / {@code ${pom.*}}, which Maven resolves itself. */ + private static boolean isMavenExpression(String value) { + return BranchVersions.propertyReference(value) + .filter(name -> name.startsWith("project.") || name.startsWith("pom.")) + .isPresent(); + } + private void commit(PomEditSession session, String message, List commits) { List written = session.save(); if (written.isEmpty()) { diff --git a/src/test/java/com/jardoapps/pao/MultiModuleTest.java b/src/test/java/com/jardoapps/pao/MultiModuleTest.java index 13c7c1d..adb2d9b 100644 --- a/src/test/java/com/jardoapps/pao/MultiModuleTest.java +++ b/src/test/java/com/jardoapps/pao/MultiModuleTest.java @@ -11,8 +11,10 @@ /** * Multi-module projects carry the shared version in each module's - * {@code }. If those references are not moved along with the - * aggregator's version, every module points at a parent that no longer exists. + * {@code }, and modules may name each other as dependencies. If + * those references are not moved along with the aggregator's version, a module + * points at a parent that no longer exists, or resolves a sibling from the + * repository - the shared snapshot some other branch published. */ class MultiModuleTest extends RunnerTestSupport { @@ -22,6 +24,13 @@ class MultiModuleTest extends RunnerTestSupport { private void writeReactor() { writeFixture(project, "multimodule/pom.xml", "pom.xml"); writeFixture(project.resolve("core"), "multimodule/core/pom.xml", "pom.xml"); + writeFixture(project.resolve("app"), "multimodule/app/pom.xml", "pom.xml"); + } + + private void writeBranchVersionedReactor() { + writeFixture(project, "multimodule-expected/pom.xml", "pom.xml"); + writeFixture(project.resolve("core"), "multimodule-expected/core/pom.xml", "pom.xml"); + writeFixture(project.resolve("app"), "multimodule-expected/app/pom.xml", "pom.xml"); } @Test @@ -33,18 +42,32 @@ void updatesParentReferencesOnEnforce() { assertMatchesFixture(project.resolve("pom.xml"), "multimodule-expected/pom.xml"); assertMatchesFixture(project.resolve("core/pom.xml"), "multimodule-expected/core/pom.xml"); + assertMatchesFixture(project.resolve("app/pom.xml"), "multimodule-expected/app/pom.xml"); + } + + @Test + @DisplayName("enforcing a branch version moves a dependency on a sibling module too") + void updatesSiblingDependencyOnEnforce() { + writeReactor(); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + // Left at 1.2.3-SNAPSHOT, app would resolve core from the repository - the + // branch-agnostic snapshot another branch published - instead of the reactor. + assertTrue(read(project.resolve("app/pom.xml")) + .contains("core\n 1.2.3-feature-my-feature-SNAPSHOT")); } @Test @DisplayName("stripping a branch version on a core branch updates module parent references too") void updatesParentReferencesOnRemoval() { - writeFixture(project, "multimodule-expected/pom.xml", "pom.xml"); - writeFixture(project.resolve("core"), "multimodule-expected/core/pom.xml", "pom.xml"); + writeBranchVersionedReactor(); run(project, settings -> settings.setBranchName("main")); assertMatchesFixture(project.resolve("pom.xml"), "multimodule/pom.xml"); assertMatchesFixture(project.resolve("core/pom.xml"), "multimodule/core/pom.xml"); + assertMatchesFixture(project.resolve("app/pom.xml"), "multimodule/app/pom.xml"); } @Test @@ -55,6 +78,7 @@ void leavesUnrelatedDependencyAlone() { run(project, settings -> settings.setBranchName("feature/my-feature")); assertTrue(read(project.resolve("core/pom.xml")).contains("9.9.9-SNAPSHOT")); + assertTrue(read(project.resolve("app/pom.xml")).contains("9.9.9-SNAPSHOT")); } @Test @@ -62,6 +86,6 @@ void leavesUnrelatedDependencyAlone() { void readsWholeReactor() { writeReactor(); - assertEquals(2, com.jardoapps.pao.pom.PomReader.readReactor(project.resolve("pom.xml")).size()); + assertEquals(3, com.jardoapps.pao.pom.PomReader.readReactor(project.resolve("pom.xml")).size()); } } diff --git a/src/test/resources/poms/multimodule-expected/app/pom.xml b/src/test/resources/poms/multimodule-expected/app/pom.xml new file mode 100644 index 0000000..23fc6db --- /dev/null +++ b/src/test/resources/poms/multimodule-expected/app/pom.xml @@ -0,0 +1,25 @@ + + + 4.0.0 + + + com.example + my-parent + 1.2.3-feature-my-feature-SNAPSHOT + + + app + + + + com.example + core + 1.2.3-feature-my-feature-SNAPSHOT + + + com.example + third-party + 9.9.9-SNAPSHOT + + + diff --git a/src/test/resources/poms/multimodule-expected/pom.xml b/src/test/resources/poms/multimodule-expected/pom.xml index 0de8feb..8c087b9 100644 --- a/src/test/resources/poms/multimodule-expected/pom.xml +++ b/src/test/resources/poms/multimodule-expected/pom.xml @@ -9,5 +9,6 @@ core + app diff --git a/src/test/resources/poms/multimodule/app/pom.xml b/src/test/resources/poms/multimodule/app/pom.xml new file mode 100644 index 0000000..eb93f25 --- /dev/null +++ b/src/test/resources/poms/multimodule/app/pom.xml @@ -0,0 +1,25 @@ + + + 4.0.0 + + + com.example + my-parent + 1.2.3-SNAPSHOT + + + app + + + + com.example + core + 1.2.3-SNAPSHOT + + + com.example + third-party + 9.9.9-SNAPSHOT + + + diff --git a/src/test/resources/poms/multimodule/pom.xml b/src/test/resources/poms/multimodule/pom.xml index b0810e9..47780f3 100644 --- a/src/test/resources/poms/multimodule/pom.xml +++ b/src/test/resources/poms/multimodule/pom.xml @@ -9,5 +9,6 @@ core + app From 6ebfe57fd6105db1c20be5ecf3aa205e251ca856 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:06:24 +0200 Subject: [PATCH 03/14] Make the git timeout bound the whole call, and never prompt for credentials readAllBytes() blocked until git exited, so waitFor always ran against a finished process and the 120s guard could never fire. The output is now drained on a separate thread so waitFor is the thing that bounds the call. Reading after waitFor would have deadlocked on output larger than the pipe buffer. Closing the child's stdin and setting GIT_TERMINAL_PROMPT=0 makes a push to an https remote with no usable credential helper fail immediately instead of blocking on a username prompt. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java:101. --- .../pao/git/CommandLineGitClient.java | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java index 1356cc3..37c6c1a 100644 --- a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java +++ b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java @@ -1,12 +1,15 @@ package com.jardoapps.pao.git; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.TimeUnit; import org.apache.maven.plugin.logging.Log; @@ -80,12 +83,16 @@ private Result run(boolean failOnError, String... arguments) { log.debug("Running: " + String.join(" ", command)); + ProcessBuilder builder = new ProcessBuilder(command) + .directory(workingDirectory.toFile()) + .redirectErrorStream(true); + // Without this, an https remote with no usable credential helper prompts for a + // username on stdin and the build blocks until the CI job itself is killed. + builder.environment().put("GIT_TERMINAL_PROMPT", "0"); + Process process; try { - process = new ProcessBuilder(command) - .directory(workingDirectory.toFile()) - .redirectErrorStream(true) - .start(); + process = builder.start(); } catch (IOException e) { throw new PaoException("Cannot run: " + String.join(" ", command), e); } @@ -93,15 +100,29 @@ private Result run(boolean failOnError, String... arguments) { String output; int exitCode; try { - output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + // Nothing is ever written to git, and a closed stdin makes anything that + // would have prompted fail immediately instead of waiting for input. + process.getOutputStream().close(); + + // Draining stdout on another thread is what makes the timeout meaningful: + // read on this thread and it blocks until git exits, so waitFor would only + // ever see an already-terminated process. Reading after waitFor instead + // would deadlock on output larger than the pipe buffer. + CompletableFuture reader = CompletableFuture.supplyAsync(() -> readAll(process)); + if (!process.waitFor(TIMEOUT_SECONDS, TimeUnit.SECONDS)) { process.destroyForcibly(); + reader.cancel(true); throw new PaoException("Timed out after " + TIMEOUT_SECONDS + "s: " + String.join(" ", command)); } exitCode = process.exitValue(); + output = new String(reader.join(), StandardCharsets.UTF_8); } catch (IOException e) { throw new PaoException("Cannot read output of: " + String.join(" ", command), e); + } catch (CompletionException e) { + throw new PaoException("Cannot read output of: " + String.join(" ", command), e.getCause()); } catch (InterruptedException e) { + process.destroyForcibly(); Thread.currentThread().interrupt(); throw new PaoException("Interrupted while running: " + String.join(" ", command), e); } @@ -112,4 +133,14 @@ private Result run(boolean failOnError, String... arguments) { } return new Result(exitCode, output); } + + private static byte[] readAll(Process process) { + try { + return process.getInputStream().readAllBytes(); + } catch (IOException e) { + // Expected when the process is destroyed on timeout; the caller has already + // decided the run failed, so the partial output is of no use either way. + throw new UncheckedIOException(e); + } + } } From b3d24ef680688b82e936d67437df2701ad0fee2c Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:08:06 +0200 Subject: [PATCH 04/14] Commit only the pom files the run rewrote git commit -a staged every modified tracked file, so anything an earlier pipeline step or a developer had touched was swept into a commit labelled "Switched to branch-specific version." and pushed. PomEditSession.save() already returns the exact files it wrote, so those paths are now passed down to git add/commit as an explicit pathspec. That also removes the hasUncommittedChanges/commit -a mismatch: git status --porcelain reported untracked files that commit -a would then not stage, so a tree whose only change was untracked passed the guard and failed with exit 1. The guard is gone; an empty file list is the condition that matters. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java:54. --- it/run-integration-tests.sh | 38 +++++++++++++++++++ .../pao/PreventOverwritesRunner.java | 2 +- .../pao/git/CommandLineGitClient.java | 22 +++++++---- .../java/com/jardoapps/pao/git/GitClient.java | 13 ++++--- .../com/jardoapps/pao/MultiModuleTest.java | 13 +++++++ .../pao/PreventOverwritesRunnerTest.java | 13 +++++++ .../com/jardoapps/pao/git/FakeGitClient.java | 17 ++++++--- 7 files changed, 98 insertions(+), 20 deletions(-) diff --git a/it/run-integration-tests.sh b/it/run-integration-tests.sh index d93b7e5..1094cde 100755 --- a/it/run-integration-tests.sh +++ b/it/run-integration-tests.sh @@ -174,6 +174,43 @@ test_invalid_pin_fails() { PASSED=$((PASSED + 1)) } +# --- Test: the commit covers the poms and nothing else ---------------------- + +test_commit_scope() { + log "unrelated working tree changes stay out of the commit" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.3-SNAPSHOT" + + # Stand-ins for what an earlier pipeline step, or a developer, might leave + # behind: one modification to a tracked file and one untracked file. + echo "scratch" >> "$dir/core/pom.xml.bak" + git -C "$dir" add "$dir/core/pom.xml.bak" + git -C "$dir" commit -qm "Add a tracked file" + echo "touched by something else" >> "$dir/core/pom.xml.bak" + echo "untracked" > "$dir/untracked.txt" + + run_goal "$dir" -Dpao.branchName=feature/FEA-123 + + local committed + committed=$(git -C "$dir" show --name-only --format= HEAD | sort | tr '\n' ' ') + if [[ "$committed" == "core/pom.xml pom.xml " ]]; then + log " ok: only the poms were committed" + else + fail "expected only the poms in the commit, got '$committed'" + fi + + if git -C "$dir" status --porcelain | grep -q 'core/pom.xml.bak'; then + log " ok: the unrelated modification is still uncommitted" + else + fail "the unrelated modification was swept into the commit" + fi + + PASSED=$((PASSED + 1)) +} + # --- Test: pao.skip short-circuits ------------------------------------------ test_skip() { @@ -197,6 +234,7 @@ test_round_trip test_idempotent test_branch_from_environment test_invalid_pin_fails +test_commit_scope test_skip echo "" diff --git a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java index 610371f..255e548 100644 --- a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java +++ b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java @@ -275,7 +275,7 @@ private void commit(PomEditSession session, String message, List commits written.forEach(path -> log.info("Updated " + path)); String fullMessage = message + settings.getCommitMessageSuffix(); - git.commitAll(fullMessage); + git.commit(fullMessage, written); commits.add(fullMessage); } diff --git a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java index 37c6c1a..215b163 100644 --- a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java +++ b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java @@ -44,17 +44,23 @@ public void configureUser(String name, String email) { } @Override - public boolean hasUncommittedChanges() { - return !run(true, "status", "--porcelain").output().isBlank(); - } - - @Override - public void commitAll(String message) { - if (!hasUncommittedChanges()) { + public void commit(String message, List files) { + if (files.isEmpty()) { log.debug("Nothing to commit."); return; } - run(true, "commit", "-a", "-m", message); + List paths = files.stream().map(Path::toString).toList(); + + // Scoping both the staging and the commit to known paths keeps anything else + // in the working tree out of it, whoever or whatever put it there. + run(true, concat(List.of("add", "--"), paths)); + run(true, concat(List.of("commit", "-m", message, "--"), paths)); + } + + private static String[] concat(List head, List tail) { + List all = new ArrayList<>(head); + all.addAll(tail); + return all.toArray(new String[0]); } @Override diff --git a/src/main/java/com/jardoapps/pao/git/GitClient.java b/src/main/java/com/jardoapps/pao/git/GitClient.java index b517fd2..a8807a6 100644 --- a/src/main/java/com/jardoapps/pao/git/GitClient.java +++ b/src/main/java/com/jardoapps/pao/git/GitClient.java @@ -1,5 +1,7 @@ package com.jardoapps.pao.git; +import java.nio.file.Path; +import java.util.List; import java.util.Optional; /** The git operations the plugin needs, kept behind an interface so runs can be faked in tests. */ @@ -8,11 +10,12 @@ public interface GitClient { /** Sets the local user identity used for commits. */ void configureUser(String name, String email); - /** True if the working tree has uncommitted changes. */ - boolean hasUncommittedChanges(); - - /** Commits all tracked modifications. Does nothing if the tree is clean. */ - void commitAll(String message); + /** + * Commits exactly the given files. Anything else in the working tree - an earlier + * pipeline step's output, a developer's own edits - is deliberately left out, so + * the commit matches its message. + */ + void commit(String message, List files); /** Pushes HEAD to the given branch on {@code origin}. */ void push(String branch); diff --git a/src/test/java/com/jardoapps/pao/MultiModuleTest.java b/src/test/java/com/jardoapps/pao/MultiModuleTest.java index adb2d9b..157bd2e 100644 --- a/src/test/java/com/jardoapps/pao/MultiModuleTest.java +++ b/src/test/java/com/jardoapps/pao/MultiModuleTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.nio.file.Path; +import java.util.List; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -81,6 +82,18 @@ void leavesUnrelatedDependencyAlone() { assertTrue(read(project.resolve("app/pom.xml")).contains("9.9.9-SNAPSHOT")); } + @Test + @DisplayName("every pom the run rewrote goes into the commit") + void commitsAllRewrittenPoms() { + writeReactor(); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertEquals( + List.of(project.resolve("pom.xml"), project.resolve("core/pom.xml"), project.resolve("app/pom.xml")), + git.getCommittedFiles("Switched to branch-specific version.")); + } + @Test @DisplayName("the reactor is discovered through ") void readsWholeReactor() { diff --git a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java index 6d2f441..98d6c42 100644 --- a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java +++ b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java @@ -242,6 +242,19 @@ void writesOutputFile() { assertEquals("changes-made=true", read(output).strip()); } + @Test + @DisplayName("only the poms the run wrote are committed") + void commitsOnlyTheFilesItWrote() { + Path pom = writePom(project, "sample-pom.xml"); + // Whatever else is in the tree - an earlier pipeline step's output, or a + // developer's own edits - must stay out of the version commit. + writeFile(project, "left-behind.txt", "not ours to commit"); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertEquals(List.of(pom), git.getCommittedFiles("Switched to branch-specific version.")); + } + @Test @DisplayName("changes are pushed to the detected branch when pushing is enabled") void pushesToDetectedBranch() { diff --git a/src/test/java/com/jardoapps/pao/git/FakeGitClient.java b/src/test/java/com/jardoapps/pao/git/FakeGitClient.java index ada5abf..44c3f07 100644 --- a/src/test/java/com/jardoapps/pao/git/FakeGitClient.java +++ b/src/test/java/com/jardoapps/pao/git/FakeGitClient.java @@ -1,13 +1,17 @@ package com.jardoapps.pao.git; +import java.nio.file.Path; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Optional; /** Records what the runner asked git to do, without touching a repository. */ public class FakeGitClient implements GitClient { private final List commits = new ArrayList<>(); + private final Map> committedFiles = new LinkedHashMap<>(); private final List pushes = new ArrayList<>(); private String configuredUser; private String currentBranch; @@ -18,13 +22,9 @@ public void configureUser(String name, String email) { } @Override - public boolean hasUncommittedChanges() { - return true; - } - - @Override - public void commitAll(String message) { + public void commit(String message, List files) { commits.add(message); + committedFiles.put(message, List.copyOf(files)); } @Override @@ -50,6 +50,11 @@ public List getPushes() { return List.copyOf(pushes); } + /** The files staged for a given commit message, in the order the runner passed them. */ + public List getCommittedFiles(String message) { + return committedFiles.getOrDefault(message, List.of()); + } + public String getConfiguredUser() { return configuredUser; } From f3fb2e6b8b0b6e394022d6fc22b531872ed228c8 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:09:34 +0200 Subject: [PATCH 05/14] Pass the git identity per commit instead of writing it to .git/config configureUser ran unconditionally at the top of every run, so a core-branch run that changed nothing still left user.name=ci-bot in the repository's .git/config - permanently, in a developer's own clone - and a run outside a working copy died on `git config --local` before it had decided whether it needed to commit at all. The identity now travels with the commit as `git -c user.name=... -c user.email=... commit`, so it applies to that commit only and no git command runs until there is something to commit. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java:80. --- README.md | 4 +- it/run-integration-tests.sh | 48 +++++++++++++++++++ .../pao/PreventOverwritesRunner.java | 4 +- .../pao/git/CommandLineGitClient.java | 15 +++--- .../java/com/jardoapps/pao/git/GitClient.java | 11 ++--- .../com/jardoapps/pao/git/FakeGitClient.java | 15 +++--- 6 files changed, 67 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 76fd2ef..e29dc9c 100644 --- a/README.md +++ b/README.md @@ -108,8 +108,8 @@ The CI call stays fully qualified. The short `mvn pao:apply` form additionally n | `enforceBranchVersion` | `pao.enforceBranchVersion` | `true` | Whether the project itself gets a branch-specific version. | | `pushChanges` | `pao.pushChanges` | `true` | Whether to push the resulting commits to `origin`. | | `commitMessageSuffix` | `pao.commitMessageSuffix` | *(empty)* | Appended to every commit message, e.g. `[skip ci]`. | -| `gitUserName` | `pao.gitUserName` | `ci-bot` | Git user name for the commits. | -| `gitUserEmail` | `pao.gitUserEmail` | `ci-bot@example.com` | Git email for the commits. | +| `gitUserName` | `pao.gitUserName` | `ci-bot` | Git user name for the commits. Applied per commit; the repository's `.git/config` is not modified. | +| `gitUserEmail` | `pao.gitUserEmail` | `ci-bot@example.com` | Git email for the commits. Applied per commit; the repository's `.git/config` is not modified. | | `coreBranches` | `pao.coreBranches` | `main master develop release*` | Branch patterns that keep the plain version. Globs allowed; space- or comma-separated. | | `configFile` | `pao.configFile` | `.prevent-overwrites.conf` | Optional per-branch pinning file, relative to the top-level project. | | `outputFile` | `pao.outputFile` | *(none)* | File to append `changes-made=` to. | diff --git a/it/run-integration-tests.sh b/it/run-integration-tests.sh index 1094cde..ad5ac57 100755 --- a/it/run-integration-tests.sh +++ b/it/run-integration-tests.sh @@ -211,6 +211,53 @@ test_commit_scope() { PASSED=$((PASSED + 1)) } +# --- Test: the run leaves no git identity behind ---------------------------- + +test_leaves_no_git_config() { + log "the run does not write a git identity into the repository" + local dir + dir=$(mktemp -d) + trap 'rm -rf "$dir"' RETURN + + setup_repo "$dir" "1.2.3-SNAPSHOT" + git -C "$dir" config --unset user.name + git -C "$dir" config --unset user.email + + run_goal "$dir" -Dpao.branchName=feature/FEA-123 -Dpao.gitUserName=pao-bot \ + -Dpao.gitUserEmail=pao-bot@example.com + + local author + author=$(git -C "$dir" log -1 --format='%an <%ae>') + if [[ "$author" == "pao-bot " ]]; then + log " ok: the commit carries the configured identity" + else + fail "expected the configured identity on the commit, got '$author'" + fi + + if git -C "$dir" config --local --get user.name > /dev/null 2>&1; then + fail "a local user.name was left behind in .git/config" + else + log " ok: nothing left behind in .git/config" + fi + + # A core branch with nothing to strip must not touch git at all, which is what + # made the goal die outside a working copy. + local plain + plain=$(mktemp -d) + trap 'rm -rf "$dir" "$plain"' RETURN + mkdir -p "$plain/core" + cp "$dir/pom.xml" "$plain/pom.xml" + cp "$dir/core/pom.xml" "$plain/core/pom.xml" + sed -i 's|-feature-FEA-123-SNAPSHOT|-SNAPSHOT|g' "$plain/pom.xml" "$plain/core/pom.xml" + if run_goal "$plain" -Dpao.branchName=main > /dev/null 2>&1; then + log " ok: a no-op run succeeds outside a git working copy" + else + fail "a no-op run failed outside a git working copy" + fi + + PASSED=$((PASSED + 1)) +} + # --- Test: pao.skip short-circuits ------------------------------------------ test_skip() { @@ -235,6 +282,7 @@ test_idempotent test_branch_from_environment test_invalid_pin_fails test_commit_scope +test_leaves_no_git_config test_skip echo "" diff --git a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java index 255e548..85cfb36 100644 --- a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java +++ b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java @@ -77,8 +77,6 @@ public RunResult run(List reactor) { PinConfig pins = new PinConfigParser(log).parse(resolveConfigFile(), branchName); - git.configureUser(settings.getGitUserName(), settings.getGitUserEmail()); - List commits = new ArrayList<>(); if (coreBranch) { removeBranchVersion(reactor, root, commits); @@ -275,7 +273,7 @@ private void commit(PomEditSession session, String message, List commits written.forEach(path -> log.info("Updated " + path)); String fullMessage = message + settings.getCommitMessageSuffix(); - git.commit(fullMessage, written); + git.commit(fullMessage, written, settings.getGitUserName(), settings.getGitUserEmail()); commits.add(fullMessage); } diff --git a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java index 215b163..2b93814 100644 --- a/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java +++ b/src/main/java/com/jardoapps/pao/git/CommandLineGitClient.java @@ -37,14 +37,7 @@ public CommandLineGitClient(Path workingDirectory, Log log) { } @Override - public void configureUser(String name, String email) { - log.info("Setting up git configuration..."); - run(true, "config", "--local", "user.name", name); - run(true, "config", "--local", "user.email", email); - } - - @Override - public void commit(String message, List files) { + public void commit(String message, List files, String userName, String userEmail) { if (files.isEmpty()) { log.debug("Nothing to commit."); return; @@ -54,7 +47,11 @@ public void commit(String message, List files) { // Scoping both the staging and the commit to known paths keeps anything else // in the working tree out of it, whoever or whatever put it there. run(true, concat(List.of("add", "--"), paths)); - run(true, concat(List.of("commit", "-m", message, "--"), paths)); + + // -c rather than `config --local`: the identity applies to this commit only and + // nothing is left behind in the repository's .git/config afterwards. + run(true, concat(List.of("-c", "user.name=" + userName, "-c", "user.email=" + userEmail, + "commit", "-m", message, "--"), paths)); } private static String[] concat(List head, List tail) { diff --git a/src/main/java/com/jardoapps/pao/git/GitClient.java b/src/main/java/com/jardoapps/pao/git/GitClient.java index a8807a6..5edf0b5 100644 --- a/src/main/java/com/jardoapps/pao/git/GitClient.java +++ b/src/main/java/com/jardoapps/pao/git/GitClient.java @@ -7,15 +7,12 @@ /** The git operations the plugin needs, kept behind an interface so runs can be faked in tests. */ public interface GitClient { - /** Sets the local user identity used for commits. */ - void configureUser(String name, String email); - /** - * Commits exactly the given files. Anything else in the working tree - an earlier - * pipeline step's output, a developer's own edits - is deliberately left out, so - * the commit matches its message. + * Commits exactly the given files under the given identity. Anything else in the + * working tree - an earlier pipeline step's output, a developer's own edits - is + * deliberately left out, so the commit matches its message. */ - void commit(String message, List files); + void commit(String message, List files, String userName, String userEmail); /** Pushes HEAD to the given branch on {@code origin}. */ void push(String branch); diff --git a/src/test/java/com/jardoapps/pao/git/FakeGitClient.java b/src/test/java/com/jardoapps/pao/git/FakeGitClient.java index 44c3f07..eaf5d05 100644 --- a/src/test/java/com/jardoapps/pao/git/FakeGitClient.java +++ b/src/test/java/com/jardoapps/pao/git/FakeGitClient.java @@ -13,18 +13,14 @@ public class FakeGitClient implements GitClient { private final List commits = new ArrayList<>(); private final Map> committedFiles = new LinkedHashMap<>(); private final List pushes = new ArrayList<>(); - private String configuredUser; + private String commitAuthor; private String currentBranch; @Override - public void configureUser(String name, String email) { - configuredUser = name + " <" + email + ">"; - } - - @Override - public void commit(String message, List files) { + public void commit(String message, List files, String userName, String userEmail) { commits.add(message); committedFiles.put(message, List.copyOf(files)); + commitAuthor = userName + " <" + userEmail + ">"; } @Override @@ -55,7 +51,8 @@ public List getCommittedFiles(String message) { return committedFiles.getOrDefault(message, List.of()); } - public String getConfiguredUser() { - return configuredUser; + /** The identity the runner asked git to attribute commits to, or null if it never committed. */ + public String getCommitAuthor() { + return commitAuthor; } } From ed82c9705e37d2c9dd854fb1008261ecf763e512 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:10:57 +0200 Subject: [PATCH 06/14] Leave release versions alone instead of turning them into snapshots For 1.2.3 on a feature branch the fallback produced 1.2.3-feature-x-SNAPSHOT, and the trip back on a core branch produced 1.2.3-SNAPSHOT - a lossy round trip that converted a released project into a snapshot one without asking. The existing "does not match --SNAPSHOT" warning did not fire, because the derived version was itself valid. withBranch now returns Optional and is empty for a non-snapshot version; enforceBranchVersion logs why it is leaving the version alone and returns. Also documents the behaviour in the README's version format section. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/BranchVersions.java:75. --- README.md | 4 +++- .../com/jardoapps/pao/BranchVersions.java | 21 +++++++++++++------ .../pao/PreventOverwritesRunner.java | 13 ++++++++++-- .../com/jardoapps/pao/BranchVersionsTest.java | 13 ++++++++++-- .../pao/PreventOverwritesRunnerTest.java | 13 ++++++++++++ 5 files changed, 53 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index e29dc9c..a8f907c 100644 --- a/README.md +++ b/README.md @@ -129,9 +129,11 @@ A branch version is `--SNAPSHOT`, where the base is a numeric vers | `1.2.3-SNAPSHOT` | — | *(not a branch version)* | | `1.2.3-rc.4-SNAPSHOT` | — | *(not a branch version)* | +Release versions are left alone. The plugin exists because several branches would otherwise publish over one shared snapshot, so a project version without `-SNAPSHOT` is a sign the goal is running somewhere it was not meant to. Deriving a branch version from `1.2.3` would also be lossy — the trip back on a core branch produces `1.2.3-SNAPSHOT`, not `1.2.3` — so the version is logged and left unchanged instead. + ## Multi-module projects -The whole reactor is handled in one run. When the project version changes, `` in every module that points at a reactor project moves with it — otherwise the modules would reference a parent version that no longer exists and the build would stop resolving. +The whole reactor is handled in one run. When the project version changes, every reference from one reactor module to another moves with it: `` in each module that inherits from a reactor project, and any `` on a sibling module that spells its version out. Otherwise a module would point at a parent version that no longer exists, or resolve a sibling from the repository — the branch-agnostic snapshot another branch published — instead of from the reactor. ## Versions defined by properties diff --git a/src/main/java/com/jardoapps/pao/BranchVersions.java b/src/main/java/com/jardoapps/pao/BranchVersions.java index 17680ab..34ca0e2 100644 --- a/src/main/java/com/jardoapps/pao/BranchVersions.java +++ b/src/main/java/com/jardoapps/pao/BranchVersions.java @@ -60,19 +60,28 @@ public static Optional withoutBranch(String version) { /** * Builds the branch version for the given suffix. An existing branch suffix is * replaced, otherwise the suffix is inserted before {@code -SNAPSHOT}. + * + *

Empty for a version that is not a snapshot. The plugin's premise is that + * several branches would otherwise publish over one shared snapshot, so a release + * version is a sign the goal is running somewhere it was not meant to - and + * deriving one anyway would be lossy: {@code 1.2.3} would become + * {@code 1.2.3--SNAPSHOT} and come back as {@code 1.2.3-SNAPSHOT}, turning + * a released project into a snapshot one on the way through. */ - public static String withBranch(String version, String branchSuffix) { + public static Optional withBranch(String version, String branchSuffix) { Optional base = baseOf(version); if (base.isPresent()) { - return base.get() + "-" + branchSuffix + "-SNAPSHOT"; + return Optional.of(base.get() + "-" + branchSuffix + "-SNAPSHOT"); } Matcher plain = PLAIN_SNAPSHOT.matcher(version); if (plain.matches()) { - return plain.group(1) + "-" + branchSuffix + "-SNAPSHOT"; + return Optional.of(plain.group(1) + "-" + branchSuffix + "-SNAPSHOT"); + } + if (!version.endsWith(SNAPSHOT)) { + return Optional.empty(); } - String stripped = version.endsWith(SNAPSHOT) ? version.substring(0, version.length() - SNAPSHOT.length()) - : version; - return stripped + "-" + branchSuffix + SNAPSHOT; + String stripped = version.substring(0, version.length() - SNAPSHOT.length()); + return Optional.of(stripped + "-" + branchSuffix + SNAPSHOT); } /** Turns a branch name into a version suffix: {@code feature/abc} -> {@code feature-abc}. */ diff --git a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java index 85cfb36..c354109 100644 --- a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java +++ b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java @@ -129,7 +129,7 @@ private void enforceBranchVersion(List reactor, ProjectModel root, if (!currentSuffix.equals(branchSuffix) && pins.isExclusiveSuffix(currentSuffix)) { // The suffix belongs to another branch, so an inherited version would // publish under - and overwrite - that branch's artifacts. - newVersion = BranchVersions.withBranch(currentVersion, branchSuffix); + newVersion = BranchVersions.withBranch(currentVersion, branchSuffix).orElseThrow(); log.info("Suffix '" + currentSuffix + "' is exclusive to another branch. Re-deriving to: " + newVersion); } else { @@ -137,7 +137,16 @@ private void enforceBranchVersion(List reactor, ProjectModel root, return; } } else { - newVersion = BranchVersions.withBranch(currentVersion, branchSuffix); + Optional derived = BranchVersions.withBranch(currentVersion, branchSuffix); + if (derived.isEmpty()) { + log.warn("Project version '" + currentVersion + "' is not a snapshot, so no branch version can be" + + " derived from it without turning a release into a snapshot - the trip back on a core" + + " branch would produce '" + currentVersion + "-SNAPSHOT', not '" + currentVersion + + "'. Leaving the version unchanged. This goal is meant for builds that publish" + + " snapshots; set -Dpao.enforceBranchVersion=false to silence this."); + return; + } + newVersion = derived.get(); log.info("Project does not have a branch version. Changing to: " + newVersion); } diff --git a/src/test/java/com/jardoapps/pao/BranchVersionsTest.java b/src/test/java/com/jardoapps/pao/BranchVersionsTest.java index 73e3a21..2d30610 100644 --- a/src/test/java/com/jardoapps/pao/BranchVersionsTest.java +++ b/src/test/java/com/jardoapps/pao/BranchVersionsTest.java @@ -48,7 +48,7 @@ void stripsBranchSuffix(String version, String expected) { @Test @DisplayName("a two-digit patch number survives the round trip") void keepsMultiDigitPatchNumbers() { - String branchVersion = BranchVersions.withBranch("1.2.10-SNAPSHOT", "feature-abc"); + String branchVersion = BranchVersions.withBranch("1.2.10-SNAPSHOT", "feature-abc").orElseThrow(); assertEquals("1.2.10-feature-abc-SNAPSHOT", branchVersion); assertEquals(Optional.of("1.2.10-SNAPSHOT"), BranchVersions.withoutBranch(branchVersion)); @@ -60,7 +60,16 @@ void keepsMultiDigitPatchNumbers() { "1.2.3-rc.4-SNAPSHOT, feature-abc, 1.2.3-rc.4-feature-abc-SNAPSHOT", "1.2.3-feature-old-SNAPSHOT, feature-abc, 1.2.3-feature-abc-SNAPSHOT" }) void addsOrReplacesBranchSuffix(String version, String suffix, String expected) { - assertEquals(expected, BranchVersions.withBranch(version, suffix)); + assertEquals(Optional.of(expected), BranchVersions.withBranch(version, suffix)); + } + + @ParameterizedTest + @ValueSource(strings = { "1.2.3", "1.2.3-rc.4", "1.2.3.RELEASE" }) + @DisplayName("no branch version is derived from a release version") + void refusesToDeriveFromReleaseVersions(String version) { + // Deriving one would be lossy: the trip back on a core branch yields + // -SNAPSHOT, silently turning a released project into a snapshot one. + assertEquals(Optional.empty(), BranchVersions.withBranch(version, "feature-abc")); } @Test diff --git a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java index 98d6c42..7a18f92 100644 --- a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java +++ b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java @@ -105,6 +105,19 @@ void treatsGlobMatchedBranchAsCore() { assertTrue(result.coreBranch()); } + @Test + @DisplayName("a release version is left alone instead of being turned into a snapshot") + void leavesReleaseVersionsAlone() { + Path pom = writeFile(project, "pom.xml", fixture("sample-pom.xml") + .replace("1.2.3-SNAPSHOT", "1.2.3")); + + RunResult result = run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertTrue(read(pom).contains("1.2.3")); + assertFalse(result.changesMade()); + assertTrue(git.getCommits().isEmpty()); + } + // --- Per-branch configuration ---------------------------------------- @Test From 7eec1cfa2b76e51a7c4a06e2e92828bde7d6c509 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:11:48 +0200 Subject: [PATCH 07/14] Read the source branch on pull-request builds On GitHub Actions pull_request / pull_request_target events GITHUB_REF is refs/pull//merge, so GITHUB_REF_NAME is the synthetic '/merge'. That derived a version nobody can merge back and then pushed to a ref the forge rejects. GITHUB_HEAD_REF carries the real source branch and is set only for pull-request events, so it is checked first; on push events it is present but empty and the existing blank check skips it. CI_MERGE_REQUEST_SOURCE_BRANCH_NAME gets the same treatment, since GitLab merged results pipelines set CI_COMMIT_REF_NAME to refs/merge-requests//merge. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/BranchDetector.java:18. --- README.md | 2 ++ .../com/jardoapps/pao/BranchDetector.java | 7 +++++ .../pao/PreventOverwritesRunnerTest.java | 31 +++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/README.md b/README.md index a8f907c..8f366b0 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,8 @@ jobs: The goal appends `changes-made=true|false` to `$GITHUB_OUTPUT`, so a later step can react to whether anything was rewritten. +On `pull_request` and `pull_request_target` events the branch is read from `GITHUB_HEAD_REF` rather than `GITHUB_REF_NAME`, which on those events is the synthetic `/merge` ref rather than a branch. GitLab merge request pipelines are handled the same way through `CI_MERGE_REQUEST_SOURCE_BRANCH_NAME`. Pushing back to the source branch of a pull request from a fork will not work regardless, so use `-Dpao.pushChanges=false` there. + ### GitLab CI/CD ```yaml diff --git a/src/main/java/com/jardoapps/pao/BranchDetector.java b/src/main/java/com/jardoapps/pao/BranchDetector.java index 5ae8798..9526b6d 100644 --- a/src/main/java/com/jardoapps/pao/BranchDetector.java +++ b/src/main/java/com/jardoapps/pao/BranchDetector.java @@ -15,7 +15,14 @@ public final class BranchDetector { private static final Map CI_VARIABLES = new LinkedHashMap<>(); static { + // On pull_request / pull_request_target events GITHUB_REF is refs/pull//merge, + // making GITHUB_REF_NAME the synthetic '/merge' rather than a branch. That + // would derive a version nobody can merge back and then push to a ref the + // forge rejects. GITHUB_HEAD_REF carries the real source branch and is set + // only for pull-request events, so checking it first is safe on push builds. + CI_VARIABLES.put("GITHUB_HEAD_REF", "GitHub Actions (pull request)"); CI_VARIABLES.put("GITHUB_REF_NAME", "GitHub Actions"); + CI_VARIABLES.put("CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", "GitLab CI/CD (merge request)"); CI_VARIABLES.put("CI_COMMIT_REF_NAME", "GitLab CI/CD"); CI_VARIABLES.put("BITBUCKET_BRANCH", "Bitbucket Pipelines"); CI_VARIABLES.put("CIRCLE_BRANCH", "CircleCI"); diff --git a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java index 7a18f92..8e6b68a 100644 --- a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java +++ b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java @@ -242,6 +242,37 @@ void detectsBranchFromEnvironment() { assertEquals("feature/my-feature", result.branchName()); } + @Test + @DisplayName("on a pull-request build the source branch wins over the synthetic merge ref") + void prefersPullRequestHeadRef() { + writePom(project, "sample-pom.xml"); + // What GitHub Actions sets on a pull_request event: GITHUB_REF is + // refs/pull/123/merge, so GITHUB_REF_NAME is the unusable '123/merge'. + environment = Map.of( + "GITHUB_HEAD_REF", "feature/my-feature", + "GITHUB_REF_NAME", "123/merge"); + + RunResult result = run(project, settings -> { + }); + + assertEquals("feature/my-feature", result.branchName()); + } + + @Test + @DisplayName("on a push build the empty pull-request variable is ignored") + void ignoresEmptyPullRequestHeadRef() { + writePom(project, "sample-pom.xml"); + // GITHUB_HEAD_REF is present but empty on push events. + environment = Map.of( + "GITHUB_HEAD_REF", "", + "GITHUB_REF_NAME", "feature/my-feature"); + + RunResult result = run(project, settings -> { + }); + + assertEquals("feature/my-feature", result.branchName()); + } + @Test @DisplayName("the changes-made output is appended to the configured output file") void writesOutputFile() { From cfc587931646fac5873c781dbcb643d070838d14 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:13:49 +0200 Subject: [PATCH 08/14] Read and write poms in the encoding their prolog declares A pom declaring ISO-8859-1 and containing any non-ASCII byte - an accented name in , a word in - made readString throw MalformedInputException, failing the goal on a file Maven parses without complaint. save() had the mirror problem: it always wrote UTF-8, so the declared encoding and the actual bytes could drift apart for content the plugin never touched. The prolog is now read out of the raw bytes as ISO-8859-1, which maps every byte to a character and cannot itself fail, and the charset it names is used for both the read and the write. Absent or unparseable prolog means UTF-8, as before. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/pom/PomDocument.java:51. --- .../com/jardoapps/pao/pom/PomDocument.java | 50 +++++++++- .../jardoapps/pao/pom/PomDocumentTest.java | 95 +++++++++++++++++++ 2 files changed, 141 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/jardoapps/pao/pom/PomDocument.java b/src/main/java/com/jardoapps/pao/pom/PomDocument.java index c60ce41..4253920 100644 --- a/src/main/java/com/jardoapps/pao/pom/PomDocument.java +++ b/src/main/java/com/jardoapps/pao/pom/PomDocument.java @@ -2,7 +2,10 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.StandardCharsets; +import java.nio.charset.UnsupportedCharsetException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayDeque; @@ -13,6 +16,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * A pom.xml held as raw text plus an index of its elements. @@ -35,27 +40,64 @@ public String key() { private record Edit(int start, int end, String replacement) { } + /** {@code encoding="..."} in the XML prolog, if the document declares one. */ + private static final Pattern PROLOG_ENCODING = + Pattern.compile("\\A<\\?xml\\s[^>]*?encoding\\s*=\\s*[\"']([^\"']+)[\"']"); + private final Path path; private final String source; + private final Charset charset; private final List elements; private final List edits = new ArrayList<>(); - private PomDocument(Path path, String source, List elements) { + private PomDocument(Path path, String source, Charset charset, List elements) { this.path = path; this.source = source; + this.charset = charset; this.elements = elements; } public static PomDocument load(Path path) { try { - return parse(path, Files.readString(path, StandardCharsets.UTF_8)); + byte[] bytes = Files.readAllBytes(path); + Charset charset = declaredCharset(bytes, path); + return parse(path, new String(bytes, charset), charset); } catch (IOException e) { throw new UncheckedIOException("Cannot read " + path, e); } } public static PomDocument parse(Path path, String source) { - return new PomDocument(path, source, scan(source, path)); + return parse(path, source, StandardCharsets.UTF_8); + } + + public static PomDocument parse(Path path, String source, Charset charset) { + return new PomDocument(path, source, charset, scan(source, path)); + } + + /** + * The encoding named in the XML prolog, defaulting to UTF-8. The same charset is + * used again on the way out, so the bytes the plugin did not touch survive + * unchanged. + * + *

The prolog itself is ASCII by definition, and ISO-8859-1 maps every byte to a + * character without ever failing, so it can be read out of the raw bytes before the + * real encoding is known. That covers the byte-oriented encodings a pom.xml + * realistically uses; a UTF-16 document, whose prolog is not byte-per-character, + * falls through to the UTF-8 default as it did before. + */ + private static Charset declaredCharset(byte[] bytes, Path path) { + String prolog = new String(bytes, 0, Math.min(bytes.length, 200), StandardCharsets.ISO_8859_1); + Matcher matcher = PROLOG_ENCODING.matcher(prolog); + if (!matcher.find()) { + return StandardCharsets.UTF_8; + } + String name = matcher.group(1); + try { + return Charset.forName(name); + } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { + throw new IllegalArgumentException(path + ": unsupported encoding '" + name + "' in the XML prolog", e); + } } // --- Scanning --------------------------------------------------------- @@ -292,7 +334,7 @@ public boolean save() { return false; } try { - Files.writeString(path, rendered, StandardCharsets.UTF_8); + Files.writeString(path, rendered, charset); } catch (IOException e) { throw new UncheckedIOException("Cannot write " + path, e); } diff --git a/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java b/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java index 45171bf..3a324ca 100644 --- a/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java +++ b/src/test/java/com/jardoapps/pao/pom/PomDocumentTest.java @@ -1,18 +1,27 @@ package com.jardoapps.pao.pom; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.List; import java.util.Optional; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; class PomDocumentTest { + @TempDir + Path directory; + private static PomDocument parse(String xml) { return PomDocument.parse(Path.of("pom.xml"), xml); } @@ -227,4 +236,90 @@ void reportsNoChangeWhenNothingWasEdited() { assertFalse(document.isModified()); } + + // --- Encoding --------------------------------------------------------- + + private static final String LATIN_1_POM = """ + + + my-app + 1.0.0-SNAPSHOT + Espa\u00f1a, caf\u00e9 and na\u00efve r\u00e9sum\u00e9s + + """; + + @Test + @DisplayName("a pom is read in the encoding its prolog declares") + void readsDeclaredEncoding() throws Exception { + Path pom = directory.resolve("latin1-pom.xml"); + Files.write(pom, LATIN_1_POM.getBytes(StandardCharsets.ISO_8859_1)); + + // Read as UTF-8 these bytes are malformed and loading fails outright. + PomDocument document = PomDocument.load(pom); + + assertEquals("1.0.0-SNAPSHOT", document.valueOf(document.projectVersion().orElseThrow())); + assertTrue(document.render().contains("Espa\u00f1a, caf\u00e9 and na\u00efve r\u00e9sum\u00e9s")); + } + + @Test + @DisplayName("a pom is written back in the encoding it was read in") + void writesDeclaredEncoding() throws Exception { + Path pom = directory.resolve("latin1-pom.xml"); + Files.write(pom, LATIN_1_POM.getBytes(StandardCharsets.ISO_8859_1)); + PomDocument document = PomDocument.load(pom); + + document.setValue(document.projectVersion().orElseThrow(), "1.0.0-feature-x-SNAPSHOT"); + assertTrue(document.save()); + + // Everything but the version must come back byte for byte, so the declared + // encoding and the actual bytes cannot drift apart. + byte[] expected = LATIN_1_POM.replace("1.0.0-SNAPSHOT", "1.0.0-feature-x-SNAPSHOT") + .getBytes(StandardCharsets.ISO_8859_1); + assertArrayEquals(expected, Files.readAllBytes(pom)); + } + + @Test + @DisplayName("a pom without a declared encoding is treated as UTF-8") + void defaultsToUtf8() throws Exception { + String xml = """ + + my-app + 1.0.0-SNAPSHOT + caf\u00e9 + + """; + Path pom = directory.resolve("no-prolog-pom.xml"); + Files.write(pom, xml.getBytes(StandardCharsets.UTF_8)); + + PomDocument document = PomDocument.load(pom); + document.setValue(document.projectVersion().orElseThrow(), "1.0.0-feature-x-SNAPSHOT"); + document.save(); + + assertArrayEquals(xml.replace("1.0.0-SNAPSHOT", "1.0.0-feature-x-SNAPSHOT") + .getBytes(StandardCharsets.UTF_8), Files.readAllBytes(pom)); + } + + @Test + @DisplayName("an encoding the JVM does not know is reported against the file") + void rejectsUnknownEncoding() throws Exception { + Path pom = directory.resolve("odd-pom.xml"); + Files.write(pom, """ + + my-app1.0.0-SNAPSHOT + """.getBytes(StandardCharsets.UTF_8)); + + IllegalArgumentException failure = + assertThrows(IllegalArgumentException.class, () -> PomDocument.load(pom)); + + assertTrue(failure.getMessage().contains("NO-SUCH-CHARSET"), failure.getMessage()); + } + + @Test + @DisplayName("the charset can be supplied directly when parsing from a string") + void parsesWithExplicitCharset() { + Charset charset = StandardCharsets.ISO_8859_1; + PomDocument document = PomDocument.parse(Path.of("pom.xml"), LATIN_1_POM, charset); + + assertEquals("1.0.0-SNAPSHOT", document.valueOf(document.projectVersion().orElseThrow())); + } } From 6a24e742af0753c06ad4a4e9bae8a90f2ca33c8d Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:15:25 +0200 Subject: [PATCH 09/14] Report the version a run produced, not the one it started from RunResult.projectVersion was filled from root.version(), read before any rewrite, so a field whose name reads as "the version this run produced" reported the stale value. enforceBranchVersion and removeBranchVersion now return the version they leave the project at, and the pre-run value is kept as previousProjectVersion. Also exposes it as a project-version output alongside changes-made, so CI can pick up the version the build will publish under. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java:104. --- README.md | 4 +- .../pao/PreventOverwritesRunner.java | 38 +++++++++++++------ .../pao/PreventOverwritesRunnerTest.java | 27 ++++++++++++- 3 files changed, 53 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 8f366b0..66ea698 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ jobs: run: mvn -B deploy ``` -The goal appends `changes-made=true|false` to `$GITHUB_OUTPUT`, so a later step can react to whether anything was rewritten. +The goal appends `changes-made=true|false` and `project-version=` to `$GITHUB_OUTPUT`, so a later step can react to whether anything was rewritten and under which version the build will publish. `project-version` is the version the run left the project at, which is the version it found when nothing changed. On `pull_request` and `pull_request_target` events the branch is read from `GITHUB_HEAD_REF` rather than `GITHUB_REF_NAME`, which on those events is the synthetic `/merge` ref rather than a branch. GitLab merge request pipelines are handled the same way through `CI_MERGE_REQUEST_SOURCE_BRANCH_NAME`. Pushing back to the source branch of a pull request from a fork will not work regardless, so use `-Dpao.pushChanges=false` there. @@ -114,7 +114,7 @@ The CI call stays fully qualified. The short `mvn pao:apply` form additionally n | `gitUserEmail` | `pao.gitUserEmail` | `ci-bot@example.com` | Git email for the commits. Applied per commit; the repository's `.git/config` is not modified. | | `coreBranches` | `pao.coreBranches` | `main master develop release*` | Branch patterns that keep the plain version. Globs allowed; space- or comma-separated. | | `configFile` | `pao.configFile` | `.prevent-overwrites.conf` | Optional per-branch pinning file, relative to the top-level project. | -| `outputFile` | `pao.outputFile` | *(none)* | File to append `changes-made=` to. | +| `outputFile` | `pao.outputFile` | *(none)* | File to append `changes-made=` and `project-version=` to. | | `skip` | `pao.skip` | `false` | Skips execution entirely. | The branch name is taken from `branchName` if set, otherwise from the first of `GITHUB_REF_NAME`, `CI_COMMIT_REF_NAME`, `BITBUCKET_BRANCH`, `CIRCLE_BRANCH` or `TRAVIS_BRANCH` that is present, otherwise from `git rev-parse --abbrev-ref HEAD`. diff --git a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java index c354109..ee9f831 100644 --- a/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java +++ b/src/main/java/com/jardoapps/pao/PreventOverwritesRunner.java @@ -29,11 +29,18 @@ */ public class PreventOverwritesRunner { - /** What a run did, for reporting and for CI outputs. */ + /** + * What a run did, for reporting and for CI outputs. + * + * @param previousProjectVersion the top-level project version as it was found on disk + * @param projectVersion the version the run left the top-level project at, equal to + * {@code previousProjectVersion} when nothing was rewritten + */ public record RunResult( boolean changesMade, String branchName, boolean coreBranch, + String previousProjectVersion, String projectVersion, List commitMessages) { } @@ -78,17 +85,19 @@ public RunResult run(List reactor) { PinConfig pins = new PinConfigParser(log).parse(resolveConfigFile(), branchName); List commits = new ArrayList<>(); + String projectVersion; if (coreBranch) { - removeBranchVersion(reactor, root, commits); + projectVersion = removeBranchVersion(reactor, root, commits); removeDependencyBranchVersions(reactor, commits); } else { - enforceBranchVersion(reactor, root, branchName, pins, commits); + projectVersion = enforceBranchVersion(reactor, root, branchName, pins, commits); applyDependencyPins(reactor, pins, commits); } boolean changesMade = !commits.isEmpty(); log.info(changesMade ? "Changes have been made." : "No changes have been made."); writeOutput("changes-made", String.valueOf(changesMade)); + writeOutput("project-version", projectVersion); if (changesMade) { if (settings.isPushChanges()) { @@ -99,16 +108,18 @@ public RunResult run(List reactor) { } } - return new RunResult(changesMade, branchName, coreBranch, root.version(), List.copyOf(commits)); + return new RunResult(changesMade, branchName, coreBranch, root.version(), projectVersion, + List.copyOf(commits)); } // --- Feature branches ------------------------------------------------- - private void enforceBranchVersion(List reactor, ProjectModel root, String branchName, PinConfig pins, - List commits) { + /** @return the version the project is left at, which may be the one it started with */ + private String enforceBranchVersion(List reactor, ProjectModel root, String branchName, + PinConfig pins, List commits) { if (!settings.isEnforceBranchVersion()) { log.info("Project version enforcement is turned off."); - return; + return root.version(); } String currentVersion = root.version(); @@ -121,7 +132,7 @@ private void enforceBranchVersion(List reactor, ProjectModel root, newVersion = pinned.get(); if (newVersion.equals(currentVersion)) { log.info("Project already at pinned version."); - return; + return currentVersion; } log.info("Using pinned project version: " + newVersion); } else if (BranchVersions.isBranchVersion(currentVersion)) { @@ -134,7 +145,7 @@ private void enforceBranchVersion(List reactor, ProjectModel root, + newVersion); } else { log.info("Project already has a branch version."); - return; + return currentVersion; } } else { Optional derived = BranchVersions.withBranch(currentVersion, branchSuffix); @@ -144,7 +155,7 @@ private void enforceBranchVersion(List reactor, ProjectModel root, + " branch would produce '" + currentVersion + "-SNAPSHOT', not '" + currentVersion + "'. Leaving the version unchanged. This goal is meant for builds that publish" + " snapshots; set -Dpao.enforceBranchVersion=false to silence this."); - return; + return currentVersion; } newVersion = derived.get(); log.info("Project does not have a branch version. Changing to: " + newVersion); @@ -156,6 +167,7 @@ private void enforceBranchVersion(List reactor, ProjectModel root, } changeProjectVersion(reactor, currentVersion, newVersion, COMMIT_ENFORCE, commits); + return newVersion; } private void applyDependencyPins(List reactor, PinConfig pins, List commits) { @@ -182,15 +194,17 @@ private void applyDependencyPins(List reactor, PinConfig pins, Lis // --- Core branches ---------------------------------------------------- - private void removeBranchVersion(List reactor, ProjectModel root, List commits) { + /** @return the version the project is left at, which may be the one it started with */ + private String removeBranchVersion(List reactor, ProjectModel root, List commits) { String currentVersion = root.version(); Optional stripped = BranchVersions.withoutBranch(currentVersion); if (stripped.isEmpty()) { - return; + return currentVersion; } log.info("Project has a branch version. Removing it, since we are on a core branch."); log.info("New version: " + stripped.get()); changeProjectVersion(reactor, currentVersion, stripped.get(), COMMIT_REMOVE_VERSION, commits); + return stripped.get(); } private void removeDependencyBranchVersions(List reactor, List commits) { diff --git a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java index 8e6b68a..a2702ee 100644 --- a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java +++ b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java @@ -274,7 +274,7 @@ void ignoresEmptyPullRequestHeadRef() { } @Test - @DisplayName("the changes-made output is appended to the configured output file") + @DisplayName("the outputs are appended to the configured output file") void writesOutputFile() { writePom(project, "sample-pom.xml"); Path output = project.resolve("build.env"); @@ -283,7 +283,30 @@ void writesOutputFile() { .setBranchName("feature/my-feature") .setOutputFile(output)); - assertEquals("changes-made=true", read(output).strip()); + assertEquals(List.of("changes-made=true", "project-version=1.2.3-feature-my-feature-SNAPSHOT"), + read(output).strip().lines().toList()); + } + + @Test + @DisplayName("the result reports the version the run produced, alongside the one it started from") + void reportsResultingProjectVersion() { + writePom(project, "sample-pom.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertEquals("1.2.3-feature-my-feature-SNAPSHOT", result.projectVersion()); + assertEquals("1.2.3-SNAPSHOT", result.previousProjectVersion()); + } + + @Test + @DisplayName("a run that changes nothing reports the version unchanged") + void reportsUnchangedProjectVersion() { + writePom(project, "sample-pom.xml"); + + RunResult result = run(project, settings -> settings.setBranchName("main")); + + assertEquals("1.2.3-SNAPSHOT", result.projectVersion()); + assertEquals("1.2.3-SNAPSHOT", result.previousProjectVersion()); } @Test From fb21539241938376f45fb988bd669fcb6117cf5e Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:15:43 +0200 Subject: [PATCH 10/14] Distinguish an unconfigured config file from a missing one A null file logged "No config file at 'null'". The two cases now get their own message, so someone who expected their pins to apply sees that the file the plugin looked for is not there, rather than a line about null. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/config/PinConfigParser.java:47. --- .../java/com/jardoapps/pao/config/PinConfigParser.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/jardoapps/pao/config/PinConfigParser.java b/src/main/java/com/jardoapps/pao/config/PinConfigParser.java index 55d3296..03ed01b 100644 --- a/src/main/java/com/jardoapps/pao/config/PinConfigParser.java +++ b/src/main/java/com/jardoapps/pao/config/PinConfigParser.java @@ -43,8 +43,14 @@ public PinConfigParser(Log log) { /** Parses the file if it exists, keeping only the rows matching {@code branchName}. */ public PinConfig parse(Path file, String branchName) { - if (file == null || !Files.isRegularFile(file)) { - log.info("No config file at '" + file + "'. Using default behaviour."); + if (file == null) { + log.info("No config file configured. Using default behaviour."); + return PinConfig.empty(); + } + if (!Files.isRegularFile(file)) { + // Worth saying out loud: someone who expected their pins to apply needs to + // see that the file the plugin looked for is not where it looked. + log.info("Config file '" + file + "' does not exist. Using default behaviour."); return PinConfig.empty(); } From 9cf3990ec1c66354d8649b40ce4e464e51adfc7a Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:16:20 +0200 Subject: [PATCH 11/14] Reject config rows with extra columns Only too-few columns were rejected; extra ones were silently dropped. A stray space inside a value - `1.2.3 -f1-SNAPSHOT` - parsed as four columns and pinned the project to `1.2.3`, failing later with a confusing "invalid pinned version", or succeeding with the wrong pin when the truncated value happened to be valid on its own. stripComment already removes trailing comments, so a legitimate fourth column cannot exist. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/config/PinConfigParser.java:71. --- .../jardoapps/pao/config/PinConfigParser.java | 9 ++++++--- .../pao/config/PinConfigParserTest.java | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/jardoapps/pao/config/PinConfigParser.java b/src/main/java/com/jardoapps/pao/config/PinConfigParser.java index 03ed01b..8d3e234 100644 --- a/src/main/java/com/jardoapps/pao/config/PinConfigParser.java +++ b/src/main/java/com/jardoapps/pao/config/PinConfigParser.java @@ -73,10 +73,13 @@ public PinConfig parse(Path file, String branchName) { continue; } + // Exactly three: stripComment has already removed any trailing comment, so a + // fourth column can only be a stray space inside a value - which would + // otherwise pin something subtly wrong instead of reporting the typo. String[] columns = line.trim().split("\\s+"); - if (columns.length < 3) { - throw new PaoException(file + ":" + (i + 1) - + ": malformed line (expected 3 columns: ): " + line.trim()); + if (columns.length != 3) { + throw new PaoException(file + ":" + (i + 1) + ": malformed line (expected 3 columns:" + + " , found " + columns.length + "): " + line.trim()); } String pattern = columns[0]; diff --git a/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java b/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java index 2b45747..93d14b5 100644 --- a/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java +++ b/src/test/java/com/jardoapps/pao/config/PinConfigParserTest.java @@ -105,6 +105,24 @@ void rejectsLinesWithTooFewColumns() { assertThrows(PaoException.class, () -> parse("feature/f1 project-version\n", "feature/f1")); } + @Test + @DisplayName("a stray space inside a value is reported rather than silently pinning a prefix") + void rejectsLinesWithTooManyColumns() { + PaoException failure = assertThrows(PaoException.class, + () -> parse("feature/f1 project-version 1.2.3 -f1-SNAPSHOT\n", "feature/f1")); + + assertTrue(failure.getMessage().contains("expected 3 columns"), failure.getMessage()); + } + + @Test + @DisplayName("a trailing comment is not counted as a fourth column") + void allowsTrailingCommentsAfterTheValue() { + PinConfig config = parse("feature/f1 project-version 1.2.3-f1-SNAPSHOT # why this pin exists\n", + "feature/f1"); + + assertEquals(Optional.of("1.2.3-f1-SNAPSHOT"), config.getProjectVersion()); + } + @Test @DisplayName("a malformed row for another branch still fails, so typos surface early") void validatesRowsForOtherBranchesToo() { From 3bd3d14d3f6bf2b7a8ab4477c76e10ba71d6951c Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:17:46 +0200 Subject: [PATCH 12/14] Stop counting a failed integration test as passed too PASSED was incremented at the end of every test function regardless of whether fail had been called inside it, so a run with broken assertions reported the same test in both columns. Each function now brackets itself with begin_test/end_test, and end_test only counts a pass when the failure count has not moved. The exit status was already correct; this is about the summary being readable. Addresses review comment from @Jardo-51 on it/run-integration-tests.sh:117. --- it/run-integration-tests.sh | 42 ++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/it/run-integration-tests.sh b/it/run-integration-tests.sh index ad5ac57..5e7ce4e 100755 --- a/it/run-integration-tests.sh +++ b/it/run-integration-tests.sh @@ -17,6 +17,7 @@ GOAL="com.jardoapps:pao-maven-plugin:${PLUGIN_VERSION}:apply" PASSED=0 FAILED=0 +FAILED_BEFORE=0 log() { echo "[IT] $*" @@ -27,6 +28,19 @@ fail() { FAILED=$((FAILED + 1)) } +# Each test function brackets itself with these, so a test whose assertions failed +# is not also counted as passed - which made the summary report the same test in +# both columns. +begin_test() { + log "$*" + FAILED_BEFORE=$FAILED +} + +end_test() { + [[ "$FAILED" -eq "$FAILED_BEFORE" ]] && PASSED=$((PASSED + 1)) + return 0 +} + assert_version() { local file="$1" expected="$2" description="$3" local actual @@ -83,7 +97,7 @@ run_goal() { # --- Test: the whole feature-branch / core-branch round trip ---------------- test_round_trip() { - log "round trip across a multi-module reactor" + begin_test "round trip across a multi-module reactor" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -114,13 +128,13 @@ test_round_trip() { fail "expected 3 commits, found $commits" fi - PASSED=$((PASSED + 1)) + end_test } # --- Test: a second run on a core branch changes nothing -------------------- test_idempotent() { - log "re-running on a core branch makes no further commits" + begin_test "re-running on a core branch makes no further commits" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -136,13 +150,13 @@ test_idempotent() { fail "expected no new commit, found $((commits - 1))" fi - PASSED=$((PASSED + 1)) + end_test } # --- Test: the branch name is taken from the CI environment ----------------- test_branch_from_environment() { - log "branch name detected from the CI environment" + begin_test "branch name detected from the CI environment" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -151,13 +165,13 @@ test_branch_from_environment() { (cd "$dir" && GITHUB_REF_NAME=feature/from-env mvn -B -q "$GOAL" -Dpao.pushChanges=false) assert_version "$dir/pom.xml" "1.2.3-feature-from-env-SNAPSHOT" "branch read from GITHUB_REF_NAME" - PASSED=$((PASSED + 1)) + end_test } # --- Test: an invalid pin fails the build ----------------------------------- test_invalid_pin_fails() { - log "an invalid pin fails the build" + begin_test "an invalid pin fails the build" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -171,13 +185,13 @@ test_invalid_pin_fails() { log " ok: build failed as expected" fi - PASSED=$((PASSED + 1)) + end_test } # --- Test: the commit covers the poms and nothing else ---------------------- test_commit_scope() { - log "unrelated working tree changes stay out of the commit" + begin_test "unrelated working tree changes stay out of the commit" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -208,13 +222,13 @@ test_commit_scope() { fail "the unrelated modification was swept into the commit" fi - PASSED=$((PASSED + 1)) + end_test } # --- Test: the run leaves no git identity behind ---------------------------- test_leaves_no_git_config() { - log "the run does not write a git identity into the repository" + begin_test "the run does not write a git identity into the repository" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -255,13 +269,13 @@ test_leaves_no_git_config() { fail "a no-op run failed outside a git working copy" fi - PASSED=$((PASSED + 1)) + end_test } # --- Test: pao.skip short-circuits ------------------------------------------ test_skip() { - log "pao.skip leaves the project alone" + begin_test "pao.skip leaves the project alone" local dir dir=$(mktemp -d) trap 'rm -rf "$dir"' RETURN @@ -270,7 +284,7 @@ test_skip() { run_goal "$dir" -Dpao.branchName=feature/FEA-123 -Dpao.skip=true assert_version "$dir/pom.xml" "1.2.3-SNAPSHOT" "version untouched" - PASSED=$((PASSED + 1)) + end_test } log "Using goal: $GOAL" From b8e98efb49f9b283acc9e3886b9b49a62a81303a Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:19:28 +0200 Subject: [PATCH 13/14] Translate glob character classes instead of copying them into the regex The class body went into the Java regex verbatim, so bash-valid classes broke or changed meaning: []] - bash's literal ']' - became the empty class [] and threw PatternSyntaxException, [a&&b] silently became a regex intersection matching nothing, and a backslash meant something else entirely. The body is now copied with '\', '[', ']', '&' and '^' escaped, a ']' in the first position is recognised as a literal, and an unmatched '[' stays literal as in bash. Since these patterns come from pao.coreBranches and the config file's branch column, a pattern that still cannot compile is reported as a PaoException naming the pattern rather than an unadorned PatternSyntaxException. Addresses review comment from @Jardo-51 on src/main/java/com/jardoapps/pao/GlobMatcher.java:36. --- .../java/com/jardoapps/pao/GlobMatcher.java | 71 +++++++++++++++---- .../com/jardoapps/pao/GlobMatcherTest.java | 67 +++++++++++++++++ 2 files changed, 124 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/jardoapps/pao/GlobMatcher.java b/src/main/java/com/jardoapps/pao/GlobMatcher.java index e6782e7..c545c37 100644 --- a/src/main/java/com/jardoapps/pao/GlobMatcher.java +++ b/src/main/java/com/jardoapps/pao/GlobMatcher.java @@ -1,6 +1,7 @@ package com.jardoapps.pao; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; /** * Glob matching for branch patterns, mirroring bash {@code [[ $branch == $pattern ]]}: @@ -24,19 +25,7 @@ static Pattern toRegex(String glob) { switch (c) { case '*' -> regex.append(".*"); case '?' -> regex.append('.'); - case '[' -> { - int close = glob.indexOf(']', i + 1); - if (close < 0) { - regex.append("\\["); - } else { - String body = glob.substring(i + 1, close); - if (body.startsWith("!")) { - body = "^" + body.substring(1); - } - regex.append('[').append(body).append(']'); - i = close; - } - } + case '[' -> i = appendCharacterClass(regex, glob, i); default -> { if ("\\.^$+{}|()".indexOf(c) >= 0) { regex.append('\\'); @@ -46,6 +35,60 @@ static Pattern toRegex(String glob) { } i++; } - return Pattern.compile(regex.toString(), Pattern.DOTALL); + try { + return Pattern.compile(regex.toString(), Pattern.DOTALL); + } catch (PatternSyntaxException e) { + // These patterns come from user input, so the pattern at fault has to be + // named - an unadorned PatternSyntaxException says nothing about which + // branch pattern or config row produced it. + throw new PaoException("Invalid branch pattern '" + glob + "': " + e.getDescription(), e); + } + } + + /** + * Appends the class starting at {@code open}, and returns the index of its closing + * bracket so the caller can continue after it. An unmatched {@code [} is a literal, + * as it is in bash. + */ + private static int appendCharacterClass(StringBuilder regex, String glob, int open) { + int bodyStart = open + 1; + boolean negated = bodyStart < glob.length() + && (glob.charAt(bodyStart) == '!' || glob.charAt(bodyStart) == '^'); + if (negated) { + bodyStart++; + } + + // A ']' in the first position is a literal - bash's way of writing a class that + // contains one - so it cannot be the terminator. + int searchFrom = bodyStart < glob.length() && glob.charAt(bodyStart) == ']' ? bodyStart + 1 : bodyStart; + int close = glob.indexOf(']', searchFrom); + if (close < 0) { + regex.append("\\["); + return open; + } + + regex.append('['); + if (negated) { + regex.append('^'); + } + appendClassBody(regex, glob, bodyStart, close); + regex.append(']'); + return close; + } + + /** + * Copies a class body across, escaping what Java reads differently from bash. A + * bare '&' pairs up into Java's intersection operator, '\' and '[' change the + * meaning of what follows, and '^' would negate if it landed first. '-' is left + * alone, so ranges keep working and a leading or trailing '-' stays literal in both. + */ + private static void appendClassBody(StringBuilder regex, String glob, int start, int end) { + for (int j = start; j < end; j++) { + char c = glob.charAt(j); + if (c == '\\' || c == '[' || c == ']' || c == '&' || c == '^') { + regex.append('\\'); + } + regex.append(c); + } } } diff --git a/src/test/java/com/jardoapps/pao/GlobMatcherTest.java b/src/test/java/com/jardoapps/pao/GlobMatcherTest.java index 42c22af..a27aecd 100644 --- a/src/test/java/com/jardoapps/pao/GlobMatcherTest.java +++ b/src/test/java/com/jardoapps/pao/GlobMatcherTest.java @@ -1,6 +1,7 @@ package com.jardoapps.pao; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.DisplayName; @@ -39,4 +40,70 @@ void treatsRegexCharactersAsLiterals() { assertTrue(GlobMatcher.matches("release-1.0", "release-1.0")); assertFalse(GlobMatcher.matches("release-1.0", "release-1x0")); } + + // --- Character classes ------------------------------------------------ + + @Test + void matchesCharacterClasses() { + assertTrue(GlobMatcher.matches("release-[0-9]", "release-2")); + assertFalse(GlobMatcher.matches("release-[0-9]", "release-x")); + assertTrue(GlobMatcher.matches("release-[abc]", "release-b")); + } + + @Test + void matchesNegatedCharacterClasses() { + assertTrue(GlobMatcher.matches("release-[!0-9]", "release-x")); + assertFalse(GlobMatcher.matches("release-[!0-9]", "release-2")); + } + + @Test + @DisplayName("[]] is bash's literal ']', not an empty class") + void treatsLeadingBracketAsLiteral() { + assertTrue(GlobMatcher.matches("v[]]", "v]")); + assertFalse(GlobMatcher.matches("v[]]", "v[")); + assertTrue(GlobMatcher.matches("v[!]]", "v[")); + assertFalse(GlobMatcher.matches("v[!]]", "v]")); + } + + @Test + @DisplayName("'&' in a class is a literal, not Java's intersection operator") + void treatsAmpersandAsLiteral() { + // As a regex intersection '[a&&b]' matches nothing at all. + assertTrue(GlobMatcher.matches("v[a&&b]", "v&")); + assertTrue(GlobMatcher.matches("v[a&&b]", "va")); + assertTrue(GlobMatcher.matches("v[a&&b]", "vb")); + assertFalse(GlobMatcher.matches("v[a&&b]", "vc")); + } + + @Test + @DisplayName("a backslash in a class is a literal backslash") + void treatsBackslashInClassAsLiteral() { + assertTrue(GlobMatcher.matches("v[\\n]", "v\\")); + assertTrue(GlobMatcher.matches("v[\\n]", "vn")); + assertFalse(GlobMatcher.matches("v[\\n]", "v\n")); + } + + @Test + @DisplayName("a nested '[' in a class is a literal, not a Java nested class") + void treatsNestedBracketAsLiteral() { + assertTrue(GlobMatcher.matches("v[[a]", "v[")); + assertTrue(GlobMatcher.matches("v[[a]", "va")); + assertFalse(GlobMatcher.matches("v[[a]", "vb")); + } + + @Test + @DisplayName("an unmatched '[' is a literal, as it is in bash") + void treatsUnmatchedBracketAsLiteral() { + assertTrue(GlobMatcher.matches("release[", "release[")); + assertTrue(GlobMatcher.matches("[]", "[]")); + } + + @Test + @DisplayName("a pattern that cannot be compiled names itself in the failure") + void reportsThePatternThatFailed() { + // A reversed range is invalid in a regex and meaningless in bash alike. + PaoException failure = assertThrows(PaoException.class, () -> GlobMatcher.matches("release-[z-a]", "x")); + + assertTrue(failure.getMessage().contains("release-[z-a]"), failure.getMessage()); + } } From 334ce1793d95331950b0c4a1e6da72c8d3be80d4 Mon Sep 17 00:00:00 2001 From: Jaroslav Brtis <6890442+Jardo-51@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:20:00 +0200 Subject: [PATCH 14/14] Assert that the configured git identity reaches git FakeGitClient recorded the identity but nothing read it back, so the one path where a wrong value produces commits attributed to the wrong author went unverified. Three tests now cover it: the default identity, an overridden one, and a run that commits nothing setting no identity at all. Addresses review comment from @Jardo-51 on src/test/java/com/jardoapps/pao/git/FakeGitClient.java:53. --- .../pao/PreventOverwritesRunnerTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java index a2702ee..1d1ba71 100644 --- a/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java +++ b/src/test/java/com/jardoapps/pao/PreventOverwritesRunnerTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -322,6 +323,39 @@ void commitsOnlyTheFilesItWrote() { assertEquals(List.of(pom), git.getCommittedFiles("Switched to branch-specific version.")); } + @Test + @DisplayName("commits are attributed to the default identity") + void attributesCommitsToTheDefaultIdentity() { + writePom(project, "sample-pom.xml"); + + run(project, settings -> settings.setBranchName("feature/my-feature")); + + assertEquals("ci-bot ", git.getCommitAuthor()); + } + + @Test + @DisplayName("a configured identity reaches git") + void attributesCommitsToTheConfiguredIdentity() { + writePom(project, "sample-pom.xml"); + + run(project, settings -> settings + .setBranchName("feature/my-feature") + .setGitUserName("release-bot") + .setGitUserEmail("release-bot@example.org")); + + assertEquals("release-bot ", git.getCommitAuthor()); + } + + @Test + @DisplayName("a run that commits nothing sets no identity") + void setsNoIdentityWhenNothingIsCommitted() { + writePom(project, "sample-pom.xml"); + + run(project, settings -> settings.setBranchName("main")); + + assertNull(git.getCommitAuthor()); + } + @Test @DisplayName("changes are pushed to the detected branch when pushing is enabled") void pushesToDetectedBranch() {