You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "mbien (via GitHub)" <gi...@apache.org> on 2023/03/09 02:12:28 UTC

[GitHub] [netbeans] mbien opened a new pull request, #5637: improved JDK detection startup tasks

mbien opened a new pull request, #5637:
URL: https://github.com/apache/netbeans/pull/5637

    - repurposed debian task to work on more distributions
    - registered JDK names will now recieve the `(System)` or `(SDKMAN)` postfix
    - only register a JDK if it wasn't registered yet since registering JDKs seems to be expensive
   
   
   ![platform-detector](https://user-images.githubusercontent.com/114367/223895806-18bf26fc-9fd9-4426-bf92-e78ef839548d.png)


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#issuecomment-1461287740

   > Once I was thinking about adding a getId() to the platform, making that official, but then decided not to.
   > Anyway just felt odd making decisions on Display name.
   
   a `getPath()` would be useful for local platforms. This is basically a UID on most circumstances.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien merged pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien merged PR #5637:
URL: https://github.com/apache/netbeans/pull/5637


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1164643532


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   > The namer should really only be applied if that path doesn't already have the file attribute for the name? Otherwise it should be the value of that attribute.
   
   my thinking was to not mess with already registered JDKs at all if there is a conflict in name or path. If the user changed something, the user should remove it first before the automation kicks in again. Being too smart here by updating things quietly during restarts could cause issues.
   
   Keep in mind the version before that registered JDKs without any checks :)



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130321944


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {

Review Comment:
   yeah thought about it but I don't want to rename it until the pattern is actually matching on other distributions. It is all internal code so we can rename it any time we want. Its tested on debian, fedora and manjaro which is all linux.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168975039


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   It'll also reset on IDE upgrade - those attributes don't get imported (one of the issues with using them for this IMO). I think it would be good to get this in for NB18, then possibly rethink how we store info in NB19, and whether we need a reset.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "lkishalmi (via GitHub)" <gi...@apache.org>.
lkishalmi commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1167633268


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   SDK man does not update JDK in place as far as I know. It downloads the new one and sets the current symlink to the one used.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168015691


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   @neilcsmith-net i tested this:
   ```java
                           String jdkName = null;
                           FileObject fo = FileUtil.toFileObject(jdk);
                           if (fo != null) {
                               Object value = fo.getAttribute("J2SEPlatform.displayName"); // NOI18N
                               if (value instanceof String) {
                                   jdkName = (String) value;
                               }
                           }
                           if (jdkName == null) {
                               jdkName = namer.apply(jdk);
                           }
   ```
   and I believe this is worse since it can lead to very weird behavior. If you add a JDK with a custom name, then remove it again, it will be added with the same name again on next start which looks for the user as if removal didn't work.
   
   Having automated JDK detection use a fixed naming pattern is better IMO, since it automatically communicates where this is from.
   
   @lkishalmi thanks for checking!



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168794489


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   Why is the lack of a reset button an issue? If a JDK registration has been manually removed, shouldn't it need to be manually re-added?
   
   That attribute was added primarily to try and maintain the names given to Disco downloaded JDKs incidentally. So they could be removed and then re-added under same name. I've had doubts since about that way to do it, but somewhere I think we should store metadata like names, removal status, etc. of manually unregistered JDKs.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1136369048


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        Set<String> registeredJDKNames = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                               .filter(JavaPlatform::isValid)
+                                               .map(JavaPlatform::getDisplayName)

Review Comment:
   at this point in the code, I have the path to a JDK installation folder and want to check if it is already registered. How do I generate a name from that folder and compare it with already registered JDKs, without registering it first ;)
   
   Thats why it uses display name right now. It knows how to build that name if it registers JDKs, so it can check for JDKs which are already registered and skip the step if they are.
   
   Having access to the path of registered JDKs would allow this too, but honestly I would probably also check the display names too since we don't want to register JDKs with the same name in the list in case of name clashes.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "lkishalmi (via GitHub)" <gi...@apache.org>.
