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