Skip to content

Commit

Permalink
Refactor GenerateProtoTask's inputs (#291)
Browse files Browse the repository at this point in the history
Previously, proto source files (that will be compiled by protoc) are added via inputs.files() (in Utils#addFilesToTaskInputs, and "include" directories (that include protos that may be "imported" but will not be compiled) are added via inputs.dir() (ProtobufPlugin.groovy:400). It has resulted in desired effect, that the only the former is included in the source files returned by inputs.sourceFiles (GenerateProtoTask.groovy:425) that are passed to protoc. However, this difference between files() and dir() is not mentioned on Gradle TaskInputs API.

Instead of relying on a behavior that is undocumented and may change without notice, I refactored the inputs of the task to call out the two inputs explicitly as custom fields includeDirs and sourceFiles, and use inputs from the base class only for registering up-to-date checks.

Also make sure the include dirs exist, because protoc would print a warning if a directory passed to -I doesn't exist.
  • Loading branch information
zhangkun83 authored Jan 10, 2019
1 parent 8d0b8db commit 7123080
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.Named
import org.gradle.api.NamedDomainObjectContainer
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.FileCollection
import org.gradle.api.file.SourceDirectorySet
import org.gradle.api.internal.file.DefaultSourceDirectorySet
import org.gradle.api.internal.file.FileResolver
Expand All @@ -57,7 +59,11 @@ public class GenerateProtoTask extends DefaultTask {
// Two quotes and a space.
static final int CMD_ARGUMENT_EXTRA_LENGTH = 3

private final List includeDirs = []
// include dirs are passed to the '-I' option of protoc. They contain protos
// that may be "imported" from the source protos, but will not be compiled.
private final List<File> includeDirs = []
// source files are proto files that will be compiled by protoc
private final ConfigurableFileCollection sourceFiles = project.files()
private final NamedDomainObjectContainer<PluginOptions> builtins
private final NamedDomainObjectContainer<PluginOptions> plugins

Expand Down Expand Up @@ -215,6 +221,10 @@ public class GenerateProtoTask extends DefaultTask {
return sourceSet
}

FileCollection getSourceFiles() {
return sourceFiles
}

Object getVariant() {
Preconditions.checkState(Utils.isAndroidProject(project),
'variant should not be used in a Java project')
Expand Down Expand Up @@ -321,13 +331,22 @@ public class GenerateProtoTask extends DefaultTask {
/**
* Add a directory to protoc's include path.
*/
public void include(Object dir) {
public void addIncludeDir(File dir) {
checkCanConfig()
if (dir instanceof File) {
includeDirs.add(dir)
} else {
includeDirs.add(project.file(dir))
}
includeDirs.add(dir)
// Register all files under the directory as input so that Gradle will check their changes for
// incremental build
inputs.dir(dir)
}

/**
* Add a collection of proto source files to be compiled.
*/
public void addSourceFiles(Object files) {
checkCanConfig()
sourceFiles.from(files)
// Register the files as input so that Gradle will check their changes for incremental build
inputs.files(files)
}

/**
Expand Down Expand Up @@ -422,7 +441,7 @@ public class GenerateProtoTask extends DefaultTask {
ToolsLocator tools = project.protobuf.tools
// Sort to ensure generated descriptors have a canonical representation
// to avoid triggering unnecessary rebuilds downstream
List<File> protoFiles = inputs.sourceFiles.files.sort()
List<File> protoFiles = sourceFiles.files.sort()

[builtins, plugins]*.each { plugin ->
File outputDir = new File(getOutputDir(plugin))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class ProtobufExtract extends DefaultTask {

@TaskAction
void extract() {
destDir.mkdir()
inputs.files.each { file ->
logger.debug "Extracting protos from ${file} to ${destDir}"
if (file.isDirectory()) {
Expand Down
63 changes: 28 additions & 35 deletions src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.attributes.Attribute
import org.gradle.api.file.ConfigurableFileTree
import org.gradle.api.file.FileCollection
import org.gradle.api.file.SourceDirectorySet
import org.gradle.api.internal.file.FileResolver
Expand Down Expand Up @@ -226,16 +225,14 @@ class ProtobufPlugin implements Plugin<Project> {
java { }
}

Task extractProtosTask = maybeAddExtractProtosTask(sourceSet.name)
generateProtoTask.dependsOn(extractProtosTask)

setupExtractProtosTask(generateProtoTask, sourceSet.name)
setupExtractIncludeProtosTask(generateProtoTask, sourceSet.name)

// Include source proto files in the compiled archive, so that proto files from
// dependent projects can import them.
Task processResourcesTask =
project.tasks.getByName(sourceSet.getTaskName('process', 'resources'))
processResourcesTask.from(generateProtoTask.inputs.sourceFiles) {
processResourcesTask.from(generateProtoTask.sourceFiles) {
include '**/*.proto'
}
}
Expand All @@ -253,8 +250,7 @@ class ProtobufPlugin implements Plugin<Project> {
generateProtoTask.doneInitializing()

variant.sourceSets.each {
Task extractProtosTask = maybeAddExtractProtosTask(it.name)
generateProtoTask.dependsOn(extractProtosTask)
setupExtractProtosTask(generateProtoTask, it.name)
}

if (variant.hasProperty("compileConfiguration")) {
Expand Down Expand Up @@ -303,45 +299,43 @@ class ProtobufPlugin implements Plugin<Project> {
outputBaseDir = "${project.protobuf.generatedFilesBaseDir}/${sourceSetOrVariantName}"
it.fileResolver = this.fileResolver
sourceSets.each { sourceSet ->
// Include sources
Utils.addFilesToTaskInputs(inputs, sourceSet.proto)
addSourceFiles(sourceSet.proto)
ProtobufSourceDirectorySet protoSrcDirSet = sourceSet.proto
protoSrcDirSet.srcDirs.each { srcDir ->
include srcDir
// The source directory designated from sourceSet may not actually exist on disk.
// "include" it only when it exists, so that Gradle and protoc won't complain
if (srcDir.exists()) {
addIncludeDir(srcDir)
}
}

// Include extracted sources
ConfigurableFileTree extractedProtoSources =
project.fileTree(getExtractedProtosDir(sourceSet.name)) {
include "**/*.proto"
}
Utils.addFilesToTaskInputs(inputs, extractedProtoSources)
include extractedProtoSources.dir
}
}
}

/**
* Adds a task to extract protos from protobuf dependencies. They are
* Sets up a task to extract protos from protobuf dependencies. They are
* treated as sources and will be compiled.
*
* <p>This task is per-sourceSet, for both Java and Android. In Android a
* variant may have multiple sourceSets, each of these sourceSets will have
* its own extraction task.
*/
private Task maybeAddExtractProtosTask(String sourceSetName) {
private void setupExtractProtosTask(
GenerateProtoTask generateProtoTask, String sourceSetName) {
String extractProtosTaskName = 'extract' +
Utils.getSourceSetSubstringForTaskNames(sourceSetName) + 'Proto'
Task existingTask = project.tasks.findByName(extractProtosTaskName)
if (existingTask != null) {
return existingTask
}
return project.tasks.create(extractProtosTaskName, ProtobufExtract) {
description = "Extracts proto files/dependencies specified by 'protobuf' configuration"
destDir = getExtractedProtosDir(sourceSetName) as File
inputs.files project.configurations[Utils.getConfigName(sourceSetName, 'protobuf')]
isTest = Utils.isTest(sourceSetName)
Task task = project.tasks.findByName(extractProtosTaskName)
if (task == null) {
task = project.tasks.create(extractProtosTaskName, ProtobufExtract) {
description = "Extracts proto files/dependencies specified by 'protobuf' configuration"
destDir = getExtractedProtosDir(sourceSetName) as File
inputs.files project.configurations[Utils.getConfigName(sourceSetName, 'protobuf')]
isTest = Utils.isTest(sourceSetName)
}
}

linkExtractTaskToGenerateTask(task, generateProtoTask)
generateProtoTask.addSourceFiles(project.fileTree(task.destDir) { include "**/*.proto" })
}

/**
Expand Down Expand Up @@ -391,13 +385,12 @@ class ProtobufPlugin implements Plugin<Project> {
}
}

generateProtoTask.dependsOn task
linkExtractTaskToGenerateTask(task, generateProtoTask)
}

File extractedIncludeProtosDir = task.destDir
generateProtoTask.include extractedIncludeProtosDir
// Register the include dir as input, but not as "source".
// Inputs are checked in incremental builds, but only "source" files are compiled.
generateProtoTask.inputs.dir extractedIncludeProtosDir
private void linkExtractTaskToGenerateTask(ProtobufExtract extractTask, GenerateProtoTask generateTask) {
generateTask.dependsOn(extractTask)
generateTask.addIncludeDir(extractTask.destDir)
}

private void linkGenerateProtoTasksToTaskName(String compileTaskName, GenerateProtoTask genProtoTask) {
Expand Down
5 changes: 0 additions & 5 deletions src/main/groovy/com/google/protobuf/gradle/Utils.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import org.apache.commons.lang.StringUtils
import org.gradle.api.GradleException
import org.gradle.api.Project
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.TaskInputs
import org.gradle.plugins.ide.idea.GenerateIdeaModule
import org.gradle.plugins.ide.idea.model.IdeaModel
import org.gradle.util.GUtil
Expand Down Expand Up @@ -95,10 +94,6 @@ class Utils {
return "compile" + GUtil.toCamelCase(variantName) + "Kotlin"
}

static void addFilesToTaskInputs(TaskInputs inputs, Object files) {
inputs.files(files).skipWhenEmpty()
}

/**
* Returns positive/0/negative if current Gradle version is higher than/equal to/lower than the
* given target version. Only major and minor versions are checked. Patch version is ignored.
Expand Down

0 comments on commit 7123080

Please sign in to comment.