Several Gradle improvements #27

eygraber · 2023-06-13T10:27:38Z

This needs to be cleaned up (and possibly broken up into multiple PRs) but I wanted to get opinions on this before doing any more work.

eygraber · 2023-06-13T10:28:11Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomPlugin.java

-                    Provider<Set<ResolvedArtifactResult>> artifacts =
-                        project
-                            .getConfigurations()
-                            .getByName(configurationName)
-                            .getIncoming()
-                            .getArtifacts()
-                            .getResolvedArtifacts();
-                    t.getResolvedArtifacts().putAll(artifacts.map(new ArtifactTransformer()));
-


This was causing resolution issues in my project because of variants and mismatched attributes.

can you describe the issue a little more for us to understand?

I wish I could 😅

Gradle has a concept of variants, and for some reason this code causes Gradle to use the wrong attributes when resolving some dependencies from some configurations.

The issue with the pomsConfig below is similar, and I think it is related to detached configurations not copying the attributes. I tried manually copying the attributes from the original configuration to the detached one, but there where still issues.

This is an area that I don't have the most expertise in with Gradle, so I'm not much help here.

I'm working on a standalone repro, but in the meantime here are some of the failures from my project:

spdxSbom { targets { create("release") { configurations.set(listOf("devDebugRuntimeClasspath")) } } }

> Could not create task ':app:spdxSbomForRelease'. > Could not resolve all artifacts for configuration ':app:detachedConfiguration9'. > Could not resolve com.eygraber:android-date-time-input-view:0.9.16. Required by: project :app > Cannot choose between the following variants of com.eygraber:android-date-time-input-view:0.9.16: - debugVariantMavenRuntimePublication - releaseVariantMavenRuntimePublication All of them match the consumer attributes: - Variant 'debugVariantMavenRuntimePublication' capability com.eygraber:android-date-time-input-view:0.9.16: - Unmatched attributes: - Provides com.android.build.api.attributes.BuildTypeAttr 'debug' but the consumer didn't ask for it - Provides org.gradle.category 'library' but the consumer didn't ask for it - Provides org.gradle.dependency.bundling 'external' but the consumer didn't ask for it - Provides org.gradle.libraryelements 'aar' but the consumer didn't ask for it - Provides org.gradle.status 'release' but the consumer didn't ask for it - Provides org.gradle.usage 'java-runtime' but the consumer didn't ask for it - Variant 'releaseVariantMavenRuntimePublication' capability com.eygraber:android-date-time-input-view:0.9.16: - Unmatched attributes: - Provides com.android.build.api.attributes.BuildTypeAttr 'release' but the consumer didn't ask for it - Provides org.gradle.category 'library' but the consumer didn't ask for it - Provides org.gradle.dependency.bundling 'external' but the consumer didn't ask for it - Provides org.gradle.libraryelements 'aar' but the consumer didn't ask for it - Provides org.gradle.status 'release' but the consumer didn't ask for it - Provides org.gradle.usage 'java-runtime' but the consumer didn't ask for it > Could not resolve com.eygraber:android-date-time-input-common:0.9.16. Required by: project :app > Cannot choose between the following variants of com.eygraber:android-date-time-input-common:0.9.16: - debugVariantMavenRuntimePublication - releaseVariantMavenRuntimePublication All of them match the consumer attributes: - Variant 'debugVariantMavenRuntimePublication' capability com.eygraber:android-date-time-input-common:0.9.16: - Unmatched attributes: - Provides com.android.build.api.attributes.BuildTypeAttr 'debug' but the consumer didn't ask for it - Provides org.gradle.category 'library' but the consumer didn't ask for it - Provides org.gradle.dependency.bundling 'external' but the consumer didn't ask for it - Provides org.gradle.libraryelements 'aar' but the consumer didn't ask for it - Provides org.gradle.status 'release' but the consumer didn't ask for it - Provides org.gradle.usage 'java-runtime' but the consumer didn't ask for it - Variant 'releaseVariantMavenRuntimePublication' capability com.eygraber:android-date-time-input-common:0.9.16: - Unmatched attributes: - Provides com.android.build.api.attributes.BuildTypeAttr 'release' but the consumer didn't ask for it - Provides org.gradle.category 'library' but the consumer didn't ask for it - Provides org.gradle.dependency.bundling 'external' but the consumer didn't ask for it - Provides org.gradle.libraryelements 'aar' but the consumer didn't ask for it - Provides org.gradle.status 'release' but the consumer didn't ask for it - Provides org.gradle.usage 'java-runtime' but the consumer didn't ask for it > Could not resolve app.cash.sqldelight:android-driver:2.0.0-alpha05. Required by: project :app > Cannot choose between the following variants of app.cash.sqldelight:android-driver:2.0.0-alpha05: - debugVariantMavenRuntimePublication - releaseVariantMavenRuntimePublication All of them match the consumer attributes: - Variant 'debugVariantMavenRuntimePublication' capability app.cash.sqldelight:android-driver:2.0.0-alpha05: - Unmatched attributes: - Provides com.android.build.api.attributes.BuildTypeAttr 'debug' but the consumer didn't ask for it - Provides org.gradle.category 'library' but the consumer didn't ask for it - Provides org.gradle.dependency.bundling 'external' but the consumer didn't ask for it - Provides org.gradle.libraryelements 'aar' but the consumer didn't ask for it - Provides org.gradle.status 'release' but the consumer didn't ask for it - Provides org.gradle.usage 'java-runtime' but the consumer didn't ask for it - Variant 'releaseVariantMavenRuntimePublication' capability app.cash.sqldelight:android-driver:2.0.0-alpha05: - Unmatched attributes: - Provides com.android.build.api.attributes.BuildTypeAttr 'release' but the consumer didn't ask for it - Provides org.gradle.category 'library' but the consumer didn't ask for it - Provides org.gradle.dependency.bundling 'external' but the consumer didn't ask for it - Provides org.gradle.libraryelements 'aar' but the consumer didn't ask for it - Provides org.gradle.status 'release' but the consumer didn't ask for it - Provides org.gradle.usage 'java-runtime' but the consumer didn't ask for it

