Skip to content

Commit

Permalink
Avoid running protobuf extract if unrelated files change
Browse files Browse the repository at this point in the history 
This change specifies ProtobufExtract more precisely, as only
*.proto files will be included in up-to-date checks. E.g if a .jar
files changes that contains no proto files, task will be up-to-date.
  • Loading branch information
@gavra0
gavra0 committed Nov 23, 2020
1 parent a582afb commit 2d0363d
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 45 deletions.
126 changes: 81 additions & 45 deletions src/main/groovy/com/google/protobuf/gradle/ProtobufExtract.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ import groovy.transform.CompileDynamic
import org.gradle.api.DefaultTask
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.FileCollection
import org.gradle.api.file.FileSystemLocation
import org.gradle.api.file.FileTree
import org.gradle.api.logging.Logger
import org.gradle.api.model.ObjectFactory
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
Expand All @@ -43,6 +45,9 @@ import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.util.PatternFilterable
import org.gradle.api.tasks.util.PatternSet

import javax.inject.Inject

/**
Expand All @@ -59,6 +64,8 @@ abstract class ProtobufExtract extends DefaultTask {
private final ConfigurableFileCollection inputFiles = objectFactory.fileCollection()
private final CopyActionFacade copyActionFacade = instantiateCopyActionFacade()
private final ArchiveActionFacade archiveActionFacade = instantiateArchiveActionFacade()
private final ConfigurableFileCollection filteredProtoNonDirs = instantiateFilteredProtoNonDirs()
private final FileCollection filteredProtoDirs = instantiateFilteredProtoDirs()

public void setIsTest(boolean isTest) {
this.isTest = isTest
Expand All @@ -70,13 +77,33 @@ abstract class ProtobufExtract extends DefaultTask {
return isTest
}

@InputFiles
// TODO Review if NAME_ONLY is the best path sensitivity to use here
@PathSensitive(PathSensitivity.NAME_ONLY)
@Internal
public ConfigurableFileCollection getInputFiles() {
return inputFiles
}

/**
* Non-dir inputs for this task used only for up to date checks. Add inputs to inputFiles.
* Uses relative path sensitivity as directory layout changes impact output.
*/
@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
FileCollection getProtosFromNonDirs() {
return filteredProtoNonDirs
}

/**
* Directory inputs for this task used only for up to date checks. Add inputs to inputFiles.
* Uses relative path sensitivity as directory layout changes impact output.
*/
@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
FileTree getProtosFromDirs() {
return objectFactory.fileCollection().from(filteredProtoDirs)
.asFileTree
.matching { PatternFilterable pattern -> pattern.include("**/*.proto") }
}

@Internal
CopyActionFacade getCopyActionFacade() {
return copyActionFacade
Expand All @@ -93,48 +120,13 @@ abstract class ProtobufExtract extends DefaultTask {
@TaskAction
void extract() {
destDir.mkdir()
Closure extractFrom = { src ->
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(src) {
include '**/*.proto'
}
spec.into(destDir)
}
}
boolean warningLogged = false
inputFiles.each { file ->
logger.debug "Extracting protos from ${file} to ${destDir}"
if (file.isDirectory()) {
extractFrom(file)
} else if (file.path.endsWith('.proto')) {
if (!warningLogged) {
warningLogged = true
logger.warn "proto file '${file.path}' directly specified in configuration. " +
"It's likely you specified files('path/to/foo.proto') or " +
"fileTree('path/to/directory') in protobuf or compile configuration. " +
"This makes you vulnerable to " +
"https://github.com/google/protobuf-gradle-plugin/issues/248. " +
"Please use files('path/to/directory') instead."
}
extractFrom(file)
} else if (file.path.endsWith('.jar') || file.path.endsWith('.zip')) {
FileTree zipTree = archiveActionFacade.zipTree(file.path)
extractFrom(zipTree)
} else if (file.path.endsWith('.aar')) {
FileCollection zipTree = archiveActionFacade.zipTree(file.path).filter { it.path.endsWith('.jar') }
zipTree.each { it ->
extractFrom(archiveActionFacade.zipTree(it))
}
} else if (file.path.endsWith('.tar')
|| file.path.endsWith('.tar.gz')
|| file.path.endsWith('.tar.bz2')
|| file.path.endsWith('.tgz')) {
FileTree tarTree = archiveActionFacade.tarTree(file.path)
extractFrom(tarTree)
} else {
logger.debug "Skipping unsupported file type (${file.path}); handles only jar, tar, tar.gz, tar.bz2 & tgz"
}
FileCollection dirs = filteredProtoDirs
FileCollection nonDirs = filteredProtoNonDirs
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(dirs, nonDirs)
spec.include('**/*.proto')
spec.into(destDir)
}
}

Expand Down Expand Up @@ -164,4 +156,48 @@ abstract class ProtobufExtract extends DefaultTask {
}
return new ArchiveActionFacade.ProjectBased(project)
}

