Module views with exports support

Mandy Chung mandy.chung at oracle.com
Tue Jan 24 20:43:40 PST 2012


Updated webrev at:
   http://cr.openjdk.java.net/~mchung/jigsaw/webrevs/module-views.02/

This incorporates Mark's and Alan's review comments.

On 1/24/2012 9:56 AM, mark.reinhold at oracle.com wrote:
> Suggestions on the API:
>
>    - Rename ModuleViewQuery back to ModuleIdQuery, for now.
>
I also reverted the Category renamed methods and keep them as they are.
       gatherLocalModuleIds
       listModuleIds
       findModuleIds
       findLatestModuleId


>    - Rename Dependence to ViewDependence, rename Service.Dependence to
>      ServiceDependence, and make both of them extend a new class
>      Dependence which declares only the modifiers() method.
>    - Replace Service.ProviderInfo with a map in the ModuleView interface:
>      Map<String,Set<String>>  ModuleView.services().
>
>    - Delete the now-empty Service class.
>
>    - Rename ModuleInfo.requires() to .requiresModules().
>

Done

>    - In ModuleInfo, is the view(ModuleId) method really necessary?  If so
>      then replace it and theand views() methods with a single method:
>      Map<ModuleId,View>  views().  If not, remove it.
>

The view(ModuleId) is not strictly needed. It's used in the Resolver
when it first reads the ModuleInfo of the candidate and finds the
ModuleView of the given mid (L259-265).  It's also used in the new
Category.readModuleView convenient method.  I removed the view(ModuleId)
method for now and replaced the call site with the code to lookup the view.

>    - In ModuleInfo, is the views(String) method really necessary?  If not,
>      remove it.
>

removed.

On 1/17/2012 7:26 AM, Alan Bateman wrote:
> I like views and works really well for the JDK. I'm in two minds on 
> some of the renaming as it forces anyone using the API to have to deal 
> with the concept more than I had expected. For example when one 
> invokes a list or find modules in a Catalog then I think it's 
> debatable as to whether it should return the ModuleIds of the modules 
> or ModuleIds synthesized from the view name and the module's version. 

I keep them as they are for now.

> I'm also in two minds on the changes to the module library where there 
> is a directory with a copy of the module info. I can see how this is 
> used but it kinda feels that this should be an index (key'ed on view) 
> instead. 

Yep.  It's just an easy way to get this prototype implemented while
giving us more time to implement a long-term solution.  Will do that
next.

> In ModuleInfo then defaultView and views() make sense (the views 
> method just needs to be clear that the returned set includes the 
> default view). I'm less sure about the view(ModuleId) and view(String) 
> methods as they aren't strictly required. 

Removed.
> In Service.Dependence I see the constructor takes a boolean to 
> indicate if the dependency is optional. It may be more consistent 
> (with Dependence) to have it take a set of modifiers - if you agree 
> then I can take into the services patch as it's not re-based to your 
> changes. 

I have revised it per Mark's suggestion.

> I don't see anything obviously wrong with the exports work although I 
> found ContextView in the linker hard to deal, maybe it just needs 
> another name.

I thought about naming it just 'View' :)

> What you would think about jmod ls -v printing the exports too? 

I missed that in the webrev.  It's now included in the new version.


> Minor nit is that I didn't understand the changes to Hi :-) 

That's left from my hack to use NetBeans to test jigsaw. I reverted
the change.

Thanks
Mandy






More information about the jigsaw-dev mailing list