Global dependency graph #3913
Merged
Global dependency graph #3913
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
6 times, most recently
from
f979787
to
cdabe9c
Compare
sschuberth
requested changes
Apr 22, 2021
analyzer/src/funTest/assets/projects/external/multi-kotlin-project-expected-output-cli.yml
Show resolved
Hide resolved
| // is typically empty, so it has to be populated manually. | ||
| val packages = result.packages.takeIf { it.isNotEmpty() } ?: managerResult.sharedPackages | ||
| result.copy(project = resolvedProject, packages = packages.toSortedSet() | ||
| ) |
There was a problem hiding this comment.
Should go to the previous line.
There was a problem hiding this comment.
Doh! While at it, I extracted this block into a separate function. This will be useful for other tests for package managers as well.
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
2 times, most recently
from
a0c97f6
to
65093f7
Compare
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
from
65093f7
to
004244e
Compare
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
from
a7bb5a0
to
2064dc3
Compare
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
from
2064dc3
to
e80d500
Compare
mnonnenmacher
requested changes
Apr 30, 2021
mnonnenmacher
requested changes
May 6, 2021
mnonnenmacher
requested changes
May 6, 2021
mnonnenmacher
requested changes
May 6, 2021
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
3 times, most recently
from
9db5faf
to
8e38b04
Compare
mnonnenmacher
previously approved these changes
May 11, 2021
sschuberth
reviewed
May 12, 2021
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
from
8e38b04
to
43dc70a
Compare
sschuberth
previously approved these changes
May 12, 2021
So far the result of a package manager has been a map with results for the single definition files processed by resolveDependencies(). In future, some additional information is needed, which is more global and not related to single definition files - namely a shared dependency graph for all the packages referenced by the analyzed projects. Therefore, add a new PackageManagerResult class as a replacement of the ResolutionResult type alias that supports this additional data. Currently, only the results of the single definition files are filled. Later commits will modify package managers to construct a shared dependency graph. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
The idea is that concrete package manager implementations can override this function to produce a result with additional information. The base implementation just creates a result with the results collected for the single projects. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
When support for the DependencyGraph format was added to Gradle in c8d69ca, the expectations of tests have been adapted to the new format. This is going to be problematic for upcoming changes on the format, as all these files would have to be changed again - and when changing the code and the test expectations at the same time, there is no guarantee that the new implementation is actually correct. Therefore, use a different approach: Revert the test expectations to their original format. This format now serves as a reference. In the test cases, before comparing the actual result to the expected one, apply a transformation to the former to resolve the project scopes. This makes sure that the analyzer result in the new representation has the same content as the reference. If there are future changes on the data format, the tests do not have to be touched again, only the transformation. This transformation is, however, needed anyway to make ORT results in the new form compatible with client code. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
This is needed when constructing dependency graphs from the dependencies of multiple projects. As scope names are typically not unique across multiple projects, there must be a way to deduplicate them, so that it can be determined which scope belongs to which project. Therefore, add functions to construct scope names qualified by the project ID and to remove the qualifier again. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
In future, a dependency graph can store the dependencies of multiple projects. With this new function, the scopes of a single project can be processed. When generating the map with scopes, qualified scope names can be converted to plain names. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
Add a set with scope names to reference the scopes of this project in the shared dependency graph. Also add an overloaded withResolvedScopes() function to create the scopes structure from a shared dependency graph. Note that there are currently two options for associating a project with a dependency graph: a project-local graph and a shared one, but this is only temporary. As the shared graph is more efficient, the option for the local graph is going to be removed once it is no longer used by any package manager. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
This is going to be used later to add packages that are not part of a ProjectAnalyzerResult. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
Package managers supporting this feature can now construct a shared DependencyGraph over the projects they analyze and put it into the AnalyzerResult object. Add support for this property to AnalyzerResultBuilder as well. Also add a withResolvedScopes() function that allows obtaining a result in which all projects have been converted to use the classic scope-based format for dependencies. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
Rework the Gradle package manager to switch from a dependency graph per project to a shared dependency graph. This commit does actually multiple things, but this is necessary to avoid inconsistencies or failing tests: Change Gradle to place the DependencyGraphBuilder in a property, so that it is reused for all the projects that are analyzed. Update GradleDependencyHandler to support a setter for the remote repositories, as they may be different for the projects processed. Update Analyzer to deal with a shared dependency graph. If present, the graph is passed to the AnalyzerResultBuilder. In OrtResult, delegate to AnalyzerResult when constructing a result with resolved scopes rather than updating the projects manually. Adapt Pub (which invokes Gradle) to the changes in the result produced by Gradle. Make the resolveSingleProject() function used by tests compatible with shared dependency graphs. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
Dependency graphs should always be shared between projects, as this is more efficient. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
This makes sure that all commands that process ORT result files can access fully initialized projects with scope information. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
oheger-bosch
force-pushed
the
oheger-bosch/analyzer/global_dependency_graph
branch
from
43dc70a
to
39ce120
Compare
sschuberth
approved these changes
May 12, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change the DependencyGraph format to be global for a package manager result, i.e. combine the dependencies of multiple projects.
Note: The new approach is not that different from the old one, but there is some shift in the responsibility of dependency information: The dependency graph is no longer maintained by a project, but projects only have references to the shared dependency graph. This also affects the conversion of an
OrtResultfrom the graph format to the classic structure with project scopes, which is expected by the components that operate on dependency data.The intension of this PR is to keep the single changes small without breaking any tests. To achieve this, some redundancy had to be introduced temporarily. Especially the
Projectclass is added a new dependency-related property, and the obsolete one is removed only later after other components have been reworked. If this is not desired, some commits can be squashed, but then there is a rather big bang change.