Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CommandLineSupport: Do not escape backslash on Windows #1559

Open
michael-o opened this issue Dec 30, 2023 · 2 comments
Open

CommandLineSupport: Do not escape backslash on Windows #1559

michael-o opened this issue Dec 30, 2023 · 2 comments
Labels
type: bug 🐛 Something isn't working

Comments

@michael-o
Copy link

michael-o commented Dec 30, 2023

Steps to reproduce

  • JaCoCo version: 0.8.8
  • Operating system: Windows
  • Tool integration: Maven plugin

Have a look at this config snippet in Maven Surefire: https://github.com/apache/maven-surefire/blob/83c466698f1cf38d7b60852549393f4245f286d4/surefire-its/pom.xml#L136-L156

It produces the following output:

[INFO] --- maven-help-plugin:3.4.0:evaluate (default-cli) @ surefire-its ---
[INFO] No artifact parameter specified, using 'org.apache.maven.surefire:surefire-its:jar:3.2.4-SNAPSHOT' as project.
[INFO] Enter the Maven expression i.e. ${project.groupId} or 0 to exit?:
${jacoco-it.agent}
[INFO]
"-javaagent:C:\\Users\\mosipov\\.m2\\repository\\org\\jacoco\\org.jacoco.agent\\0.8.8\\org.jacoco.agent-0.8.8-runtime.jar=destfile=D:\\Entwicklung\\Projekte\\Maven Surefire\\surefire-its\\target\\jacoco.exec,includes=**/failsafe/*:**/failsafe/**/*:**/surefire/*:**/surefire/**/*,excludes=**/HelpMojo.class:**/shadefire/**/*:org/jacoco/**/*:com/vladium/emma/rt/*"

Expected behaviour

cmd.exe does not require bashslashes to be escaped. I have extracted the code from Maven Surefire:

        String argsss = new String[] {
            "cmd.exe",
            "/X",
            "/C",
            "\"D:\\Entwicklung\\Programme\\apache-maven-3.8.8\\bin\\mvn -e --batch-mode -Dmaven.repo.local=C:\\Users\\mosipov\\.m2\\repository org.apache.maven.plugins:maven-clean-plugin:clean -Dsurefire.version=3.2.4-SNAPSHOT \"-Djacoco.agent=-javaagent:C:\\Users\\mosipov\\.m2\\repository\\org\\jacoco\\org.jacoco.agent\\0.8.8\\org.jacoco.agent-0.8.8-runtime.jar=destfile=D:\\Entwicklung\\Projekte\\Maven Surefire\\surefire-its\\target\\jacoco.exec,includes=**/failsafe/*:**/failsafe/**/*:**/surefire/*:**/surefire/**/*,excludes=**/HelpMojo.class:**/shadefire/**/*:org/jacoco/**/*:com/vladium/emma/rt/*\" -nsu test -DtestNgVersion=5.7 -DtestNgClassifier=jdk15\""
        };

        ProcessBuilder b = new ProcessBuilder();
        b.directory(
                new File(
                        "D:\\Entwicklung\\Projekte\\Maven Surefire\\surefire-its\\target\\CheckTestNgBeforeMethodIT_testNgBeforeMethod"));
        b.command(argsss);
        b.inheritIO();
        Process process = b.start();

        int exitCode = process.waitFor();
        System.out.println(exitCode);

Output:

[INFO] Error stacktraces are turned on.
[INFO] Scanning for projects...
[INFO] 
[INFO] -------< org.apache.maven.plugins.surefire:testng-beforeMethod >--------
[INFO] Building TestNG @BeforeMethod annotation 1.0-SNAPSHOT
[INFO]   from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-cli) @ testng-beforeMethod ---
[INFO] Deleting D:\Entwicklung\Projekte\Maven Surefire\surefire-its\target\CheckTestNgBeforeMethodIT_testNgBeforeMethod\target
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.265 s
[INFO] Finished at: 2023-12-31T00:29:10+01:00
[INFO] ------------------------------------------------------------------------
0

The underlying implementation for calling the command is in Maven Shared Utils and will handle this w/o the double escaping.

Actual behaviour

Backslashes are uselessly escaped because CommandLineSupport does not differentiate between Bourne Shell input and cmd.exe.
This was foundw hile working on https://issues.apache.org/jira/browse/SUREFIRE-1810 and reported as https://issues.apache.org/jira/browse/SUREFIRE-2225.
Luckily both File and Path normalize two backslashes to one...

@michael-o michael-o added the type: bug 🐛 Something isn't working label Dec 30, 2023
@marchof
Copy link
Member

marchof commented Dec 31, 2023

Hi @michael-o thanks for reporting this! Indeed we don't run our regression tests on windows. Indeed it looks like windows has different escape logic.

Do you know how Maven handles this and if there is an implementation or test suite we can use as a reference?

@michael-o
Copy link
Author

Hi @michael-o thanks for reporting this! Indeed we don't run our regression tests on windows. Indeed it looks like windows has different escape logic.

Do you know how Maven handles this and if there is an implementation or test suite we can use as a reference?

Yes, Maven delegates this to Maven Shared Utils which includes a system specific command launcher. For now, I have modified this in Surefire: https://github.com/apache/maven-surefire/pull/705/files

Maven logic used throughout: https://github.com/apache/maven-shared-utils/tree/maven-shared-utils-3.4.2/src/main/java/org/apache/maven/shared/utils/cli. Entrypoint for you https://github.com/apache/maven-shared-utils/blob/2dc9eb5ea2b4ed2573bd96282a16e0340083c87a/src/main/java/org/apache/maven/shared/utils/cli/CommandLineUtils.java#L76-L79. It will either pick Bourne shell or cmd.exe and quote as necessary.

I consider the current escaping completely designed for Bourne shell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants