Skip to content

Commit

Permalink
Revert work-around removal from #719 and properly fix #711
Browse files Browse the repository at this point in the history
  • Loading branch information
Vampire committed Jan 22, 2025
1 parent 2810499 commit a14750b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 17 deletions.
27 changes: 16 additions & 11 deletions src/main/groovy/com/google/protobuf/gradle/ProtobufExtract.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,9 @@ import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.DuplicatesStrategy
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.provider.ProviderFactory
import org.gradle.api.tasks.InputFiles
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.OutputDirectory
Expand Down Expand Up @@ -71,6 +69,13 @@ abstract class ProtobufExtract extends DefaultTask {
@Internal
public abstract ConfigurableFileCollection getInputFiles()

/**
* A dummy task dependency to delay provider calculation for input proto files to execution time
* even with configuration cache enabled.
*/
@Internal
abstract ConfigurableFileCollection getDummyTaskDependency()

/**
* 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.
Expand Down Expand Up @@ -99,9 +104,6 @@ abstract class ProtobufExtract extends DefaultTask {
@Inject
protected abstract ObjectFactory getObjectFactory()

@Inject
protected abstract ProviderFactory getProviderFactory()

private ArchiveActionFacade instantiateArchiveActionFacade() {
if (GradleVersion.current() >= GradleVersion.version("6.0")) {
// Use object factory to instantiate as that will inject the necessary service.
Expand All @@ -117,14 +119,17 @@ abstract class ProtobufExtract extends DefaultTask {
// Provider.map seems broken for excluded tasks. Add inputFiles with all contents excluded for
// the dependency it provides, but then provide the files we actually care about in our own
// provider. https://github.com/google/protobuf-gradle-plugin/issues/550
PatternSet protoFilter = new PatternSet().include("**/*.proto")
// Additionally depend on a dummy task for the own provider so that it is executed at
// execution time even with configuration cache enabled.
// https://github.com/google/protobuf-gradle-plugin/issues/711
def inputFiles = inputFiles
return objectFactory.fileCollection()
.from(inputFiles.filter { false })
.from(
inputFiles.getElements().map { elements ->
.from(dummyTaskDependency.elements.map { unused ->
Set<File> files = inputFiles.files
PatternSet protoFilter = new PatternSet().include("**/*.proto")
Set<Object> protoInputs = [] as Set
for (FileSystemLocation e : elements) {
File file = e.asFile
for (File file : files) {
if (file.isDirectory()) {
protoInputs.add(file)
} else if (file.path.endsWith('.proto')) {
Expand All @@ -151,7 +156,7 @@ abstract class ProtobufExtract extends DefaultTask {
|| file.path.endsWith('.tgz')) {
protoInputs.add(archiveFacade.tarTree(file.path).matching(protoFilter))
} else {
logger.debug "Skipping unsupported file type (${file.path});" +
logger.debug "Skipping unsupported file type (${file.path}); " +
"handles only jar, tar, tar.gz, tar.bz2 & tgz"
}
}
Expand Down
18 changes: 12 additions & 6 deletions src/main/groovy/com/google/protobuf/gradle/ProtobufPlugin.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import org.gradle.api.GradleException
import org.gradle.api.NamedDomainObjectContainer
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.type.ArtifactTypeDefinition
import org.gradle.api.attributes.LibraryElements
Expand Down Expand Up @@ -145,12 +146,13 @@ class ProtobufPlugin implements Plugin<Project> {
addTasksForVariant(variant, variantSourceSets, postConfigure)
}
} else {
Provider<Task> dummyTask = project.tasks.register("dummy")
project.sourceSets.configureEach { sourceSet ->
ProtoSourceSet protoSourceSet = protobufExtension.sourceSets.create(sourceSet.name)
addSourceSetExtension(sourceSet, protoSourceSet)
Configuration protobufConfig = createProtobufConfiguration(protoSourceSet)
Configuration compileProtoPath = createCompileProtoPathConfiguration(protoSourceSet)
addTasksForSourceSet(sourceSet, protoSourceSet, protobufConfig, compileProtoPath, postConfigure)
addTasksForSourceSet(sourceSet, protoSourceSet, protobufConfig, compileProtoPath, postConfigure, dummyTask)
}
}
project.afterEvaluate {
Expand Down Expand Up @@ -237,11 +239,11 @@ class ProtobufPlugin implements Plugin<Project> {
*/
private void addTasksForSourceSet(
SourceSet sourceSet, ProtoSourceSet protoSourceSet, Configuration protobufConfig,
Configuration compileProtoPath, Collection<Closure> postConfigure) {
Provider<ProtobufExtract> extractProtosTask = setupExtractProtosTask(protoSourceSet, protobufConfig)
Configuration compileProtoPath, Collection<Closure> postConfigure, Provider<Task> dummyTask) {
Provider<ProtobufExtract> extractProtosTask = setupExtractProtosTask(protoSourceSet, protobufConfig, dummyTask)

Provider<ProtobufExtract> extractIncludeProtosTask = setupExtractIncludeProtosTask(
protoSourceSet, compileProtoPath)
protoSourceSet, compileProtoPath, dummyTask)

// Make protos in 'test' sourceSet able to import protos from the 'main' sourceSet.
// Pass include proto files from main to test.
Expand Down Expand Up @@ -422,14 +424,16 @@ class ProtobufPlugin implements Plugin<Project> {
*/
private Provider<ProtobufExtract> setupExtractProtosTask(
ProtoSourceSet protoSourceSet,
Configuration protobufConfig
Configuration protobufConfig,
Provider<Task> dummyTask
) {
String sourceSetName = protoSourceSet.name
String taskName = getExtractProtosTaskName(sourceSetName)
Provider<ProtobufExtract> task = project.tasks.register(taskName, ProtobufExtract) {
it.description = "Extracts proto files/dependencies specified by 'protobuf' configuration"
it.destDir.set(getExtractedProtosDir(sourceSetName) as File)
it.inputFiles.from(protobufConfig)
it.dummyTaskDependency.from(dummyTask)
}
protoSourceSet.proto.srcDir(task)
return task
Expand All @@ -448,13 +452,15 @@ class ProtobufPlugin implements Plugin<Project> {
*/
private Provider<ProtobufExtract> setupExtractIncludeProtosTask(
ProtoSourceSet protoSourceSet,
FileCollection archives
FileCollection archives,
Provider<Task> dummyTask
) {
String taskName = 'extractInclude' + Utils.getSourceSetSubstringForTaskNames(protoSourceSet.name) + 'Proto'
Provider<ProtobufExtract> task = project.tasks.register(taskName, ProtobufExtract) {
it.description = "Extracts proto files from compile dependencies for includes"
it.destDir.set(getExtractedIncludeProtosDir(protoSourceSet.name) as File)
it.inputFiles.from(archives)
it.dummyTaskDependency.from(dummyTask)
}
protoSourceSet.includeProtoDirs.from(task)
return task
Expand Down

0 comments on commit a14750b

Please sign in to comment.