Service provider module resolution <was> Re: Services/configuration/context webrevs

[Paul Sandoz]

Hi,

The following webrev fixes the nits. Many service-based tests are added to _Configurator.java, which makes most of the service-based *.sh tests redundant so those redundant ones removed. I added a runtime service-based test [*] that ensures correct behavior for all three ServiceLoader methods.

  http://cr.openjdk.java.net/~psandoz/jigsaw/resolver-services/webrev.3/

--

I also added tests that show the permits behavior is general and not specific to services:

 693         /*
 694          * The following two tests re-produce an issue with permits
 695          * and requires optional. Depending on the order of resolution
 696          * a module, x, may be linked to a module, z, that is not
 697          * permitted to do so.
 698          * 
 699          * "requires optional" should not cause a resolution failure instead
 700          * linking should not be performed for a "requires optional x" where
 701          * x does not permit the requiring module.
 702          */
 703         
 704         new Test("permits-requires-optional-permissive-linking", true, "z at 1") {
 705             void init(MockLibrary mlib) {
 706                 mlib.add(module("z at 1")
 707                         .requiresOptional("x")
 708                         .requires("y"))
 709                     .add(module("y at 1").requires("x"))
 710                     .add(module("x at 1").permits("y"));
 711             }
 712             void ref(ConfigurationBuilder cfbd) {
 713                 cfbd.add(context("z at 1").remote("+x", "+y"))
 714                     .add(context("y at 1").remote("+x"))
 715                     .add(context("x at 1"));
 716                 // The result should be
 717 //                cfbd.add(context("z at 1").remote(""+y"))
 718 //                    .add(context("y at 1").remote("+x"))
 719 //                    .add(context("x at 1"));
 720             }
 721         };
 722 
 723         new Test("permits-requires-optional-failure", false, "z at 1") {
 724             void init(MockLibrary mlib) {
 725                 mlib.add(module("z at 1")
 726                         .requires("y")
 727                         .requiresOptional("x"))
 728                     .add(module("y at 1").requires("x"))
 729                     .add(module("x at 1").permits("y"));
 730             }
 731             void ref(ConfigurationBuilder cfbd) {
 732                 cfbd.add(context("z at 1").remote("+x", "+y"))
 733                     .add(context("y at 1").remote("+x"))
 734                     .add(context("x at 1"));
 735                 // The result should be
 736 //                cfbd.add(context("z at 1").remote(""+y"))
 737 //                    .add(context("y at 1").remote("+x"))
 738 //                    .add(context("x at 1"));
 739             }

I will work on fixing that this week.

Paul.

[*] I held back on the temptation for now to convert to testng! jtreg is rather smart but IMHO we really need tests we can edit, debug and run within the IDE.

On Jul 5, 2012, at 5:55 AM, Mandy Chung wrote:

> On 7/1/2012 2:34 AM, Paul Sandoz wrote:
>> Hi,
>> 
>> I thought it would help to consolidate the service/configuration changes into one email.
> 
> Yes, that helps.  Thanks.
> 
>> Service provider module resolution
>> -----------------------------------------------
>> 
>> http://cr.openjdk.java.net/~psandoz/jigsaw/resolver-services/webrev.2/
>> 
>> - "permits" clauses do not affect service provider module resolution
>> 
>> - service provider module resolution occurs after "application" resolution. Essentially the application has to successfully resolve as if there were no "requires service " clauses present and after that service provider module resolution occurs.
>> 
>> I understand that on and off list there is reasonable agreement to go forward with this latter approach.
> 
> Doing the service provider dependency resolution in phases would make it a little easier to trace and diagnose.  Each phase may add new modules to the graph (service provider module and its dependencies) and looks like it requires to re-validate if a module is required optionally but failed to be chosen in phase 0. For example,
> 
> M
>   requires optional X;
>   requires service S;
> 
> P
>   requires X @ 1.0
>   provides service S with S1;
> 
> X @ 1.0
>   permits P
> 
> In this case, phase 0 will chose M only.  Phase 1 choses P and X at 1.0 and the solution includes M, P, and X at 1.0.  However, X at 1.0doesn't permit M but it's linked with M in your current implementation.
> Nits w.r.t. the change:
> Resolver.javaL221: toString() call is not needed
> L446: best to follow current convention to put the first parameter
>      next to the signature L445 and line break at the second parameter.
> L450: 4-space indent would be good.
> L458: typo 'dependencey'
> L533: if serviceInterface was resolved in previous phases, it can skip and process the next service interface.
> 
> many.sh L119 - it'd be good to add a comment that p4 is 1 implementation
> and permits is ignored.
> 
> I think it would be useful to add a few test cases that service provider module pulls in new modules and how this fix changes the solution - e.g. a case when a provider module is not included due to a module selected in phase 0 doesn't satisfy its dependency.
> 
> Mandy
>