autonomousapps / dependency-analysis-gradle-plugin

Gradle plugin for JVM projects written in Java, Kotlin, Groovy, or Scala; and Android projects written in Java or Kotlin. Provides advice for managing dependencies and other applied plugins
Apache License 2.0
1.8k stars 117 forks source link

DAGP should be aware of KMP (multiplatform) artifacts and not advise downgrading from multiplatform facade to "-android"-specfic artifact (androidx is one example) #907

Closed yschimke closed 1 year ago

yschimke commented 1 year ago

Bump from alphas to betas causing issues https://github.com/google/horologist/pull/1312/files#diff-697f70cdd88ba88fe77eebda60c7e143f6ad1286bca75017421e93ad84fb87df

Failures https://github.com/google/horologist/actions/runs/5088063599/jobs/9144041802

Caused by: com.autonomousapps.exception.BuildHealthException: Unused dependencies which should be removed:
  api("androidx.compose.foundation:foundation-layout:1.5.0-beta01")
  api("androidx.compose.foundation:foundation:1.5.0-beta01")
  api("androidx.compose.runtime:runtime:1.5.0-beta01")
  api("androidx.compose.ui:ui-graphics:1.5.0-beta01")
  api("androidx.compose.ui:ui:1.5.0-beta01")
  debugImplementation("androidx.compose.material:material-icons-extended:1.5.0-beta01")
  debugImplementation("androidx.compose.ui:ui-tooling-preview:1.5.0-beta01")
  implementation("androidx.compose.material:material-icons-core:1.5.0-beta01")
  implementation("androidx.compose.ui:ui-text:1.5.0-beta01")
  implementation("androidx.compose.ui:ui-unit:1.5.0-beta01")
  testImplementation("androidx.compose.material:material-icons-extended:1.5.0-beta01")
  testImplementation("androidx.compose.ui:ui-test-junit4:1.5.0-beta01")
  testImplementation("androidx.compose.ui:ui-test:1.5.0-beta01")

Transitively used dependencies that should be declared directly as indicated:
  api("androidx.compose.foundation:foundation-android:1.5.0-beta01")
  api("androidx.compose.foundation:foundation-layout-android:1.5.0-beta01")
  api("androidx.compose.runtime:runtime-android:1.5.0-beta01")
  api("androidx.compose.ui:ui-android:1.5.0-beta01")
  api("androidx.compose.ui:ui-graphics-android:1.5.0-beta01")
  debugImplementation("androidx.compose.material:material-icons-extended-android:1.5.0-beta01")
  debugImplementation("androidx.compose.ui:ui-tooling-preview-android:1.5.0-beta01")
  implementation("androidx.compose.material:material-icons-core-android:1.5.0-beta01")
  implementation("androidx.compose.ui:ui-text-android:1.5.0-beta01")
  implementation("androidx.compose.ui:ui-unit-android:1.5.0-beta01")
  testImplementation("androidx.compose.material:material-icons-extended-android:1.5.0-beta01")
  testImplementation("androidx.compose.ui:ui-test-android:1.5.0-beta01")
  testImplementation("androidx.compose.ui:ui-test-junit4-android:1.5.0-beta01")

Build scan link

Plugin version 1.20.0

Gradle version 8.1 (there is a warning it is unsupported)

(Optional) Android Gradle Plugin (AGP) version 8.0.2

reason output for bugs relating to incorrect advice It feels like apps should just depend on androidx.compose.ui:ui instead of androidx.compose.ui:ui-android

poliver commented 1 year ago

I noticed this the other day, as well. It seems likely to be an issue with the Compose BOM going forward. I'm currently using Chris Banes' Compose BOM (alphas), so I'm not entirely sure what the behavior will be once Google promotes these artifacts to their stable BOM. I did confirm that these -android-suffixed artifacts will not be added to the BOM containing alphas, so the resolution will need to be in the configuration of the DA plugin. The workaround I'm currently using is to add a bundle for each dependency:

bundle("androidx-compose-animation") {
  includeDependency("androidx.compose.animation:animation")
  includeDependency("androidx.compose.animation:animation-android")
}

But there are many artifacts, so this is quite a bit of boilerplate. It would be useful to have something like exclude(), but which accepts a regex similar to includeDependency(), making it possible to exclude all of these with a single statement. It seems this added functionality would also remove the need for a dedicated ignoreKtx().

autonomousapps commented 1 year ago

Are you saying that you have something like this:

api "androidx.compose.foundation:foundation-layout:1.5.0-beta01"

and the plugin is saying you should instead use

api "androidx.compose.foundation:foundation-layout-android:1.5.0-beta01"

?

yschimke commented 1 year ago

Yes, which I don't think is correct.

