Services/configuration/context webrevs

[Paul Sandoz]

Hi,

Thanks for looking at this.

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)

Yes.

(BTW there are currently three phases for jdk.base :-) ).


> 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.

Well spotted. Hmm... need to put my thinking cap on and investigate!

A service provider module choice from the perspective of the resolver is the same as a root module choice. I don't currently see how the above case is different from:

  module x at 1.0 {
    permits y;
  }

  module y at 1.0 {
    requires x;
  }

  module z at 1.0 {
    requires optional x;
  }

when the order of resolution is z, y, then x. I have attached a simple test case that resolves using the above modules.

So i think we have a general bug and require either:

1) a validation step on the set of resolved modules; or

2) the compile and install time linkers do the right thing.

Which one we choose depends on whether we consider the above a valid solution or not. Can x be present but the linker knows that z should not link to x? i.e. as if the "resolve optional x" gets erased by the resolver by adapting the module info of z. It seems reasonable.

Perhaps for now we should go with 1). I will do it as another webrev.


> Nits w.r.t. the change:
> Resolver.javaL221: toString() call is not needed

s/221/211.

OK.


> 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.

Can we reach agreement on a code convention for Jigsaw that is supported by the NetBeans formatting options? then i can just hit Ctrl+Shift+F :-) i fully admit i am lazy!

I don't have any strong preference on the style, I just want to be consistent and do it quickly to avoid us spending unnecessary cycles on these aspects.


> L458: typo 'dependencey'
> L533: if serviceInterface was resolved in previous phases, it can skip and process the next service interface.
> 

I was wondering about this. A service provider dependency that failed to resolve in phase n might be resolved later in a phase > n. So i left that as is for now. Also I did not bother to check if a service provider dependency was already resolved since it is done in the resolve method and i wanted the tracing to show what was going on.


> 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.

Yes, i want to flesh out the _Configurator related tests to support services, that should make redundant some of the shell-script-based tests, those tests can then focus on the use of ServiceLoader and we can probably consolidate. 

I have a preference to do that in another webrev, since it is gonna result in some shuffling around is that OK?

Also i have a preference to do that after the refactoring changes (which somewhat cleans up that test code), but realize that could hold things up.

Paul.