Re: RFR 8168091: jlink should check security permission early when programmatic access is used
Hi, Thanks for your review. I've updated webrev with jdk.tools.jlink -> jdk.tools.jlink.internal refactoring. But, when I attempted to get rid of shell script in the test with your suggestion, I got: Exception in thread "main" java.security.AccessControlException: access denied ("java.io.FilePermission" "/Users/SATHIJEG/src/jdk9-dev/jdk/test/tools/jlink/JTwork/tools/jlink/JLinkToolProviderTest.d/main.0.jta" "read") at java.security.AccessControlContext.checkPermission(java.base@9-internal/AccessControlContext.java:471) at java.security.AccessController.checkPermission(java.base@9-internal/AccessController.java:894) at java.lang.SecurityManager.checkPermission(java.base@9-internal/SecurityManager.java:548) at java.lang.SecurityManager.checkRead(java.base@9-internal/SecurityManager.java:887) at java.io.FileInputStream.(java.base@9-internal/FileInputStream.java:127) at java.io.FileInputStream.(java.base@9-internal/FileInputStream.java:93) at java.io.FileReader.(java.base@9-internal/FileReader.java:58) at com.sun.javatest.regtest.agent.MainWrapper.main(MainWrapper.java:46) Looks like I've to give AllPermission to all code in jtreg itself and leave the test only as sandbox! => I've to have use a complicated policy file. Shell script avoids all that.. Updated webrev: http://cr.openjdk.java.net/~sundar/8168091/webrev.01/ Thanks, -Sundar On 18/10/16, 3:33 AM, Mandy Chung wrote: On Oct 17, 2016, at 10:23 AM, Sundararajan Athijegannathanwrote: Please review http://cr.openjdk.java.net/~sundar/8168091/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8168091 The shell test can be removed and use @run main/othervm/secure=java.lang.SecurityManager You may want to move the classes in jdk.tools.jlink package to jdk.tools.jlink.internal since they are now internal. Mandy
Re: RFR 8168091: jlink should check security permission early when programmatic access is used
> On Oct 17, 2016, at 10:23 AM, Sundararajan Athijegannathan >wrote: > > Please review http://cr.openjdk.java.net/~sundar/8168091/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8168091 The shell test can be removed and use @run main/othervm/secure=java.lang.SecurityManager You may want to move the classes in jdk.tools.jlink package to jdk.tools.jlink.internal since they are now internal. Mandy
Re: module-info hygiene
On Mon, 17 Oct 2016 12:59:25 +0200, Alan Batemanwrote: On 17/10/2016 08:32, Peter Levart wrote: : Do we need an --exclude-modules (in addition to --add-modules) option on javac, java and jlink commands? --exclude-modules would be different to --limit-modules. If some module requires module M and there is no module M on the module path or it is not observable because it was not mentioned in the --limit-modules option, then an exception is raised. OTOH if some module X requires module M and module M is mentioned in the --exclude-modules option, then such requires is silently ignored in hope that module X will not actually need types from module M. The module declaration is intended to be authoritative and so we have to trust module author when they declare that the module `requires M`. So my view is that options such as --exclude-modules that would have the effect of dropping requires puts us on the road to anarchy. That said, I do see Robert's concern that there might be orphaned `requires` clauses in some modules. My module started using the preferences API but later the implementation changed to use something else. I neglected to remove the `requires java.prefs` from the module declaration and the result is that my module cannot compile against or run on a run-time image that doesn't include this module. Static analysis tools might help here, as might the IDE. We are used to IDEs highlighting unused `import` statements and in time then I expect they will do the same for apparently unused `requires` clauses in module-info.java. If the usage is purely reflective then the module author might need to put a comment on the `requires` clause to avoid other maintainers from removing it (a bit like "// used by javadoc" in comments today when an import is for an unqualified reference in the javadoc). Another part to Robert's mail is the case where something is making use of types in modules that it doesn't depend on. Assuming these are static references then they will be caught at compile-time. This is big improvement compared to today's class path. A more general comment is that module authors will need to learn a few new things about compatibility and refactoring. One example is changing `requires transitive M` to `requires M` is an incompatible change. Another is splitting a module (several sub-cases) where the module author will need to add `requires transitive` to avoid breaking consumers. There are lots of opportunities here for authoritative books and documentation to help module authors do this right. -Alan - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org I understand why *in concept* the --exclude-modules is an unwanted option. The module-info clearly states "requires A.B", otherwise it should have been marked as "optional" or simply removed. Now that the user fully relies on the discipline of the library-builders: users cannot control the modules, nor will the compilation fail in case of an incorrect module-info. It is really a matter of hoping that library-builders are aware of this and maybe it will make libraries more popular based on the quality of the module-info instead of the quality of the classes. As a user you probably don't want to be forced to choose on these facts. And for the smaller and medium application this will work, but for the larger this can really become problematic. Up until now the compiler was always about "is everything on the classpath to compile the classes?". If there is more, we'll, it'll be ignored. "More" was never a problem. And if it was a problem, the user could fix it. Now we have the module-info, and it is actually a safety-belt for the library-builder! Now he can never be blamed (almost): the module-info contains at least all info to compile and run this library, maybe even more for free. But with a lot of libraries with their own safety-belts there can be (and will be) conflicts and there's nothing you can do right now (apart from dropping all safety-belts). For the end-user all these small safety-belts doesn't feel very "safe". He would feel much better if he had some of the control back (and yes, he's very well aware of the possible consequences). The introduction of the module-info comes with great powers, but that comes with great responsibilities as well. I would like to see that the compiler could help with controlling those required modules (which would mean that "More" is considered to be a problem). Static analysis is IMHO just a hint, ignorable, but to me it shouldn't be that way. Robert
Re: Review Request: JDK-8167558 Add new JMOD section for header files and man pages
Updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.01/ This updates the exclude-jmod-section plugin not to filter specific modules. Also clean up DefaultImageBuilder further and improve the exception message when it detects duplicated resource entries. Mandy > On Oct 14, 2016, at 8:28 AM, Mandy Chungwrote: > > >> On Oct 14, 2016, at 5:55 AM, Alan Bateman wrote: >> >> On 13/10/2016 03:22, Mandy Chung wrote: >> >>> Webrev: >>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167558/webrev.00/ >>> >>> Header files and man pages are currently copied to the image. The header >>> files are modularized and in the following directory: >>> src/$MODULE/share/native/include >>> >>> The man page for the corresponding tool should also be modularized. >>> >>> This patch proposes to add a JMOD section for include header files and one >>> for man pages such that they should be packaged in a JMOD file of the >>> module they belong to. >> The changes to add the new section looks good, as does the options for the >> jmod tool. >> >> For jlink then --no-man-pages and --no-header-files look good (as >> demonstrated by how they are used in Images.gmk to create the JRE). I'm less >> sure about the exclude-jmod-section plugin needing the flexibility to >> exclude the resources in the man page or headers section for specific >> modules. I would be tempted to leave that out unless it is really needed. > > I think it should be rare to exclude resources in the man page or headers > section for specific modules. On the other hand, —-exclude-files can > exclude resources and that should be used in case of excluding any specific > resources. I will leave that out. > > > > Mandy >
Re: module-info hygiene
I didn't had dead code in mind. And as Remi explained it cannot be detected on a jar-base, only on an application base. (which reminds me that we also need to have a look at the minimizeJar option of the maven-shade-plugin). My idea was more about collecting all classes required to compile the sourcefiles and verify that of all required modules at least one class is used. In case of "transitive" it should also verify the method-signatures. And yes, in this case I assume that modules which classes are accessed by reflection are optional, which sounds fair enough to me. Robert On Mon, 17 Oct 2016 13:06:59 +0200, Remi Foraxwrote: The compiler can not detect dead code because it can be a library. jlink can detect dead code and provide a list of unneeded modules because it has the view of the whole application. Rémi On October 17, 2016 10:45:26 AM GMT+02:00, Andrew Haley wrote: On 16/10/16 19:52, Robert Scholte wrote: To enforce the discipline, the java compiler should IMHO at least check if all required modules are indeed required and if the transitive required modules are indeed transitive. How can the compiler possibly know this? There are ways of requiring a module without naming it in a declaration. Andrew.
Re: RFR 8168091: jlink should check security permission early when programmatic access is used
+1 > On Oct 17, 2016, at 2:23 PM, Sundararajan Athijegannathan >wrote: > > Please review http://cr.openjdk.java.net/~sundar/8168091/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8168091 > > Thanks, > > -Sundar >
RFR 8168091: jlink should check security permission early when programmatic access is used
Please review http://cr.openjdk.java.net/~sundar/8168091/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8168091 Thanks, -Sundar
Re: Ugly things done to support multiple ContentHandlerFactory and URLStreamHandlerFactory
Alan, Tom, On 13/10/16 19:59, Alan Bateman wrote: .. Speaking of net-dev, then maybe this thread should move there as this topic is really more of a URL issue rather than module system issue. I have replied to this over on net-dev [1] ( I hope that is ok ). We can continue the discussion there. -Chris. []1 http://mail.openjdk.java.net/pipermail/net-dev/2016-October/010380.html
Re: module-info hygiene
The compiler can not detect dead code because it can be a library. jlink can detect dead code and provide a list of unneeded modules because it has the view of the whole application. Rémi On October 17, 2016 10:45:26 AM GMT+02:00, Andrew Haleywrote: >On 16/10/16 19:52, Robert Scholte wrote: > >> To enforce the discipline, the java compiler should IMHO at least >> check if all required modules are indeed required and if the >> transitive required modules are indeed transitive. > >How can the compiler possibly know this? There are ways of requiring >a module without naming it in a declaration. > >Andrew. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: module-info hygiene
On 17/10/2016 08:32, Peter Levart wrote: : Do we need an --exclude-modules (in addition to --add-modules) option on javac, java and jlink commands? --exclude-modules would be different to --limit-modules. If some module requires module M and there is no module M on the module path or it is not observable because it was not mentioned in the --limit-modules option, then an exception is raised. OTOH if some module X requires module M and module M is mentioned in the --exclude-modules option, then such requires is silently ignored in hope that module X will not actually need types from module M. The module declaration is intended to be authoritative and so we have to trust module author when they declare that the module `requires M`. So my view is that options such as --exclude-modules that would have the effect of dropping requires puts us on the road to anarchy. That said, I do see Robert's concern that there might be orphaned `requires` clauses in some modules. My module started using the preferences API but later the implementation changed to use something else. I neglected to remove the `requires java.prefs` from the module declaration and the result is that my module cannot compile against or run on a run-time image that doesn't include this module. Static analysis tools might help here, as might the IDE. We are used to IDEs highlighting unused `import` statements and in time then I expect they will do the same for apparently unused `requires` clauses in module-info.java. If the usage is purely reflective then the module author might need to put a comment on the `requires` clause to avoid other maintainers from removing it (a bit like "// used by javadoc" in comments today when an import is for an unqualified reference in the javadoc). Another part to Robert's mail is the case where something is making use of types in modules that it doesn't depend on. Assuming these are static references then they will be caught at compile-time. This is big improvement compared to today's class path. A more general comment is that module authors will need to learn a few new things about compatibility and refactoring. One example is changing `requires transitive M` to `requires M` is an incompatible change. Another is splitting a module (several sub-cases) where the module author will need to add `requires transitive` to avoid breaking consumers. There are lots of opportunities here for authoritative books and documentation to help module authors do this right. -Alan
Re: module-info hygiene
On 16/10/16 19:52, Robert Scholte wrote: > To enforce the discipline, the java compiler should IMHO at least > check if all required modules are indeed required and if the > transitive required modules are indeed transitive. How can the compiler possibly know this? There are ways of requiring a module without naming it in a declaration. Andrew.
Re: module-info hygiene
Hi Robert, On 10/16/2016 08:52 PM, Robert Scholte wrote: Hi, with the introduction of the module-info something interesting is happening. Up until now the scope of a Java project was limited to the compilation of the classes. In case of Maven the end-user was in full control regarding the classpath and the order of entries. With the order of the dependencies you can control the order of classpath entries. You could add your own dependencies but could also exclude them. The exclude is especially powerful in those cases where library builders have made mistakes (e.g. junit without test-scope) or simply forgot to remove dependencies when refactoring code. The first project poms are often quite clean, but it requires discipline to keep cleaning up your pom, e.g. removing unused dependencies. However, you're not really punished if you don't do this. With the shift to module-info, suddenly every library-builder gets control. If the module-info of that library "requires A.B", it means that every project using this library MUST have A.B on its module-path. As end-user you cannot exclude A.B, nor can you say "B.A replaces A.B for x.y.z" in case of duplicate classes as allowed on the classpath. In short: the end-users must rely on the discipline of library builders. This loss of control for the end-user will have huge impact. Maven has a analyze-goal in the maven-dependency-plugin which show both unused declared dependencies and used undeclared dependencies (which means pulled in as transitive dependency, even though the code directly uses it). Almost every time I run this goal on any project it detects dependencies in one of both groups. To enforce the discipline, the java compiler should IMHO at least check if all required modules are indeed required and if the transitive required modules are indeed transitive. The role of the module-info is way too important to simply allow all content. The scope is not just about the classes anymore; the complete module tree is now locked into the jar. Any mis-configured module-info down the tree can never be fixed by the end-user, which could block the end-user to use the modulepath. just sharing my concerns, Robert I think this is a real concern. Do we need an --exclude-modules (in addition to --add-modules) option on javac, java and jlink commands? --exclude-modules would be different to --limit-modules. If some module requires module M and there is no module M on the module path or it is not observable because it was not mentioned in the --limit-modules option, then an exception is raised. OTOH if some module X requires module M and module M is mentioned in the --exclude-modules option, then such requires is silently ignored in hope that module X will not actually need types from module M. Would that satisfy your concerns? Regards, Peter
Re: RFR: 8159523. Fix tests depending on absence of -limitmods in VM arguments.
On 14/10/2016 23:26, Alexandre (Shura) Iline wrote: Could you please take another look? I have added more options, and fixed other things you have pointed out. I have also picked up a couple more tests to cover the newly added methods. http://cr.openjdk.java.net/~shurailine/8159523/webrev.02/ The usages are easy to read so it looks to me that this effort is coming along well. On repeating options again then this version is an improvement but it still doesn't add to the list for usages like .addExports(...).addExports(...). So rather than taking a String[] then I think it would be simpler to take a String for one value and just add to the builder's list. I see shouldContain requires specifying the OutputKind when sometimes the test doesn't care if the output is sent to stdout or stderr, maybe there can be a shouldContain overload that searches both streams (we have this in ProcessTools already). For the updated tests then I assume `throws Exception` can be removed as there are no unchecked exceptions now. Related is whether TaskError should be TaskException extends RuntimeException instead. -Alan