You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "matthiasblaesing (via GitHub)" <gi...@apache.org> on 2024/01/02 15:52:00 UTC

[PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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

   When trying to run "Execute Java Shell" for a non-modular maven project, this fails because the VM can't be started.
   
   The problem is, that NetBeans tries to start the VM with a module setup, but the project does not provide one (it is non-modular).
   
   Please see individual commits for detailed explanation. 
   
   Closes: #6692


-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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


##########
java/jshell.support/src/org/netbeans/modules/jshell/editor/CompletionFilter.java:
##########
@@ -55,6 +61,29 @@
  */
 final class CompletionFilter extends DocTrees {
 
+    private static final Logger LOG = Logger.getLogger(CompletionFilter.class.getName());
+
+    private static final MethodHandle MH_GET_TYPE;
+    private static final MethodHandle MH_GET_CHARACTERS;
+
+    static {
+        MethodHandle getTypeBuilder = null;
+        MethodHandle getCharactersBuilder = null;
+        Lookup lookup = MethodHandles.lookup();
+        try {
+            getTypeBuilder = lookup.findVirtual(DocTrees.class, "getType", MethodType.methodType(TypeMirror.class, DocTreePath.class));

Review Comment:
   I'd suggest a more prominent LOG message, as `getType()` method will NPE if this `findVirtual` fails.
   I am not very familiar with shimming different javac versions - @jlahoda should definitely review this.



##########
java/jshell.support/src/org/netbeans/modules/jshell/support/ShellSession.java:
##########
@@ -944,8 +945,34 @@ public void notifyClosed(JShellEnvironment env, boolean remote) {
     }
     
     public void reportErrorMessage(Throwable t) {
-        LOG.log(Level.INFO, "Error in JSHell", t);
-        reportShellMessage(buildErrorMessage(t));
+        StringBuilder processOutput = null;
+        VMStartException startException = findVMStartException(t);
+        if(startException != null) {
+            processOutput = new StringBuilder();
+            // VMStartException is raised when target VM fails to start. At time
+            // of writing com.sun.tools.jdi.AbstractLauncher does not read the
+            // process output, so diagnostic output is lost, try to recover it.
+            assert !startException.process().isAlive() : "Process expected to be not alive";
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            try {
+                startException.process().getInputStream().transferTo(baos);
+            } catch (IOException ex) {}
+            processOutput.append("Standard output:\n");

Review Comment:
   L10n as it goes to UI ?



##########
java/jshell.support/src/org/netbeans/modules/jshell/tool/JShellTool.java:
##########
@@ -1691,50 +1754,60 @@ boolean cmdClasspath(String arg) {
     protected void classpathAdded(String s) {}
 
     boolean cmdDebug(String arg) {
-        if (arg.isEmpty()) {
-            debug = !debug;
-            InternalDebugControl.setDebugFlags(state, debug ? DBG_GEN : 0);
-            fluff("Debugging %s", debug ? "on" : "off");
-        } else {
-            int flags = 0;
-            for (char ch : arg.toCharArray()) {
-                switch (ch) {
-                    case '0':
-                        flags = 0;
-                        debug = false;
-                        fluff("Debugging off");
-                        break;
-                    case 'r':
-                        debug = true;
-                        fluff("REPL tool debugging on");
-                        break;
-                    case 'g':
-                        flags |= DBG_GEN;
-                        fluff("General debugging on");
-                        break;
-                    case 'f':
-                        flags |= DBG_FMGR;
-                        fluff("File manager debugging on");
-                        break;
-                    case 'c':
-                        flags |= DBG_COMPA;
-                        fluff("Completion analysis debugging on");
-                        break;
-                    case 'd':
-                        flags |= DBG_DEP;
-                        fluff("Dependency debugging on");
-                        break;
-                    case 'e':
-                        flags |= DBG_EVNT;
-                        fluff("Event debugging on");
-                        break;
-                    default:
-                        hard("Unknown debugging option: %c", ch);
-                        fluff("Use: 0 r g f c d");
-                        return false;
+        try {
+            if (arg.isEmpty()) {
+                debug = !debug;
+                MH_SET_DEBUG_FLAGS.invoke(state, debug ? DBG_GEN : 0);

Review Comment:
   maybe guard against missing method ?



-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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

   @sdedic @lahodaj thank you both for review.
   
   I addressed the review comments, squashed the updates into their base counterparts. github actions is still green, so I'll merge.


-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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


-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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

   (I'll (try to) take a look tomorrow, sorry.)


-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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

   @sdedic the core of this change is in [1a41b91](https://github.com/apache/netbeans/pull/6905/commits/1a41b91367d9a511b5d4f99a068098b11c0a17ef). You touched that code in 2017 in a commit titled  "#270831: quoted possible spaces in agent specification / other arguments generated by JShell", I don't understand why in that changeset `isModularProject` was modified.
   
   Could you please check if this update make sense to 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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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


##########
java/jshell.support/src/org/netbeans/modules/jshell/support/ShellSession.java:
##########
@@ -944,8 +945,34 @@ public void notifyClosed(JShellEnvironment env, boolean remote) {
     }
     
     public void reportErrorMessage(Throwable t) {
-        LOG.log(Level.INFO, "Error in JSHell", t);
-        reportShellMessage(buildErrorMessage(t));
+        StringBuilder processOutput = null;
+        VMStartException startException = findVMStartException(t);
+        if(startException != null) {
+            processOutput = new StringBuilder();
+            // VMStartException is raised when target VM fails to start. At time
+            // of writing com.sun.tools.jdi.AbstractLauncher does not read the
+            // process output, so diagnostic output is lost, try to recover it.
+            assert !startException.process().isAlive() : "Process expected to be not alive";
+            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+            try {
+                startException.process().getInputStream().transferTo(baos);
+            } catch (IOException ex) {}
+            processOutput.append("Standard output:\n");

Review Comment:
   Valid point - updated via 25ee1a49b5838031bd0d52294d66628ca43512a8



-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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


##########
java/jshell.support/src/org/netbeans/modules/jshell/tool/JShellTool.java:
##########
@@ -1691,50 +1754,60 @@ boolean cmdClasspath(String arg) {
     protected void classpathAdded(String s) {}
 
     boolean cmdDebug(String arg) {
-        if (arg.isEmpty()) {
-            debug = !debug;
-            InternalDebugControl.setDebugFlags(state, debug ? DBG_GEN : 0);
-            fluff("Debugging %s", debug ? "on" : "off");
-        } else {
-            int flags = 0;
-            for (char ch : arg.toCharArray()) {
-                switch (ch) {
-                    case '0':
-                        flags = 0;
-                        debug = false;
-                        fluff("Debugging off");
-                        break;
-                    case 'r':
-                        debug = true;
-                        fluff("REPL tool debugging on");
-                        break;
-                    case 'g':
-                        flags |= DBG_GEN;
-                        fluff("General debugging on");
-                        break;
-                    case 'f':
-                        flags |= DBG_FMGR;
-                        fluff("File manager debugging on");
-                        break;
-                    case 'c':
-                        flags |= DBG_COMPA;
-                        fluff("Completion analysis debugging on");
-                        break;
-                    case 'd':
-                        flags |= DBG_DEP;
-                        fluff("Dependency debugging on");
-                        break;
-                    case 'e':
-                        flags |= DBG_EVNT;
-                        fluff("Event debugging on");
-                        break;
-                    default:
-                        hard("Unknown debugging option: %c", ch);
-                        fluff("Use: 0 r g f c d");
-                        return false;
+        try {
+            if (arg.isEmpty()) {
+                debug = !debug;
+                MH_SET_DEBUG_FLAGS.invoke(state, debug ? DBG_GEN : 0);

Review Comment:
   Thanks for bringing this up. I added a null guard, that gives info to the user. While checking/testing this, I noticed, that my testing was tainted by modifications to the `netbeans.conf`. The package `jdk.internal.jshell.debug` needs to be exported to the unnamed module, that necessary change was also added.



-- 
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


Re: [PR] JShell: Running "Execute Java Shell" from Maven project fails [netbeans]

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


##########
java/jshell.support/src/org/netbeans/modules/jshell/editor/CompletionFilter.java:
##########
@@ -55,6 +61,29 @@
  */
 final class CompletionFilter extends DocTrees {
 
+    private static final Logger LOG = Logger.getLogger(CompletionFilter.class.getName());
+
+    private static final MethodHandle MH_GET_TYPE;
+    private static final MethodHandle MH_GET_CHARACTERS;
+
+    static {
+        MethodHandle getTypeBuilder = null;
+        MethodHandle getCharactersBuilder = null;
+        Lookup lookup = MethodHandles.lookup();
+        try {
+            getTypeBuilder = lookup.findVirtual(DocTrees.class, "getType", MethodType.methodType(TypeMirror.class, DocTreePath.class));

Review Comment:
   Raised log level to warning - in a normal situation `DocTrees` is loaded from nb-javac and thus the two methods are present and can be resolved here. If it fails the methods are not present and thus can't be called in the first place, so I think this should be safe.



-- 
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