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

Fixup #716 Calculate java home from java command #721

Merged
merged 2 commits into from
Dec 13, 2022
Merged

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Oct 18, 2022

Fixup #715

  • Add a test
  • Simplify the stream processing

* Add a test
* Simplify the stream processing
private String javaHomeFromPath() {
final String jHome = OsUtils.findJavaHomeFromPath();
private static String javaHomeFromPath() {
LOG.warn("Falling back to finding JAVA_HOME by running java executable available in PATH."
Copy link
Contributor

Choose a reason for hiding this comment

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

why warn here? the generic java version (mvnd.sh) will also search java from path without a warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why warn here? the generic java version (mvnd.sh) will also search java from path without a warning

As long as I am on an OS and arch having a mvnd native executable, I do not need to care that mvnd.sh is slow. mvnd.sh is fallback where some expectations are not met. But actually we could warn in mvnd.sh too. I'd prefer knowing that mvnd is missing some piece of config that makes it slower.

# Conflicts:
#	client/src/main/java/org/mvndaemon/mvnd/client/DaemonParameters.java
#	common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java
#	common/src/test/java/org/mvndaemon/mvnd/common/OsUtilsTest.java
@gnodet gnodet merged commit 031c263 into apache:master Dec 13, 2022
gnodet added a commit that referenced this pull request Jan 6, 2023
* Add a test
* Simplify the stream processing

Co-authored-by: Guillaume Nodet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants