[export-dep-as-jar] Respect strict_deps in libraries field
#9145
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
wisechengyi
approved these changes
Feb 19, 2020
tests/python/pants_test/backend/project_info/tasks/test_export_dep_as_jar.py
Outdated
Show resolved
Hide resolved
wisechengyi
pushed a commit
that referenced
this pull request
May 15, 2020
…9485) ### Problem In #9145, we made the `libraries` field in a `target` entry respect `stric_deps`. While this is correct to construct an accurate compile classpath, there are certain cases where it now becomes impossible to reconstruct a runtime classpath. Runtime classpaths shouldn't respect strict deps, so we need to store that information somewhere. ### Solution Implementation: - We implement this by storing extra information when we compute the flat non-modulizable dependency list. We introduce a new type to hold that information: ``` @DataClass() class FlatDependenciesInfo: """Holds flat meta information about the runtime and compile time dependencies of a target. Compile time dependencies should respect strict_deps, whereas runtime deps should not. """ compile_deps: FrozenOrderedSet[Target] runtime_deps: FrozenOrderedSet[Target] ``` - We modify `_flat_non_modulizable_deps_for_modulizable_targets` so that it returns a `Dict[Target, FlatDependenciesInfo]` (instead of the previous `Dict[Target, FrozenOrderedSet[Target]]`). - In that function, when we pre-seed the dict with targets that have `strict_deps` enabled, we calculate the `runtime_deps` with the target's `closure()`. This is potentially expensive, but the implementation remains significantly cleaner this way, because we can keep avoiding expanding `strict_deps` targets. This can be changed if needed. ### Result Introduce API changes to `export-dep-as-jar`: - `libraries` entries for targets will now be named `compile_libraries`. - There is a new entry in a target, called `runtime_libraries`, which will include all non-modulizable targets that are dependencies, not restricting strict_deps. This field should be combined with the values of `runtime_libraries` of all dependencies under the `targets` field to create the runtime classpath. **Note** Depends on #9748
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.
Problem
The
librariesfield of a target didn't respect thestrict_depsof the target, making it not correct.Solution
Precalculate a set of "things we want in the classpath", which takes
strict_depsinto account.Predicate including something in the
librariesfield, so that we only include things from this set.Result
The test introduced in the third commit failed before the change, succeeds after.
Notes: