From 4f95b10862da4f302aa96f8697ddb47d52e3e93d Mon Sep 17 00:00:00 2001 From: rougsig Date: Fri, 7 Oct 2022 10:55:29 +0300 Subject: [PATCH 1/5] Failing test case for generate proto task source files should include only *.proto files See: https://github.com/google/protobuf-gradle-plugin/issues/620 --- .../gradle/ProtobufJavaPluginTest.groovy | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy index 58c72b59..451a6610 100644 --- a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy +++ b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy @@ -35,6 +35,23 @@ class ProtobufJavaPluginTest extends Specification { assert project.tasks.extractTestProto instanceof ProtobufExtract } + void "test generate proto task sources should include only *.proto files"() { + given: "a project with readme file in proto source directory" + Project project = setupBasicProject() + project.file("src/main/proto").mkdirs() + project.file("src/main/proto/messages.proto") << "syntax = \"proto3\";" + project.file("src/main/proto/README.md") << "Hello World" + + when: "project evaluated" + project.evaluate() + + then: "it contains only *.proto files" + assert Objects.equals( + project.tasks.generateProto.sourceDirs.asFileTree.files, + [project.file("src/main/proto/messages.proto")] as Set + ) + } + void "testCustom sourceSet should get its own GenerateProtoTask"() { given: "a basic project with java and com.google.protobuf" Project project = setupBasicProject() From dbd52cbf84be79676ef0bd3129247e78efb88c18 Mon Sep 17 00:00:00 2001 From: rougsig Date: Fri, 7 Oct 2022 15:36:03 +0300 Subject: [PATCH 2/5] Add quickfix for #620 --- .../com/google/protobuf/gradle/GenerateProtoTask.groovy | 6 +++++- .../google/protobuf/gradle/ProtobufJavaPluginTest.groovy | 3 +++ testProjectAndroidBase/src/main/proto/README.md | 1 + testProjectAndroidBase/src/test/proto/README.md | 1 + testProjectBase/src/main/proto/README.md | 1 + testProjectBase/src/test/proto/README.md | 1 + 6 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 testProjectAndroidBase/src/main/proto/README.md create mode 100644 testProjectAndroidBase/src/test/proto/README.md create mode 100644 testProjectBase/src/main/proto/README.md create mode 100644 testProjectBase/src/test/proto/README.md diff --git a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy index 35b656d6..3413b271 100644 --- a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy +++ b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy @@ -589,7 +589,11 @@ public abstract class GenerateProtoTask extends DefaultTask { // Sort to ensure generated descriptors have a canonical representation // to avoid triggering unnecessary rebuilds downstream - List protoFiles = sourceDirs.asFileTree.files.sort() + List protoFiles = sourceDirs.asFileTree + // quick fix for https://github.com/google/protobuf-gradle-plugin/issues/620 + .filter { File it -> it.name.endsWith(".proto") } + .files + .sort() [builtins, plugins]*.forEach { PluginOptions plugin -> String outputPath = getOutputDir(plugin) diff --git a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy index 451a6610..2db136be 100644 --- a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy +++ b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy @@ -6,6 +6,7 @@ import org.gradle.testfixtures.ProjectBuilder import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.GradleRunner import org.gradle.testkit.runner.TaskOutcome +import spock.lang.Ignore import spock.lang.Specification import spock.lang.Unroll @@ -35,6 +36,8 @@ class ProtobufJavaPluginTest extends Specification { assert project.tasks.extractTestProto instanceof ProtobufExtract } + // Do not forget add android test + @Ignore("enable in 0.10.0, good fix contains breaking api change, more info #621") void "test generate proto task sources should include only *.proto files"() { given: "a project with readme file in proto source directory" Project project = setupBasicProject() diff --git a/testProjectAndroidBase/src/main/proto/README.md b/testProjectAndroidBase/src/main/proto/README.md new file mode 100644 index 00000000..557db03d --- /dev/null +++ b/testProjectAndroidBase/src/main/proto/README.md @@ -0,0 +1 @@ +Hello World diff --git a/testProjectAndroidBase/src/test/proto/README.md b/testProjectAndroidBase/src/test/proto/README.md new file mode 100644 index 00000000..557db03d --- /dev/null +++ b/testProjectAndroidBase/src/test/proto/README.md @@ -0,0 +1 @@ +Hello World diff --git a/testProjectBase/src/main/proto/README.md b/testProjectBase/src/main/proto/README.md new file mode 100644 index 00000000..557db03d --- /dev/null +++ b/testProjectBase/src/main/proto/README.md @@ -0,0 +1 @@ +Hello World diff --git a/testProjectBase/src/test/proto/README.md b/testProjectBase/src/test/proto/README.md new file mode 100644 index 00000000..557db03d --- /dev/null +++ b/testProjectBase/src/test/proto/README.md @@ -0,0 +1 @@ +Hello World From ea98e0662e772ef41adb53496e470de5c0df7e3d Mon Sep 17 00:00:00 2001 From: rougsig Date: Fri, 7 Oct 2022 17:52:18 +0300 Subject: [PATCH 3/5] Add better fix for #620 --- .../google/protobuf/gradle/GenerateProtoTask.groovy | 8 ++------ .../google/protobuf/gradle/ProtobufPlugin.groovy | 13 +++++++------ .../protobuf/gradle/ProtobufJavaPluginTest.groovy | 1 - 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy index 3413b271..2e6f2510 100644 --- a/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy +++ b/src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy @@ -589,11 +589,7 @@ public abstract class GenerateProtoTask extends DefaultTask { // Sort to ensure generated descriptors have a canonical representation // to avoid triggering unnecessary rebuilds downstream - List protoFiles = sourceDirs.asFileTree - // quick fix for https://github.com/google/protobuf-gradle-plugin/issues/620 - .filter { File it -> it.name.endsWith(".proto") } - .files - .sort() + List protoFiles = sourceDirs.asFileTree.files.sort() [builtins, plugins]*.forEach { PluginOptions plugin -> String outputPath = getOutputDir(plugin) @@ -608,7 +604,7 @@ public abstract class GenerateProtoTask extends DefaultTask { // 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. - List dirs = (sourceDirs + includeDirs).filter { File it -> it.exists() }*.path + List dirs = includeDirs.filter { File it -> it.exists() }*.path .collect { "-I${it}".toString() } logger.debug "ProtobufCompile using directories ${dirs}" logger.debug "ProtobufCompile using files ${protoFiles}" diff --git a/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy b/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy index 60840cb1..8ae3ebe1 100644 --- a/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy +++ b/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy @@ -248,7 +248,7 @@ class ProtobufPlugin implements Plugin { Provider extractIncludeProtosTask = setupExtractIncludeProtosTask( sourceSet.name, compileProtoPath, sourceSet.compileClasspath) Provider generateProtoTask = addGenerateProtoTask( - sourceSet.name, protoSrcDirSet.sourceDirectories, project.files(extractProtosTask), + sourceSet.name, protoSrcDirSet, project.files(extractProtosTask), extractIncludeProtosTask) { it.sourceSet = sourceSet it.doneInitializing() @@ -320,9 +320,8 @@ class ProtobufPlugin implements Plugin { setupExtractIncludeProtosTask(variant.name, classPathConfig, testClassPathConfig) // GenerateProto task, one per variant (compilation unit). - FileCollection sourceDirs = project.files(project.providers.provider { - variant.sourceSets.collect { it.proto.sourceDirectories } - }) + SourceDirectorySet sourceDirs = project.objects.sourceDirectorySet(variant.name, "AllSourceSets") + variant.sourceDirs.forEach { sourceDirs.source(it.proto) } FileCollection extractProtosDirs = project.files(project.providers.provider { variant.sourceSets.collect { project.files(project.tasks.named(getExtractProtosTaskName(it.name))) @@ -367,9 +366,10 @@ class ProtobufPlugin implements Plugin { * compiled. For Java it's the sourceSet that sourceSetOrVariantName stands * for; for Android it's the collection of sourceSets that the variant includes. */ + @SuppressWarnings(["UnnecessaryObjectReferences"]) // suppress a lot of it.doLogic in task registration block private Provider addGenerateProtoTask( String sourceSetOrVariantName, - FileCollection sourceDirs, + SourceDirectorySet protoSourceSet, FileCollection extractProtosDirs, Provider extractIncludeProtosTask, Action configureAction) { @@ -381,7 +381,8 @@ class ProtobufPlugin implements Plugin { return project.tasks.register(generateProtoTaskName, GenerateProtoTask) { it.description = "Compiles Proto source for '${sourceSetOrVariantName}'".toString() it.outputBaseDir = outDir - it.addSourceDirs(sourceDirs) + it.addSourceDirs(protoSourceSet.asFileTree) + it.addIncludeDir(protoSourceSet.sourceDirectories) it.addSourceDirs(extractProtosDirs) it.addIncludeDir(project.files(extractIncludeProtosTask)) configureAction.execute(it) diff --git a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy index 2db136be..33e44d66 100644 --- a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy +++ b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy @@ -37,7 +37,6 @@ class ProtobufJavaPluginTest extends Specification { } // Do not forget add android test - @Ignore("enable in 0.10.0, good fix contains breaking api change, more info #621") void "test generate proto task sources should include only *.proto files"() { given: "a project with readme file in proto source directory" Project project = setupBasicProject() From 5ab7bc230b097441bfa7710298539bbb8ce310b7 Mon Sep 17 00:00:00 2001 From: rougsig Date: Fri, 7 Oct 2022 22:13:11 +0300 Subject: [PATCH 4/5] Fix for #620 again --- .../groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy | 3 ++- .../com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy b/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy index 8ae3ebe1..4d223957 100644 --- a/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy +++ b/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy @@ -381,9 +381,10 @@ class ProtobufPlugin implements Plugin { return project.tasks.register(generateProtoTaskName, GenerateProtoTask) { it.description = "Compiles Proto source for '${sourceSetOrVariantName}'".toString() it.outputBaseDir = outDir - it.addSourceDirs(protoSourceSet.asFileTree) + it.addSourceDirs(protoSourceSet) it.addIncludeDir(protoSourceSet.sourceDirectories) it.addSourceDirs(extractProtosDirs) + it.addIncludeDir(extractProtosDirs) it.addIncludeDir(project.files(extractIncludeProtosTask)) configureAction.execute(it) } diff --git a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy index 33e44d66..451a6610 100644 --- a/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy +++ b/src/test/groovy/com/google/protobuf/gradle/ProtobufJavaPluginTest.groovy @@ -6,7 +6,6 @@ import org.gradle.testfixtures.ProjectBuilder import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.GradleRunner import org.gradle.testkit.runner.TaskOutcome -import spock.lang.Ignore import spock.lang.Specification import spock.lang.Unroll @@ -36,7 +35,6 @@ class ProtobufJavaPluginTest extends Specification { assert project.tasks.extractTestProto instanceof ProtobufExtract } - // Do not forget add android test void "test generate proto task sources should include only *.proto files"() { given: "a project with readme file in proto source directory" Project project = setupBasicProject() From c02cfa435ba14f7ae4cc4fdb6df3ba3550153553 Mon Sep 17 00:00:00 2001 From: rougsig Date: Fri, 7 Oct 2022 22:23:09 +0300 Subject: [PATCH 5/5] Fix android variant mistype field name --- .../groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy b/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy index 4d223957..82b18d67 100644 --- a/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy +++ b/src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy @@ -321,7 +321,7 @@ class ProtobufPlugin implements Plugin { // GenerateProto task, one per variant (compilation unit). SourceDirectorySet sourceDirs = project.objects.sourceDirectorySet(variant.name, "AllSourceSets") - variant.sourceDirs.forEach { sourceDirs.source(it.proto) } + variant.sourceSets.forEach { sourceDirs.source(it.proto) } FileCollection extractProtosDirs = project.files(project.providers.provider { variant.sourceSets.collect { project.files(project.tasks.named(getExtractProtosTaskName(it.name)))