Services/configuration/context webrevs

[Mandy Chung]

On 7/5/2012 2:47 AM, Paul Sandoz wrote:
>
>> 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 andX at 1.0  and the solution includes M, P, andX 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:
>
>    modulex at 1.0  {
>      permits y;
>    }
>
>    moduley at 1.0  {
>      requires x;
>    }
>
>    modulez 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.

The attachment is missing.  The resulting solution would be z at 1.0 only 
which is correct.  In the example I showed above, the service provider 
module P requires a module X at 1.0 that doesn't satisfy the constraints in 
the previously resolved modules.  I would consider that module P will 
not be included as its dependency X at 1.0 fails to be resolved.

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

In your example, there is no service provider module in the picture.  
The current implementation should resolve z with no x module in the 
solution.   Just to make sure this is the behavior you're thinking about 
- if x is required by a service provider module p being resolved in 
phase 1 due to  (z but not x is one of the resolved module in phase 0),  
would the service provider module requiring x be included in the solution?
>> 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!

One inconsistency I know of is the for-each statement "for (T a: 
iterable)" vs "for (T a : iterable)" whether a space is needed after 
":".   Other than that the coding convention for jigsaw is fairly 
consistently applied in the org.openjdk.jigsaw source e.g. 4-space 
indentation, and line breaks.  Is there any other inconsistency you observe?

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

Is it possible to have a service provider dependency that failed to 
resolve in phase n but successfully resolved later in phase > n?  During 
resolution, the module libraries should be locked and there should be no 
change to the list of available modules.   In any case, I think it's 
fine not to check that and the resolver/library would do better caching 
and avoid reading the same set of module-info multiple times.

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

I see and you have several changes lined up.  Would you consider to 
include the tests update in part of the refactoring webrev?   Test cases 
are sometimes helpful for code review.

Mandy