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/16 07:23:59 UTC

[GitHub] [maven-mvnd] gnodet commented on a diff in pull request #716: calculate java home from java command

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