Skip to content

Commit

Permalink
Fix input and output annotations (#364)
Browse files Browse the repository at this point in the history
With Gradle 6.0 we are starting to enforce annotations on all task getters. Non ad-hoc tasks should use annotations in place of the `Task.getInputs()` and `getOutputs()` APIs. Due to these changes with Gradle 6.0 the applying the Protobuf plugin results in the following deprecation warnings being displayed:

```text
   > Warning: Type 'GenerateProtoTask': property 'buildType' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'builtins' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'descriptorPath' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'descriptorSetOptions' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'fileResolver' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'flavors' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'generateDescriptorSet' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'isTest' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'isTestVariant' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'outputSourceDirectorySet' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'plugins' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'sourceFiles' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'sourceSet' is not annotated with an input or output annotation.
   > Warning: Type 'GenerateProtoTask': property 'variant' is not annotated with an input or output annotation.
   > Warning: Type 'ProtobufExtract': property 'destDir' is not annotated with an input or output annotation.
   > Warning: Type 'ProtobufExtract': property 'isTest' is not annotated with an input or output annotation.
```

This PR fixes the problems and the warnings are removed. This is based on #351 by @lptr
  • Loading branch information
zhangkun83 authored Dec 12, 2019
1 parent 44eb516 commit 4daf4e7
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 21 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ apply plugin: 'groovy'
apply plugin: 'kotlin'
apply plugin: 'maven'
apply plugin: 'signing'
apply plugin: 'java-gradle-plugin'
apply plugin: 'com.jfrog.bintray'
apply plugin: 'com.gradle.plugin-publish'
apply plugin: 'com.github.ben-manes.versions'
Expand Down
82 changes: 71 additions & 11 deletions src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,27 @@ import org.gradle.api.internal.file.DefaultSourceDirectorySet
import org.gradle.api.internal.file.FileResolver
import org.gradle.api.internal.file.collections.DefaultDirectoryFileTreeFactory
import org.gradle.api.logging.LogLevel
import org.gradle.api.tasks.CacheableTask
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.Nested
import org.gradle.api.tasks.Optional
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.SkipWhenEmpty
import org.gradle.api.tasks.SourceSet
import org.gradle.api.tasks.TaskAction
import org.gradle.util.ConfigureUtil

import javax.annotation.Nullable

/**
* The task that compiles proto files into Java files.
*/
// TODO(zhangkun83): add per-plugin output dir reconfiguraiton.
@CacheableTask
public class GenerateProtoTask extends DefaultTask {
// Windows CreateProcess has command line limit of 32768:
// https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx
Expand Down Expand Up @@ -86,6 +98,7 @@ public class GenerateProtoTask extends DefaultTask {
*
* Default: false
*/
@Internal("Handled as input via getDescriptorSetOptionsForCaching()")
boolean generateDescriptorSet

/**
Expand All @@ -98,23 +111,29 @@ public class GenerateProtoTask extends DefaultTask {
*
* Default: null
*/
@Nullable
@Optional
@Input
GString path

/**
* If true, source information (comments, locations) will be included in the descriptor set.
*
* Default: false
*/
@Input
boolean includeSourceInfo

/**
* If true, imports are included in the descriptor set, such that it is self-containing.
*
* Default: false
*/
@Input
boolean includeImports
}

@Internal("Handled as input via getDescriptorSetOptionsForCaching()")
final DescriptorSetOptions descriptorSetOptions = new DescriptorSetOptions()

// protoc allows you to prefix comma-delimited options to the path in
Expand Down Expand Up @@ -177,7 +196,11 @@ public class GenerateProtoTask extends DefaultTask {
checkInitializing()
Preconditions.checkState(this.outputBaseDir == null, 'outputBaseDir is already set')
this.outputBaseDir = outputBaseDir
outputs.dir outputBaseDir
}

@OutputDirectory
String getOutputBaseDir() {
return outputBaseDir
}

void setSourceSet(SourceSet sourceSet) {
Expand Down Expand Up @@ -214,38 +237,52 @@ public class GenerateProtoTask extends DefaultTask {
this.fileResolver = fileResolver
}

@Internal("Inputs tracked in getSourceFiles()")
SourceSet getSourceSet() {
Preconditions.checkState(!Utils.isAndroidProject(project),
'sourceSet should not be used in an Android project')
Preconditions.checkNotNull(sourceSet, 'sourceSet is not set')
return sourceSet
}

@SkipWhenEmpty
@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
FileCollection getSourceFiles() {
return sourceFiles
}

@InputFiles
@PathSensitive(PathSensitivity.RELATIVE)
FileCollection getIncludeDirs() {
return includeDirs
}

@Internal("Not an actual input to the task, only used to find tasks belonging to a variant")
Object getVariant() {
Preconditions.checkState(Utils.isAndroidProject(project),
'variant should not be used in a Java project')
Preconditions.checkNotNull(variant, 'variant is not set')
return variant
}

@Internal("Not an actual input to the task, only used to find tasks belonging to a variant")
boolean getIsTestVariant() {
Preconditions.checkState(Utils.isAndroidProject(project),
'isTestVariant should not be used in a Java project')
Preconditions.checkNotNull(variant, 'variant is not set')
return isTestVariant
}

@Internal("Not an actual input to the task, only used to find tasks belonging to a variant")
ImmutableList<String> getFlavors() {
Preconditions.checkState(Utils.isAndroidProject(project),
'flavors should not be used in a Java project')
Preconditions.checkNotNull(flavors, 'flavors is not set')
return flavors
}

@Internal("Not an actual input to the task, only used to find tasks belonging to a variant")
String getBuildType() {
Preconditions.checkState(Utils.isAndroidProject(project),
'buildType should not be used in a Java project')
Expand All @@ -255,11 +292,6 @@ public class GenerateProtoTask extends DefaultTask {
return buildType
}

FileResolver getFileResolver() {
Preconditions.checkNotNull(fileResolver)
return fileResolver
}

void doneInitializing() {
Preconditions.checkState(state == State.INIT, "Invalid state: ${state}")
state = State.CONFIG
Expand All @@ -270,6 +302,7 @@ public class GenerateProtoTask extends DefaultTask {
state = State.FINALIZED
}

@Internal("Tracked as an input via getDescriptorSetOptionsForCaching()")
String getDescriptorPath() {
if (!generateDescriptorSet) {
throw new IllegalStateException(
Expand Down Expand Up @@ -299,6 +332,7 @@ public class GenerateProtoTask extends DefaultTask {
/**
* Returns the container of protoc builtins.
*/
@Internal("Tracked as an input via getBuiltinsForCaching()")
public NamedDomainObjectContainer<PluginOptions> getBuiltins() {
checkCanConfig()
return builtins
Expand All @@ -316,6 +350,7 @@ public class GenerateProtoTask extends DefaultTask {
/**
* Returns the container of protoc plugins.
*/
@Internal("Tracked as an input via getPluginsForCaching()")
public NamedDomainObjectContainer<PluginOptions> getPlugins() {
checkCanConfig()
return plugins
Expand All @@ -334,9 +369,6 @@ public class GenerateProtoTask extends DefaultTask {
public void addIncludeDir(FileCollection dir) {
checkCanConfig()
includeDirs.from(dir)
// Register all files under the directory as input so that Gradle will check their changes for
// incremental build
inputs.files(dir).withPathSensitivity(PathSensitivity.RELATIVE)
}

/**
Expand All @@ -345,13 +377,12 @@ public class GenerateProtoTask extends DefaultTask {
public void addSourceFiles(FileCollection files) {
checkCanConfig()
sourceFiles.from(files)
// Register the files as input so that Gradle will check their changes for incremental build
inputs.files(files).withPathSensitivity(PathSensitivity.RELATIVE)
}

/**
* Returns true if the Java source set or Android variant is test related.
*/
@Input
public boolean getIsTest() {
if (Utils.isAndroidProject(project)) {
return isTestVariant
Expand Down Expand Up @@ -379,13 +410,15 @@ public class GenerateProtoTask extends DefaultTask {
return this
}

@Input
public List<String> getOptions() {
return options
}

/**
* Returns the name of the plugin or builtin.
*/
@Input
@Override
public String getName() {
return name
Expand All @@ -401,6 +434,7 @@ public class GenerateProtoTask extends DefaultTask {
/**
* Returns the relative outputDir for this plugin. If outputDir is not specified, name is used.
*/
@Input
public String getOutputSubDir() {
if (outputSubDir != null) {
return outputSubDir
Expand All @@ -421,6 +455,7 @@ public class GenerateProtoTask extends DefaultTask {
* Returns a {@code SourceDirectorySet} representing the generated source
* directories.
*/
@Internal
SourceDirectorySet getOutputSourceDirectorySet() {
String srcSetName = "generate-proto-" + name
SourceDirectorySet srcSet
Expand Down Expand Up @@ -504,6 +539,31 @@ public class GenerateProtoTask extends DefaultTask {
}
}

/**
* Used to expose inputs to Gradle, not to be called directly.
*/
@Optional
@Nested
protected DescriptorSetOptions getDescriptorSetOptionsForCaching() {
return generateDescriptorSet ? descriptorSetOptions : null
}

/**
* Used to expose inputs to Gradle, not to be called directly.
*/
@Nested
protected Collection<PluginOptions> getBuiltinsForCaching() {
return builtins
}

/**
* Used to expose inputs to Gradle, not to be called directly.
*/
@Nested
protected Collection<PluginOptions> getPluginsForCaching() {
return plugins
}

private static enum State {
INIT, CONFIG, FINALIZED
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ package com.google.protobuf.gradle

import com.google.common.base.Preconditions
import org.gradle.api.DefaultTask
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.TaskAction

/**
Expand All @@ -43,21 +49,30 @@ class ProtobufExtract extends DefaultTask {
*/
private File destDir
private Boolean isTest = null
private final ConfigurableFileCollection inputFiles = project.files()

public void setIsTest(boolean isTest) {
this.isTest = isTest
}

@Input
public boolean getIsTest() {
Preconditions.checkNotNull(isTest)
return isTest
}

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

@TaskAction
void extract() {
destDir.mkdir()
boolean warningLogged = false
inputs.files.each { file ->
inputFiles.each { file ->
logger.debug "Extracting protos from ${file} to ${destDir}"
if (file.isDirectory()) {
project.copy {
Expand Down Expand Up @@ -113,6 +128,7 @@ class ProtobufExtract extends DefaultTask {
outputs.dir destDir
}

@OutputDirectory
protected File getDestDir() {
return destDir
}
Expand Down
14 changes: 5 additions & 9 deletions src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import org.gradle.api.internal.file.FileResolver
import org.gradle.api.internal.file.collections.DefaultDirectoryFileTreeFactory
import org.gradle.api.logging.LogLevel
import org.gradle.api.plugins.AppliedPlugin
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.SourceSet

import javax.inject.Inject
Expand Down Expand Up @@ -336,8 +335,7 @@ class ProtobufPlugin implements Plugin<Project> {
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')])
.withPathSensitivity(PathSensitivity.NAME_ONLY)
inputFiles.from(project.configurations[Utils.getConfigName(sourceSetName, 'protobuf')])
isTest = Utils.isTest(sourceSetName)
}
}
Expand Down Expand Up @@ -365,9 +363,8 @@ class ProtobufPlugin implements Plugin<Project> {
task = project.tasks.create(extractIncludeProtosTaskName, ProtobufExtract) {
description = "Extracts proto files from compile dependencies for includes"
destDir = getExtractedIncludeProtosDir(sourceSetOrVariantName) as File
inputs.files (compileClasspathConfiguration
inputFiles.from(compileClasspathConfiguration
?: project.configurations[Utils.getConfigName(sourceSetOrVariantName, 'compile')])
.withPathSensitivity(PathSensitivity.NAME_ONLY)

// TL; DR: Make protos in 'test' sourceSet able to import protos from the 'main'
// sourceSet. Sub-configurations, e.g., 'testCompile' that extends 'compile', don't
Expand All @@ -380,16 +377,15 @@ class ProtobufPlugin implements Plugin<Project> {
// ad-hoc solution that manually includes the source protos of 'main' and its
// dependencies.
if (Utils.isTest(sourceSetOrVariantName)) {
inputs.files getSourceSets()['main'].proto
inputs.files testedCompileClasspathConfiguration ?: project.configurations['compile']
inputFiles.from getSourceSets()['main'].proto
inputFiles.from testedCompileClasspathConfiguration ?: project.configurations['compile']
}
} else {
// In Java projects, the compileClasspath of the 'test' sourceSet includes all the
// 'resources' of the output of 'main', in which the source protos are placed. This is
// nicer than the ad-hoc solution that Android has, because it works for any extended
// configuration, not just 'testCompile'.
inputs.files (getSourceSets()[sourceSetOrVariantName].compileClasspath)
.withPathSensitivity(PathSensitivity.NAME_ONLY)
inputFiles.from getSourceSets()[sourceSetOrVariantName].compileClasspath
}
isTest = Utils.isTest(sourceSetOrVariantName)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.gradle.api.NamedDomainObjectContainer
import org.gradle.api.Project
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.Dependency
import org.gradle.api.tasks.PathSensitivity

/**
* Holds locations of all external executables, i.e., protoc and plugins.
Expand Down Expand Up @@ -110,6 +111,8 @@ class ToolsLocator {
if (protoc.is(locator) || protoTask.hasPlugin(locator.name)) {
// register the configuration dependency as a task input
protoTask.inputs.files(config)
.withPathSensitivity(PathSensitivity.NONE)
.withPropertyName("config.${locator.name}")

protoTask.doFirst {
if (locator.path == null) {
Expand Down

0 comments on commit 4daf4e7

Please sign in to comment.