dropbox / dependency-guard

A Gradle plugin that guards against unintentional dependency changes.
Apache License 2.0
406 stars 15 forks source link

Feature request: filter out BOM artifacts #54

Open ZacSweers opened 1 year ago

ZacSweers commented 1 year ago

These are not actually a real dependency, rather they just dictate the version of other dependencies owned by it. This leaks across buildscript classpaths too. For example - buildscript { dependencies { classpath(platform(libs.coroutines.bom)) } } will make this show up as a configuration dependency in all subprojects using dependencyGuard as well even if they don't use coroutines. My sense here is that it should filter these.

handstandsam commented 1 year ago

Nice, I don't have any projects using a bom. Can you provide an example GAV for one and I can work on an example based on that.

ZacSweers commented 1 year ago

Kotlin has one - org.jetbrains.kotlin:kotlin-bom

handstandsam commented 1 year ago

I'm hoping to get to this one soon. There is a large configuration cache PR I'd want to get in first (as it changes a bunch), but this should be fun one to learn about boms for me. :-)

Thanks for your patience.

handstandsam commented 1 year ago

Finally hitting this issue now that we're bringing in the compose-bom. From a technical standpoint, I'm not sure how to approach it quite yet.

This is our code used to traverse the dependency tree and collect info about artifacts.

https://github.com/dropbox/dependency-guard/blob/10ee51044148a001dd8471312772497c80b9faaa/dependency-guard/src/main/kotlin/com/dropbox/gradle/plugins/dependencyguard/internal/DependencyVisitor.kt#L18-L24

ModuleComponentIdentifier (source link) doesn't tell me the packaging type of the artifact which is what I'd use to distinguish aar/jar/bom/etc.

Are you aware of another Gradle API to traverse the dependency tree which contains packaging info? That would allow this change to be made.

Again, thanks for your patience on this issue!

handstandsam commented 1 year ago

A workaround could be to add

baselineMap = { 
  if (it.contains("-bom") {
    null 
  } else {
    it
  } 
}

Returning null from the baselineMap lambda will remove it from the baseline file.

Gross, but a possible workaround.

handstandsam commented 1 year ago

@ZacSweers - @devpalacio and I have been adding Version Catalogs on our projects here and see the same behavior. Because these show up in the ./gradlew :module:dependencies list, this seems like it is the "correct" behavior.

Please use the baselineMap suggestion above as a way to filter out those bom entries from the baseline files if you do not want them in your baselines.


I'm going to close this since:

Thank you, and we can re-open if you are aware of some way we can do this which is cleaner.

ZacSweers commented 5 months ago

It appears that it is possible to determine this. This is how licensee did it: https://github.com/cashapp/licensee/pull/311

handstandsam commented 5 months ago

If this is added, do we have it behind a flag, or make it default behavior?

ZacSweers commented 4 months ago

Make it the default for sure