Community
Participate
Working Groups
The bug is same as reported in bug 489207. That bug provided a short-term fix by adding the jrt-fs.jar as the single library to the container. But ideally we should try to make the JREContainer work without referring to any physical library because there isn't any in case of JRE 9.
I don't think we should fiddle with the IClasspathContainer#getClasspathEntries() and affect existing clients. Here's one idea that could work with minimal code changes: I propose a new IClasspathEntry kind namely: IClasspathEntry#CPE_SYSTEM The path could point to the root of the JRE, i.e. what is represented by the JREContainer. The JREContainer would be responsible for creating this (only in case of JRE 9) and making it available via getClasspathEntries(). And all we need to do in JDT Core is to look for this new kind and do what is required (i.e. create a ClasspathJimage, a kind of ClasspathLocation that specializes in reading from JRE 9 images). Any suggestions?
The problem with a new IClasspathEntry kind is that this will break every client that assumes the set given in IClasspathEntry#getEntryKind() is closed. The Javadoc of IClasspathEntry supports that viewpoint. I know that jdt.ui will throw exceptions if you do that, and I assume others won't like it either. Can't we keep using CPE_LIBRARY and just use the root of the JRE as path? That probably needs some magic to distinguish the root of a JRE from a class folder, but I think that would cause less breakage.
(In reply to Markus Keller from comment #2) > Can't we keep using CPE_LIBRARY and just use the root of the JRE as path? > That probably needs some magic to distinguish the root of a JRE from a class > folder, but I think that would cause less breakage. That magic could well be the presence of the jrt-fs.jar directly under the path. But we should think about the clients that are probably expecting this CPE_LIBRARY (a folder) to contain binary classes. They would be broken too.
The suggestion from comment #1 was based on the assumption that there would be at least one entry from #getClasspathEntries(). But that's wrong. The JREContainer should clearly tell its clients that they are dealing with a JRT file system. What we currently have is a crude way of such communication - i.e. the JREContainer#getClasspathEntries() returns exactly one entry pointing to jrt-fs.jar. This was clearly a stop-gap measure. So, I propose the new API goes to IClasspathContainer. Once again IClasspathContainer#getKind() is closed. So, I propose this: /** * Answers whether or not this is a Java 9 runtime or later. The Java Run Time (shortly JRT) * is not represented by list of libraries but a format internal to the runtime. * Therefore, for containers representing such a runtime, {@link #getClasspathEntries()} * returns an empty array. The kind of such a container, however, would remain one of * {@link #K_SYSTEM} or {@link #K_DEFAULT_SYSTEM} as applicable. * * @since 3.13 BETA_JAVA9 * @see #getClasspathEntries() * @see #getKind() */ default boolean isJava9Runtime() { return false; }
(In reply to Jay Arthanareeswaran from comment #4) > Once again IClasspathContainer#getKind() is closed. So, I propose this: Note that this alone is not enough to make things work again. We would still require to tag the resolved IClasspathEntry with something to represent the JRT. It remains to be seen, though, how many clients are broken by using the incorrect CPE_LIBRARY as the entry kind that points to the root of the JDK/JRE.
(In reply to Jay Arthanareeswaran from comment #4) > Once again IClasspathContainer#getKind() is closed. So, I propose this: Duh, the IClasspathContainer#getPath() returns the container path and not the VM install location. Sarika, what are your thoughts?
(In reply to Jay Arthanareeswaran from comment #6) > Sarika, what are your thoughts? If instead of providing a new API as I suggested, do you see problems in the JREContainer directly creating the special entry and supply via getClasspathEntries()? Perhaps, JavaCore can provide some methods for you to create such a special entry?
I feel the JRE container is redundant in a module setting. With java9, it is merely used to point to the JRE that needs to be used and for that, it does not need to be a container. With the target platform including system libraries as well and Module path container handling all module dependencies, we can probably do away with the JREContainer? Define an IModulePathEntry that can represent a module path entry and let it produce the necessary IClasspathEntries. With that, just an isModule() API in IClasspathEntry would be enough?
(In reply to Sasikanth Bharadwaj from comment #8) > Define an > IModulePathEntry that can represent a module path entry and let it produce > the necessary IClasspathEntries. With that, just an isModule() API in > IClasspathEntry would be enough? That will probably help us leave some of the baggages behind. BTW, I also found some API in IJavaProject that might make our life bit difficult. For e.g: IJavaProject#getPackageFragmentRoot(String externalLibraryPath) Where the implementation is expected to come up with a Package fragment root by just looking at the path.
I think I am hitting too many roadblocks to park this work. I guess I will wait to see how the target platform shapes and decide the course for this. The existing solution works, except that ... ... if one adds the jrt-fs.jar to the build path as an individual JAR, that adds the whole system libraries to the build path :(
New Gerrit change created: https://git.eclipse.org/r/95450
(In reply to Eclipse Genie from comment #11) > New Gerrit change created: https://git.eclipse.org/r/95450 This patch contains two sets of API changes: 1. Introduces a new IClasspathEntry kind and related JavaCore#newJrtEntry(). 2. Removes all but the top level Java element for a module declaration. The patch also contains all the implementation changes in jdt core to back up the API changes. The first API change is essentially what I mentioned in comment #1. Had a discussion with Markus and this appears to be the simplest/cleanest solution we can think of yet. Yes, this will break some clients but those still using older Java runtimes should not be affected. The documentation needs improvement in certain areas and certain components in jdt launching should be updated to support this too. The second set is a straight forward change that could probably be moved to a separate commit.
As expected, this is turning out to be a highly disruptive change. I have addressed the issues on the launching side. But some things are broken in JDT Core and UI too. I will see if I can bring it to a decent shape within a reasonable amount of time (let's say this week).
(In reply to Jay Arthanareeswaran from comment #13) > As expected, this is turning out to be a highly disruptive change. I have > addressed the issues on the launching side. But some things are broken in > JDT Core and UI too. I will see if I can bring it to a decent shape within a > reasonable amount of time (let's say this week). What about other clients?
(In reply to Dani Megert from comment #14) > What about other clients? They would be broken too, esp. those that assume that JREContainer is always resolved to a bunch of CPE_LIBRARY. I and Markus had a discussion and we agreed that this is by far the cleanest solution. And this will only affect when clients upgrade to a JRT based file system. What we are using (resolving the JREContainer to the bogus jrt-fs.jar) is a hack and don't look nice in the final product. Couple of other options I suggested during the discussion: 1. Just as we do now, resolve the JREContainer to the list of JMOD that JRE provides. But Markus pointed out that that would defeat the purpose of not worrying about the JRE's implementation details. 2. Upgrade JDT's major version. Markus is of the opinion that we should rather break only the clients that depend on this API (and upgraded to JRT based JRE) than everybody.
(In reply to Jay Arthanareeswaran from comment #15) > (In reply to Dani Megert from comment #14) > > What about other clients? > > They would be broken too, esp. those that assume that JREContainer is always > resolved to a bunch of CPE_LIBRARY. I and Markus had a discussion and we > agreed that this is by far the cleanest solution. And this will only affect > when clients upgrade to a JRT based file system. What we are using > (resolving the JREContainer to the bogus jrt-fs.jar) is a hack and don't > look nice in the final product. OK. > 2. Upgrade JDT's major version. Markus is of the opinion that we should > rather break only the clients that depend on this API (and upgraded to JRT > based JRE) than everybody. I agree with Markus.
The latest patch fixes all but the following failures: org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test#test2 org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test#test3 org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test#testProjectDependencyReconcile2 org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test#testProjectDependencyReconcile3 org.eclipse.jdt.core.tests.model.ExternalAnnotations18Test#testBug465296 This is because we don't yet know what kind of support we will need/provide for external annotations with a JRT. If these failures are not fixed with this, these should be added to bug 486013.
Created attachment 268054 [details] JDT Launch changes This is the patch I used for my testing and nowhere close to completion. There are failures in JREContainer initialization tests. On a quick glance, doesn't look complicated.
Created attachment 268056 [details] JDT UI Patch The patch I used in JDT UI to test classpath resolution and source attachment to system libraries. Again, nowhere close to completion. We should have a close look at all references to IClasspathEntry#CPE_LIBRARY.
This fix should also include the following in the context of IClasspathEntry#CPE_JRT_SYSTEM: adjust Javadoc of IClasspathEntry#getSourceAttachmentRootPath() Brought from bug 509366.
This would have been a good addition in 4.7.1a, but considering the seriousness of the API change, better not rush. I will re-test this and release early m4.
New Gerrit change created: https://git.eclipse.org/r/114457
New Gerrit change created: https://git.eclipse.org/r/114458
New Gerrit change created: https://git.eclipse.org/r/114459
Updated all three patches - but the debug and ui ones are bound to fail since they depend on the core's changes.
(In reply to Jay Arthanareeswaran from comment #25) > Updated all three patches - but the debug and ui ones are bound to fail > since they depend on the core's changes. The following tests in core fail since they depend on the jdt.launching resolving the JRE container to the new CPE_JRT_SYSTEM library entry. With those projects checked out in my local environment, they pass.
Stephan, is it too much to ask for a review of this patch? :) Would be a nice addition in M5 well ahead of the API freeze.
(In reply to Jay Arthanareeswaran from comment #27) > Stephan, is it too much to ask for a review of this patch? :) Would be a > nice addition in M5 well ahead of the API freeze. Looking at the JDT/Core change now.
The JDT/Core change looks quite solid to me, details commented in gerrit. I haven't tried running this, though, so I'm curious what would be the user visible changes?
From an adopter's perspective: I believe the javadoc of IClasspathContainer still needs updating, see e.g. slide #13 in https://www.eclipsecon.org/europe2017/sites/default/files/slides/JDT_Embraces_Java_9_ECE_2017.pdf Do you have a suggested new wording? Just add CPE_JRT_SYSTEM to the list?
(In reply to Stephan Herrmann from comment #30) > From an adopter's perspective: I believe the javadoc of IClasspathContainer > still needs updating, see e.g. slide #13 in > https://www.eclipsecon.org/europe2017/sites/default/files/slides/JDT_Embraces_Java_9_ECE_2017.pdf > > > Do you have a suggested new wording? Just add CPE_JRT_SYSTEM to the list? Sorry, missed this in the latest patch. Will consider this. (In reply to Stephan Herrmann from comment #29) > The JDT/Core change looks quite solid to me, details commented in gerrit. > > I haven't tried running this, though, so I'm curious what would be the user > visible changes? In the package explorer and other places like the JRE definition dialog, the path will point to the java home instead of the jrt-fs.jar.
There was a new test failure after I removed the jrt-fs.jar from AbstractJavaModelTests. Some tests still collect the system libraries from Util.getJavaClassLibs(), which still relies on "jrt-fs.jar" to identify a system library. The batch compiler and other compiler interfaces still pass the libraries as String.
Noopur/Sarika, can you please look at the respective patches and see if it's good enough for consumption. Because of some dependency between jdt core tests and launch, we should push all these changes together.
(In reply to Jay Arthanareeswaran from comment #33) > Noopur/Sarika, can you please look at the respective patches and see if it's > good enough for consumption. Because of some dependency between jdt core > tests and launch, we should push all these changes together. Can we do this today or latest this week please?
For JRE definition in Installed JRE, it will still continue to to show jrt-fs.jar?
(In reply to Sarika Sinha from comment #35) > For JRE definition in Installed JRE, it will still continue to to show > jrt-fs.jar? I don't know how launch stores and retrieves the configurations. If the path has jrt-fs.jar, then yes. May be we can (for the time being), look for such containers and convert them to the java home path based entries?
I just tried the patches and I am seeing the following exception while hovering over any required module in a module-info.java/.class file: java.lang.IllegalArgumentException: Entry must be of kind CPE_LIBRARY or CPE_VARIABLE at org.eclipse.jdt.internal.corext.javadoc.JavaDocLocations.getLibraryJavadocLocation(JavaDocLocations.java:176) at ... This should be checked and updated at all the locations with such checks.
Another issue: Go to the Libraries tab in the Java Build Path dialog of a modular Java 9 project. Expand JRE System Library present under the Modulepath node. => Modules are not shown. Instead, it shows an entry "unknown element". And we get the following exception in Error Log: java.lang.NullPointerException at org.eclipse.jdt.internal.ui.wizards.buildpaths.CPListElement.equals(CPListElement.java:582) ...
Looks like it will break many locations where CPE_LIBRARY was expected and it can now receive CPE_JRT_SYSTEM. All such locations should be updated in the clients. Not sure but PDE could be another client in Eclipse SDK.
(In reply to Noopur Gupta from comment #39) > Looks like it will break many locations where CPE_LIBRARY was expected and > it can now receive CPE_JRT_SYSTEM. All such locations should be updated in > the clients. > > Not sure but PDE could be another client in Eclipse SDK. That's why I have been calling for some attention for a long time. And that's also why we should push this change in sooner than M6.
(In reply to Jay Arthanareeswaran from comment #40) > (In reply to Noopur Gupta from comment #39) > > Looks like it will break many locations where CPE_LIBRARY was expected and > > it can now receive CPE_JRT_SYSTEM. All such locations should be updated in > > the clients. > > > > Not sure but PDE could be another client in Eclipse SDK. > > That's why I have been calling for some attention for a long time. And > that's also why we should push this change in sooner than M6. I think the same was discussed in comment #2. The non-SDK clients of JDT Core should also be considered. (In reply to Jay Arthanareeswaran from comment #33) > Noopur/Sarika, can you please look at the respective patches and see if it's > good enough for consumption. For JDT UI, a pass will be required over other usage locations as well to fix them if needed.
(In reply to Noopur Gupta from comment #41) > I think the same was discussed in comment #2. The non-SDK clients of JDT > Core should also be considered. If there was ever a need for a breaking change, this is it. None of our existing classpath entries satisfactorily cover the new kind introduced by the JRT. The alternatives discussed are not convincing to me: 1. JREContainer resolves to itself or '0" classpath entries? 2. The system libraries are represented by the jrt-fs.jar, like it is now? What if the clients want to have the jrt-fs.jar itself in the classpath? 3. CPE_LIBRARY pointing to the java.home? This will also break many clients who will simply try to look for classes directly under java.home.
(In reply to Noopur Gupta from comment #41) > (In reply to Jay Arthanareeswaran from comment #40) > > (In reply to Noopur Gupta from comment #39) > > > Looks like it will break many locations where CPE_LIBRARY was expected and > > > it can now receive CPE_JRT_SYSTEM. All such locations should be updated in > > > the clients. > > > > > > Not sure but PDE could be another client in Eclipse SDK. > > > > That's why I have been calling for some attention for a long time. And > > that's also why we should push this change in sooner than M6. > I think the same was discussed in comment #2. The non-SDK clients of JDT > Core should also be considered. Can you clarify what you mean by "should be considered"? If you suggest we should find a solution that's guaranteed not to break any clients, I don't think that is possible, not with that Java 9 we have at hands. BTW, I even announced at ECE that we have to break API contracts in this particular case one way or other. IMHO, its important we have a precise description of the change and an I-build containing the change, and refer to both when sending an alert to cross-projects, ideally well before M5.
(In reply to Stephan Herrmann from comment #43) > (In reply to Noopur Gupta from comment #41) > > I think the same was discussed in comment #2. The non-SDK clients of JDT > > Core should also be considered. > > Can you clarify what you mean by "should be considered"? I mean to notify them (including PDE) in advance of this breaking change so that they can also take necessary actions. > (In reply to Jay Arthanareeswaran from comment #33) > > Noopur/Sarika, can you please look at the respective patches and see if it's > > good enough for consumption. > For JDT UI, a pass will be required over other usage locations as well to > fix them if needed. Probably this comment wasn't clear. I am fine with the JDT Core change but the JDT UI patch needs more work before committing.
(In reply to Noopur Gupta from comment #44) Thanks, sounds good to me.
Anything I can do to help completing this task?
(In reply to Stephan Herrmann from comment #46) > Anything I can do to help completing this task? I had a discussion with Noopur yesterday on this and she would like someone to handle the other locations in JDT UI where we switch on the classpath entry kind. I said I will take a stab at it this week. Perhaps, some review time later will be nice :)
New Gerrit change created: https://git.eclipse.org/r/116579
Noopue, the latest UI patchset contains some more changes. I went through some 160+ instances of getEntryKind() and IClasspathEntry#CPE_JRT*. Most locations don't need any change because they are dealing with raw classpath entries, which means nothing has changed. The CPE_JRT come into the picture only for resolved entries. For scenarios I wasn't sure of, like the image, label generation etc., I added the new kind in the same case block as CPE_LIBRARY. Just one question remains from me: In the build path dialog, the container doesn't get child nodes (i.e. modules), like we used to get the rt.jar and all. Hence, there's no way to attach the Javadoc location to the JRE container. Should we allow the Javadoc attribution to the container kinds as well? Over to you Noopur. All tests pass with a JDK 9 locally.
I tried the new JDT Core Gerrit and the latest patch set in JDT UI Gerrit without checking any code. 1) I still see the exception from comment #38 in Error log. Just the line number has changed now: java.lang.NullPointerException at org.eclipse.jdt.internal.ui.wizards.buildpaths.CPListElement.equals(CPListElement.java:591) ... 2) On expanding the JRE 9 node in build path dialog, I see only java.activation module below "Is modular". It should show all the modules or none. 3) I am also getting the following exception in main Eclipse's Console quite frequently. I think one scenario is while trying to change the JRE in build path dialog and applying the changes: java.lang.NegativeArraySizeException at org.eclipse.jdt.internal.core.builder.State.readRestriction(State.java:456) at org.eclipse.jdt.internal.core.builder.State.read(State.java:294) at org.eclipse.jdt.internal.core.builder.JavaBuilder.readState(JavaBuilder.java:151) at org.eclipse.jdt.internal.core.JavaModelManager.readState(JavaModelManager.java:4140) at org.eclipse.jdt.internal.core.JavaModelManager.getLastBuiltState(JavaModelManager.java:2288) at org.eclipse.jdt.internal.core.builder.JavaBuilder.getLastState(JavaBuilder.java:433) at org.eclipse.jdt.internal.core.builder.JavaBuilder.build(JavaBuilder.java:179) ... org.eclipse.core.runtime.CoreException: Error reading last build state for project P9 at org.eclipse.jdt.internal.core.JavaModelManager.readState(JavaModelManager.java:4148) at org.eclipse.jdt.internal.core.JavaModelManager.getLastBuiltState(JavaModelManager.java:2288) at org.eclipse.jdt.internal.core.builder.JavaBuilder.getLastState(JavaBuilder.java:433) at org.eclipse.jdt.internal.core.builder.JavaBuilder.build(JavaBuilder.java:179) ... Caused by: java.lang.NegativeArraySizeException at org.eclipse.jdt.internal.core.builder.State.readRestriction(State.java:456) at org.eclipse.jdt.internal.core.builder.State.read(State.java:294) at org.eclipse.jdt.internal.core.builder.JavaBuilder.readState(JavaBuilder.java:151) at org.eclipse.jdt.internal.core.JavaModelManager.readState(JavaModelManager.java:4140) ... 15 more 4) (In reply to Jay Arthanareeswaran from comment #49) > In the build path dialog, the container doesn't get child nodes (i.e. > modules), like we used to get the rt.jar and all. Hence, there's no way to > attach the Javadoc location to the JRE container. > > Should we allow the Javadoc attribution to the container kinds as well? We are also missing the source attachment attribute like the Javadoc location. Other attributes like Native lib loc, access rules, and external annotations are available on the JRE container. I am not sure if individual modules can have different values for all these attributes. If not, I think we should allow the Javadoc and Source attributes on CPE_JRT_SYSTEM and not in general on all containers.
jdk-9.0.4+11 With these patches from core, ui and debug, when I am trying to run a simple Java program with main() in a Java 9 modular project (having module-info.java), I am getting the following exception and it does not run (same with JUnit tests also): Error occurred during initialization of boot layer java.lang.LayerInstantiationException: Package jdk.internal.jrtfs in both module java.base and module jrt.fs
(In reply to Noopur Gupta from comment #51) > Error occurred during initialization of boot layer > java.lang.LayerInstantiationException: Package jdk.internal.jrtfs in both > module java.base and module jrt.fs I suspect somehow the jrt-fs.jar ends as an automatic module in the runtime classpath. Will see what causes this.
I am going to have to request Sarika to take this further. I narrowed down to two places where the changes need to go: 1. StandardVMType.getDefaultLibraryLocations(File) - we should remove the code that adds the jrt-fs.jar to the library locations and a matching change in canDetectDefaultSystemLibraries() to allow "0" libraries. 2. JREContainer#computeClasspathEntries() should be made to work without the jrt-fs.jar in the getLibraryLocations(). We should somehow construct the source path and other classpath attributess right here. Sarika, will you able to take this forward?
(In reply to Jay Arthanareeswaran from comment #53) > I am going to have to request Sarika to take this further. I narrowed down > to two places where the changes need to go: > > 1. StandardVMType.getDefaultLibraryLocations(File) - we should remove the > code that adds the jrt-fs.jar to the library locations and a matching change > in canDetectDefaultSystemLibraries() to allow "0" libraries. > > 2. JREContainer#computeClasspathEntries() should be made to work without the > jrt-fs.jar in the getLibraryLocations(). We should somehow construct the > source path and other classpath attributess right here. > > Sarika, will you able to take this forward? Will look into the feasibility of it.
(In reply to Sarika Sinha from comment #54) > Will look into the feasibility of it. Thanks Sarika. I found a major bug in the jdt.core patch and fixed in the most recent iteration. The index was not considered for the new entry kind and this kept some features from working.
(In reply to Jay Arthanareeswaran from comment #0) > The bug is same as reported in bug 489207. That bug provided a short-term > fix by adding the jrt-fs.jar as the single library to the container. But > ideally we should try to make the JREContainer work without referring to any > physical library because there isn't any in case of JRE 9. Sorry for going back to the basics. Why do we say that there is no physical library in case of JRE 9. We have java.base modules ? Isn't it mandatory physical library for Java 9?
(In reply to Sarika Sinha from comment #56) > Sorry for going back to the basics. > Why do we say that there is no physical library in case of JRE 9. We have > java.base modules ? Isn't it mandatory physical library for Java 9? Did you mean java.base.jmod? Unlike JRE 8 or below, we can't say the system libraries are in a bunch of JAR files. The jmods are indeed physical libraries, but I don't think we will add them all to the container. On the other hand, it makes sense to change the container to work without any libraries and load the modules on a need basis.
Right now, I see two challenges. To compute the classpath attributes, 3 things are needed - java doc location, index location for the library and the external annotation location for the library. So we can get the java doc location from vm, but what about other 2?
(In reply to Sarika Sinha from comment #58) > Right now, I see two challenges. To compute the classpath attributes, 3 > things are needed - java doc location, index location for the library and > the external annotation location for the library. > So we can get the java doc location from vm, but what about other 2? The other option is to let the library location be computed till the JREContainer is computed and later on removed. This could be a dumb suggestion, but I don't know the code much.
I think we need to think a little more on this, 4.7.3 does not seem feasible.
(In reply to Sarika Sinha from comment #60) > I think we need to think a little more on this, 4.7.3 does not seem feasible. I agree. But M6 is not very far away either. Early M6 would be nice.
Another question is, how should be the representation of libraries in "JRE Definition" dialog ? How should we display the JRE system libraries ? We still need a node to show the source, java doc and external annotation. But what should be it's label ? And that also means there can be only one java doc location and one external annotation jar for all the modules of java 9.
Trying to catch up on the traffic here ... (In reply to Jay Arthanareeswaran from comment #57) > (In reply to Sarika Sinha from comment #56) > > Sorry for going back to the basics. > > Why do we say that there is no physical library in case of JRE 9. We have > > java.base modules ? Isn't it mandatory physical library for Java 9? > > Did you mean java.base.jmod? Unlike JRE 8 or below, we can't say the system > libraries are in a bunch of JAR files. The jmods are indeed physical > libraries, but I don't think we will add them all to the container. On the > other hand, it makes sense to change the container to work without any > libraries and load the modules on a need basis. If I understand correctly then jmod files are not relevant for compiling nor launching but provided as a basis for creating your own image (with jlink), right? If true then I second the goal to not depend on those file. They might even stop shipping those in the default installation, couldn't they? Having jrt-fs.jar and the undocumented lib/modules would be sufficient for normal use.
(In reply to Sarika Sinha from comment #62) > Another question is, how should be the representation of libraries in "JRE > Definition" dialog ? > How should we display the JRE system libraries ? Shouldn't that node have the library icon, just as it has in other locations? > We still need a node to show the source, java doc and external annotation. > But what should be it's label ? A path just like now, just without the trailing jrt-fs.jar ? > And that also means there can be only one java doc location and one external > annotation jar for all the modules of java 9. I think that's fair. Going back to your previous question: (In reply to Sarika Sinha from comment #58) > Right now, I see two challenges. To compute the classpath attributes, 3 > things are needed - java doc location, index location for the library and > the external annotation location for the library. > So we can get the java doc location from vm, but what about other 2? External annotation location is not automatically populated, so initially this will simply remain blank. Not a problem. Remains javadoc location, how is that determined nowadays?
(In reply to Stephan Herrmann from comment #63) > If I understand correctly then jmod files are not relevant for compiling nor > launching but provided as a basis for creating your own image (with jlink), > right? > > If true then I second the goal to not depend on those file. They might even > stop shipping those in the default installation, couldn't they? Having > jrt-fs.jar and the undocumented lib/modules would be sufficient for normal > use. Spot on! Although expanding the container to individual jmod files will work (bug 500905, which is waiting for proper tests), that's an internal detail we don't want to worry about.
(In reply to Sarika Sinha from comment #62) > 2) On expanding the JRE 9 node in build path dialog, I see only > java.activation module below "Is modular". It should show all the modules or > none. I am going to need some help with this. I am looking at the code where we add the pseudo nodes to the container (CPListElement#addModuleNodes) and can see all modules being added to the children. But I can't pinpoint the location where only the [0] is picked. Can someone point me where I should be looking?
(In reply to Jay Arthanareeswaran from comment #66) > (In reply to Sarika Sinha from comment #62) > > 2) On expanding the JRE 9 node in build path dialog, I see only > > java.activation module below "Is modular". It should show all the modules or > > none. > > I am going to need some help with this. I am looking at the code where we > add the pseudo nodes to the container (CPListElement#addModuleNodes) and can > see all modules being added to the children. But I can't pinpoint the > location where only the [0] is picked. Can someone point me where I should > be looking? I don't think only [0] is being picked anywhere. This issue may be caused by the exception mentioned in (1) in comment #50. Check Error Log view when you expand JRE node. So, once this is fixed we should be able to see all modules as it was before the patch.
(In reply to Noopur Gupta from comment #67) > I don't think only [0] is being picked anywhere. This issue may be caused by > the exception mentioned in (1) in comment #50. Check Error Log view when you > expand JRE node. So, once this is fixed we should be able to see all modules > as it was before the patch. Thanks Noopur! Are you beginning to wonder you might as well have written the patch? If so, it's still not too late :) Anyway, the latest patch fixes the NPE and the missing modules. I don't see the other exception. Will play around bit more.
(In reply to Jay Arthanareeswaran from comment #68) > (In reply to Noopur Gupta from comment #67) > > I don't think only [0] is being picked anywhere. This issue may be caused by > > the exception mentioned in (1) in comment #50. Check Error Log view when you > > expand JRE node. So, once this is fixed we should be able to see all modules > > as it was before the patch. > > Thanks Noopur! Are you beginning to wonder you might as well have written > the patch? Not yet! :) > Anyway, the latest patch fixes the NPE and the missing modules. I don't see > the other exception. Will play around bit more. Thanks, I will also try out the new patch sets after we get the update in debug.
(In reply to Noopur Gupta from comment #69) > Thanks, I will also try out the new patch sets after we get the update in > debug. Updated the JDT Debug patch.
I see the module nodes now but I can see only the "Javadoc location" attribute under each module. Other attributes like "Source attachment" etc. are missing. I don't think this is expected. I couldn't get any clue in a quick debugging session. Probably the new constant CPE_JRT_SYSTEM is still not handled somewhere.
The remaining issues required changes to both debug and ui. The gerrits are updated now. Sarika, in patch set 6, I see you removed one of my changes in JREContainerInitializer#requestClasspathContainerUpdate(). Is this intentional? I have put that code back in. Let me know if it's wrong.
(In reply to Jay Arthanareeswaran from comment #72) > The remaining issues required changes to both debug and ui. The gerrits are > updated now. > > Sarika, in patch set 6, I see you removed one of my changes in > JREContainerInitializer#requestClasspathContainerUpdate(). Is this > intentional? I have put that code back in. Let me know if it's wrong. Looks fine!!
I tried the latest patch sets and the reported issues are resolved now.
Gerrit change https://git.eclipse.org/r/116579 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.git/commit/?id=1e07a149262601f724e34444775f518eaaad79c5
Gerrit change https://git.eclipse.org/r/114458 was merged to [m]. Commit: http://git.eclipse.org/c/jdt/eclipse.git/commit/?id=39f74bf71be9dba646c24fe71f2505276bdb3f93
Gerrit change https://git.eclipse.org/r/114459 was merged to []. Commit: http://git.eclipse.org/c/jdt/eclipse.git/commit/?id=366988d2ce5be661caeeb7d4292c4a4940a5f1ff
(In reply to Eclipse Genie from comment #75) > Gerrit change https://git.eclipse.org/r/116579 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.git/commit/ > ?id=1e07a149262601f724e34444775f518eaaad79c5 Since this patch Gerrit reports 12 failing tests, even the last patch set shows them. Any reason why the patch was still merged? Do we plan to fix tests later?
(In reply to Andrey Loskutov from comment #78) > (In reply to Eclipse Genie from comment #75) > > Gerrit change https://git.eclipse.org/r/116579 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/jdt/eclipse.git/commit/ > > ?id=1e07a149262601f724e34444775f518eaaad79c5 > > Since this patch Gerrit reports 12 failing tests, even the last patch set > shows them. Any reason why the patch was still merged? Do we plan to fix > tests later? The Core test framework depends on the debug changes and these failures are expect to go away after we have the next I build. Of course, the debug changes can't go in first because they need core changes for compilation :)
New Gerrit change created: https://git.eclipse.org/r/117321
New Gerrit change created: https://git.eclipse.org/r/117335
(In reply to Eclipse Genie from comment #81) > New Gerrit change created: https://git.eclipse.org/r/117335 Looks like some of the test changes got lost in merge or updating the gerrit. This patch fixes the failures with JRE 8. I will post another gerrit for the failures in BuildpathTests with JRE 9.
Gerrit change https://git.eclipse.org/r/117335 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=188fa56efe8177767cc56dcd855b230ba7fb1280
New Gerrit change created: https://git.eclipse.org/r/117353
Gerrit change https://git.eclipse.org/r/117353 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=2ed5e9f0108829acbbe3e972fb83419bf7505793
I see lot of errors in my runtime since this change of the kind build path of project is incomplete, java.lang.Object cannot be resolved. Is any user action required like deleting the JRE and adding it again? ( I tired that, but it didn't work)
And changing the default workspace JRE to 1.8 consistently causes OutOfMemoryError :-(
Something is fishy here. It looks like a race condition. In a fresh workspace I got the exceptions from comment 50 after adding a new project and a package! Trying with another fresh workspace did not reproduce it. NOTE: The exceptions are in the console that started the IDE, not in the .log or Eclipse Console view.
(In reply to Dani Megert from comment #88) > Trying with another fresh workspace did not reproduce it. Here are the steps to reproduce the issue: 1. Start a new workspace with 4.8 M5 2. Create Java project 'P' 3. Exit 4. Start with I20180214-2000 5. Create package 'p' in P ==> exceptions in console
I can reproduce some of the reported issues. I started investigating, but probably not going to be able to come around with a quick fix. So, for now I have reverted fixes to all the three repos: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=b970fee63ce679d80b7e665b70dcbd93d014a258 http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=aa56cd6109cd70ed2fcec454ddb6f149bfc66e05 http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=399251d24ca7750b72cc08f1d693cd6cc738d22e I will let these get picked up by the next build.
New Gerrit change created: https://git.eclipse.org/r/118325
Okay, finally I nailed it down to the change in JavaRuntime. Patch set removed the following code: if (!libraryPaths[i].toFile().isFile()) { libraryPaths[i] = Path.EMPTY; } But patch set brought that change back with a condition: if (compareJavaVersions(vm, JavaCore.VERSION_9) < 0) { if (!libraryPaths[i].toFile().isFile()) { libraryPaths[i] = Path.EMPTY; } } Unfortunately, the check has no effect as at this point, the Java version in the vm is null. Sarika, can you please look into this? Also, what was the effect if the code is removed altogether?
Having a check like this makes sense I guess instead of removing it - if (!(libraryPaths[i].toFile().isFile() || libraryPaths[i].toFile().isDirectory())) { libraryPaths[i] = Path.EMPTY; }
New Gerrit change created: https://git.eclipse.org/r/118394
New Gerrit change created: https://git.eclipse.org/r/118395
Moving to M7 as discussed with Jay.
Since there's no pressing need for this and that there's some work to be done from the UI and launch front, I am forced to move this out of 4.8.
Moving out of 4.9 again.
Despite spending lot of time on this already, I am not sure I am going to find in near future to tackle the remaining issues. Removing the target.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