eygraber · 2023-06-13T10:28:23Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomPlugin.java

-                    Configuration pomsConfig =
-                        project
-                            .getConfigurations()
-                            .detachedConfiguration(
-                                project
-                                    .getConfigurations()
-                                    .getByName(configurationName)
-                                    .getIncoming()
-                                    .getResolutionResult()
-                                    .getAllComponents()
-                                    .stream()
-                                    .filter(rcr -> rcr.getId() instanceof ModuleComponentIdentifier)
-                                    .map(rcr -> rcr.getId().getDisplayName() + "@pom")
-                                    .map(pom -> project.getDependencies().create(pom))
-                                    .toArray(Dependency[]::new));
-                    t.getPoms()
-                        .putAll(PomResolver.newPomResolver(project).effectivePoms(pomsConfig));


This was causing resolution issues in my project because of variants and mismatched attributes.

You can see the information Gradle uses to resolve the variant here - https://repo1.maven.org/maven2/com/eygraber/android-date-time-input-common/0.9.16/android-date-time-input-common-0.9.16.module

eygraber · 2023-06-13T10:30:09Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomPlugin.java

+  private Map<String, ArtifactRepository> getAllRepositories(Project project) {
+    Map<String, ArtifactRepository> projectRepositories = project.getRepositories().getAsMap();
+
+    // nasty case because of https://github.com/gradle/gradle/issues/17295
+    Map<String, ArtifactRepository> settingsRepositories = ((GradleInternal) project.getGradle())
+        .getSettings()
+        .getDependencyResolutionManagement()
+        .getRepositories()
+        .getAsMap();


I only use dependencyResolutionManagement.repositories to add my repositories, so the project one is empty. There could technically be repositories added in both places, so that's why I'm doing this.

eygraber · 2023-06-13T10:30:41Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomTask.java

+  @Inject protected abstract DependencyHandler getDependencyHandler();
+  @Inject protected abstract ConfigurationContainer getConfigurations();


Injecting like this allows the task to be configuration cache compliant.

eygraber · 2023-06-13T10:31:32Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomTask.java

+    Set<ComponentIdentifier> componentIds = new HashSet<>();
+    for (var rootComponent : getRootComponents().get()) {
+      componentIds.addAll(
+          gatherSelectedDependencies(
+              rootComponent,
+              new HashSet<>(),
+              new HashSet<>()
+          )
+      );
+    }
+
+    Map<ComponentArtifactIdentifier, File> resolvedArtifactsById = new HashMap<>();
+
+    @SuppressWarnings("unchecked")
+    List<ResolvedArtifactResult> resolvedArtifactResults =
+        getDependencyHandler()
+            .createArtifactResolutionQuery()
+            .forComponents(componentIds)
+            .withArtifacts(MavenModule.class, MavenPomArtifact.class)
+            .execute()
+            .getResolvedComponents()
+            .stream()
+            .flatMap(componentArtifactsResult -> componentArtifactsResult.getArtifacts(MavenPomArtifact.class).stream())
+            .filter(artifactResult -> artifactResult instanceof ResolvedArtifactResult)
+            .map(artifactResult -> (ResolvedArtifactResult) artifactResult)
+            .peek(resolvedArtifactResult -> resolvedArtifactsById.put(resolvedArtifactResult.getId(), resolvedArtifactResult.getFile()))
+            .collect(Collectors.toList());
+
+    PomResolver pomResolver = PomResolver.newPomResolver(getDependencyHandler(), getConfigurations(), getLogger());


The tests and functionalTests both pass with this, and my project looks OK with it. I think it should be fine, but I'm not 100% sure.

loosebazooka · 2023-06-13T18:14:37Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomTask.java

+    Map<ComponentArtifactIdentifier, File> resolvedArtifactsById = new HashMap<>();
+
+    @SuppressWarnings("unchecked")
+    List<ResolvedArtifactResult> resolvedArtifactResults =


I guess fundamentally, is this okay by gradle's build conventions? Resolving artifacts at task execution time?

I've been using this in my project for ~8 months and it's been working great. I think it just looks at the result of the resolution that is done before execution time. The javadoc for ArtifactResolutionQuery shows an example of using it in a task.

So further research is telling me this is not compatible with task configuration avoidance, someone from game recommended against this approach

I'm pretty sure Gradle uses this approach themselves, and I've been using it with task configuration avoidance for a while now. I can look for more info from Gradle on this.

gotcha, I'll see if I can validate what I've been told too

From a configuration cache perspective it should be fine

Waiting to hear from someone at Gradle conclusively about usage of the API in general, but in this case the existing code is using the same underlying API, and it won't work with projects using variants, so it's probably fine.

I haven't looked very closely, but Gradle has been thinking about disallowing resolving dependencies at configuration time: gradle/gradle#2298 . Are there any other times at which we might resolve these artifacts? (Resolving dependencies during task execution makes sense to me)

I haven't heard from anyone at gradle directly, but it looks like they're working on a new API too address this. Until that's released this is one of at least two options.

The other option is something that CashApp is doing in Licensee where they used detached configurations and copy all the variant information into it. I think the sentiment is that this solution is more "correct" (despite being a little hacky), but it is vastly more complex.

loosebazooka · 2023-06-13T18:15:16Z

src/main/java/org/spdx/sbom/gradle/SpdxSbomPlugin.java

-                    Provider<Set<ResolvedArtifactResult>> artifacts =
-                        project
-                            .getConfigurations()
-                            .getByName(configurationName)
-                            .getIncoming()
-                            .getArtifacts()
-                            .getResolvedArtifacts();
-                    t.getResolvedArtifacts().putAll(artifacts.map(new ArtifactTransformer()));
-


can you describe the issue a little more for us to understand?

loosebazooka · 2023-06-16T12:00:42Z

@eygraber just fyi I'm on parental leave and will be quite slow to respond on these.

loosebazooka · 2023-10-30T23:45:25Z

Hey @eygraber, sorry I let this PR kinda fall to the side. Are there parts specifically that are still failing with 0.3.0, that we can begin to address again. Would prefer each issue is its own PR if possible (as mentioned up top).

eygraber · 2023-10-31T05:34:53Z

@loosebazooka I tested 0.3.0 (I've been using my own internal builds for now) and it is still failing because of the mismatched variants and attributes.

As discussed here this PR has a fix for that, but Gradle plans on removing that workaround in the future. They don't have a proper workaround for now. There are other workarounds that involve a lot more work, but aren't using APIs that Gradle plans on removing (for now).

I think there's one other issue addressed here that I can separate out (i.e. ArtifactRepository not getting picked up from settings files).

loosebazooka · 2023-11-01T00:50:30Z

Cool, lets do the ArtifactRepository one at least for now. That seems pretty straight forward?

eygraber · 2023-11-07T18:33:10Z

I got some guidance from Gradle on how to handle the rest of it

So in general we strongly discourage doing any sort of dependency resolution at execution time.
Would you be able to add an @input to your task with some Property, and then configure the value of that property with the result of your ARQ? That way, the ARQ is executed during serialization-time, in between configuration time and execution time
So something along the lines of

abstract class MyTask {
    @Input
    abstract MapProperty<String, PomInfo> getPoms()
}

tasks.register("doStuff", MyTask) {
    poms = project.provider(() -> {
        // Do ARQ and effective POM calculations here
    })
}

I'm a little busy at the moment, but I'll get back to all of this when I have a few minutes.

eygraber · 2023-11-08T22:01:33Z

Replaced by #70, #71, and #72

eygraber commented Jun 13, 2023

View reviewed changes

eygraber force-pushed the gradle-improvements branch 2 times, most recently from 4b690c1 to 45ffa80 Compare June 13, 2023 10:33

loosebazooka reviewed Jun 13, 2023

View reviewed changes

eygraber force-pushed the gradle-improvements branch from 45ffa80 to be27066 Compare September 1, 2023 19:23

Several Gradle improvements

c3608b0

eygraber force-pushed the gradle-improvements branch from be27066 to c3608b0 Compare November 8, 2023 17:57

eygraber closed this Nov 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Several Gradle improvements #27

Several Gradle improvements #27

eygraber commented Jun 13, 2023

eygraber Jun 13, 2023

loosebazooka Jun 13, 2023

eygraber Jun 13, 2023

eygraber Jun 16, 2023

eygraber Jun 13, 2023

eygraber Jun 16, 2023

eygraber Jun 13, 2023

eygraber Jun 13, 2023

eygraber Jun 13, 2023

loosebazooka Jun 13, 2023

eygraber Jun 13, 2023

loosebazooka Aug 23, 2023

eygraber Aug 23, 2023

loosebazooka Aug 28, 2023

eygraber Aug 30, 2023

mathjeff Sep 21, 2023

eygraber Sep 21, 2023

loosebazooka Jun 13, 2023

loosebazooka commented Jun 16, 2023

loosebazooka commented Oct 30, 2023 •

edited

Loading

eygraber commented Oct 31, 2023

loosebazooka commented Nov 1, 2023

eygraber commented Nov 7, 2023

eygraber commented Nov 8, 2023

		@Inject protected abstract DependencyHandler getDependencyHandler();
		@Inject protected abstract ConfigurationContainer getConfigurations();

Several Gradle improvements #27

Several Gradle improvements #27

Conversation

eygraber commented Jun 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loosebazooka commented Jun 16, 2023

loosebazooka commented Oct 30, 2023 • edited Loading

eygraber commented Oct 31, 2023

loosebazooka commented Nov 1, 2023

eygraber commented Nov 7, 2023

eygraber commented Nov 8, 2023

loosebazooka commented Oct 30, 2023 •

edited

Loading