Skip to content
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

Registering multiple transforms with the same values of attributes should fail #26974

Open
hungvietnguyen opened this issue Nov 7, 2023 · 3 comments
Labels
a:feature A new functionality in:artifact-transforms re:comprehensibility reasonable errors and warnings, clear dsl, mental overload

Comments

@hungvietnguyen
Copy link

Current Behavior

Open the attached project: TransformWithSameAttributes.zip

It has the following transform setup (expand to see)
import org.gradle.api.artifacts.type.ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE

plugins {
    java
}

dependencies {
    implementation("com.google.guava:listenablefuture:1.0")
}

abstract class ExampleTransform : TransformAction<ExampleTransform.Parameters> {

    interface Parameters : TransformParameters {
        @get:Input
        var minSdkVersion: Int
    }

    @get:InputArtifact
    abstract val inputArtifact: Provider<FileSystemLocation>

    override
    fun transform(outputs: TransformOutputs) {
        println("Running transform with minSdk = ${parameters.minSdkVersion} for artifact ${inputArtifact.get().asFile.path}")
        outputs.file("transformed-minSdk-${parameters.minSdkVersion}-${inputArtifact.get().asFile.name}").createNewFile()
    }
}

val minSdkAttribute = Attribute.of("minSdkAttribute", String::class.java)

// NOTE: We will register 2 transforms and 2 tasks for the 2 minSdk values below.
for (minSdk in listOf(21, 24)) {
    dependencies {
        registerTransform(ExampleTransform::class) {
            parameters.minSdkVersion = minSdk
            // BUG: If we forget to set the minSdkAttribute below, Gradle should fail because 2
            // transforms are registered with the same set of attributes, but currently Gradle
            // doesn't fail.
//            from.attribute(minSdkAttribute, minSdk.toString())
//            to.attribute(minSdkAttribute, minSdk.toString())
            from.attribute(ARTIFACT_TYPE_ATTRIBUTE, "jar")
            to.attribute(ARTIFACT_TYPE_ATTRIBUTE, "transformed")
        }
    }
    tasks.register("printArtifacts$minSdk") {
        val artifactFiles = configurations.getByName("runtimeClasspath").incoming.artifactView {
            attributes {
                attribute(minSdkAttribute, minSdk.toString())
                attribute(ARTIFACT_TYPE_ATTRIBUTE, "transformed")
            }
        }.artifacts.artifactFiles
        inputs.files(artifactFiles)
        doLast {
            println("Transformed artifact: ${artifactFiles.files}")
        }
    }
}

The idea is that we register 2 transforms with the same set of attributes (by accident), but Gradle doesn't fail, therefore causing latent errors that are hard to debug.

For example: Running ./gradlew --stop -Dgradle.user.home=gradle-user-home && rm -rf gradle-user-home/caches/transforms-3 && ./gradlew printArtifacts21 printArtifacts24 -Dgradle.user.home=gradle-user-home will produce the following log:

> Task :printArtifacts21
Running transform with minSdk = 24 for artifact TransformWithSameAttributes/gradle-user-home/caches/modules-2/files-2.1/com.google.guava/listenablefuture/1.0/c949a840a6acbc5268d088e47b04177bf90b3cad/listenablefuture-1.0.jar
Transformed artifact: [TransformWithSameAttributes/gradle-user-home/caches/transforms-3/12cf933ebe09a55ab0718d0a63694f35/transformed/transformed-minSdk-24-listenablefuture-1.0.jar]

> Task :printArtifacts24
Transformed artifact: [TransformWithSameAttributes/gradle-user-home/caches/transforms-3/12cf933ebe09a55ab0718d0a63694f35/transformed/transformed-minSdk-24-listenablefuture-1.0.jar]

It shows that the transform for minSdk = 21 didn't run, and the task :printArtifacts21 is supposed to get the transformed artifact with minSdk = 21 but it gets the transformed artifact with minSdk = 24 instead.

Expected Behavior

It's easy for the users to forget to set the necessary attributes. One could say it's the responsibility of the users. But Gradle could easily help users avoid that mistake by failing the build when multiple transforms are registered with the same values of attributes.

Context (optional)

This is a simplified example of a real bug that we have in the Android Gradle plugin (https://issuetracker.google.com/293206305 - "NoClassDefFoundError with AGP 8.1.0, desugaring and minify"). When setting up L8DesugarLibTransform, we forgot to set up the minSdkVersion attribute even though it is an input to the transform.

Steps to Reproduce

(See above)

Gradle version

8.4

Build scan URL (optional)

No response

Your Environment (optional)

No response

@hungvietnguyen hungvietnguyen changed the title Registering multiple transforms with the same set of attributes should fail Registering multiple transforms with the same values of attributes should fail Nov 7, 2023
@hungvietnguyen
Copy link
Author

Note that the new behavior will be a breaking change: It will fail the build for user projects if they use plugins / plugin versions where the transforms are not registered properly.

This belongs to the group of Gradle's validation problems, so I'm guessing there is already a process for introducing such a breaking change.

@cobexer
Copy link
Member

cobexer commented Nov 9, 2023

Thank you for your interest in Gradle!

This feature request is in the backlog of the relevant team, but this area of Gradle is currently not in focus. It might take a while before it gets implemented.

@cobexer cobexer added a:feature A new functionality in:artifact-transforms re:comprehensibility reasonable errors and warnings, clear dsl, mental overload and removed a:bug to-triage labels Nov 9, 2023
github-actions bot pushed a commit to chachako/android-studio that referenced this issue Nov 10, 2023
It's important that we do not miss any attributes when registering
transforms and querying artifacts, otherwise Gradle will reuse
transform results incorrectly.

This commit:
  - Adds the missing minSdkVersion attribute to L8DesugarLibTransform.
  - Refactors L8DesugarLibTransform registration in such a way that
    prevents this mistake in the future (using a pattern similar to
    DexingRegistration).

Also filed gradle/gradle#26974 for Gradle to
detect this kind of issue up front so we can catch it early during
development.

Test: New L8DexDesugarTest.testL8DesugarLibTransform
Bug: 293206305
Change-Id: I3b7b4d31be5ae41d0c681e29b261beecc758ecc0

Former-commit-id: 017a9fdf96451367918711f0b35502f29c3f8f5b
@britter
Copy link
Member

britter commented May 14, 2024

I ran into the same problem, see #28960 for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:artifact-transforms re:comprehensibility reasonable errors and warnings, clear dsl, mental overload
Projects
None yet
Development

No branches or pull requests

3 participants