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

[Alan Bateman]

On 01/08/2012 01:59, Mandy Chung wrote:
>
> Thanks for adding these tests.  It's a general issue.  As we discussed
> it offline today, it'll be addressed in a separate changeset.  I know
> Alan is going to review it too.  Once he reviews it, I can push this
> changeset for you.
Sorry for the delay, but I have finally got time to go this (webrev.04).

Overall I've come around to agreeing with approach. Clearly we could 
conjure up cases where the resolver would find a solution today but not 
find a solution with the changes. On the other hand it is very important 
that the solution (and selected versions) can be explained to the user 
and it isn't nice that the dependencies of a service module can change 
the outcome. So I think the approach makes things more predictable and 
should help the diagnostic output too.

I went through the resolver changes in detail and don't see anything 
obvious wrong. Naming of variables is a bit more verbose than the 
existing code but that's no big deal. A minor point is that 
serviceInterfaceQueue is a LinkedList so the contains check will be 
O(n). No big deal now but worth adding a comment as such usages have a 
habit of biting us later.

I also went through the test changes and the replacement of the shell 
tests with tests based _Configurator/ModuleInfoBuilder and I think it 
looks good (took me a few minutes to properly tune into this but overall 
I think the test coverage has improved quite a bit). I guess I probably 
would drop the "runtime" directory and move runtime.sh up one level as 
services.sh.

So overall I think is a good improvement and good work.

(Mandy - as I was going through the tests I notice we've got the GPL 
with Classpath exception header on the tests. Also did we discuss -esa 
and -ea recently? Usually they are passed through via the jtreg 
-vmoption rather than needing to be in each shell test).

-Alan.