I can retest if it's fixed with the next set of releases.

autonomousapps commented 1 year ago

This looks a lot like what happens with "-ktx" dependencies. If so, we could bake in a similar assumption. Do you have a minimal reproducer?

What is the difference between the normal and android-suffixed dependencies?

poliver commented 1 year ago

@autonomousapps The workaround in my comment above is an example of what I am currently doing to silence the warnings, but it gets very verbose due to all of the dependencies in the Compose BOM. These libraries are now being built with KMP, hence the new -android artifacts.

autonomousapps commented 1 year ago

These libraries are now being built with KMP

a-ha, thanks. What I suggested with -ktx would work, but I think a more robust solution would be to add in some more awareness for multi-platform dependencies. Might do one and then the other, we'll see.

poliver commented 1 year ago

Hey @autonomousapps here's a project that repro's the issue. It was generated by Android Studio Flamingo 2022.2.1 Patch 2. I omitted use of the Compose BOM for simplicity. https://github.com/poliver/KMPComposeDeps

ZacSweers commented 1 year ago

Here's another: https://github.com/ZacSweers/CatchUp

Relevant bit from the final-advice.json

{
  "projectPath": ":libraries:compose-extensions:pull-refresh",
  "dependencyAdvice": [
    {
      "coordinates": {
        "type": "module",
        "identifier": "androidx.compose.material:material",
        "resolvedVersion": "1.6.0-alpha01",
        "gradleVariantIdentification": {
          "capabilities": [],
          "attributes": {}
        }
      },
      "fromConfiguration": "implementation"
    },
    {
      "coordinates": {
        "type": "module",
        "identifier": "androidx.compose.material:material-android",
        "resolvedVersion": "1.6.0-alpha01",
        "gradleVariantIdentification": {
          "capabilities": [],
          "attributes": {}
        }
      },
      "toConfiguration": "implementation"
    },

Re: handling it like ktx files – I think this can be smarter about it since DAGP can ask the dependency's gradle module metadata.

For example with this one, this is its its module metadata from here. The org.jetbrains.kotlin.platform.type attribute is the salient bit.

{
  "formatVersion": "1.1",
  "component": {
    "group": "androidx.compose.material",
    "module": "material",
    "version": "1.6.0-alpha01",
    "attributes": {
      "org.gradle.status": "release"
    }
  },
  "createdBy": {
    "gradle": {
      "version": "8.0"
    }
  },
  "variants": [
    {
      "name": "androidxSourcesElements",
      "attributes": {
        "org.gradle.category": "documentation",
        "org.gradle.dependency.bundling": "external",
        "org.gradle.docstype": "sources",
        "org.gradle.usage": "androidx-multiplatform-docs"
      },
      "dependencyConstraints": [
        {
          "group": "androidx.compose.material",
          "module": "material-icons-core",
          "version": {
            "requires": "1.6.0-alpha01"
          }
        },
        {
          "group": "androidx.compose.material",
          "module": "material-icons-extended",
          "version": {
            "requires": "1.6.0-alpha01"
          }
        },
        {
          "group": "androidx.compose.material",
          "module": "material-ripple",
          "version": {
            "requires": "1.6.0-alpha01"
          }
        }
      ],
      "files": [
        {
          "name": "material-1.6.0-alpha01-multiplatform-sources.jar",
          "url": "material-1.6.0-alpha01-multiplatform-sources.jar",
          "size": 232974,
          "sha512": "4213922dc40a26203197a1f062f8745bd0355fe5ffd2b722c6b0ba9ef1d132456c22cc3db4c810a363a649ca30c04cf66cf2cd1564e055bfc8e30d4df1b22c00",
          "sha256": "08414920819f283419e5cf255fc3eb1f58274ecac1b83705012cf94d5acfa4be",
          "sha1": "d6cc2e97fa022ffee99b8bdbeeb352952adda4f3",
          "md5": "a03e5b02db44eba8440599f68e775136"
        }
      ]
    },
    {
      "name": "metadataApiElements",
      "attributes": {
        "org.gradle.category": "library",
        "org.gradle.usage": "kotlin-metadata",
        "org.jetbrains.kotlin.platform.type": "common"
      },
      ...
  ]
}

If it's only seeing the -android artifact, its module metadata is here. It has the same attribute for the kotlin platform type, but I think the more relevant bit is the url for the component pointing back at the common (aka desirable) dep

{
  "formatVersion": "1.1",
  "component": {
    "url": "../../material/1.6.0-alpha01/material-1.6.0-alpha01.module",
    "group": "androidx.compose.material",
    "module": "material",
    "version": "1.6.0-alpha01",
    "attributes": {
      "org.gradle.status": "release"
    }
  },