-
Notifications
You must be signed in to change notification settings - Fork 19
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
Per variant classpath support #89
Closed
arunkumar9t2
wants to merge
30
commits into
grab:master
from
arunkumar9t2:refactor-version-handling-2
Closed
Per variant classpath support #89
arunkumar9t2
wants to merge
30
commits into
grab:master
from
arunkumar9t2:refactor-version-handling-2
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
Contributor
arunkumar9t2
force-pushed
the
refactor-version-handling-2
branch
4 times, most recently
from
July 20, 2023 05:35
f6d0dda
to
eaf1fc5
Compare
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
…t dependencies task
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
This reverts commit a904e12.
…endency resolution service
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
Signed-off-by: arunkumar9t2 <[email protected]>
arunkumar9t2
force-pushed
the
refactor-version-handling-2
branch
from
August 2, 2023 23:13
3f63b19
to
8eb8a3b
Compare
minkuan88
reviewed
Aug 4, 2023
grazel-gradle-plugin/src/main/kotlin/com/grab/grazel/gradle/MigrationCriteria.kt
Show resolved
Hide resolved
minkuan88
approved these changes
Aug 4, 2023
Signed-off-by: arunkumar9t2 <[email protected]>
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.
Proposed Changes
Continuation from
maven_install
classpath #44Variant
API #57variant
API and addandroidTest
base variant #92to implement Gradle like multiple classpath support in Bazel. In Gradle, each android variant can have its own classpath, for example
debugImplementation
extends fromimplementation
and at the same time can use a different version of dependency indebug
variant. This is not possible to achieve in currentmaven_install
rules we are generating since artifacts appearing inmaven_install
go through dependency resolution at once. So even if we haveartifact:1.0.0
andartifact:1.0.0-debug
, due to dependency resolution only one of them will get picked which is not correct.To solve the multiple classpath problem, we could generate several
maven_install
thus forcing each one to resolve differently. While that works they are disjointed and transitives might get duplicated across each one and that is not how Gradle'sextendsFrom
work. To resolve this problem, we could link severalmaven_install
s by using override_targets feature whereby we generate overrides for transitives if we detect they are being duplicated. This allows us to have multiple classpath as well as not break the One Version Enforcement rule. Which is exactly what is being attempted in this change.Notable behavior changes
maven_install
s if we detect we need them, usually when variant specific configuration is used in Gradle likedebugImplementation
.maven_install
to avoid dependency resolution conflicts in Bazel (mostly due to coursier, being addressed by Introduce a new maven dependency resolver bazel-contrib/rules_jvm_external#807 ). Explicit transitives +version_conflict_policy
set topinned
ensure same classpath from Gradle is being used.migrateToBazel
performance especially on large projects.Implementation notes
Resolving dependencies
Resolving dependencies now follow best practices by implementing a dedicated task
ResolveVariantDependenciesTask
with variant specific resolution result as input. Relationship between variants are also carried into this task, for example consider a variant graph like belowThen the same relationship is established in each
ResolveVariantDependenciesTask
instance, this is due to unverified assumption and aim to mimic topological sort that happens during dependency resolution.Computing workspace entries
ComputeWorkspaceDependenciesTask
is added as an aggregator and is now the source of truth for all computed dependencies. It collects alljson
s from eachResolveVariantDependenciesTask
to calculate the versions, exclude rules, repositories, override targets and ultimately the final classpath. This is serialized to disk as json.Dependency resolution service
Classpath computed by
ComputeWorkspaceDependenciesTask
needs to be queried in other tasks as well, to speed and to cache the json parsing result, a shared service is used. Implemented such a way that it is compatible with bothFROM-CACHE
orUP-TO-DATE
state ofComputeWorkspaceDependenciesTask
Result serialization
Serializing to disk is essential for using Gradle's task up to date check. Currently we use
json
andkotlinx.serialization
but protobuf can be investigated if serialisation is a bottleneck.Results