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

Remove transitive modifier from optional dependencies #2930

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

ppkarwasz
Copy link
Contributor

This PR:

  • Add a Groovy script that checks the generated module-info.class files for static transitive dependencies.
  • Modifies the BND configuration not to mark org.jspecify as transitive.

We remove the transitive modifier from all optional dependencies.

Closes #2929.

Note: Once these changes are accepted, I'll move the Groovy script to logging-parent and implement apache/logging-parent#196 too.

@ppkarwasz ppkarwasz changed the title Feature/2.x/2929 remove transitive static Remove transitive modifier from optional dependencies Sep 8, 2024
Copy link
Member

@vy vy left a comment

Choose a reason for hiding this comment

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

Question: Shouldn't we instead

  • enforce a whitelist of static dependencies (so every optional will need to be a conscious decision, not a mistake) and
  • ban transitive static?

log4j-parent/pom.xml Outdated Show resolved Hide resolved
@ppkarwasz
Copy link
Contributor Author

Question: Shouldn't we instead

* enforce a whitelist of `static` dependencies (so every optional will need to be a conscious decision, not a mistake) and

I am not sure whether to whitelist our optional dependencies or all our runtime dependencies. In general there is a mismatch between the scope of Maven dependencies and the OSGi and JPMS optionality.

In general the agreement is that BND should not automatically mark as optional the packages in optional Maven dependencies (see bndtools/bnd#2713), but I believe it could at least warn about required packages, which are optional Maven dependencies.

log4j-parent/pom.xml Outdated Show resolved Hide resolved
This adds a Groovy script that fails the build
if any optional JPMS module has the `transitive` qualifier.

We remove the `transitive` modifier from all optional dependencies.

Closes #2929.

Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
@ppkarwasz ppkarwasz force-pushed the feature/2.x/2929_remove_transitive_static branch from 3c6d9d4 to c7a59eb Compare September 12, 2024 19:18
@ppkarwasz ppkarwasz merged commit c7a59eb into 2.x Sep 12, 2024
3 of 8 checks passed
@ppkarwasz ppkarwasz deleted the feature/2.x/2929_remove_transitive_static branch September 12, 2024 19:19
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.

Compiling with 2.24.0 fails: module not found: org.jspecify
2 participants