Skip to content

Commit

Permalink
Merge pull request #18715 Use the file resolver in LazyPublishArtifact
Browse files Browse the repository at this point in the history
  • Loading branch information
bot-gradle committed Dec 16, 2021
2 parents a598534 + 379e9b1 commit 2cb4012
Show file tree
Hide file tree
Showing 19 changed files with 159 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import org.gradle.StartParameter;
import org.gradle.api.Describable;
import org.gradle.api.artifacts.ConfigurablePublishArtifact;
import org.gradle.api.artifacts.component.ComponentSelector;
import org.gradle.api.artifacts.dsl.ArtifactHandler;
import org.gradle.api.artifacts.dsl.ComponentMetadataHandler;
Expand Down Expand Up @@ -364,12 +363,14 @@ ConfigurationContainerInternal createConfigurationContainer(
PublishArtifactNotationParserFactory createPublishArtifactNotationParserFactory(
Instantiator instantiator,
DependencyMetaDataProvider metaDataProvider,
DomainObjectContext domainObjectContext
DomainObjectContext domainObjectContext,
FileResolver fileResolver
) {
return new PublishArtifactNotationParserFactory(
instantiator,
metaDataProvider,
taskResolverFor(domainObjectContext)
taskResolverFor(domainObjectContext),
fileResolver
);
}

Expand Down Expand Up @@ -462,9 +463,8 @@ DefaultComponentModuleMetadataHandler createComponentModuleMetadataHandler(Insta
return instantiator.newInstance(DefaultComponentModuleMetadataHandler.class, moduleIdentifierFactory);
}

ArtifactHandler createArtifactHandler(Instantiator instantiator, DependencyMetaDataProvider dependencyMetaDataProvider, ConfigurationContainerInternal configurationContainer, DomainObjectContext context) {
NotationParser<Object, ConfigurablePublishArtifact> publishArtifactNotationParser = new PublishArtifactNotationParserFactory(instantiator, dependencyMetaDataProvider, taskResolverFor(context)).create();
return instantiator.newInstance(DefaultArtifactHandler.class, configurationContainer, publishArtifactNotationParser);
ArtifactHandler createArtifactHandler(Instantiator instantiator, ConfigurationContainerInternal configurationContainer, PublishArtifactNotationParserFactory publishArtifactNotationParserFactory) {
return instantiator.newInstance(DefaultArtifactHandler.class, configurationContainer, publishArtifactNotationParserFactory.create());
}

ComponentMetadataProcessorFactory createComponentMetadataProcessorFactory(ComponentMetadataHandlerInternal componentMetadataHandler, DependencyResolutionManagementInternal dependencyResolutionManagement, DomainObjectContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,37 @@ public class LazyPublishArtifact implements PublishArtifactInternal {
private final FileResolver fileResolver;
private PublishArtifactInternal delegate;

/**
* @deprecated Provide a {@link FileResolver} instead using {@link LazyPublishArtifact#LazyPublishArtifact(Provider, FileResolver)}.
*/
@Deprecated
public LazyPublishArtifact(Provider<?> provider) {
this.provider = Providers.internal(provider);
this.version = null;
this.fileResolver = null;
this(provider, null);
// TODO after Spring Boot resolves their usage of this constructor, uncomment this nag
// https://github.com/spring-projects/spring-boot/issues/29074
// DeprecationLogger.deprecateInternalApi("constructor LazyPublishArtifact(Provider<?>)")
// .replaceWith("constructor LazyPublishArtifact(Provider<?>, FileResolver)")
// .willBeRemovedInGradle8()
// .withUpgradeGuideSection(7, "lazypublishartifact_fileresolver")
// .nagUser();
}

public LazyPublishArtifact(Provider<?> provider, FileResolver fileResolver) {
this.provider = Providers.internal(provider);
this.version = null;
this.fileResolver = fileResolver;
this(provider, null, fileResolver);
}

public LazyPublishArtifact(Provider<?> provider, String version) {
public LazyPublishArtifact(Provider<?> provider, String version, FileResolver fileResolver) {
// TODO after Spring Boot resolves their usage of this constructor, uncomment this nag
// https://github.com/spring-projects/spring-boot/issues/29074
// DeprecationLogger.deprecateInternalApi("constructor LazyPublishArtifact(Provider<?>, FileResolver) or constructor LazyPublishArtifact(Provider<?>, String, FileResolver)"
// + " with a null FileResolver")
// .replaceWith("a non-null FileResolver")
// .willBeRemovedInGradle8()
// .withUpgradeGuideSection(7, "lazypublishartifact_fileresolver")
// .nagUser();
this.provider = Providers.internal(provider);
this.version = version;
this.fileResolver = null;
this.fileResolver = fileResolver;
}

@Override
Expand Down Expand Up @@ -100,8 +115,8 @@ private PublishArtifactInternal getDelegate() {
delegate = new ArchivePublishArtifact((AbstractArchiveTask) value);
} else if (value instanceof Task) {
delegate = fromFile(((Task) value).getOutputs().getFiles().getSingleFile());
} else if (value instanceof CharSequence && fileResolver != null) {
delegate = fromFile(fileResolver.resolve(value.toString()));
} else if (fileResolver != null) {
delegate = fromFile(fileResolver.resolve(value));
} else {
throw new InvalidUserDataException(String.format("Cannot convert provided value (%s) to a file.", value));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.gradle.api.internal.artifacts.publish.ArchivePublishArtifact;
import org.gradle.api.internal.artifacts.publish.DecoratingPublishArtifact;
import org.gradle.api.internal.artifacts.publish.DefaultPublishArtifact;
import org.gradle.api.internal.file.FileResolver;
import org.gradle.api.internal.tasks.TaskResolver;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.bundling.AbstractArchiveTask;
Expand All @@ -43,11 +44,13 @@ public class PublishArtifactNotationParserFactory implements Factory<NotationPar
private final Instantiator instantiator;
private final DependencyMetaDataProvider metaDataProvider;
private final TaskResolver taskResolver;
private final FileResolver fileResolver;

public PublishArtifactNotationParserFactory(Instantiator instantiator, DependencyMetaDataProvider metaDataProvider, TaskResolver taskResolver) {
public PublishArtifactNotationParserFactory(Instantiator instantiator, DependencyMetaDataProvider metaDataProvider, TaskResolver taskResolver, FileResolver fileResolver) {
this.instantiator = instantiator;
this.metaDataProvider = metaDataProvider;
this.taskResolver = taskResolver;
this.fileResolver = fileResolver;
}

@Override
Expand Down Expand Up @@ -111,7 +114,7 @@ protected PublishArtifact parseMap(@MapKey("file") File file) {
private class FileProviderNotationConverter extends TypedNotationConverter<Provider<?>, ConfigurablePublishArtifact> {
@SuppressWarnings("unchecked")
FileProviderNotationConverter() {
super((Class)Provider.class);
super((Class<Provider<?>>) (Class<?>) Provider.class);
}

@Override
Expand All @@ -124,7 +127,7 @@ public void describe(DiagnosticsVisitor visitor) {
@Override
protected ConfigurablePublishArtifact parseType(Provider<?> notation) {
Module module = metaDataProvider.getModule();
return instantiator.newInstance(DecoratingPublishArtifact.class, new LazyPublishArtifact(notation, module.getVersion()));
return instantiator.newInstance(DecoratingPublishArtifact.class, new LazyPublishArtifact(notation, module.getVersion(), fileResolver));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class DefaultConfigurationContainerTest extends Specification {
new PublishArtifactNotationParserFactory(
instantiator,
metaDataProvider,
taskResolver
taskResolver,
TestFiles.resolver(),
),
immutableAttributesFactory,
documentationRegistry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,8 @@ All Artifacts:
def publishArtifactNotationParser = new PublishArtifactNotationParserFactory(
instantiator,
metaDataProvider,
Mock(TaskResolver)
Mock(TaskResolver),
TestFiles.resolver(),
)
def defaultConfigurationFactory = new DefaultConfigurationFactory(
DirectInstantiator.INSTANCE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package org.gradle.api.internal.artifacts.dsl

import org.gradle.api.InvalidUserDataException
import org.gradle.api.Task
import org.gradle.api.artifacts.ConfigurablePublishArtifact
import org.gradle.api.artifacts.PublishArtifact
Expand All @@ -26,24 +25,29 @@ import org.gradle.api.internal.artifacts.configurations.DependencyMetaDataProvid
import org.gradle.api.internal.artifacts.publish.ArchivePublishArtifact
import org.gradle.api.internal.artifacts.publish.DecoratingPublishArtifact
import org.gradle.api.internal.artifacts.publish.DefaultPublishArtifact
import org.gradle.api.internal.file.TestFiles
import org.gradle.api.internal.provider.ProviderInternal
import org.gradle.api.internal.tasks.TaskDependencyResolveContext
import org.gradle.api.internal.tasks.TaskResolver
import org.gradle.api.tasks.bundling.AbstractArchiveTask
import org.gradle.internal.reflect.Instantiator
import org.gradle.internal.typeconversion.NotationParser
import org.gradle.internal.typeconversion.UnsupportedNotationException
import org.gradle.test.fixtures.file.TestNameTestDirectoryProvider
import org.gradle.util.TestUtil
import org.gradle.util.internal.TextUtil
import org.junit.Rule
import spock.lang.Specification

import java.awt.*

class PublishArtifactNotationParserFactoryTest extends Specification {
@Rule
public final TestNameTestDirectoryProvider tmpDir = new TestNameTestDirectoryProvider(getClass())
final DependencyMetaDataProvider provider = Mock()
final TaskResolver taskResolver = Mock()
final Instantiator instantiator = TestUtil.instantiatorFactory().decorateLenient()
final PublishArtifactNotationParserFactory publishArtifactNotationParserFactory = new PublishArtifactNotationParserFactory(instantiator, provider, taskResolver)
final PublishArtifactNotationParserFactory publishArtifactNotationParserFactory = new PublishArtifactNotationParserFactory(instantiator, provider, taskResolver, TestFiles.resolver(tmpDir.getTestDirectory()))
final NotationParser<Object, PublishArtifact> publishArtifactNotationParser = publishArtifactNotationParserFactory.create();

def setup() {
Expand Down Expand Up @@ -241,14 +245,19 @@ class PublishArtifactNotationParserFactoryTest extends Specification {

given:
def publishArtifact = publishArtifactNotationParser.parseNotation(provider)
provider.get() >> "broken"
provider.get() >> new Object() {
@Override
String toString() {
return "broken"
}
}

when:
publishArtifact.file

then:
def e = thrown(InvalidUserDataException)
e.message == "Cannot convert provided value (broken) to a file."
def e = thrown(UnsupportedNotationException)
e.message.startsWith("Cannot convert the provided notation to a File or URI: broken.")
}

def createArtifactFromFileInMap() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ To ignore directories in Gradle 8.0 and later, the input property needs to be an
Finally, using `@link:{javadocPath}/org/gradle/api/tasks/InputDirectory.html[InputDirectory]` implies `@IgnoreEmptyDirectories`, so no changes are necessary when using this annotation.
The same is true for `@link:{javadocPath}/org/gradle/api/tasks/TaskInputs.html#dir-java.lang.Object-[inputs.dir()]` when registering an input directory via the runtime API.

[[lazypublishartifact_fileresolver]]
==== Using LazyPublishArtifact without a FileResolver is deprecated

When using a LazyPublishArtifact without a FileResolver, a different file resolution strategy is used, which duplicates
some logic in the FileResolver. To improve consistency, LazyPublishArtifact should be used with a FileResolver, and will
require it in the future.

This also affects other internal APIs that use LazyPublishArtifact, which now also have deprecation warnings where needed.

[[tar_tree_no_backing_file]]
==== TAR trees from resources without backing files

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.gradle.api.file.FileCollection;
import org.gradle.api.internal.artifacts.dsl.LazyPublishArtifact;
import org.gradle.api.internal.plugins.DefaultArtifactPublicationSet;
import org.gradle.api.internal.project.ProjectInternal;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.plugins.BasePlugin;
import org.gradle.api.plugins.JavaPlugin;
Expand Down Expand Up @@ -129,7 +130,7 @@ public void execute(Ear ear) {
deploymentDescriptor.setDescription(project.getDescription());
}
}
project.getExtensions().getByType(DefaultArtifactPublicationSet.class).addCandidate(new LazyPublishArtifact(ear));
project.getExtensions().getByType(DefaultArtifactPublicationSet.class).addCandidate(new LazyPublishArtifact(ear, ((ProjectInternal) project).getFileResolver()));

project.getTasks().withType(Ear.class).configureEach(new Action<Ear>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.internal.artifacts.dsl.LazyPublishArtifact;
import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
import org.gradle.api.internal.project.ProjectInternal;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.provider.Provider;
import org.gradle.api.provider.ProviderFactory;
Expand Down Expand Up @@ -166,7 +167,7 @@ private Stream<CppBinary> getDebugStaticHostStream() {
task.getArchiveClassifier().set("cpp-api-headers");
task.getArchiveFileName().set("cpp-api-headers.zip");
});
library.getMainPublication().addArtifact(new LazyPublishArtifact(headersZip));
library.getMainPublication().addArtifact(new LazyPublishArtifact(headersZip, ((ProjectInternal) project).getFileResolver()));
});

library.getBinaries().realizeNow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import org.gradle.internal.reflect.Instantiator
import org.gradle.internal.typeconversion.NotationParser
import org.gradle.test.fixtures.AbstractProjectBuilderSpec
import org.gradle.util.TestUtil
import spock.lang.Issue

import java.nio.file.Paths

class MavenArtifactNotationParserFactoryTest extends AbstractProjectBuilderSpec {
Instantiator instantiator = TestUtil.instantiatorFactory().decorateLenient()
Expand All @@ -54,6 +57,7 @@ class MavenArtifactNotationParserFactoryTest extends AbstractProjectBuilderSpec
def "setup"() {
def fileResolver = Stub(FileResolver) {
asNotationParser() >> fileNotationParser
resolve(_) >> { Object path -> fileNotationParser.parseNotation(path) }
}
parser = new MavenArtifactNotationParserFactory(instantiator, fileResolver).create()
}
Expand Down Expand Up @@ -254,7 +258,28 @@ class MavenArtifactNotationParserFactoryTest extends AbstractProjectBuilderSpec
when:
1 * provider.get() >> regularFile
1 * regularFile.getAsFile() >> file
artifact.file
artifact.file == file

then:
0 * _
}

@Issue("https://github.com/gradle/gradle/issues/15711")
def "creates lazy MavenArtifact for Provider<Path> notation"() {
def provider = Mock(ProviderInternal)
def file = Paths.get("picard.txt")
def regularFile = Mock(RegularFile)

when:
def artifact = parser.parseNotation(provider)

then:
0 * provider._

when:
1 * provider.get() >> regularFile
1 * regularFile.getAsFile() >> file.toFile()
artifact.file == file.toFile()

then:
0 * _
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.gradle.api.internal.artifacts.dsl.LazyPublishArtifact;
import org.gradle.api.internal.file.FileOperations;
import org.gradle.api.internal.plugins.DefaultArtifactPublicationSet;
import org.gradle.api.internal.project.ProjectInternal;
import org.gradle.api.plugins.BasePlugin;
import org.gradle.api.tasks.Sync;
import org.gradle.api.tasks.TaskProvider;
Expand Down Expand Up @@ -126,7 +127,7 @@ private <T extends AbstractArchiveTask> void addArchiveTask(final Project projec
task.with(childSpec);
});

PublishArtifact archiveArtifact = new LazyPublishArtifact(archiveTask);
PublishArtifact archiveArtifact = new LazyPublishArtifact(archiveTask, ((ProjectInternal) project).getFileResolver());
project.getExtensions().getByType(DefaultArtifactPublicationSet.class).addCandidate(archiveArtifact);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ private void configureBuiltInTest(Project project, TestingExtension testing, Jav
}

private void configureArchivesAndComponent(Project project, final JavaPluginExtension pluginExtension) {
PublishArtifact jarArtifact = new LazyPublishArtifact(registerJarTaskFor(project, pluginExtension));
PublishArtifact jarArtifact = new LazyPublishArtifact(registerJarTaskFor(project, pluginExtension), ((ProjectInternal) project).getFileResolver());
Configuration apiElementConfiguration = project.getConfigurations().getByName(API_ELEMENTS_CONFIGURATION_NAME);
Configuration runtimeElementsConfiguration = project.getConfigurations().getByName(RUNTIME_ELEMENTS_CONFIGURATION_NAME);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.gradle.api.internal.attributes.ImmutableAttributesFactory;
import org.gradle.api.internal.java.WebApplication;
import org.gradle.api.internal.plugins.DefaultArtifactPublicationSet;
import org.gradle.api.internal.project.ProjectInternal;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.plugins.internal.DefaultWarPluginConvention;
import org.gradle.api.tasks.SourceSet;
Expand Down Expand Up @@ -86,7 +87,7 @@ public void apply(final Project project) {
warTask.setGroup(BasePlugin.BUILD_GROUP);
});

PublishArtifact warArtifact = new LazyPublishArtifact(war);
PublishArtifact warArtifact = new LazyPublishArtifact(war, ((ProjectInternal) project).getFileResolver());
project.getExtensions().getByType(DefaultArtifactPublicationSet.class).addCandidate(warArtifact);
configureConfigurations(project.getConfigurations());
configureComponent(project, warArtifact);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,14 @@ public void withJavadocJar() {
TaskContainer tasks = project.getTasks();
ConfigurationContainer configurations = project.getConfigurations();
SourceSet main = getSourceSets().getByName(SourceSet.MAIN_SOURCE_SET_NAME);
configureDocumentationVariantWithArtifact(JAVADOC_ELEMENTS_CONFIGURATION_NAME, null, JAVADOC, ImmutableList.of(), main.getJavadocJarTaskName(), tasks.named(main.getJavadocTaskName()), findJavaComponent(components), configurations, tasks, objectFactory);
configureDocumentationVariantWithArtifact(JAVADOC_ELEMENTS_CONFIGURATION_NAME, null, JAVADOC, ImmutableList.of(), main.getJavadocJarTaskName(), tasks.named(main.getJavadocTaskName()), findJavaComponent(components), configurations, tasks, objectFactory, project.getFileResolver());
}

public void withSourcesJar() {
TaskContainer tasks = project.getTasks();
ConfigurationContainer configurations = project.getConfigurations();
SourceSet main = getSourceSets().getByName(SourceSet.MAIN_SOURCE_SET_NAME);
configureDocumentationVariantWithArtifact(SOURCES_ELEMENTS_CONFIGURATION_NAME, null, SOURCES, ImmutableList.of(), main.getSourcesJarTaskName(), main.getAllSource(), findJavaComponent(components), configurations, tasks, objectFactory);
configureDocumentationVariantWithArtifact(SOURCES_ELEMENTS_CONFIGURATION_NAME, null, SOURCES, ImmutableList.of(), main.getSourcesJarTaskName(), main.getAllSource(), findJavaComponent(components), configurations, tasks, objectFactory, project.getFileResolver());
}

public ModularitySpec getModularity() {
Expand Down

0 comments on commit 2cb4012

Please sign in to comment.