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.

Sign up for GitHub

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

Jump to bottom

Several Gradle improvements #27

Closed
wants to merge 1 commit into from
+92 −76
Closed

Several Gradle improvements #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 1 addition & 55 deletions src/main/java/org/spdx/sbom/gradle/SpdxSbomPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,18 @@
*/
package org.spdx.sbom.gradle;

import java.io.File;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.stream.Collectors;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.Transformer;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.Dependency;
import org.gradle.api.artifacts.component.ComponentArtifactIdentifier;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.api.artifacts.repositories.MavenArtifactRepository;
import org.gradle.api.artifacts.result.ArtifactResult;
import org.gradle.api.artifacts.result.ResolvedArtifactResult;
import org.gradle.api.artifacts.result.ResolvedComponentResult;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.internal.component.local.model.OpaqueComponentIdentifier;
import org.spdx.sbom.gradle.SpdxSbomExtension.Target;
import org.spdx.sbom.gradle.maven.PomResolver;
import org.spdx.sbom.gradle.project.DocumentInfo;
import org.spdx.sbom.gradle.project.ProjectInfo;
import org.spdx.sbom.gradle.project.ScmInfo;
Expand Down Expand Up @@ -111,15 +98,6 @@ private void createTaskForTarget(

List<String> configurationNames = target.getConfigurations().get();
for (var configurationName : configurationNames) {
Provider<Set<ResolvedArtifactResult>> artifacts =
project
.getConfigurations()
.getByName(configurationName)
.getIncoming()
.getArtifacts()
.getResolvedArtifacts();
t.getResolvedArtifacts().putAll(artifacts.map(new ArtifactTransformer()));

Comment on lines -114 to -122
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Provider<ResolvedComponentResult> rootComponent =
project
.getConfigurations()
Expand All @@ -128,24 +106,6 @@ private void createTaskForTarget(
.getResolutionResult()
.getRootComponent();

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));
Comment on lines -131 to -147
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


t.getRootComponents().add(rootComponent);
}
t.getMavenRepositories()
Expand All @@ -160,23 +120,9 @@ private void createTaskForTarget(
e -> e,
e ->
((MavenArtifactRepository)
project.getRepositories().getByName(e))
project.getRepositories().getByName(e))
.getUrl()))));
});
aggregate.configure(t -> t.dependsOn(task));
}

private static class ArtifactTransformer
implements Transformer<
Map<ComponentArtifactIdentifier, File>, Collection<ResolvedArtifactResult>> {

@Override
public Map<ComponentArtifactIdentifier, File> transform(
Collection<ResolvedArtifactResult> resolvedArtifactResults) {
return resolvedArtifactResults.stream()
// ignore gradle API components as they cannot be serialized
.filter(x -> !(x.getId().getComponentIdentifier() instanceof OpaqueComponentIdentifier))
.collect(Collectors.toMap(ArtifactResult::getId, ResolvedArtifactResult::getFile));
}
}
}
79 changes: 70 additions & 9 deletions src/main/java/org/spdx/sbom/gradle/SpdxSbomTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,22 @@
import java.io.File;
import java.io.FileOutputStream;
import java.net.URI;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import javax.inject.Inject;
import org.gradle.api.DefaultTask;
import org.gradle.api.artifacts.ConfigurationContainer;
import org.gradle.api.artifacts.component.ComponentArtifactIdentifier;
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.artifacts.dsl.DependencyHandler;
import org.gradle.api.artifacts.result.DependencyResult;
import org.gradle.api.artifacts.result.ResolvedArtifactResult;
import org.gradle.api.artifacts.result.ResolvedComponentResult;
import org.gradle.api.artifacts.result.ResolvedDependencyResult;
import org.gradle.api.file.DirectoryProperty;
import org.gradle.api.provider.ListProperty;
import org.gradle.api.provider.MapProperty;
Expand All @@ -31,11 +43,13 @@
import org.gradle.api.tasks.Internal;
import org.gradle.api.tasks.OutputDirectory;
import org.gradle.api.tasks.TaskAction;
import org.gradle.maven.MavenModule;
import org.gradle.maven.MavenPomArtifact;
import org.spdx.jacksonstore.MultiFormatStore;
import org.spdx.jacksonstore.MultiFormatStore.Format;
import org.spdx.library.model.SpdxDocument;
import org.spdx.sbom.gradle.extensions.SpdxSbomTaskExtension;
import org.spdx.sbom.gradle.maven.PomInfo;
import org.spdx.sbom.gradle.maven.PomResolver;
import org.spdx.sbom.gradle.project.DocumentInfo;
import org.spdx.sbom.gradle.project.ProjectInfo;
import org.spdx.sbom.gradle.project.ScmInfo;
Expand All @@ -45,16 +59,18 @@
import org.spdx.storage.simple.InMemSpdxStore;

