You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/10/15 17:03:13 UTC
[GitHub] [maven-mvnd] gzm55 opened a new pull request, #716: calculate java home from java command
gzm55 opened a new pull request, #716:
URL: https://github.com/apache/maven-mvnd/pull/716
closes #715
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] gnodet commented on a diff in pull request #716: calculate java home from java command
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #716:
URL: https://github.com/apache/maven-mvnd/pull/716#discussion_r996400406
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
Review Comment:
Please use lowercase `path` for variable names.
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
+ final File java = new File(dirname, basename);
+ if (java.isFile() && java.canExecute()) {
+ final String[] cmd = { java.getAbsolutePath(), "-XshowSettings:properties", "-version" };
+ final List<String> output = new ArrayList<String>(1);
+ exec(cmd, output);
+ final List<String> javaHomeLines = output.stream().filter(l -> l.contains(" java.home = "))
Review Comment:
No need for `final` here.
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
Review Comment:
This is not completely right. If one of the path element contains an escaped reserved character such as `:`, this won't work. The split should have to take that into account, maybe by using a regex that ensures that the `pathSeparator` is not preceded by an odd number of backslashes.
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
+ final File java = new File(dirname, basename);
+ if (java.isFile() && java.canExecute()) {
+ final String[] cmd = { java.getAbsolutePath(), "-XshowSettings:properties", "-version" };
+ final List<String> output = new ArrayList<String>(1);
Review Comment:
The size of `1` does not make much sense here. The output will most probably include much more lines.
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
+ final File java = new File(dirname, basename);
Review Comment:
Use nio2 `Path` as in most mvnd code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] gnodet commented on a diff in pull request #716: calculate java home from java command
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #716:
URL: https://github.com/apache/maven-mvnd/pull/716#discussion_r996403025
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
+ final File java = new File(dirname, basename);
+ if (java.isFile() && java.canExecute()) {
+ final String[] cmd = { java.getAbsolutePath(), "-XshowSettings:properties", "-version" };
+ final List<String> output = new ArrayList<String>(1);
Review Comment:
Ah, I missed this commit, thx !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] gzm55 commented on a diff in pull request #716: calculate java home from java command
Posted by GitBox <gi...@apache.org>.
gzm55 commented on code in PR #716:
URL: https://github.com/apache/maven-mvnd/pull/716#discussion_r996402720
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
Review Comment:
In the latest commit, the env PATH is not parsed, and the `java` is auto searched by `ProcessBuilder`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] gnodet merged pull request #716: Calculate java home from java command
Posted by GitBox <gi...@apache.org>.
gnodet merged PR #716:
URL: https://github.com/apache/maven-mvnd/pull/716
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-mvnd] gzm55 commented on a diff in pull request #716: calculate java home from java command
Posted by GitBox <gi...@apache.org>.
gzm55 commented on code in PR #716:
URL: https://github.com/apache/maven-mvnd/pull/716#discussion_r996402811
##########
common/src/main/java/org/mvndaemon/mvnd/common/OsUtils.java:
##########
@@ -101,6 +102,29 @@ public static long findProcessRssInKb(long pid) {
}
}
+ public static String findJavaHomeFromPATH() {
+ final String basename = Os.current().isUnixLike() ? "java" : "java.exe";
+ final String PATH = System.getenv("PATH");
+ if (null == PATH) {
+ return null;
+ }
+ for (final String dirname : PATH.split(File.pathSeparator)) {
+ final File java = new File(dirname, basename);
+ if (java.isFile() && java.canExecute()) {
+ final String[] cmd = { java.getAbsolutePath(), "-XshowSettings:properties", "-version" };
+ final List<String> output = new ArrayList<String>(1);
Review Comment:
In the latest commit, `File` is removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org