Skip to content

#1058: fixed missing JAVA_HOME env var#1100

Merged
hohwille merged 28 commits intodevonfw:mainfrom
jan-vcapgemini:fix/1058-faulty-java-home
May 12, 2025
Merged

#1058: fixed missing JAVA_HOME env var#1100
hohwille merged 28 commits intodevonfw:mainfrom
jan-vcapgemini:fix/1058-faulty-java-home

Conversation

@jan-vcapgemini
Copy link
Contributor

@jan-vcapgemini jan-vcapgemini commented Feb 28, 2025

Fixes: #1058

Implements:

  • fixed mvn encrypt password
  • replaced new ProcessContext with runTool
  • added check for mvn settings security file
  • added new getSingleOutput method to ProcessResult
  • added new retrievePassword method to Mvn

Fixes: devonfw#1058

### Implements:
* added missing JAVA_HOME to environment variables
@github-project-automation github-project-automation bot moved this to 🆕 New in IDEasy board Feb 28, 2025
@jan-vcapgemini jan-vcapgemini self-assigned this Feb 28, 2025
@jan-vcapgemini jan-vcapgemini added the java related to Java code, the Java Virtual Machine and directly related tooling (OpenJDK, Adoptium) label Feb 28, 2025
@jan-vcapgemini jan-vcapgemini moved this from 🆕 New to Team Review in IDEasy board Feb 28, 2025
@jan-vcapgemini jan-vcapgemini added the bugfix PR that fixes a bug issue label Feb 28, 2025
@coveralls
Copy link
Collaborator

coveralls commented Feb 28, 2025

Pull Request Test Coverage Report for Build 14975449259

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 55 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.03%) to 67.679%

Files with Coverage Reduction New Missed Lines %
com/devonfw/tools/ide/tool/ide/IdeToolCommandlet.java 1 77.59%
com/devonfw/tools/ide/process/ProcessContext.java 5 30.0%
com/devonfw/tools/ide/process/ProcessResultImpl.java 23 51.35%
com/devonfw/tools/ide/tool/mvn/Mvn.java 26 65.5%
Totals Coverage Status
Change from base Build 14972411443: 0.03%
Covered Lines: 7974
Relevant Lines: 11374

💛 - Coveralls

@jan-vcapgemini jan-vcapgemini marked this pull request as draft March 4, 2025 11:34
@jan-vcapgemini jan-vcapgemini added this to the release:2025.04.002 milestone Apr 9, 2025
@jan-vcapgemini jan-vcapgemini marked this pull request as ready for review April 9, 2025 15:13
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jan-vcapgemini thanks for your PR. You implemented everything exactly as I expected.

However, I tested this and found a new bug:

[ERROR] Error executing Maven.
[ERROR] The specified user settings file does not exist: D:\projects\test\conf\mvn\settings.xml

So in that case already the creation of the master password is failing.
Should we add a check for the existence before adding the -s option?

public String getMavenArgs() {
Path mavenConfFolder = getMavenConfFolder(false);
Path mvnSettingsFile = mavenConfFolder.resolve(Mvn.SETTINGS_FILE);
if (!Files.exists(mvnSettingsFile)) {
return null;
}
String settingsPath = mvnSettingsFile.toString();
return "-s " + settingsPath + " " + getSettingsSecurityProperty();
}
private String getSettingsSecurityProperty() {
return "-Dsettings.security=" + this.context.getMavenConfigurationFolder().resolve(SETTINGS_SECURITY_FILE).toString().replace("\\", "\\\\");
}

IMHO that would make sense.

@jan-vcapgemini
Copy link
Contributor Author

@jan-vcapgemini thanks for your PR. You implemented everything exactly as I expected.

However, I tested this and found a new bug:

[ERROR] Error executing Maven.
[ERROR] The specified user settings file does not exist: D:\projects\test\conf\mvn\settings.xml

So in that case already the creation of the master password is failing. Should we add a check for the existence before adding the -s option?

public String getMavenArgs() {
Path mavenConfFolder = getMavenConfFolder(false);
Path mvnSettingsFile = mavenConfFolder.resolve(Mvn.SETTINGS_FILE);
if (!Files.exists(mvnSettingsFile)) {
return null;
}
String settingsPath = mvnSettingsFile.toString();
return "-s " + settingsPath + " " + getSettingsSecurityProperty();
}
private String getSettingsSecurityProperty() {
return "-Dsettings.security=" + this.context.getMavenConfigurationFolder().resolve(SETTINGS_SECURITY_FILE).toString().replace("\\", "\\\\");
}

