affected-paths icon indicating copy to clipboard operation
affected-paths copied to clipboard

fix: Include dependency substitutions in dependency extractors

Open rainecp opened this issue 1 year ago • 2 comments

This addresses an issue with the affected-paths calculator where projects depending on included builds were not being returned when changes occurred in those included builds. This is becauseConfiguration.allDependencies used in dependency extractor was including artifacts from the included builds rather than the project dependency substitutions.

Details:

This updates DependencyExtractors.kt to include project dependencies from resolved artifacts.

  • The new method checks if the configuration can be resolved (e.g. compileOnly configuration cannot be resolved).
  • It retrieves dependencies from the resolution result and differentiates between direct and transitive dependencies.
  • It filters project dependencies and converts their paths so it will supported in the affected-paths logic.

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.

rainecp avatar Aug 08 '24 19:08 rainecp

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 avatar Aug 09 '24 04:08 pablobaxter

Please make sure to address @pablobaxter's comments. Thx!

Thanks for reviewing this. Yes, that has been addressed here https://github.com/square/affected-paths/pull/31/commits/2f95be9b1752f6bc5dfb6ae6778560228fc6e14e. I also discussed with Pablo this implementation last week and resolved https://github.com/square/affected-paths/pull/31#discussion_r1710733057.

rainecp avatar Aug 12 '24 19:08 rainecp

Pulled in all commits and work into this PR: https://github.com/square/affected-paths/pull/32.

pablobaxter avatar Aug 21 '24 21:08 pablobaxter