Skip to content

Commit

Permalink
Avoid using project.copy when possible, and fix serialization
Browse files Browse the repository at this point in the history
Since Gradle 6.0 there is FileSystemOperations API which is compatible
with configuration caching. This PR introduces CopyActionFacade which
exposes the copy method, but implementation is using the new API whne
possible, and reverts to project.copy when Gradle version is lower than 6.0.

This PR also starts using Gradle 6.0 to build the plugin. It is needed in
order to use FileSystemOperations API.

In GenerateProtoTask, SourceSet is now not serialized as part of the
task state, and a provider capturing what is needed is created instead.

Integration test for the Java plugin is updated to Gradle 6.5-rc-1.
  • Loading branch information
gavra0 committed Jun 1, 2020
1 parent f2febac commit 405ca49
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 34 deletions.
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-5.6-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-6.0-bin.zip
70 changes: 70 additions & 0 deletions src/main/groovy/com/google/protobuf/gradle/CopyActionFacade.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Original work copyright (c) 2015, Alex Antonov. All rights reserved.
* Modified work copyright (c) 2015, Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* 3. Neither the name of the copyright holder nor the names of its contributors
* may be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package com.google.protobuf.gradle;

import org.gradle.api.Action;
import org.gradle.api.Project;
import org.gradle.api.file.CopySpec;
import org.gradle.api.file.FileSystemOperations;
import org.gradle.api.tasks.WorkResult;

import javax.inject.Inject;

