[MNG-8056] Fix -f/-s parameters failing with absolute Cygwin paths

[gnodet]

Backport of #13239 to maven-4.0.x.

Cherry-pick of acd29da, applied with one conflict resolved: the test script test-mvn-path-conversion.sh was absent on maven-4.0.x (deleted), so it was re-added as part of this backport.

[gnodet-bot]

The backport correctly ports the Cygwin path-conversion loop from master and faithfully re-adds the test script deleted from maven-4.0.x. The shell logic is identical to acd29da on master.

One gap: the test script is never executed on this branch.

apache-maven/pom.xml on maven-4.0.x does not have the shell-script-tests Maven profile (added to master by #13056). The file test-mvn-path-conversion.sh is wired into CI via that profile on master (exec-maven-plugin in phase test, OS-activated on Unix). Without it, the script lives in the repo but is never run — removing the regression-prevention value of the test entirely.

The profile needs to be added to apache-maven/pom.xml:

<!-- Runs the bin/mvn path conversion test as part of the build. -->
<profile>
  <id>shell-script-tests</id>
  <activation>
    <os>
      <family>unix</family>
    </os>
  </activation>
  <build>
    <plugins>
      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>exec-maven-plugin</artifactId>
        <version>3.6.4</version>
        <executions>
          <execution>
            <id>test-mvn-path-conversion</id>
            <goals><goal>exec</goal></goals>
            <phase>test</phase>
            <configuration>
              <skip>${skipTests}</skip>
              <executable>sh</executable>
              <arguments>
                <argument>${project.basedir}/src/test/scripts/test-mvn-path-conversion.sh</argument>
              </arguments>
            </configuration>
          </execution>
        </executions>
      </plugin>
    </plugins>
  </build>
</profile>

(Note: omit the test-mvn-fast-version execution from master — that script is not present on maven-4.0.x.)

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

Added the shell-script-tests Maven profile to apache-maven/pom.xml (without the test-mvn-fast-version execution, which doesn't exist on this branch). The test script will now be executed on Unix as part of the build.

As for the forward-port question: master already has both the bin/mvn Cygwin fix and the shell-script-tests profile (via #13239 and #13056 respectively), so no additional work is needed there.

[gnodet-bot]

Re-review after latest commits.

Previous finding addressed ✓

The shell-script-tests Maven profile is now correctly added to apache-maven/pom.xml, wired to the test phase (OS-activated on Unix), and excludes the test-mvn-fast-version execution that doesn't exist on maven-4.0.x. The test script will now execute as part of the build.

Everything else checks out:

• bin/mvn Cygwin path-conversion loop is identical to master (cherry-pick applied cleanly)
• test-mvn-path-conversion.sh is identical to the master version
• The exec-maven-plugin version (3.6.4) is consistent with existing usage in the project
• run_mvn_debug's 2>&1 >/dev/null ordering is intentional — correctly captures debug stderr into the backtick variable while discarding normal stdout

Backport is solid. ✅

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after the latest commit (d2e80a65).

Previous findings addressed ✓

The shell-script-tests Maven profile is in pom.xml, wired to the test phase, and respects ${skipTests}. The test-mvn-path-conversion.sh script is correctly re-added (it was absent on maven-4.0.x).

New content review

This commit replaces the earlier backport implementation with Tarun4201's version from the master PR (#13239). The shell logic is reviewed below.

bin/mvn changes — correct

• MINGW*|MSYS* in the uname case is the right fix: MSYS2 reports MSYS_NT-* from uname, so matching MINGW* alone missed it. The test script asserts this (MSYS_NT-10.0 is exercised). ✓
• The early cygpath safety check (lines 60–67) properly gates all downstream cygpath invocations by resetting cygwin=false/mingw=false before anything else touches them. ✓
• JLINE_NATIVE_PATH is assigned from the POSIX $MAVEN_HOME before conversion, then converted separately inside the if $cygwin || $mingw block to avoid the mixed-separator bug (C:\maven/lib/jline-native). The comment explains this correctly. ✓
• MAVEN_PROJECTBASEDIR_NATIVE is computed upfront and assigned to MAVEN_PROJECTBASEDIR after path conversion, so concat_lines receives the native path for JvmConfigParser placeholder expansion. ✓
• printf replaces echo for debug logging — correct fix to avoid backslash expansion on dash/ash (the debug log test asserts no backslash escape was expanded). ✓
• Cygwin args loop (_np/_count) is identical to master and handles both space-separated (-f /path) and =-separated (--file=/path) forms, leaving relative paths and non-Cygwin platforms untouched. ✓

Test script — correct and self-contained

The stub-based approach (fake uname, cygpath, java) allows the test to run on any POSIX platform without a real Cygwin installation. The stubs are shell built-in-only, so the test is portable and reproducible.

One minor note on coverage: the test asserts conversion for -f, -s, and --file=, plus the no-conversion cases for MinGW and relative paths. Flags like -gs, -t, -l, -af are not individually tested — but since the loop mechanism is uniform (every flag goes through the same case/_np state machine), this gap doesn't represent a correctness risk. The critical structural invariants (relative paths untouched, MinGW excluded, missing cygpath falls back gracefully) are all covered.

One cosmetic issue: misleading commit message. The message says "Added _cygwin_convert_paths() helper" but the actual implementation uses an inline loop, not a named function. Not a blocker, but worth cleaning up if the branch is squashed before merge.

Implementation is solid. Ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.