-
Notifications
You must be signed in to change notification settings - Fork 315
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
Project type usage preparations #9742
Conversation
Align with the "legacy" Gradle `GradleDependencyHandler`, also see 666120a. Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
Previously, the `projectType` was "PIP" if the former was called via Pipenv or Poetry. Fix this to now use dedicated project types for the latter. Signed-off-by: Sebastian Schuberth <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9742 +/- ##
=========================================
Coverage 68.08% 68.08%
Complexity 1293 1293
=========================================
Files 249 249
Lines 8845 8845
Branches 923 923
=========================================
Hits 6022 6022
Misses 2434 2434
Partials 389 389
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
val pipenvAnalyzerConfig = analyzerConfig | ||
.withPackageManagerOption(managerName, "overrideProjectType", projectType) | ||
|
||
return Pip(managerName, analysisRoot, pipenvAnalyzerConfig, repoConfig) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of scope: Is it actually fine to create a new instance of Pip each time? (IIRC dependency graph building relies on using only once instance)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC dependency graph building relies on using only once instance
Actually, the requirement is that not multiple package managers that manage the same project type are enabled at the same time:
ort/model/src/main/kotlin/DependencyGraphNavigator.kt
Lines 48 to 55 in b2e6c3d
// TODO: Relax this assumption that package manager names start with the name of the type of project | |
// they manage, for example that "GradleInspector" manages "Gradle" projects. | |
val managers = graphs.keys.filter { it.startsWith(project.id.type) } | |
val manager = requireNotNull(managers.singleOrNull()) { | |
"All of the $managers managers are able to manage '${project.id.type}' projects. Please enable only one " + | |
"of them." | |
} |
So, as this commit fixes the multiple Pip
instances to have different project types, we should be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I believe I was after a different requirement, was it that same DependencyGraphBuilder
or Handler
instance needs to be used, maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea...
Please have a look at the individual commit messages for the details.