Rework how maven project jars are calculated #475
Merged
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
jin
approved these changes
Oct 28, 2020
| ctx, | ||
| ctx.executable._singlejar, | ||
| artifact_jars, | ||
| depset(transitive = [ji.transitive_runtime_jars for ji in info.dep_infos.to_list()]).to_list(), |
There was a problem hiding this comment.
This line looks like it's creating the transitive dep_info jars to be excluded from artifact_jars, where artifact_jars already excluded this same list of transitive_runtime_jars from calculate_artifact_jars. Am I misreading something or is this doing redundant work?
There was a problem hiding this comment.
We've excluded the same jars, but we pass in the jars to exclude to avoid including classes from them too. Quite often you find jars that contain bits of guava or the Nullable annotations by mistake. We don't want to pollute the classpath with duplicate classes. The MergeJars binary checks for this and handles it for us.
Previously, we assumed that all the information required for a project all code to be included was inside the main repository. This was put in place largely to prevent code depending on `rules_proto` from pulling in the classes from protobuf itself. Issue bazelbuild#448 demonstrated that this assumption wasn't correct. We now gather all the JavaInfos that should be included in the artifact, as well as those of dependencies of the artifact (called "dep_infos" in the MavenInfo provider) We then calculate the difference of those sets to determine the input files to add to the generated project jar. However, this is not sufficient on its own in the case of protocbuf. As such, we also scan for classes to exclude from the project jar by examaning the contents of the "dep_infos" runtime jars. Furthermore, jars that are created by aspects have no way of expressing what their maven dependencies are. To work around this, rules may now express `MavenHintInfo` which will be used to help calculate the contents of the artifact jars and maven dependencies.
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.
Previously, we assumed that all the information required for a project
all code to be included was inside the main repository. This was put
in place largely to prevent code depending on
rules_protofrompulling in the classes from protobuf itself.
Issue #448 demonstrated that this assumption wasn't correct.
We now gather all the JavaInfos that should be included in the
artifact, as well as those of dependencies of the artifact (called
"dep_infos" in the MavenInfo provider) We then calculate the
difference of those sets to determine the input files to add to the
generated project jar.
However, this is not sufficient on its own in the case of
protocbuf. As such, we also scan for classes to exclude from the
project jar by examaning the contents of the "dep_infos" runtime jars.
Furthermore, jars that are created by aspects have no way of
expressing what their maven dependencies are. To work around this,
rules may now express
MavenHintInfowhich will be used to helpcalculate the contents of the artifact jars and maven dependencies.