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

Do not rely on svm scope set to provided in the BoM #12138

Merged
merged 1 commit into from
Sep 23, 2020

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Sep 16, 2020

This PR does the following:

  • Sets an explicit <scope>provided</scope> on every svm dependency
  • Removed svm dependencies from deployment modules where it is redundant

Motivation:

Relying on a provided or test scope defined in BoM is not necessarily a good practice. People seeing

        <dependency>
            <groupId>org.graalvm.nativeimage</groupId>
            <artifactId>svm</artifactId>
        </dependency>

may naturally expect that the scope is compile, but it is not because it is set in the BoM. When people see this in Quarkus they may think that svm should actually have the compile scope and they may wrongly copy it in that way to their extension projects. That may lead to bugs like apache/camel-quarkus#1182.

I think it is safer for all parties to have an explicit <scope>provided</scope> on all places where the dependency is used.

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

I agree 100%. Perhaps it's safer to remove the scope from the BOM in this PR as well?

@ppalaga
Copy link
Contributor Author

ppalaga commented Sep 17, 2020

I agree 100%. Perhaps it's safer to remove the scope from the BOM in this PR as well?

Thrid party extensions importing our BOM may rely on that. We should send a warning via dev ML and platform ML first.

@gastaldi gastaldi merged commit 11c470b into quarkusio:master Sep 23, 2020
@gastaldi gastaldi added this to the 1.9.0 - master milestone Sep 23, 2020
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.

2 participants