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.

Sign up for GitHub

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

Jump to bottom

Non-Uppercase Environment Variables duplicated with an Uppercase version #328

Open
chrislake opened this issue Aug 5, 2022 · 6 comments · May be fixed by #427
Open

Non-Uppercase Environment Variables duplicated with an Uppercase version #328

chrislake opened this issue Aug 5, 2022 · 6 comments · May be fixed by #427

Comments

@chrislake
Copy link

Whilst attempting to run cmd.exe to execute a batch file, where within we unset an environment variable, I found that the exec-maven-plugin duplicates any non-uppercased environment variables with an uppercased one. Example:

<plugin>
  <groupId>org.codehaus.mojo</groupId>
  <artifactId>exec-maven-plugin</artifactId>
  <version>3.1.0</version>
  <executions>
    <execution>
      <id>showproblem</id>
      <phase>validate</phase>
      <goals>
        <goal>exec</goal>
      </goals>
      <configuration>
        <executable>cmd.exe</executable>
        <commandlineArgs>/c dobuild.cmd</commandlineArgs>
      </configuration>
    </execution>
  </executions>
</plugin>

Running "mvn validate" shows:
PrecessExplorerProperties

[INFO] Scanning for projects...
[INFO]
[INFO] ----------------------< my:plugin >----------------------
[INFO] Building plugin 0.0.1
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- exec-maven-plugin:3.1.0:exec (showproblem) @ plugin ---

>SET
AaA_TestVariable=This is already set
AAA_TESTVARIABLE=This is already set
ALLUSERSPROFILE=C:\ProgramData
ANT_HOME=C:\apps\apache-ant-1.10.12
BUILD_NUMBER=999
CLASSWORLDS_JAR="C:\apps\apache-maven-3.8.3\boot\plexus-classworlds-2.6.0.jar"
CLASSWORLDS_LAUNCHER=org.codehaus.plexus.classworlds.launcher.Launcher
CommonProgramFiles=C:\Program Files\Common Files
COMMONPROGRAMFILES=C:\Program Files\Common Files
COMMONPROGRAMFILES(X86)=C:\Program Files (x86)\Common Files
CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files
COMMONPROGRAMW6432=C:\Program Files\Common Files
CommonProgramW6432=C:\Program Files\Common Files
COMSPEC=C:\WINDOWS\system32\cmd.exe
ComSpec=C:\WINDOWS\system32\cmd.exe
DRIVERDATA=C:\Windows\System32\Drivers\DriverData
DriverData=C:\Windows\System32\Drivers\DriverData
ERROR_CODE=0
GIT_LFS_PATH=C:\apps\Git LFS
HOMEDRIVE=C:
JAVACMD=C:\develop\.git-repos\JDK\windows\amd64\bin\java.exe
JAVA_HOME=C:\develop\.git-repos\JDK\windows\amd64
jvmConfig=\.mvn\jvm.config
JVMCONFIG=\.mvn\jvm.config
...
__PSLOCKDOWNPOLICY=0
__PSLockDownPolicy=0

>ECHO AaA_TestVariable = [This is already set]
AaA_TestVariable = [This is already set]

>SET AaA_TestVariable=

>ECHO AaA_TestVariable = [This is already set]
AaA_TestVariable = [This is already set]

>SET AaA_TestVariable=

>ECHO AaA_TestVariable = []
AaA_TestVariable = []
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  9.552 s
[INFO] Finished at: 2022-08-05T12:47:57+10:00
[INFO] ------------------------------------------------------------------------

We can workaround the problem as shown, by unsetting the variable twice, or unsetting it first in the POM and then unsetting it again once in the batch file, but I don't think the variables should be duplicated in the first place.
@wheezil
Copy link

wheezil commented May 10, 2024

👍 this is breaking our build in which maven calls msbuild, and then msbuild tries to insert the env vars into a case-insensitive set which then errors out for a duplicate :-(. The error comes out something like:
System.ArgumentException: Item has already been added. Key in dictionary: 'PROGRAMDATA' Key being added: 'ProgramData'
The MS-side of this is dotnet/msbuild#5726
Which they've pondered, but refused to fix because "it's hard".
So now I'm trying to understand, how we can make this work at all? Does anyone have a workaround? We cannot manually change all of our env vars, some of them seem to be set in the toolchain or by parts of the environment we have no control over.

@wheezil
Copy link

wheezil commented May 10, 2024 •
edited

There is a fairly simple fix to ExtendedExecutor.launch()
First, squirrel away the env that comes with the ProcessBuilder:

    boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
    Map<String,String> existingEnv = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);

Second, remove any keys that already exist, case-insensitive fashion:

        if (isWindows && existingEnv.containsKey(key)) {
            pb.environment().remove(key);
        }

I shall endeavor to submit a patch

@wheezil
Copy link

wheezil commented May 10, 2024

Hmmmm, the simple fix doesn't actually work for MSBuild, which seems to want to re-add all of the env vars in their "natural" case, and this leads to the collision and error reported in https://developercommunity.visualstudio.com/t/Build-Error:-MSB6001-in-Maven-Build/10527486?sort=newest

The real problem is that CommandLineUtils.getSystemEnvVars() is UPPERCASING all of the env vars, and this confuses MSBuild when it sees the MixedCase equivalents.

@slawekjaranowski
Copy link
Member

Maybe simply use System.getenv instead of magic from CommandLineUtils will be the best ...
I don't have access to windows in order to check it

@wheezil
Copy link

wheezil commented May 10, 2024 via email

@slawekjaranowski
Copy link
Member

I don't know why it is doing in this way ....

such implementations from plexus-utils / maven-shared-utils .... are used in many place and I see only in exec-m-p it is a problem

The question is that env name should be uppercased on windows at all, or should be used as is ...

@wheezil wheezil linked a pull request May 10, 2024 that will close this issue
Open
@slawekjaranowski slawekjaranowski linked a pull request May 11, 2024 that will close this issue
Do not get UPPERCASE env vars #427
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Do not get UPPERCASE env vars wheezil/exec-maven-plugin
3 participants
@slawekjaranowski @wheezil @chrislake