8233922: Service binding augments module graph with observable incubator modules

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

8233922: Service binding augments module graph with observable incubator modules

Alan Bateman

This issue concerns the interaction of service binding with incubator
modules, a left over from JDK 9 that resurfaces with the upcoming JEP
343 Packaging Tool (Incubator).

The summary on this issue is that service binding augments the module
graph with the modules induced by the service-use relation. This happens
irrespective of the incubating status of the observable modules because
the incubating designation is a JDK rather than Java SE/specified
concept. It shows up with module `jdk.incubator.jpackage` because this
module wants to implement the `java.util.spi.ToolProvider` service
interface, a service interface used by `java.base`. If service binding
were to resolve `jdk.incubator.jpackage` then a warning would be emitted
in all phases that java.base is resolved (so everywhere).

It's on my radar to more precisely specify service binding and also work
out the specification changes needed to allow filtering of the observing
modules but this is not needed at this time. Instead, we can limit the
changes to the configuration for the boot layer as the set of observable
modules when creating the configuration for the boot layer is not
specified (it's up to the implementation). The change proposed here
means that incubator modules are not candidates to add to the module
graph during service binding for the boot layer's configuration. It has
no impact on the general case where incubating modules may be observable
when creating the configuration for custom layers.

While in the area, I've also fixed the existing DefaultModules test to
allow for incubator modules as this test has been failing the jpackage
branch. We didn't have any incubator modules in JDK 11, when the test
was added, to notice this oversight in the test.

The webrev with the proposed changes is here. The reason that the jlink
system modules plugin is updated is because it creates the configuration
for the boot layer at link-time, for cases where we are skip resolution
at run-time.
   http://cr.openjdk.java.net/~alanb/8233922/webrev/

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Alex Buckley
On 11/18/2019 4:34 AM, Alan Bateman wrote:

> The summary on this issue is that service binding augments the
> module graph with the modules induced by the service-use relation.
> ... If service binding were to resolve `jdk.incubator.jpackage`  > then a warning would be emitted in all phases that java.base
> is resolved (so everywhere).
>
> ... The change proposed here means that incubator modules are not
> candidates to add to the module graph during service binding for the
> boot layer's configuration. It has no impact on the general case
> where incubating modules may be observable when creating the
> configuration for custom layers.
An incubator module's service providers will now be unavailable by
default even if a module on the module path says `uses`. I believe that
JEP 11 should say the following:

-----
Incubator modules are part of the JDK run-time image produced by the
standard JDK build. *****However, by default, incubator modules are not
resolved for applications on the class path. Furthermore, by default,
incubator modules do not participate in service binding for applications
on the class path or the module path.*****

Applications on the class path must use the --add-modules command-line
option to request that an incubator module be resolved. Applications
developed as modules can specify `requires` or `requires transitive`
dependences upon an incubator module directly. *****(Some incubator
modules do not export packages but rather provide implementations of
services. It is usually not recommended to require a module that exports
no packages, but it is necessary in this case in order to resolve the
incubator module and thus have it participate in service binding.)*****

During the JDK build, incubator modules must be packaged ...
-----

Alex
Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Alan Bateman
On 18/11/2019 17:54, Alex Buckley wrote:

> An incubator module's service providers will now be unavailable by
> default even if a module on the module path says `uses`. I believe
> that JEP 11 should say the following:
>
> -----
> Incubator modules are part of the JDK run-time image produced by the
> standard JDK build. *****However, by default, incubator modules are
> not resolved for applications on the class path. Furthermore, by
> default, incubator modules do not participate in service binding for
> applications on the class path or the module path.*****
That looks good.

>
> Applications on the class path must use the --add-modules command-line
> option to request that an incubator module be resolved. Applications
> developed as modules can specify `requires` or `requires transitive`
> dependences upon an incubator module directly. *****(Some incubator
> modules do not export packages but rather provide implementations of
> services. It is usually not recommended to require a module that
> exports no packages, but it is necessary in this case in order to
> resolve the incubator module and thus have it participate in service
> binding.)*****
We could potentially say something about tools that are incubator
modules here. The launcher for the tool, "jpackage" in the case of JEP
343, will use the name of the incubator module as the root module to
resolve.

>
> During the JDK build, incubator modules must be packaged ...
There's a typo in this paragraph in that the jmod (and jar) tools use
"--warn-if-resolved=incubating" rather than
"--warn-if-resolved=incubator" to package an incubator module.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Alex Buckley
On 11/18/2019 11:10 AM, Alan Bateman wrote:
> We could potentially say something about tools that are incubator
> modules here. The launcher for the tool, "jpackage" in the case of JEP
> 343, will use the name of the incubator module as the root module to
> resolve.

I updated JEP 11 to allude to incubator modules containing tools, hence
no exported API but rather a service provider.

>> During the JDK build, incubator modules must be packaged ...
> There's a typo in this paragraph in that the jmod (and jar) tools use
> "--warn-if-resolved=incubating" rather than
> "--warn-if-resolved=incubator" to package an incubator module.

JEP corrected to say `incubating`.

Alex
Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Mandy Chung
In reply to this post by Alan Bateman

On 11/18/19 4:34 AM, Alan Bateman wrote:

>
> This issue concerns the interaction of service binding with incubator
> modules, a left over from JDK 9 that resurfaces with the upcoming JEP
> 343 Packaging Tool (Incubator).
>
> The summary on this issue is that service binding augments the module
> graph with the modules induced by the service-use relation. This
> happens irrespective of the incubating status of the observable
> modules because the incubating designation is a JDK rather than Java
> SE/specified concept. It shows up with module `jdk.incubator.jpackage`
> because this module wants to implement the
> `java.util.spi.ToolProvider` service interface, a service interface
> used by `java.base`. If service binding were to resolve
> `jdk.incubator.jpackage` then a warning would be emitted in all phases
> that java.base is resolved (so everywhere).
>
> It's on my radar to more precisely specify service binding and also
> work out the specification changes needed to allow filtering of the
> observing modules but this is not needed at this time. Instead, we can
> limit the changes to the configuration for the boot layer as the set
> of observable modules when creating the configuration for the boot
> layer is not specified (it's up to the implementation). The change
> proposed here means that incubator modules are not candidates to add
> to the module graph during service binding for the boot layer's
> configuration. It has no impact on the general case where incubating
> modules may be observable when creating the configuration for custom
> layers.
>

Limiting this change to the boot layer's configuration is a good
solution that allows time to work out the specification change.

> :
>   http://cr.openjdk.java.net/~alanb/8233922/webrev/

The patch looks good to me.

Mandy

Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

chris.hegarty
In reply to this post by Alan Bateman


> On 18 Nov 2019, at 12:34, Alan Bateman <[hidden email]> wrote:
>
> ...
>   http://cr.openjdk.java.net/~alanb/8233922/webrev/

I think this looks ok.

The original patch that added incubator module support had similar changes ( to exclude incubator modules from service binding ), but it impacted on the public Configuration::resolveAndBind ( so backed away from doing this ). Your changes to limit this to the boot layer are good ( since it is unlikely to be a more general issue ).

I’m surprise to see the resolver use `mres.hasIncubatingWarning() == false`, rather than ModuleResolution.doNotResolveByDefault(mres).

-Chris.




Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Kevin Rushforth
In reply to this post by Mandy Chung
Alan,

Thanks for getting this fixed. I verified that the jpacakge ToolProvider
implementation now works as expected.

-- Kevin


On 11/18/2019 12:13 PM, Mandy Chung wrote:

>
> On 11/18/19 4:34 AM, Alan Bateman wrote:
>>
>> This issue concerns the interaction of service binding with incubator
>> modules, a left over from JDK 9 that resurfaces with the upcoming JEP
>> 343 Packaging Tool (Incubator).
>>
>> The summary on this issue is that service binding augments the module
>> graph with the modules induced by the service-use relation. This
>> happens irrespective of the incubating status of the observable
>> modules because the incubating designation is a JDK rather than Java
>> SE/specified concept. It shows up with module
>> `jdk.incubator.jpackage` because this module wants to implement the
>> `java.util.spi.ToolProvider` service interface, a service interface
>> used by `java.base`. If service binding were to resolve
>> `jdk.incubator.jpackage` then a warning would be emitted in all
>> phases that java.base is resolved (so everywhere).
>>
>> It's on my radar to more precisely specify service binding and also
>> work out the specification changes needed to allow filtering of the
>> observing modules but this is not needed at this time. Instead, we
>> can limit the changes to the configuration for the boot layer as the
>> set of observable modules when creating the configuration for the
>> boot layer is not specified (it's up to the implementation). The
>> change proposed here means that incubator modules are not candidates
>> to add to the module graph during service binding for the boot
>> layer's configuration. It has no impact on the general case where
>> incubating modules may be observable when creating the configuration
>> for custom layers.
>>
>
> Limiting this change to the boot layer's configuration is a good
> solution that allows time to work out the specification change.
>
>> :
>>   http://cr.openjdk.java.net/~alanb/8233922/webrev/
>
> The patch looks good to me.
>
> Mandy
>

Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Alan Bateman
In reply to this post by chris.hegarty
On 19/11/2019 12:43, Chris Hegarty wrote:
> :
>
> I’m surprise to see the resolver use `mres.hasIncubatingWarning() == false`, rather than ModuleResolution.doNotResolveByDefault(mres).
>
I think you are asking about the semantics of the two flags in the
JDK-specific ModuleResolution class file attribute. Some day we should
probably document it although I'm not sure if it's the j.l.module
package description, JEP 11, or somewhere else.

The warn-incubating flag is used in all phases to indicate that a module
is an incubator module and to warn when the module is resolved.
Technically these two concerns could be separated but this need hasn't
arisen to date.

The do-not-resolve-by-default flag excludes modules exporting APIs from
the default set of named modules to resolve when the initial module is
the unnamed module, i.e. class path applications. Technically this could
be used to exclude modules that aren't incubator modules (and I think
we've got close once or two to doing that).

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

chris.hegarty

> On 19 Nov 2019, at 13:51, Alan Bateman <[hidden email]> wrote:
>
> ...
> The warn-incubating flag is used in all phases to indicate that a module is an incubator module and to warn when the module is resolved. Technically these two concerns could be separated but this need hasn't arisen to date.
>
> The do-not-resolve-by-default flag excludes modules exporting APIs from the default set of named modules to resolve when the initial module is the unnamed module, i.e. class path applications. Technically this could be used to exclude modules that aren't incubator modules (and I think we've got close once or two to doing that).

The original design of ModuleResolution intended do-not-resolve-by-default to be consulted by the resolver when making decisions about whether or not to include the module in the resolution process by default.

The warn-if-resolved value was intended to be used by tools and the launcher, to provide a warning reason if the module in question was resolved.

-Chris.

Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

chris.hegarty
In reply to this post by Alex Buckley


> On 18 Nov 2019, at 17:54, Alex Buckley <[hidden email]> wrote:
>> ..
> An incubator module's service providers will now be unavailable by default even if a module on the module path says `uses`. I believe that JEP 11 should say the following:
>
> -----
> Incubator modules are part of the JDK run-time image produced by the standard JDK build. *****However, by default, incubator modules are not resolved for applications on the class path. Furthermore, by default, incubator modules do not participate in service binding for applications on the class path or the module path.*****
>
> Applications on the class path must use the --add-modules command-line option to request that an incubator module be resolved. Applications developed as modules can specify `requires` or `requires transitive` dependences upon an incubator module directly. *****(Some incubator modules do not export packages but rather provide implementations of services. It is usually not recommended to require a module that exports no packages, but it is necessary in this case in order to resolve the incubator module and thus have it participate in service binding.)*****
>
> During the JDK build, incubator modules must be packaged ...
> -----

Thanks for the updates to JEP 11 Alex, they look good.

-Chris.
Reply | Threaded
Open this post in threaded view
|

Re: 8233922: Service binding augments module graph with observable incubator modules

Alan Bateman
In reply to this post by chris.hegarty
On 19/11/2019 14:27, Chris Hegarty wrote:
> :
> The original design of ModuleResolution intended do-not-resolve-by-default to be consulted by the resolver when making decisions about whether or not to include the module in the resolution process by default.
>
> The warn-if-resolved value was intended to be used by tools and the launcher, to provide a warning reason if the module in question was resolved.
>
The resolver has never used the ModuleResolution attribute during
resolution or service binding before now. To date, we've used the
do-no-resolve-by-default to exclude modules that export an API from the
default set of root modules. The semantics that we want for service
binding is to say whether the module participates or not, maybe you are
thinking we should expand the meaning of this flag to beyond the default
modules case?

I remember the warning reasons although I don't think it's possible to
create an incubator module without the warn-incubating flag. The
warn-incubating is the only way to know if a module is an incubator module.

-Alan