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

Some FileCollection cleanup #8403

Merged
merged 11 commits into from
Feb 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public FileCollection getRuleSetFiles() {
* If you want to only use custom rule sets, you must clear {@code ruleSets}.
*/
public void setRuleSetFiles(FileCollection ruleSetFiles) {
this.ruleSetFiles = project.getLayout().configurableFiles(ruleSetFiles);
this.ruleSetFiles = project.getObjects().fileCollection().from(ruleSetFiles);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ public interface ProjectLayout {
* @param paths The paths to the files. May be empty.
* @return The file collection. Never returns null.
* @since 4.8
* @deprecated Please use {@link ObjectFactory#fileCollection()} instead.
*/
@Deprecated
ConfigurableFileCollection configurableFiles(Object... paths);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,24 @@ class FileCollectionSymlinkIntegrationTest extends AbstractIntegrationSpec {
"""

when:
executer.usingBuildScript(buildScript).run()
maybeDeprecated(code)
run()

then:
noExceptionThrown()

where:
desc | code
"project.files()" | "project.files(file, symlink, symlinked)"
"project.layout.files()" | "project.layout.files(file, symlink, symlinked)"
"project.layout.configurableFiles()" | "project.layout.configurableFiles(file, symlink, symlinked)"
"project.fileTree()" | "project.fileTree(baseDir)"
desc | code
"project.files()" | "project.files(file, symlink, symlinked)"
"project.fileTree()" | "project.fileTree(baseDir)"
"project.layout.files()" | "project.layout.files(file, symlink, symlinked)"
"project.layout.configurableFiles()" | "project.layout.configurableFiles(file, symlink, symlinked)"
"project.objects.fileCollection()" | "project.objects.fileCollection().from(file, symlink, symlinked)"
}

void maybeDeprecated(String expression) {
if (expression.contains("configurableFiles")) {
executer.expectDeprecationWarning()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ class SomeTask extends DefaultTask {
class ConsumerTask extends DefaultTask {

@InputFiles
ConfigurableFileCollection inputFiles = project.layout.configurableFiles()
ConfigurableFileCollection inputFiles = project.objects.fileCollection()

@Optional
@OutputFile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ package org.gradle.api.file

import org.gradle.api.internal.file.collections.ImmutableFileCollection
import org.gradle.integtests.fixtures.AbstractIntegrationSpec
import org.gradle.util.TextUtil
import spock.lang.Unroll

class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
private static final String STRING_CALLABLE = 'new java.util.concurrent.Callable<String>() { String call() { return "%s/src/resource/file.txt" } }'
private static final String STRING_CALLABLE = 'new java.util.concurrent.Callable<String>() { String call() { return "src/resource/file.txt" } }'

def "can access the project dir and build dir"() {
buildFile << """
Expand Down Expand Up @@ -179,6 +178,7 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
"""

when:
maybeDeprecated(expression)
run()

then:
Expand All @@ -196,44 +196,45 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
file('src/resource/file.txt') << "some text"

buildFile << """
def fileCollection = ${TextUtil.normaliseFileSeparators(String.format(expressionTemplate, testDirectory.absolutePath))}
def fileCollection = $expression
println("size = \${fileCollection.files.size()}")
"""

when:
maybeDeprecated(expression)
run()

then:
outputContains('size = 1')

where:
collectionType | content | expressionTemplate
'FileCollection' | 'String' | 'project.layout.files("%s/src/resource/file.txt")'
'FileCollection' | 'File' | 'project.layout.files(new File("%s", "src/resource/file.txt"))'
'FileCollection' | 'Path' | 'project.layout.files(java.nio.file.Paths.get("%s/src/resource/file.txt"))'
'FileCollection' | 'URI' | 'project.layout.files(new File("%s", "/src/resource/file.txt").toURI())'
'FileCollection' | 'URL' | 'project.layout.files(new File("%s", "/src/resource/file.txt").toURI().toURL())'
collectionType | content | expression
'FileCollection' | 'String' | 'project.layout.files("src/resource/file.txt")'
'FileCollection' | 'File' | 'project.layout.files(new File("src/resource/file.txt"))'
'FileCollection' | 'Path' | 'project.layout.files(java.nio.file.Paths.get("src/resource/file.txt"))'
'FileCollection' | 'URI' | 'project.layout.files(new File(projectDir, "/src/resource/file.txt").toURI())'
'FileCollection' | 'URL' | 'project.layout.files(new File(projectDir, "/src/resource/file.txt").toURI().toURL())'
'FileCollection' | 'Directory' | 'project.layout.files(project.layout.projectDirectory)'
'FileCollection' | 'RegularFile' | 'project.layout.files(project.layout.projectDirectory.file("src/resource/file.txt"))'
'FileCollection' | 'Closure' | 'project.layout.files({ "%s/src/resource/file.txt" })'
'FileCollection' | 'List' | 'project.layout.files([ "%s/src/resource/file.txt" ])'
'FileCollection' | 'array' | 'project.layout.files([ "%s/src/resource/file.txt" ] as Object[])'
'FileCollection' | 'FileCollection' | "project.layout.files(${ImmutableFileCollection.name}.of(new File('%s/src/resource/file.txt')))"
'FileCollection' | 'Closure' | 'project.layout.files({ "src/resource/file.txt" })'
'FileCollection' | 'List' | 'project.layout.files([ "src/resource/file.txt" ])'
'FileCollection' | 'array' | 'project.layout.files([ "src/resource/file.txt" ] as Object[])'
'FileCollection' | 'FileCollection' | "project.layout.files(${ImmutableFileCollection.name}.of(new File('src/resource/file.txt')))"
'FileCollection' | 'Callable' | "project.layout.files($STRING_CALLABLE)"
'FileCollection' | 'Provider' | "project.layout.files(provider($STRING_CALLABLE))"
'FileCollection' | 'nested objects' | "project.layout.files({[{$STRING_CALLABLE}]})"

'ConfigurableFileCollection' | 'String' | 'project.layout.configurableFiles("%s/src/resource/file.txt")'
'ConfigurableFileCollection' | 'File' | 'project.layout.configurableFiles(new File("%s", "src/resource/file.txt"))'
'ConfigurableFileCollection' | 'Path' | 'project.layout.configurableFiles(java.nio.file.Paths.get("%s/src/resource/file.txt"))'
'ConfigurableFileCollection' | 'URI' | 'project.layout.configurableFiles(new File("%s", "/src/resource/file.txt").toURI())'
'ConfigurableFileCollection' | 'URL' | 'project.layout.configurableFiles(new File("%s", "/src/resource/file.txt").toURI().toURL())'
'ConfigurableFileCollection' | 'String' | 'project.layout.configurableFiles("src/resource/file.txt")'
'ConfigurableFileCollection' | 'File' | 'project.layout.configurableFiles(new File("src/resource/file.txt"))'
'ConfigurableFileCollection' | 'Path' | 'project.layout.configurableFiles(java.nio.file.Paths.get("src/resource/file.txt"))'
'ConfigurableFileCollection' | 'URI' | 'project.layout.configurableFiles(new File(projectDir, "/src/resource/file.txt").toURI())'
'ConfigurableFileCollection' | 'URL' | 'project.layout.configurableFiles(new File(projectDir, "/src/resource/file.txt").toURI().toURL())'
'ConfigurableFileCollection' | 'Directory' | 'project.layout.configurableFiles(project.layout.projectDirectory)'
'ConfigurableFileCollection' | 'RegularFile' | 'project.layout.configurableFiles(project.layout.projectDirectory.file("src/resource/file.txt"))'
'ConfigurableFileCollection' | 'Closure' | 'project.layout.configurableFiles({ "%s/src/resource/file.txt" })'
'ConfigurableFileCollection' | 'List' | 'project.layout.configurableFiles([ "%s/src/resource/file.txt" ])'
'ConfigurableFileCollection' | 'array' | 'project.layout.configurableFiles([ "%s/src/resource/file.txt" ] as Object[])'
'ConfigurableFileCollection' | 'FileCollection' | "project.layout.configurableFiles(${ImmutableFileCollection.name}.of(new File('%s/src/resource/file.txt')))"
'ConfigurableFileCollection' | 'Closure' | 'project.layout.configurableFiles({ "src/resource/file.txt" })'
'ConfigurableFileCollection' | 'List' | 'project.layout.configurableFiles([ "src/resource/file.txt" ])'
'ConfigurableFileCollection' | 'array' | 'project.layout.configurableFiles([ "src/resource/file.txt" ] as Object[])'
'ConfigurableFileCollection' | 'FileCollection' | "project.layout.configurableFiles(${ImmutableFileCollection.name}.of(new File('src/resource/file.txt')))"
'ConfigurableFileCollection' | 'Callable' | "project.layout.configurableFiles($STRING_CALLABLE)"
'ConfigurableFileCollection' | 'Provider' | "project.layout.configurableFiles(provider($STRING_CALLABLE))"
'ConfigurableFileCollection' | 'nested objects' | "project.layout.configurableFiles({[{$STRING_CALLABLE}]})"
Expand All @@ -243,7 +244,7 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
def 'can create #collectionType with #dependencyType dependency'() {
buildFile << """
task myTask {
def outputFile = new File('${TextUtil.normaliseFileSeparators(testDirectory.absolutePath)}', 'build/resource/file.txt')
def outputFile = file('build/resource/file.txt')
doLast {
outputFile.text = "some text"
}
Expand All @@ -255,10 +256,11 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
"""

when:
maybeDeprecated(expression)
run('myTask')

then:
outputContains("files = [${new File(testDirectory.absolutePath , '/build/resource/file.txt').absolutePath}]")
outputContains("files = [${testDirectory.file('/build/resource/file.txt').absolutePath}]")

where:
collectionType | dependencyType | expression
Expand All @@ -272,15 +274,15 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
def '#methodName enforces build dependencies when given Task as input'() {
buildFile << """
task producer {
def outputFile = new File('${TextUtil.normaliseFileSeparators(testDirectory.absolutePath)}', 'build/resource/file.txt')
def outputFile = file('build/resource/file.txt')
outputs.file outputFile
doLast {
outputFile.text = "some text"
}
}

task consumer {
def fileCollection = project.layout.$methodName(project.tasks.producer)
def fileCollection = $expression(project.tasks.producer)
inputs.files fileCollection
doLast {
println("files = \${fileCollection.files}")
Expand All @@ -289,14 +291,15 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
"""

when:
maybeDeprecated(expression)
run('consumer')

then:
executed(':producer', ':consumer')
outputContains("files = [${new File(testDirectory.absolutePath, '/build/resource/file.txt').absolutePath}]")
outputContains("files = [${testDirectory.file('/build/resource/file.txt').absolutePath}]")

where:
methodName << ['files', 'configurableFiles']
expression << ['project.layout.files', 'project.layout.configurableFiles']
}

@Unroll
Expand All @@ -308,18 +311,19 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
}

dependencies {
other files("${TextUtil.normaliseFileSeparators(testDirectory.absolutePath)}/src/resource/file.txt")
other files("src/resource/file.txt")
}

def fileCollection = $expression
println("files = \${fileCollection.files}")
"""

when:
maybeDeprecated(expression)
run()

then:
outputContains("files = [${new File(testDirectory.absolutePath, '/src/resource/file.txt').absolutePath}]")
outputContains("files = [${testDirectory.file('/src/resource/file.txt').absolutePath}]")

where:
collectionType | expression
Expand All @@ -335,6 +339,7 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
"""

expect:
maybeDeprecated(expression)
fails('help')
errorOutput.contains('java.lang.NullPointerException')

Expand All @@ -344,4 +349,10 @@ class ProjectLayoutIntegrationTest extends AbstractIntegrationSpec {
'FileCollection (File...)' | 'project.layout.files((File) null)'
'ConfigurableFileCollection' | 'project.layout.configurableFiles(null)'
}

void maybeDeprecated(String expression) {
if (expression.contains("configurableFiles")) {
executer.expectDeprecationWarning()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

package org.gradle.api.tasks

import org.gradle.api.internal.file.FileCollectionFactory
import org.gradle.api.internal.tasks.TaskPropertyUtils
import org.gradle.api.internal.tasks.properties.GetInputFilesVisitor
import org.gradle.api.internal.tasks.properties.PropertyWalker
import org.gradle.integtests.fixtures.AbstractIntegrationSpec
import spock.lang.Issue
import spock.lang.Unroll
Expand All @@ -25,16 +29,16 @@ class TaskInputFilePropertiesIntegrationTest extends AbstractIntegrationSpec {
@Unroll
def "allows optional @#annotation.simpleName to have null value"() {
buildFile << """
import org.gradle.api.internal.tasks.properties.GetInputFilesVisitor
import org.gradle.api.internal.tasks.TaskPropertyUtils
import org.gradle.api.internal.tasks.properties.PropertyWalker
import org.gradle.api.internal.file.FileResolver
import ${GetInputFilesVisitor.name}
import ${TaskPropertyUtils.name}
import ${PropertyWalker.name}
import ${FileCollectionFactory.name}

class CustomTask extends DefaultTask {
@Optional @$annotation.simpleName input
@TaskAction void doSomething() {
def fileResolver = project.services.get(FileResolver)
GetInputFilesVisitor visitor = new GetInputFilesVisitor("ownerName", fileResolver)
def fileCollectionFactory = project.services.get(FileCollectionFactory)
GetInputFilesVisitor visitor = new GetInputFilesVisitor("ownerName", fileCollectionFactory)
def walker = services.get(PropertyWalker)
TaskPropertyUtils.visitProperties(walker, this, visitor)
def inputFiles = visitor.fileProperties*.propertyFiles*.files.flatten()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.gradle.api.InvalidUserDataException;
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.internal.file.FileCollectionFactory;
import org.gradle.api.internal.file.FileResolver;
import org.gradle.api.internal.file.TemporaryFileProvider;
import org.gradle.api.internal.project.ProjectInternal;
Expand Down Expand Up @@ -162,9 +163,10 @@ private AbstractTask(TaskInfo taskInfo) {

FileResolver fileResolver = project.getFileResolver();
PropertyWalker propertyWalker = services.get(PropertyWalker.class);
FileCollectionFactory fileCollectionFactory = services.get(FileCollectionFactory.class);
taskMutator = new TaskMutator(this);
taskInputs = new DefaultTaskInputs(this, taskMutator, propertyWalker, fileResolver);
taskOutputs = new DefaultTaskOutputs(this, taskMutator, propertyWalker, fileResolver);
taskInputs = new DefaultTaskInputs(this, taskMutator, propertyWalker, fileCollectionFactory);
taskOutputs = new DefaultTaskOutputs(this, taskMutator, propertyWalker, fileResolver, fileCollectionFactory);
taskDestroyables = new DefaultTaskDestroyables(taskMutator);
taskLocalState = new DefaultTaskLocalState(taskMutator);

Expand Down