private FileCollection instantiateFilteredProtoDirs() {
return inputFiles.filter { File file -> file.isDirectory() }
}

private ConfigurableFileCollection instantiateFilteredProtoNonDirs() {
boolean warningLogged = false
ArchiveActionFacade archiveFacade = this.archiveActionFacade
Logger logger = this.logger
return objectFactory.fileCollection().from(inputFiles.getElements().map { files ->
PatternSet protoFilter = new PatternSet().include("**/*.proto")
Set<Object> protoInputs = [] as Set
for (FileSystemLocation location : files) {
File file = location.asFile
if (file.path.endsWith('.proto')) {
if (!warningLogged) {
warningLogged = true
logger.warn "proto file '${file.path}' directly specified in configuration. " +
"It's likely you specified files('path/to/foo.proto') or " +
"fileTree('path/to/directory') in protobuf or compile configuration. " +
"This makes you vulnerable to " +
"https://github.com/google/protobuf-gradle-plugin/issues/248. " +
"Please use files('path/to/directory') instead."
}
protoInputs.add(file)
} else if (file.path.endsWith('.jar') || file.path.endsWith('.zip')) {
protoInputs.add(archiveFacade.zipTree(file.path).matching(protoFilter))
} else if (file.path.endsWith('.aar')) {
FileCollection zipTree = archiveFacade.zipTree(file.path).filter { it.path.endsWith('.jar') }
zipTree.each { entry ->
protoInputs.add(archiveFacade.zipTree(entry).matching(protoFilter))
}
} else if (file.path.endsWith('.tar')
|| file.path.endsWith('.tar.gz')
|| file.path.endsWith('.tar.bz2')
|| file.path.endsWith('.tgz')) {
protoInputs.add(archiveFacade.tarTree(file.path).matching(protoFilter))
} else if (!file.isDirectory()) {
logger.debug "Skipping unsupported file type (${file.path}); handles only jar, tar, tar.gz, tar.bz2 & tgz"
}
}
return protoInputs
})
}
}
58 changes: 58 additions & 0 deletions src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,64 @@ class ProtobufJavaPluginTest extends Specification {
gradleVersion << GRADLE_VERSIONS
}
@Unroll
void "test proto extraction is up-to-date for testProject when changing java sources [gradle #gradleVersion]"() {
given: "project from testProject"
File projectDir = ProtobufPluginTestHelper.projectBuilder('testProject')
.copyDirs('testProjectBase', 'testProject')
.build()
when: "build is invoked"
BuildResult result = GradleRunner.create()
.withProjectDir(projectDir)
.withArguments('build', '--stacktrace')
.withPluginClasspath()
.withGradleVersion(gradleVersion)
.forwardStdOutput(new OutputStreamWriter(System.out))
.forwardStdError(new OutputStreamWriter(System.err))
.withDebug(true)
.build()
then: "it succeed"
result.task(":build").outcome == TaskOutcome.SUCCESS
when: "Java class is added and build runs again"
new File(projectDir, "src/main/java/Bar.java").write("public class Bar {}")
result = GradleRunner.create()
.withProjectDir(projectDir)
.withArguments('build', '--stacktrace')
.withPluginClasspath()
.withGradleVersion(gradleVersion)
.forwardStdOutput(new OutputStreamWriter(System.out))
.forwardStdError(new OutputStreamWriter(System.err))
.withDebug(true)
.build()
then: "extract include test protos is up to date because it ignores classpath changes"
result.task(":extractIncludeTestProto").outcome == TaskOutcome.UP_TO_DATE
when: "proto file is added"
new File(projectDir, "empty_proto.proto").write("syntax = \"proto3\";")
new File(projectDir, "build.gradle")
.append("\n dependencies { implementation files ('empty_proto.proto') } ")
result = GradleRunner.create()
.withProjectDir(projectDir)
.withArguments('build', '--stacktrace')
.withPluginClasspath()
.withGradleVersion(gradleVersion)
.forwardStdOutput(new OutputStreamWriter(System.out))
.forwardStdError(new OutputStreamWriter(System.err))
.withDebug(true)
.build()

then: "extract include protos is not up to date"
result.task(":extractIncludeProto").outcome == TaskOutcome.SUCCESS
result.task(":extractIncludeTestProto").outcome == TaskOutcome.SUCCESS

where:
gradleVersion << GRADLE_VERSIONS
}

void "test generateCmds should split commands when limit exceeded"() {
given: "a cmd length limit and two proto files"

Expand Down

0 comments on commit 2d0363d

Please sign in to comment.