Community
Participate
Working Groups
ref bug 526310 comment 23 onwards Currently, we allow the --patch-module option to be specified on each classpath entry on the build path of a project, but interpret it as being applicable to the project under compilation. This needs to be revisited for several reasons. The --patch-module option allows arbitrary artifacts on the file system to be used to add/modify contents of an existing module. So, the user could point to a jar and indicate that the jar should patch an existing module, the result of which would be to treat any classes/resources found in that jar as part of the patched module. Similarly, it would be possible to compile a set of sources *as part of* an existing module using this option. Our implementation today targets the second case, but doesn't really work in either way. In addition, 1. We should ignore this option on a modular project because this option cannot be used to replace module-info classes. But we do not do this validation 2. The option can be added several times today, and we can't be sure which one will be in effect. So if several entries specify this option, it would go down to the order in which the entries occur in the classpath file. For example, if a project wants to use an external implementation of some classes that are part of the java.base module, it should be possible to add the jar to the module path of the project, specify the option patch-module=java.base and use classes from the jar file in the project without any issues. Multiple modules can be patched with multiple entries and several entries and can patch a single module in this scenario. It should also be possible to add additional exports or reads to the patched module via the respective options. Likewise, it should be possible to compile a project as part of another module using this option. In this case, the patch-module option applies to the project under compilation and not to the entries on it's build path. This case should work as if the project defines the said module. In both cases, when the --patch-module option is present, type/package lookup should first consult locations from the patches before consulting the actual module that is being patched. It should also be possible to combine both cases (I hear Stephan saying why would anyone want to do that? :-), so let's not bother about this for now) My argument is that we should rather target the first case because it looks more likely and more useful than the second
Continued from bug 526310 (In reply to comment #31) > [......] > But doesn't that break our rule "one project = one module"? > I didn't understand that. Each entry on a project's build path can specify that it patches an existing module. In other words, a project can include a jar (or a class folder or a project) in it's build path and say that that jar (or class folder or a project) patches an existing module and use classes from that jar as if they were part of the patched module. That does not impact the modularity of the project itself. In the other case where a project can say that it should be compiled as part of an existing module, the project itself should not define a module. If it does, the patch-module option does not have any effect > Consider you have a CU from your patch project, which module is it associated > to? Consider, the project maps several source folders to the same output folder, > you find a .class in that output folder, which module? Ask the project, which > module it defines. Etc. This is the second case, all sources are treated *as part of* the patched module. In this mode, the project itself cannot define a module because that is not allowed. And classes/packages from the project replace classes/packages from the patched module. I'm not sure we do this today and I'm still trying to get a non-trivial example with an eclipse project to work using javac, but the idea is described in the example at http://openjdk.java.net/projects/jigsaw/quick-start
For me the key question is: do we have a clear story to determine from any given CU or class file to which module it is associated? As long as we support multiple source folders in a modular project I believe this story can only be clean if all code in a project is associated to the same module. Since "associated with" is not precisely specified but left to the host system, it's us who decide this story, right? Even the behind-the scenes intra-project patch-module for test code seems to respect this rule, doesn't it?
(In reply to Stephan Herrmann from comment #2) > For me the key question is: do we have a clear story to determine from any > given CU or class file to which module it is associated? > > As long as we support multiple source folders in a modular project I believe > this story can only be clean if all code in a project is associated to the > same module. I agree. But there can be multiple projects that belong to the same module. > > Since "associated with" is not precisely specified but left to the host > system, it's us who decide this story, right? > > Even the behind-the scenes intra-project patch-module for test code seems to > respect this rule, doesn't it? What rule did you mean - that the system decides? Then obviously yes :-)
(In reply to Sasikanth Bharadwaj from comment #1) > Continued from bug 526310 > > (In reply to comment #31) > > [......] > > But doesn't that break our rule "one project = one module"? > > > I didn't understand that. Each entry on a project's build path can specify > that it patches an existing module. Sorry, but no. This is a misunderstanding probably based on poor naming of the UI element (see also bug 525716 comment 2). The project can specify that it patches an existing module, which can be found via the current build path entry. Please see, that the "Patched module" shown in the dialog is always the one in the classpath entry (or one of them - in the case of JRT). So, as a side effect of this bug we should agree on a better wording for the UI :)
(In reply to Till Brychcy from comment #3) > (In reply to Stephan Herrmann from comment #2) > > For me the key question is: do we have a clear story to determine from any > > given CU or class file to which module it is associated? > > > > As long as we support multiple source folders in a modular project I believe > > this story can only be clean if all code in a project is associated to the > > same module. > > I agree. But there can be multiple projects that belong to the same module. I agree, too :) Multiple projects contributing to the same module is a valid use of patch-module. > > Since "associated with" is not precisely specified but left to the host > > system, it's us who decide this story, right? > > > > Even the behind-the scenes intra-project patch-module for test code seems to > > respect this rule, doesn't it? > > What rule did you mean - that the system decides? Then obviously yes :-) The rule that each IJavaProject can only contribute to one module (where previously I wrongly said "one project = one module").
(In reply to Stephan Herrmann from comment #5) > > > Even the behind-the scenes intra-project patch-module for test code seems to > > > respect this rule, doesn't it? > > > > What rule did you mean - that the system decides? Then obviously yes :-) > > The rule that each IJavaProject can only contribute to one module (where > previously I wrongly said "one project = one module"). Then of course: yes, test code is always in the same one module as the main code in the same project.
(In reply to comment #4) > (In reply to Sasikanth Bharadwaj from comment #1) > > Continued from bug 526310 > > > > (In reply to comment #31) > > > [......] > > > But doesn't that break our rule "one project = one module"? > > > > > I didn't understand that. Each entry on a project's build path can specify > > that it patches an existing module. > > Sorry, but no. This is a misunderstanding probably based on poor naming of the > UI element (see also bug 525716 comment 2). > I'm saying it should be supported. It is a valid use case to have an external jar on the build path and have it patch an existing module, while the project defines it's own module > The project can specify that it patches an existing module, which can be found > via the current build path entry. Please see, that the "Patched module" shown in > the dialog is always the one in the classpath entry (or one of them - in the > case of JRT). > > So, as a side effect of this bug we should agree on a better wording for the UI > :) This is true too, we just need to have the same option on the project and may be add it as an extra attribute to one of the source folders
(In reply to Sasikanth Bharadwaj from comment #7) > (In reply to comment #4) > > (In reply to Sasikanth Bharadwaj from comment #1) > > > Continued from bug 526310 > > > > > > (In reply to comment #31) > > > > [......] > > > > But doesn't that break our rule "one project = one module"? > > > > > > > I didn't understand that. Each entry on a project's build path can specify > > > that it patches an existing module. > > > > Sorry, but no. This is a misunderstanding probably based on poor naming of the > > UI element (see also bug 525716 comment 2). > > > I'm saying it should be supported. It is a valid use case to have an > external jar on the build path and have it patch an existing module, while > the project defines it's own module Sorry for being dense, I'm slow in imagining what this could be good for. We are speaking about build time, not runtime, yes? So you propose, the compiler should be able to understand the following situation: Jar1 defines Module1 Jar2 defines Package1.Class1 Project1 has Jar1, Jar2 on its modulepath module-info requires Module1 some class imports Package1.Class1 <-- this should resolve OK? Is that your proposal? To make Class1 accessible, it must be associated to Module1 and Package1 must exported from it. - to associate Class1 to Module1 we need patch-module Module1=Jar2 - additionally we need add-exports Module1=Package1 Or is it about the case where Module1 already declares and exports Package1? What do others think? Do we want to support this? If we change anything here, we should be quick, as this invalidates all we currently do about patch-module. We would need to move the patch-module attribute from the entry containing the *patched* module, to the entry containing the *patching* module. Existing projects would also need to be migrated to the new semantics. For this it would be best if the CP attribute has a different name and/or format so we can detect whether it's old semantics or new semantics. > This is true too, we just need to have the same option on the project and may > be add it as an extra attribute to one of the source folders I don't see how an option on the project could be mapped to UI nor classpath attributes. Maybe *every* source folder not having a module-info would need to declare which module it patches? By such change we would move from saying a project may patch the module of a given dependency, towards saying each "location" can choose to patch a module that must be present somewhere else on the modulepath. Would this move semantics of the CP attribute closer to how launching needs this information?
(In reply to comment #8) > (In reply to Sasikanth Bharadwaj from comment #7) > [...] > > Sorry for being dense, I'm slow in imagining what this could be good for. > > We are speaking about build time, not runtime, yes? > Yes > So you propose, the compiler should be able to understand the following > situation: > > Jar1 defines Module1 > Jar2 defines Package1.Class1 > Project1 > has Jar1, Jar2 on its modulepath > module-info requires Module1 > some class imports Package1.Class1 <-- this should resolve OK? > > Is that your proposal? > Yes > To make Class1 accessible, it must be associated to Module1 and Package1 must > exported from it. > > - to associate Class1 to Module1 we need patch-module Module1=Jar2 > - additionally we need add-exports Module1=Package1 > Yes > Or is it about the case where Module1 already declares and exports Package1? > Either case may be true. This is what the JEP says If a package found in a module definition on a patch path is not already exported or opened by that module then it will, still, not be exported or opened. It can be exported or opened explicitly via either the reflection API or the --add-exports or --add-opens options. > What do others think? Do we want to support this? > > If we change anything here, we should be quick, as this invalidates all we > currently do about patch-module. We would need to move the patch-module > attribute from the entry containing the *patched* module, to the entry > containing the *patching* module. > Which is in line with our other options, like add-exports is added to the exporting module and add-reads is applied to the reading module > Existing projects would also need to be migrated to the new semantics. For this > it would be best if the CP attribute has a different name and/or format so we > can detect whether it's old semantics or new semantics. > The current semantics aren't clear enough anyway, so I don't think there's a need to support old/new semantics > > This is true too, we just need to have the same option on the project and may > > be add it as an extra attribute to one of the source folders > > I don't see how an option on the project could be mapped to UI nor classpath > attributes. Maybe *every* source folder not having a module-info would need to > declare which module it patches? > A modular project is not allowed to patch another module. Even if we allow compilation this way, we wouldn't be able to run the code as part of the patched module because the JEP is very clear that a module-info class cannot exist on the patch path. As for non-modular projects, isn't it still valid to expect being able to patch a module using an external jar or class folder or even another project? > > By such change we would move from saying a project may patch the module of a > given dependency, towards saying each "location" can choose to patch a module > that must be present somewhere else on the modulepath. > I'm saying both must be possible. Either the project or a location on the build path must be able to patch any module on the module path It is, using javac while it is not possible for one module to use some external jar to patch another module in eclipse. > Would this move semantics of the CP attribute closer to how launching needs this > information?
(In reply to Sasikanth Bharadwaj from comment #9) I agree to much of what you write. I admit that I implemented the existing semantics more with a specific kind of use case in mind, not in full analogy to JEP 261. I still think declaring your own sources to patch a given module is the most natural usage, and for this the current model admits the following analogy: - add-exports: the module to which this attribute is attached is made more open: I can now access the types in that package. - patch-module: the module to which this attribute is attached is made more open: I can add more types into the module. OTOH, I can also see advantage in moving closer to JEP 261. (Where JEP 261 focusses on command line options I don't think it is strictly binding for the IDE case, but of course we should offer options that are roughly equivalent to the command line options discussed in the JEP). > (In reply to comment #8) > > (In reply to Sasikanth Bharadwaj from comment #7) > > Existing projects would also need to be migrated to the new semantics. For this > > it would be best if the CP attribute has a different name and/or format so we > > can detect whether it's old semantics or new semantics. > > > The current semantics aren't clear enough anyway, so I don't think there's a > need to support old/new semantics I disagree. Anybody who already uses the option will have found out what effect it has. They will have had > 8 months time to explore and use the feature before the change will be released. If we change the semantics under their feet without giving them a chance to notice (other than breakage of their application), this is pretty unfriendly. Just the fact that a UI label could be misunderstood, doesn't mean we are free to break the semantics in arbitrary ways. > > > This is true too, we just need to have the same option on the project and may > > > be add it as an extra attribute to one of the source folders > > > > I don't see how an option on the project could be mapped to UI nor classpath > > attributes. Maybe *every* source folder not having a module-info would need to > > declare which module it patches? > > > A modular project is not allowed to patch another module. Even if we allow > compilation this way, we wouldn't be able to run the code as part of the > patched module because the JEP is very clear that a module-info class cannot > exist on the patch path. No question here. Never meant to argue for one module patching another. > As for non-modular projects, isn't it still valid > to expect being able to patch a module using an external jar or class folder > or even another project? That's not what I was trying to discuss with the above. 1 step back: if we say that CP attribute declares "this entry patches a module which is given as the value of the attribute", then we can also apply the attribute to an individual source folder meaning that these sources are patching a given module (which I still believe to be the main use case). If a project has multiple source folders, which should contribute to the same module (by patching), then the attribute will have to be repeated on each source folder. One *might* think of abbreviating these repeated attributes on source folders, and pull them up to the project. I was only arguing against this pull-up, because I believe consistency is more important: modularity options are attached to classpath entries, not to an entire project. > > By such change we would move from saying a project may patch the module of a > > given dependency, towards saying each "location" can choose to patch a module > > that must be present somewhere else on the modulepath. > > > I'm saying both must be possible. Either the project or a location on the > build path must be able to patch any module on the module path > It is, using javac while it is not possible for one module to use some > external jar to patch another module in eclipse. I say: in the proposed model only "locations" can patch a module. A project is not a location in this sense, only its source folders are. Internally: "location" == classpath entry. Perhaps, attaching patch information directly to a project could be added later (as a convenience), but we don't even have a place in .classpath where this information could be stored for an entire project. As such I see the new proposal as: "Each classpath entry can declare that the CUs/classfiles found in that location are associated to the module that is named by the value of this attribute." This will also require to change the javadoc of the IClasspathAttribute.PATCH_MODULE API, where currently it says: "The value of this attribute must be the name of a module defined in the classpath entry, to which this attribute is attached." This will no longer be true. This must then be updated, too: "This attribute is supported for classpath entries of kind IClasspathEntry.CPE_CONTAINER, IClasspathEntry.CPE_LIBRARY and IClasspathEntry.CPE_PROJECT." The attribute will be applicable to all kinds of classpath entries. What about the following sentence: "A classpath entry having this attribute must also have the MODULE attribute with value "true"." A patch-entry will not contain a module-info, nor should it be interpreted as an auto module. Should it still be marked as modular? Technically, this would be needed, because otherwise we couldn't even open the dialog for creating the patch attribute.
(In reply to Stephan Herrmann from comment #10) > (In reply to Sasikanth Bharadwaj from comment #9) > > I agree to much of what you write. I admit that I implemented the existing > semantics more with a specific kind of use case in mind, not in full analogy > to JEP 261. > > I still think declaring your own sources to patch a given module is the most > natural usage, [...] I think there are really only two important use cases: 1) The current project is a patch for a module (and is treated as such when used by other projects) 2) A jar is used as patch for a module. 1) Is what is implemented by the current mechanism I think we just need to support 2) somehow. I don't think source folder level patch attributes are really needed (and if, wouldn't it be per output folder?) Maybe we can get away with simply refining the semantics of the existing attribute: If it is attached to a non-modular jar (i.e. that does not contain a module-info.class), it means that the jar patches the module named by the attribute? This would be a compatible change, as a PATCH_MODULE attribute can't exist on such a jar yet. But maybe reusing it may be confusing and it would be clearer to add a different attribute PATCHES_MODULE that may be just added for jars.
(In reply to Till Brychcy from comment #11) > Maybe we can get away with simply refining the semantics of the existing > attribute: If it is attached to a non-modular jar (i.e. that does not > contain a module-info.class), it means that the jar patches the module named > by the attribute? > > This would be a compatible change, as a PATCH_MODULE attribute can't exist > on such a jar yet. What I wrote here may be wrong: Is possible to patch automatic modules? Maybe split package problems could be worked around by this.
Maybe we can also address that question via this bug: (In reply to Sarika Sinha from bug 522554 comment #50) > (In reply to Till Brychcy from bug 522554 comment #49) > > > > If a project has a module-info, it defines its own module, so it cannot > > patch another module. > > Then why do we allow it while compilation? => Where, when and how do we validate this kind of consistency? (sounds like a topic for the builder, as we can't know in which order patch-module and module-info are created).
To coordinate with other pending requests in this area I've scribbled a few bullets into https://wiki.eclipse.org/Java9/ModularityOptions Please add your questions & insights to that page.
Since bug 546653 the patch-module attribute is already more powerful. In 4.15 we may want to remove the dialog, which still has the dubious checkbox [x] Patches an existing module Other than that I don't plan more changes, i.e., using a binary locations as a patch is not on my agenda, any objections?
bulk move out of 4.15 M1
Bulk move out of 4.15 M3
(In reply to Stephan Herrmann from comment #15) > Since bug 546653 the patch-module attribute is already more powerful. > > In 4.15 we may want to remove the dialog, which still has the dubious > checkbox > > [x] Patches an existing module > > Other than that I don't plan more changes, i.e., using a binary locations as > a patch is not on my agenda, any objections? Scheduling the removal of the old dialog for 4.17 @Dani, is there a protocol for removing dialogs? (similar to deprecating than removing API?)
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.