IMHO that would make sense.

I can't reproduce this bug. Could you please add the preparations/steps to reproduce it?

Added check for existing settings security file
@jan-vcapgemini jan-vcapgemini moved this from Team Review to 👀 In review in IDEasy board May 5, 2025
@jan-vcapgemini
Copy link
Contributor Author

I've added a check for the existence of the mvn settings security file now.

Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jan-vcapgemini thanks for your rework and improvements. 👍
I still have some CRs to be addressed before merge.

jan-vcapgemini and others added 5 commits May 6, 2025 11:17
removed unnecessary re-initialization of Mvn
made sure that getMavenArgs only returns null if both mvn configuration files (settings and settings-security) were not found
moved code from ProcessContext.runAndGetSingleOutput to ProcessResult
implemented new getSingleOutput method in ProcessResultImpl
using getSingleOutput in Mvn instead of getOut().getFirst() now
refactored encrypt password logic into retrievePassword method
cleanup
jan-vcapgemini and others added 3 commits May 9, 2025 11:40
replaced getMavenConfigurationFolder with getMavenConfFolder
adjusted settings and setttingssecurity checks as suggested
changed getSingleOutput logger to null
@hohwille
Copy link
Member

build failed with:

[ERROR] com.devonfw.tools.ide.tool.mvn.MvnTest.testMvnInstall -- Time elapsed: 0.050 s <<< FAILURE!
java.lang.AssertionError: 

Expecting path:
  /home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/project/conf/mvn/settings.xml
to exist (symbolic links were followed).
	at com.devonfw.tools.ide.tool.mvn.MvnTest.checkInstallation(MvnTest.java:76)
	at com.devonfw.tools.ide.tool.mvn.MvnTest.testMvnInstall(MvnTest.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

I did not yet see which change prevents that this file gets created.

@hohwille
Copy link
Member

11:26:54.715 [main] - INFO - c.d.tools.ide.log.IdeSubLoggerSlf4j - STEP:Start: Create mvn settings file at /home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/project/conf/.m2/settings.xml
11:26:54.715 [main] - INFO - c.d.tools.ide.log.IdeSubLoggerSlf4j - Please enter secret value for variable login:
11:26:54.715 [main] - DEBUG - c.d.tools.ide.log.IdeSubLoggerSlf4j - Running command '/home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/_ide/software/default/mvn/mvn/3.9.7/bin/mvn' using bash with arguments '--encrypt-password' 'testLogin' '-Dsettings.security=/home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/project/conf/.m2/settings-security.xml' ...
11:26:54.718 [main] - INFO - c.d.tools.ide.log.IdeSubLoggerSlf4j - Encrypted as testLogin
11:26:54.718 [main] - INFO - c.d.tools.ide.log.IdeSubLoggerSlf4j - Please enter secret value for variable password:
11:26:54.718 [main] - DEBUG - c.d.tools.ide.log.IdeSubLoggerSlf4j - Running command '/home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/_ide/software/default/mvn/mvn/3.9.7/bin/mvn' using bash with arguments '--encrypt-password' 'testPassword' '-Dsettings.security=/home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/project/conf/.m2/settings-security.xml' ...
11:26:54.721 [main] - INFO - c.d.tools.ide.log.IdeSubLoggerSlf4j - Encrypted as testPassword
11:26:54.722 [main] - INFO - c.d.tools.ide.log.IdeSubLoggerSlf4j - SUCCESS:Successfully ended step 'Create mvn settings file at /home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/project/conf/.m2/settings.xml'.
11:26:54.722 [main] - DEBUG - c.d.tools.ide.log.IdeSubLoggerSlf4j - Step 'Create mvn settings file at /home/runner/work/IDEasy/IDEasy/cli/target/test-projects/mvn/project/conf/.m2/settings.xml' ended successfully.

@hohwille
Copy link
Member

hohwille commented May 12, 2025

Is this maybe part of the problem:

  private void createSettingsFile(Path settingsFile, Path settingsSecurityFile, Path settingsTemplateFile) {
    if (Files.exists(settingsFile) || !Files.exists(settingsSecurityFile)) {
      return;
    }

Also why do we have the second if condition? Maybe the settings.xml file contains no variable, then it should also be created without a settings-security.xml file.

@hohwille hohwille merged commit b2f3ea4 into devonfw:main May 12, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in IDEasy board May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PR that fixes a bug issue java related to Java code, the Java Virtual Machine and directly related tooling (OpenJDK, Adoptium)

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

ide create still cannot handle faulty JAVA_HOME

3 participants