lkishalmi commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130390252


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        Set<String> registeredJDKNames = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                               .filter(JavaPlatform::isValid)
+                                               .map(JavaPlatform::getDisplayName)

Review Comment:
   Theoretically asking. Shouldn't we shall use `"platform.ant.name"` property instead of DisplayName?



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130318692


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {
+
+    static final Path LINUX_JAVA_DIR = Paths.get("/usr/lib/jvm"); //NOI18N
+    
+    /*
+     * examples:
+     *  java-17-openjdk-amd64 (debian)
+     *  java-17-openjdk (arch, manjaro)
+     *  java-17-openjdk-17.0.6.0.10-1.fc37.x86_64 (fedora)
+     */
+    static final String JAVA_DIR_MATCHER = "^java-(\\d+)-openjdk(-.+)?"; //NOI18N
+
+    @Override
+    public void run() {
+
+        if (Files.isDirectory(LINUX_JAVA_DIR)) {
+

Review Comment:
   i like when the code has some structure



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168023235


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   @mbien yes, that sounds wrong. One possible thought would be to _ignore_ folders with that attribute? That should stop removed folders from being automatically re-added?
   
   Otherwise, leave as is for now. The naming / removal history could probably do with some tweaking at a later date. In retrospect the file attribute was not the right way to do this anyway.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130305406


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/SdkManJavaPlatformDetector.java:
##########
@@ -18,49 +18,42 @@
  */
 package org.netbeans.modules.java.j2seplatform;
 
-import java.io.File;
 import java.io.IOException;
-import java.util.Collection;
-import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
-import org.netbeans.spi.java.platform.JavaPlatformFactory;
-import org.openide.filesystems.FileObject;
-import org.openide.filesystems.FileUtil;
-import org.openide.util.Lookup;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  *
  * @author lkishalmi
  */
 public class SdkManJavaPlatformDetector implements Runnable {
 
-    static final File SDKMAN_JAVA_DIR = new File(System.getProperty("user.home"), ".sdkman/candidates/java"); //NOI18N
+    static final Path SDKMAN_JAVA_DIR = Paths.get(System.getProperty("user.home"), ".sdkman", "candidates", "java"); //NOI18N
 
     @Override
     public void run() {
-        if (SDKMAN_JAVA_DIR.isDirectory()) {
-            File[] platformDirs = SDKMAN_JAVA_DIR.listFiles((File f) -> f.isDirectory() && !"current".equals(f.getName())); //NOI18N
-            Collection<? extends JavaPlatformFactory.Provider> providers = Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class);
-            for (JavaPlatformFactory.Provider provider : providers) {
-                JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
-                if (platformFactory != null) {
-                    for (File platformDir : platformDirs) {
-                        try {
-                            FileObject installFolder = FileUtil.toFileObject(platformDir);
-                            platformFactory.create(installFolder, getDisplayName(installFolder), true);
-                        } catch (IOException ex) {
-                            //It seems we was not suceeding to add this
-                        } catch(IllegalArgumentException ex) {
-                            //Thrown if the platform is already persisted and added to the system.
-                        }
 
-                    }
-                    break;
-                }
+        if (Files.isDirectory(SDKMAN_JAVA_DIR)) {
+            try (Stream<Path> files = Files.list(SDKMAN_JAVA_DIR)) {
+
+                List<Path> jdks = files.filter(p -> Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS))
+                                       .filter(p -> !p.getFileName().toString().equals("current"))
+                                       .collect(Collectors.toList());
+
+                JDKDetectorUtils.registerJDKs(jdks, jdk -> getDisplayName(jdk));

Review Comment:
   @lkishalmi the SDKMAN folder contains the actual JDKs, not symlinks, right? In other words: I haven't tested this but i think this should work just like it did before.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130397179


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        Set<String> registeredJDKNames = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                               .filter(JavaPlatform::isValid)
+                                               .map(JavaPlatform::getDisplayName)

Review Comment:
   also worth noting: the display name would be JDK 17 (SDKMAN) or JDK 17 (System). So it is unlikely that it collides with user settings - if it does, it will simply skip the jdk



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "BradWalker (via GitHub)" <gi...@apache.org>.
BradWalker commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130670238


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {

Review Comment:
   I could make that happen.. 



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on pull request #5637: improved JDK detection startup tasks

Posted by "lkishalmi (via GitHub)" <gi...@apache.org>.
lkishalmi commented on PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#issuecomment-1509915425

   Just some insights on SDKMAN:
   ```
   lkishalmi@desktop-su4jobc:~/NetBeansProjects/netbeans$ ls -l ~/.sdkman/candidates/java/
   total 19
   drwxr-xr-x  9 lkishalmi lkishalmi 11 Jan 18 02:13 17.0.6-tem
   drwxr-xr-x 10 lkishalmi lkishalmi 14 Mar  7 00:53 20-zulu
   lrwxrwxrwx  1 lkishalmi lkishalmi 10 Apr 15 10:53 current -> 17.0.6-tem
   ```


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168818731


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   > Why is the lack of a reset button an issue? If a JDK registration has been manually removed, shouldn't it need to be manually re-added?
   
   the longer I thought about it the more I liked the idea of skipping those folders. I think it is the safest option to get auto detection in.
   
   step 1: don't ever override already registered JDKs
   step 2: don't annoy the user by registering stuff the user potentially just removed
   
   having a growing "block list" without reset button is intuitively wrong to me, but it is probably completely fine in this particular case - and the better option to not having it.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1155386182


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        Set<String> registeredJDKNames = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                               .filter(JavaPlatform::isValid)
+                                               .map(JavaPlatform::getDisplayName)

Review Comment:
   How did I not see the `getInstallFolders()` method before? Anyway, changed it so that it won't register a platform if a platform with the same path or name already exists.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130405791


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/SdkManJavaPlatformDetector.java:
##########
@@ -18,49 +18,42 @@
  */
 package org.netbeans.modules.java.j2seplatform;
 
-import java.io.File;
 import java.io.IOException;
-import java.util.Collection;
-import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
-import org.netbeans.spi.java.platform.JavaPlatformFactory;
-import org.openide.filesystems.FileObject;
-import org.openide.filesystems.FileUtil;
-import org.openide.util.Lookup;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  *
  * @author lkishalmi
  */
 public class SdkManJavaPlatformDetector implements Runnable {
 
-    static final File SDKMAN_JAVA_DIR = new File(System.getProperty("user.home"), ".sdkman/candidates/java"); //NOI18N
+    static final Path SDKMAN_JAVA_DIR = Paths.get(System.getProperty("user.home"), ".sdkman", "candidates", "java"); //NOI18N
 
     @Override
     public void run() {
-        if (SDKMAN_JAVA_DIR.isDirectory()) {
-            File[] platformDirs = SDKMAN_JAVA_DIR.listFiles((File f) -> f.isDirectory() && !"current".equals(f.getName())); //NOI18N
-            Collection<? extends JavaPlatformFactory.Provider> providers = Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class);
-            for (JavaPlatformFactory.Provider provider : providers) {
-                JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
-                if (platformFactory != null) {
-                    for (File platformDir : platformDirs) {
-                        try {
-                            FileObject installFolder = FileUtil.toFileObject(platformDir);
-                            platformFactory.create(installFolder, getDisplayName(installFolder), true);
-                        } catch (IOException ex) {
-                            //It seems we was not suceeding to add this
-                        } catch(IllegalArgumentException ex) {
-                            //Thrown if the platform is already persisted and added to the system.
-                        }
 
-                    }
-                    break;
-                }
+        if (Files.isDirectory(SDKMAN_JAVA_DIR)) {
+            try (Stream<Path> files = Files.list(SDKMAN_JAVA_DIR)) {
+
+                List<Path> jdks = files.filter(p -> Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS))
+                                       .filter(p -> !p.getFileName().toString().equals("current"))
+                                       .collect(Collectors.toList());
+
+                JDKDetectorUtils.registerJDKs(jdks, jdk -> getDisplayName(jdk));

Review Comment:
   ok, old code used File::isDirectory which returns true on symlinks. So maybe I should simply remove `NOFOLLOW_LINKS` to be sure that it works just like before.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#issuecomment-1494846056

   update: it is now checking installation paths too, in addition to the registered platform name, to not register duplicates or overwrite existing ones.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1164631349


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   The `namer` should really only be applied if that path doesn't already have the file attribute for the name? Otherwise it should be the value of that attribute.
   
   We could do with a better way of linking path and chosen name - the idea was to allow re-registered JDK's to get the previously set user name by default.
   
   It's probably a minor concern now you're checking for path rather than name to determine registration, but could still be worth considering?



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1164643532


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   > The namer should really only be applied if that path doesn't already have the file attribute for the name? Otherwise it should be the value of that attribute.
   
   my thinking was to not mess with already registered JDKs at all if there is a conflict in name or path. If the user changed something, the user should remove it first before the automation kicks in again. Being too smart here by updating things quietly during restarts could cause issues.
   
   Keep in mind the version before that simply registered JDKs without any checks :)



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168760495


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   however :)
   
   having to remove this check later is probably the better option than not having it -> going to change it



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "BradWalker (via GitHub)" <gi...@apache.org>.
BradWalker commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130312936


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {
+
+    static final Path LINUX_JAVA_DIR = Paths.get("/usr/lib/jvm"); //NOI18N
+    
+    /*
+     * examples:
+     *  java-17-openjdk-amd64 (debian)
+     *  java-17-openjdk (arch, manjaro)
+     *  java-17-openjdk-17.0.6.0.10-1.fc37.x86_64 (fedora)
+     */
+    static final String JAVA_DIR_MATCHER = "^java-(\\d+)-openjdk(-.+)?"; //NOI18N
+
+    @Override
+    public void run() {
+
+        if (Files.isDirectory(LINUX_JAVA_DIR)) {
+

Review Comment:
   Maybe delete the extra line here?



##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {
+
+    static final Path LINUX_JAVA_DIR = Paths.get("/usr/lib/jvm"); //NOI18N
+    
+    /*
+     * examples:
+     *  java-17-openjdk-amd64 (debian)
+     *  java-17-openjdk (arch, manjaro)
+     *  java-17-openjdk-17.0.6.0.10-1.fc37.x86_64 (fedora)
+     */
+    static final String JAVA_DIR_MATCHER = "^java-(\\d+)-openjdk(-.+)?"; //NOI18N
+
+    @Override
+    public void run() {
+
+        if (Files.isDirectory(LINUX_JAVA_DIR)) {
+
+            try (Stream<Path> files = Files.list(LINUX_JAVA_DIR)) {
+

Review Comment:
   Ditto..



##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {

Review Comment:
   I like the work here but I would change the class name to `UnixPlatformDetector`..
   
   There are platforms out there that are definitely not Linux but act  the same as Linux (i..e. it's just another Unix platform). Also, there are platforms that use the same filesystem layout but are also not Linux.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168025110


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   interesting idea.
   ```java
                       FileObject fo = FileUtil.toFileObject(jdk);
                       if (fo != null) {
                           Object value = fo.getAttribute("J2SEPlatform.displayName"); // NOI18N
                           if (value instanceof String) {
                               break; // skip, this indicates that it was added (and potentially removed) by the user
                           }
                       }
   ```
   i think this has potential but there is one big issue: there is no reset button. We would have to be prepared to tell users to remove `nb_userdir/var/attributes.xml`



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130386150


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {

Review Comment:
   I rename it if you test it on unix :) This workflow created a dev-build



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on pull request #5637: improved JDK detection startup tasks

Posted by "lkishalmi (via GitHub)" <gi...@apache.org>.
lkishalmi commented on PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#issuecomment-1461257889

   Next JDK-s to be detected the ones in `$HOME/NetBeansJDKs/` and the ones in `$HOME/.gradle/jdks`...


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130394881


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        Set<String> registeredJDKNames = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                               .filter(JavaPlatform::isValid)
+                                               .map(JavaPlatform::getDisplayName)

Review Comment:
   what would be in that property?
   
   a few lines later there is:
   ```
                           String jdkName = namer.apply(jdk);
                           if (!registeredJDKNames.contains(jdkName)) {
   ```
   `namer.apply(jdk)` builds a display name of the JDK candidate and only registers a new one if it is not already taken.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1135999470


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        Set<String> registeredJDKNames = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                               .filter(JavaPlatform::isValid)
+                                               .map(JavaPlatform::getDisplayName)

Review Comment:
   Should possibly be aware of use of file attribute for storing registered names here?  As in https://github.com/apache/netbeans/blob/master/java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/platformdefinition/J2SEPlatformFactory.java#L72
   
   We could do with a better way of storing a mapping of location to display names than that mind you.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#issuecomment-1505734123

   @lkishalmi what do you think? Should we get this in? It checks now paths + names for conflicts.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1164631349


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   The `namer` should really only be applied if that path doesn't already have the file attribute for the name?
   
   We could do with a better way of linking path and chosen name - the idea was to allow re-registered JDK's to get the previously set user name by default.
   
   It's probably a minor concern now you're checking for path rather than name to determine registration, but could still be worth considering?



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1155386335


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/SdkManJavaPlatformDetector.java:
##########
@@ -18,49 +18,42 @@
  */
 package org.netbeans.modules.java.j2seplatform;
 
-import java.io.File;
 import java.io.IOException;
-import java.util.Collection;
-import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
-import org.netbeans.spi.java.platform.JavaPlatformFactory;
-import org.openide.filesystems.FileObject;
-import org.openide.filesystems.FileUtil;
-import org.openide.util.Lookup;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  *
  * @author lkishalmi
  */
 public class SdkManJavaPlatformDetector implements Runnable {
 
-    static final File SDKMAN_JAVA_DIR = new File(System.getProperty("user.home"), ".sdkman/candidates/java"); //NOI18N
+    static final Path SDKMAN_JAVA_DIR = Paths.get(System.getProperty("user.home"), ".sdkman", "candidates", "java"); //NOI18N
 
     @Override
     public void run() {
-        if (SDKMAN_JAVA_DIR.isDirectory()) {
-            File[] platformDirs = SDKMAN_JAVA_DIR.listFiles((File f) -> f.isDirectory() && !"current".equals(f.getName())); //NOI18N
-            Collection<? extends JavaPlatformFactory.Provider> providers = Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class);
-            for (JavaPlatformFactory.Provider provider : providers) {
-                JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
-                if (platformFactory != null) {
-                    for (File platformDir : platformDirs) {
-                        try {
-                            FileObject installFolder = FileUtil.toFileObject(platformDir);
-                            platformFactory.create(installFolder, getDisplayName(installFolder), true);
-                        } catch (IOException ex) {
-                            //It seems we was not suceeding to add this
-                        } catch(IllegalArgumentException ex) {
-                            //Thrown if the platform is already persisted and added to the system.
-                        }
 
-                    }
-                    break;
-                }
+        if (Files.isDirectory(SDKMAN_JAVA_DIR)) {
+            try (Stream<Path> files = Files.list(SDKMAN_JAVA_DIR)) {
+
+                List<Path> jdks = files.filter(p -> Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS))
+                                       .filter(p -> !p.getFileName().toString().equals("current"))
+                                       .collect(Collectors.toList());
+
+                JDKDetectorUtils.registerJDKs(jdks, jdk -> getDisplayName(jdk));

Review Comment:
   I kept `NOFOLLOW_LINKS` for now, since this is probably safer.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168025110


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   interesting idea.
   ```java
                       FileObject fo = FileUtil.toFileObject(jdk);
                       if (fo != null) {
                           Object value = fo.getAttribute("J2SEPlatform.displayName"); // NOI18N
                           if (value instanceof String) {
                               break; // skip, this indicates that it was added (and potentially removed) by the user
                           }
                       }
   ```
   i think this has potential but there is one big issue: there is no reset button. We would have to be prepared to tell users to remove nb_userdir/var/attributes.xml



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1168818731


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   > Why is the lack of a reset button an issue? If a JDK registration has been manually removed, shouldn't it need to be manually re-added?
   
   the longer I thought about it the more I liked the idea of skipping those folders. I think it is the safest option to get auto detection in.
   
   step 1: don't ever override already registered JDKs
   step 2: don't annoy the user by registering stuff the user potentially just removed
   
   having a growing "block list" without reset button was intuitively wrong to me, but it is probably completely fine in this particular case - and the better option to not having it.



##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   > Why is the lack of a reset button an issue? If a JDK registration has been manually removed, shouldn't it need to be manually re-added?
   
   the longer I thought about it the more I liked the idea of skipping those folders. I think it is the safest option to get auto detection in.
   
   step 1: don't ever override already registered JDKs
   step 2: don't annoy the user by registering stuff the user potentially just removed
   
   having a growing "block list" without reset button sounded intuitively wrong to me, but it is probably completely fine in this particular case - and the better option to not having it.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] BradWalker commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "BradWalker (via GitHub)" <gi...@apache.org>.
BradWalker commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130364525


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/LinuxJavaPlatformDetector.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ *
+ * @author lkishalmi
+ */
+public class LinuxJavaPlatformDetector implements Runnable {

Review Comment:
   Understand.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on pull request #5637: improved JDK detection startup tasks

Posted by "lkishalmi (via GitHub)" <gi...@apache.org>.
lkishalmi commented on PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#issuecomment-1461256600

   I thought that property is used as a platform ID. It is unfortunate that the Platform API does not have an ID. So internally we use that property value as an unofficial ID.
   
   That is being saved in projects metadata if you select the non-default Java platform (in Ant, Maven and Gradle) projects.
   
   Once I was thinking about adding a `getId()` to the platform, making that official, but then decided not to.
   
   Anyway just felt odd making decisions on Display name.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "lkishalmi (via GitHub)" <gi...@apache.org>.
lkishalmi commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1130398252


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/SdkManJavaPlatformDetector.java:
##########
@@ -18,49 +18,42 @@
  */
 package org.netbeans.modules.java.j2seplatform;
 
-import java.io.File;
 import java.io.IOException;
-import java.util.Collection;
-import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
-import org.netbeans.spi.java.platform.JavaPlatformFactory;
-import org.openide.filesystems.FileObject;
-import org.openide.filesystems.FileUtil;
-import org.openide.util.Lookup;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  *
  * @author lkishalmi
  */
 public class SdkManJavaPlatformDetector implements Runnable {
 
-    static final File SDKMAN_JAVA_DIR = new File(System.getProperty("user.home"), ".sdkman/candidates/java"); //NOI18N
+    static final Path SDKMAN_JAVA_DIR = Paths.get(System.getProperty("user.home"), ".sdkman", "candidates", "java"); //NOI18N
 
     @Override
     public void run() {
-        if (SDKMAN_JAVA_DIR.isDirectory()) {
-            File[] platformDirs = SDKMAN_JAVA_DIR.listFiles((File f) -> f.isDirectory() && !"current".equals(f.getName())); //NOI18N
-            Collection<? extends JavaPlatformFactory.Provider> providers = Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class);
-            for (JavaPlatformFactory.Provider provider : providers) {
-                JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
-                if (platformFactory != null) {
-                    for (File platformDir : platformDirs) {
-                        try {
-                            FileObject installFolder = FileUtil.toFileObject(platformDir);
-                            platformFactory.create(installFolder, getDisplayName(installFolder), true);
-                        } catch (IOException ex) {
-                            //It seems we was not suceeding to add this
-                        } catch(IllegalArgumentException ex) {
-                            //Thrown if the platform is already persisted and added to the system.
-                        }
 
-                    }
-                    break;
-                }
+        if (Files.isDirectory(SDKMAN_JAVA_DIR)) {
+            try (Stream<Path> files = Files.list(SDKMAN_JAVA_DIR)) {
+
+                List<Path> jdks = files.filter(p -> Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS))
+                                       .filter(p -> !p.getFileName().toString().equals("current"))
+                                       .collect(Collectors.toList());
+
+                JDKDetectorUtils.registerJDKs(jdks, jdk -> getDisplayName(jdk));

Review Comment:
   AFAIK, the only symlink is the `current` which is filtered out for that reason. Though I have not used SDKMAN for a while.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1164735752


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   actually you are probably right. What if SDKMAN is updating JDKs in place. The old version would have simply overwritten the registered platforms. This PR would keep the old ones until they are manually removed.
   
   probably better to think about this more (and I gonna have to install SDKMAN somewhere)



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on a diff in pull request #5637: improved JDK detection startup tasks

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #5637:
URL: https://github.com/apache/netbeans/pull/5637#discussion_r1164735752


##########
java/java.j2seplatform/src/org/netbeans/modules/java/j2seplatform/JDKDetectorUtils.java:
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.j2seplatform;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.netbeans.api.java.platform.JavaPlatform;
+import org.netbeans.api.java.platform.JavaPlatformManager;
+import org.netbeans.modules.java.j2seplatform.platformdefinition.J2SEPlatformImpl;
+import org.netbeans.spi.java.platform.JavaPlatformFactory;
+import org.openide.filesystems.FileUtil;
+import org.openide.util.Lookup;
+
+/**
+ *
+ * @author mbien
+ */
+class JDKDetectorUtils {
+
+    /**
+     * Registers new JDKs if they aren't registered yet.
+     * @param jdks List of paths to the JDKs.
+     * @param namer Converts a Path to the display name of the JDK.
+     */
+    static void registerJDKs(List<Path> jdks, Function<Path, String> namer) {
+
+        List<JavaPlatform> platforms = Stream.of(JavaPlatformManager.getDefault().getInstalledPlatforms())
+                                             .filter(JavaPlatform::isValid)
+                                             .collect(Collectors.toList());
+
+        Set<String> registeredJDKNames = platforms.stream()
+                                                  .map(JavaPlatform::getDisplayName)
+                                                  .collect(Collectors.toSet());
+
+        Set<Path> registeredPaths = platforms.stream()
+                                             .flatMap(p -> p.getInstallFolders().stream())
+                                             .map(f -> Paths.get(f.getPath()))
+                                             .collect(Collectors.toSet());
+
+        for (JavaPlatformFactory.Provider provider : Lookup.getDefault().lookupAll(JavaPlatformFactory.Provider.class)) {
+
+            JavaPlatformFactory platformFactory = provider.forType(J2SEPlatformImpl.PLATFORM_J2SE);
+            if (platformFactory != null) {
+                for (Path jdk : jdks) {
+                    try {
+                        String jdkName = namer.apply(jdk);
+                        // don't register if something with the same name or same path exists

Review Comment:
   actually you are probably right. What if SDKMAN is updating JDKs in place. The old version would have simply overwritten the registered platforms. This PR would keep the old ones until they are manually removed.
   
   probably better to think about this more



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists