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

Maven standard dependencies should take precedence over managed dependencies #1000

Merged
merged 3 commits into from
May 30, 2024

Conversation

cuixq
Copy link
Contributor

@cuixq cuixq commented May 28, 2024

Managed dependencies are not real dependencies so they should not take precedence over standard dependencies.

Dependency management is used to control the versions of artifacts used in transitive dependencies. https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management

Also, version requirements in managed dependencies are only referred when the requirement is not defined for that dependency in standard dependencies section.

@cuixq cuixq requested review from G-Rath and another-rex May 28, 2024 06:02
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.53%. Comparing base (e94c6b5) to head (f25ebde).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1000      +/-   ##
==========================================
+ Coverage   64.94%   65.53%   +0.59%     
==========================================
  Files         149      149              
  Lines       12259     9518    -2741     
==========================================
- Hits         7962     6238    -1724     
+ Misses       3845     2827    -1018     
- Partials      452      453       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@G-Rath
Copy link
Collaborator

G-Rath commented May 28, 2024

Can you just check this against what I believe is the section of the docs I referred to when I did this?

  • a and c both are declared as dependencies of the project so version 1.0 is used due to dependency mediation. Both also have runtime scope since it is directly specified.
  • b is defined in B's parent's dependency management section and since dependency management takes precedence over dependency mediation for transitive dependencies, version 1.0 will be selected should it be referenced in a or c's POM. b will also have compile scope.
  • Finally, since d is specified in B's dependency management section, should d be a dependency (or transitive dependency) of a or c, version 1.0 will be chosen - again because dependency management takes precedence over dependency mediation and also because the current POM's declaration takes precedence over its parent's declaration.

I assume my mistake is in not understanding what's being referred to is triggered in specific cases which are not true (most of the time at least) for the scanner when it's reading a pom, but would like to have that confirmed.

@cuixq
Copy link
Contributor Author

cuixq commented May 29, 2024

I think the dependency management in root pom.xml takes precedence over the dependencies in pom.xml of transitive dependencies.

However, since we are only scanning the root pom.xml with direct dependencies only, dependencies take precedence over the managed dependencies, and this is also what mvn does.

@cuixq cuixq merged commit 854cb01 into google:main May 30, 2024
13 checks passed
@cuixq cuixq deleted the maven-dep branch May 30, 2024 04:18
josieang pushed a commit to josieang/osv-scanner that referenced this pull request Jun 6, 2024
…dencies (google#1000)

Managed dependencies are not real dependencies so they should not take
precedence over standard dependencies.

Dependency management is used to control the versions of artifacts used
in transitive dependencies.
https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management

Also, version requirements in managed dependencies are only referred
when the requirement is not defined for that dependency in standard
dependencies section.
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.

None yet

4 participants