public abstract class SpdxSbomTask extends DefaultTask {
@Inject
protected abstract DependencyHandler getDependencyHandler();

@Inject
protected abstract ConfigurationContainer getConfigurations();

@Internal
abstract Property<SpdxKnownLicensesService> getSpdxKnownLicensesService();

@Input
abstract ListProperty<ResolvedComponentResult> getRootComponents();

@Input
abstract MapProperty<ComponentArtifactIdentifier, File> getResolvedArtifacts();

@OutputDirectory
public abstract DirectoryProperty getOutputDirectory();

Expand All @@ -64,9 +80,6 @@ public abstract class SpdxSbomTask extends DefaultTask {
@Input
abstract MapProperty<String, URI> getMavenRepositories();

@Input
abstract MapProperty<String, PomInfo> getPoms();

@Input
abstract Property<String> getFilename();

Expand All @@ -84,6 +97,37 @@ public abstract class SpdxSbomTask extends DefaultTask {

@TaskAction
public void generateSbom() throws Exception {
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 =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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());

ISerializableModelStore modelStore =
new MultiFormatStore(new InMemSpdxStore(), Format.JSON_PRETTY);
SpdxDocumentBuilder documentBuilder =
Expand All @@ -92,9 +136,9 @@ public void generateSbom() throws Exception {
getAllProjects().get(),
getLogger(),
modelStore,
getResolvedArtifacts().get(),
resolvedArtifactsById,
getMavenRepositories().get(),
getPoms().get(),
pomResolver.effectivePoms(resolvedArtifactResults),
getTaskExtension().getOrNull(),
getDocumentInfo().get(),
getScmInfo().get(),
Expand All @@ -114,4 +158,21 @@ public void generateSbom() throws Exception {
new FileOutputStream(getOutputDirectory().file(getFilename()).get().getAsFile());
modelStore.serialize(doc.getDocumentUri(), out);
}

private Set<ComponentIdentifier> gatherSelectedDependencies(
ResolvedComponentResult component,
Set<ResolvedComponentResult> seenComponents,
Set<ComponentIdentifier> componentIds) {
if (seenComponents.add(component)) {
for (DependencyResult dep : component.getDependencies()) {
if (dep instanceof ResolvedDependencyResult) {
ResolvedDependencyResult resolvedDep = (ResolvedDependencyResult) dep;
componentIds.add(resolvedDep.getSelected().getId());
gatherSelectedDependencies(resolvedDep.getSelected(), seenComponents, componentIds);
}
}
}

return componentIds;
}
}
16 changes: 10 additions & 6 deletions src/main/java/org/spdx/sbom/gradle/maven/GradleMavenResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,24 @@
import org.apache.maven.model.building.FileModelSource;
import org.apache.maven.model.building.ModelSource2;
import org.apache.maven.model.resolution.ModelResolver;
import org.gradle.api.Project;
import org.gradle.api.artifacts.ConfigurationContainer;
import org.gradle.api.artifacts.dsl.DependencyHandler;

public class GradleMavenResolver implements ModelResolver {
private final Project project;
private final DependencyHandler dependencies;
private final ConfigurationContainer configurations;

public GradleMavenResolver(Project project) {
this.project = project;
public GradleMavenResolver(
DependencyHandler dependencies, ConfigurationContainer configurations) {
this.dependencies = dependencies;
this.configurations = configurations;
}

@Override
public ModelSource2 resolveModel(String groupId, String artifactId, String version) {
var dep = groupId + ":" + artifactId + ":" + version + "@pom";
var dependency = project.getDependencies().create(dep);
var config = project.getConfigurations().detachedConfiguration(dependency);
var dependency = dependencies.create(dep);
var config = configurations.detachedConfiguration(dependency);

var pomXml = config.getSingleFile();
return new FileModelSource(pomXml);
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/org/spdx/sbom/gradle/maven/PomResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
Expand All @@ -29,9 +30,10 @@
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.model.building.ModelBuildingRequest;
import org.gradle.api.GradleException;
import org.gradle.api.Project;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.ConfigurationContainer;
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.artifacts.dsl.DependencyHandler;
import org.gradle.api.artifacts.result.ResolvedArtifactResult;
import org.gradle.api.logging.Logger;

/** This needs to be run *before* while configuring the task, so use it in the Plugin. */
Expand All @@ -40,9 +42,12 @@ public class PomResolver {
private final GradleMavenResolver gradleMavenResolver;
private final Logger logger;

public static PomResolver newPomResolver(Project project) {
public static PomResolver newPomResolver(
DependencyHandler dependencies, ConfigurationContainer configurations, Logger logger) {
return new PomResolver(
new GradleMavenResolver(project), new DefaultModelBuilderFactory(), project.getLogger());
new GradleMavenResolver(dependencies, configurations),
new DefaultModelBuilderFactory(),
logger);
}

PomResolver(
Expand All @@ -54,9 +59,9 @@ public static PomResolver newPomResolver(Project project) {
this.logger = logger;
}

public Map<String, PomInfo> effectivePoms(Configuration pomsConfig) {
public Map<String, PomInfo> effectivePoms(List<ResolvedArtifactResult> resolvedArtifactResults) {
Map<String, PomInfo> effectivePoms = new HashMap<>();
for (var ra : pomsConfig.getIncoming().getArtifacts().getResolvedArtifacts().get()) {
for (var ra : resolvedArtifactResults) {
var pomFile = ra.getFile();
Model model = resolveEffectivePom(pomFile);
effectivePoms.put(
Expand Down