Skip to content

Commit

Permalink
Avoid running protobuf extract if unrelated files change (#452)
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 authored Nov 24, 2020
1 parent a582afb commit bf6301a
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 45 deletions.
110 changes: 65 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,7 @@ abstract class ProtobufExtract extends DefaultTask {
private final ConfigurableFileCollection inputFiles = objectFactory.fileCollection()
private final CopyActionFacade copyActionFacade = instantiateCopyActionFacade()
private final ArchiveActionFacade archiveActionFacade = instantiateArchiveActionFacade()
private final FileCollection filteredProtos = instantiateFilteredProtos()

public void setIsTest(boolean isTest) {
this.isTest = isTest
Expand All @@ -70,13 +76,21 @@ 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
}

/**
* Inputs for this task containing only proto files, which is enough for up-to-date checks.
* Add inputs to inputFiles. Uses relative path sensitivity as directory layout changes impact output.
*/
@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
FileTree getFilteredProtosFromInputs() {
return filteredProtos.asFileTree.matching { PatternFilterable pattern -> pattern.include("**/*.proto") }
}

@Internal
CopyActionFacade getCopyActionFacade() {
return copyActionFacade
Expand All @@ -93,48 +107,12 @@ 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 allProtoFiles = filteredProtos
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(allProtoFiles)
spec.include('**/*.proto')
spec.into(destDir)
}
}

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

private FileCollection instantiateFilteredProtos() {
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.isDirectory()) {
protoInputs.add(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."
}
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 {
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 bf6301a

Please sign in to comment.