/**
* Interface exposing the file copying feature. Actual implementations may use the
* {@link org.gradle.api.file.FileSystemOperations} if available (Gradle 6.0+) or {@link org.gradle.api.Project#copy} if
* the version of Gradle is below 6.0.
*/
interface CopyActionFacade {
WorkResult copy(Action<? super CopySpec> var1);

class ProjectBased implements CopyActionFacade {
private final Project project;

public ProjectBased(Project project) {
this.project = project;
}

@Override
public WorkResult copy(Action<? super CopySpec> action) {
return project.copy(action);
}
}

abstract class FileSystemOperationsBased implements CopyActionFacade {
@Inject
public abstract FileSystemOperations getFileSystemOperations();

@Override
public WorkResult copy(Action<? super CopySpec> action) {
return getFileSystemOperations().copy(action);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import org.gradle.api.file.FileCollection
import org.gradle.api.file.SourceDirectorySet
import org.gradle.api.internal.file.FileResolver
import org.gradle.api.logging.LogLevel
import org.gradle.api.provider.Provider
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.tasks.CacheableTask
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
Expand All @@ -55,13 +57,14 @@ import org.gradle.api.tasks.TaskAction
import org.gradle.util.ConfigureUtil

import javax.annotation.Nullable
import javax.inject.Inject

/**
* The task that compiles proto files into Java files.
*/
// TODO(zhangkun83): add per-plugin output dir reconfiguraiton.
@CacheableTask
public class GenerateProtoTask extends DefaultTask {
public abstract 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
static final int WINDOWS_CMD_LENGTH_LIMIT = 32760
Expand All @@ -82,13 +85,19 @@ public class GenerateProtoTask extends DefaultTask {
// constructor arguments. We use the initializing flag to prevent users from
// accidentally modifying them.
private String outputBaseDir
// Tags for selectors inside protobuf.generateProtoTasks
private SourceSet sourceSet
// Tags for selectors inside protobuf.generateProtoTasks; do not serialize with Gradle configuration caching
transient private SourceSet sourceSet
private Object variant
private ImmutableList<String> flavors
private String buildType
private boolean isTestVariant
private FileResolver fileResolver
private Provider<Boolean> isTestProvider = providerFactory.provider {
if (Utils.isAndroidProject(project)) {
return isTestVariant
}
return Utils.isTest(sourceSet.name)
}

/**
* If true, will set the protoc flag
Expand Down Expand Up @@ -309,6 +318,9 @@ public class GenerateProtoTask extends DefaultTask {
return descriptorSetOptions.path != null ? descriptorSetOptions.path : "${outputBaseDir}/descriptor_set.desc"
}

@Inject
abstract ProviderFactory getProviderFactory()

public GenerateProtoTask() {
builtins = project.container(PluginOptions)
plugins = project.container(PluginOptions)
Expand Down Expand Up @@ -382,10 +394,12 @@ public class GenerateProtoTask extends DefaultTask {
*/
@Input
public boolean getIsTest() {
if (Utils.isAndroidProject(project)) {
return isTestVariant
}
return Utils.isTest(sourceSet.name)
return isTestProvider.get()
}

@Internal("Already captured with getIsTest()")
Provider<Boolean> getIsTestProvider() {
return isTestProvider
}

/**
Expand Down
59 changes: 40 additions & 19 deletions src/main/groovy/com/google/protobuf/gradle/ProtobufExtract.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,28 @@ 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.model.ObjectFactory
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
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 javax.inject.Inject

/**
* Extracts proto files from a dependency configuration.
*/
class ProtobufExtract extends DefaultTask {
abstract class ProtobufExtract extends DefaultTask {

/**
* The directory for the extracted files.
*/
private File destDir
private Boolean isTest = null
private final ConfigurableFileCollection inputFiles = project.files()
private final ConfigurableFileCollection inputFiles = objectFactory.fileCollection()
private CopyActionFacade copyActionFacade = instantiateCopyActionFacade()

public void setIsTest(boolean isTest) {
this.isTest = isTest
Expand All @@ -68,53 +72,61 @@ class ProtobufExtract extends DefaultTask {
return inputFiles
}

@Internal
CopyActionFacade getCopyActionFacade() {
return copyActionFacade
}

@Inject
abstract ObjectFactory getObjectFactory()

@TaskAction
void extract() {
destDir.mkdir()
boolean warningLogged = false
inputFiles.each { file ->
logger.debug "Extracting protos from ${file} to ${destDir}"
if (file.isDirectory()) {
project.copy {
includeEmptyDirs(false)
from(file.path) {
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(file.path) {
include '**/*.proto'
}
into(destDir)
spec.into(destDir)
}
} else if (file.path.endsWith('.proto')) {
if (!warningLogged) {
warningLogged = true
project.logger.warn "proto file '${file.path}' directly specified in configuration. " +
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."
}
project.copy {
includeEmptyDirs(false)
from(file.path)
into(destDir)
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(file.path)
spec.into(destDir)
}
} else if (file.path.endsWith('.jar') || file.path.endsWith('.zip')) {
project.copy {
includeEmptyDirs(false)
from(project.zipTree(file.path)) {
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(project.zipTree(file.path)) {
include '**/*.proto'
}
into(destDir)
spec.into(destDir)
}
} else if (file.path.endsWith('.tar')
|| file.path.endsWith('.tar.gz')
|| file.path.endsWith('.tar.bz2')
|| file.path.endsWith('.tgz')) {
project.copy {
includeEmptyDirs(false)
from(project.tarTree(file.path)) {
copyActionFacade.copy { spec ->
spec.includeEmptyDirs = false
spec.from(project.tarTree(file.path)) {
include '**/*.proto'
}
into(destDir)
spec.into(destDir)
}
} else {
logger.debug "Skipping unsupported file type (${file.path}); handles only jar, tar, tar.gz, tar.bz2 & tgz"
Expand All @@ -132,4 +144,13 @@ class ProtobufExtract extends DefaultTask {
protected File getDestDir() {
return destDir
}

private CopyActionFacade instantiateCopyActionFacade() {
if (Utils.compareGradleVersion(project, "6.0") > 0) {
// Use object factory to instantiate as that will inject the necessary service.
return objectFactory.newInstance(CopyActionFacade.FileSystemOperationsBased.class)
} else {
return new CopyActionFacade.ProjectBased(project)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import spock.lang.Unroll
*/
class ProtobufJavaPluginTest extends Specification {
// Current supported version is Gradle 5+.
private static final List<String> GRADLE_VERSIONS = ["5.6", "6.0", "6.1"]
private static final List<String> GRADLE_VERSIONS = ["5.6", "6.0", "6.5-rc-1"]
private static final List<String> KOTLIN_VERSIONS = ["1.3.20", "1.3.30"]

void "testApplying java and com.google.protobuf adds corresponding task to project"() {
Expand Down Expand Up @@ -78,7 +78,7 @@ class ProtobufJavaPluginTest extends Specification {
}
@Unroll
void "testProject should be successfully executed (instant-execution) [gradle #gradleVersion]"() {
void "testProject should be successfully executed (configuration cache) [gradle #gradleVersion]"() {
given: "project from testProject"
File projectDir = ProtobufPluginTestHelper.projectBuilder('testProject')
.copyDirs('testProjectBase', 'testProject')
Expand All @@ -89,8 +89,7 @@ class ProtobufJavaPluginTest extends Specification {
.withProjectDir(projectDir)
.withArguments(
'build', '--stacktrace',
'-Dorg.gradle.unsafe.instant-execution=true',
'-Dorg.gradle.unsafe.instant-execution.fail-on-problems=true'
'--configuration-cache=warn'
)
.withPluginClasspath()
.withGradleVersion(gradleVersion)
Expand All @@ -111,10 +110,10 @@ class ProtobufJavaPluginTest extends Specification {
result = runner.build()
then: "it reuses the task graph"
result.output.contains("Reusing instant execution cache")
result.output.contains("Reusing configuration cache")
and: "it succeeds"
result.task(":build").outcome == TaskOutcome.SUCCESS
and: "it is up to date"
result.task(":build").outcome == TaskOutcome.UP_TO_DATE
ProtobufPluginTestHelper.verifyProjectDir(projectDir)
where:
Expand Down

0 comments on commit 405ca49

Please sign in to comment.