Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNG-8195] Add DependencyResolverResult.getModuleName(Path) method #1625

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

desruisseaux
Copy link
Contributor

@desruisseaux desruisseaux commented Aug 5, 2024

Clarify the meaning of "module" in documentation, then add two methods for getting Java module information from a DependencyResolverResult:

  • getModuleName(Path) returns a java.lang.String
  • getModuleDescription(Path) returns a java.lang.module.ModuleDescriptor

"Get name" may seem redundant with "get description" because the name can be obtained from the description by ModuleDescriptor.name(). The difference is that "get name" fallbacks on the Automatic-Module-Name attribute of META-INF/MANIFEST.MF if the module descriptor is not found.

The rational for providing those methods in the API is because it allows to use the cache managed by the DefaultDependencyResolverResult implementation. It avoids decoding many times the module-info.class files. Plugins need those information for managing --add-reads and similar options.

Alternative

It would be more natural to have getModuleName() and getModuleDescription() methods in Dependency. But it would imply that dependency contains a getPath() method. I'm not sure why it is not already the case.


https://issues.apache.org/jira/browse/MNG-8195

Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'd try to:

  • add an alias in the xml to replace the <modules> element
  • make this element optional (discover them if not specified explicitely)

@cstamas cstamas changed the title Add DependencyResolverResult.getModuleName(Path) method [MNG-8195] Add DependencyResolverResult.getModuleName(Path) method Aug 9, 2024
@cstamas cstamas added this to the 4.0.0-beta-4 milestone Aug 9, 2024
@desruisseaux
Copy link
Contributor Author

No additional change envisioned for this pull request if the alternative (methods added in the Dependency interface) is not applied.

@gnodet
Copy link
Contributor

gnodet commented Aug 12, 2024

Could we move the subproject thing into a separate PR ?
I'd rather merge them with the addition of <subprojects> in the model.
I've raised https://issues.apache.org/jira/browse/MNG-8210 for that.

@desruisseaux
Copy link
Contributor Author

No problem. I will try to apply this separation and other comments in the next few days.

@desruisseaux
Copy link
Contributor Author

Done with #1649.

@gnodet
Copy link
Contributor

gnodet commented Aug 13, 2024

Done with #1649.

@desruisseaux Could you remove the commit to rename module -> subproject from this PR ?

… `DependencyResolverResult`.

Those methods are helpful for plugins that need to provide `--add-reads` and similar options,
as they allow to reuse the cached values instead of decoding `module-info.class` many times.
@desruisseaux
Copy link
Contributor Author

Done.

@gnodet gnodet merged commit 008d0b4 into apache:master Aug 16, 2024
13 checks passed
@desruisseaux desruisseaux deleted the feat/getModuleDescription branch September 2, 2024 08:22
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants