-
Notifications
You must be signed in to change notification settings - Fork 5
fix: Include dependency substitutions in dependency extractors #31
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
Conversation
pablobaxter
left a comment
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.
@inez @tmellor-block We're going to need to add some tests to handle the dependency substitution use case this PR is fixing. Maybe a quick follow?
...upport/android/src/test/kotlin/com/squareup/tooling/support/android/VariantExtractorsTest.kt
Show resolved
Hide resolved
...rt/core/src/main/kotlin/com/squareup/tooling/support/core/extractors/DependencyExtractors.kt
Outdated
Show resolved
Hide resolved
|
So looking further at what is being resolved here, this is actually solving an issue with dependency substitution of an included build. I believe this to be a good fix to the issue, although the concern I have is that it would increase the time spent by the Gradle daemon configuring, since now it is resolving dependencies. I wonder if having a flag for configuration resolution should be considered? |
pablobaxter
left a comment
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.
- Remove
RELEASE_SIGNING_ENABLED=false
includeBuild projects in dependency extractors
inez
left a comment
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.
Please make sure to address @pablobaxter's comments. Thx!
Thanks for reviewing this. Yes, that has been addressed here 2f95be9. I also discussed with Pablo this implementation last week and resolved #31 (comment). |
|
Pulled in all commits and work into this PR: #32. |
This addresses an issue with the
affected-pathscalculator where projects depending on included builds were not being returned when changes occurred in those included builds. This is becauseConfiguration.allDependenciesused in dependency extractor was including artifacts from the included builds rather than the project dependency substitutions.Details:
This updates
DependencyExtractors.ktto include project dependencies from resolved artifacts.compileOnlyconfiguration cannot be resolved).Testing:
Published a snapshot version and used that against a gradle project with included builds. Verified that the new affected-paths accounts for projects dependent on included builds.