Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert work-around removal from #719 and properly fix #711 #770

Merged
merged 1 commit into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/codenarc/codenarc.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
<exclude name="CyclomaticComplexity"/>
<exclude name="NestedBlockDepth"/>
<exclude name="MethodCount"/>
<exclude name="ParameterCount"/>
</ruleset-ref>
<ruleset-ref path='rulesets/unused.xml'/>
<ruleset-ref path='rulesets/unnecessary.xml'>
Expand Down
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 All @@ -96,9 +101,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 @@ -114,14 +116,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
FileCollection inputFiles = this.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 @@ -148,7 +153,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
27 changes: 17 additions & 10 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 @@ -129,28 +130,29 @@ class ProtobufPlugin implements Plugin<Project> {
// extract included protos from {@code variant.compileConfiguration}
// of each variant.
Collection<Closure> postConfigure = []
Provider<Task> dummyTask = project.tasks.register("protobufDummy")
if (isAndroid) {
project.android.sourceSets.configureEach { sourceSet ->
ProtoSourceSet protoSourceSet = protobufExtension.sourceSets.create(sourceSet.name)
addSourceSetExtension(sourceSet, protoSourceSet)
Configuration protobufConfig = createProtobufConfiguration(protoSourceSet)
setupExtractProtosTask(protoSourceSet, protobufConfig)
setupExtractProtosTask(protoSourceSet, protobufConfig, dummyTask)
}

NamedDomainObjectContainer<ProtoSourceSet> variantSourceSets =
project.objects.domainObjectContainer(ProtoSourceSet) { String name ->
new DefaultProtoSourceSet(name, project.objects)
}
ProjectExt.forEachVariant(this.project) { BaseVariant variant ->
addTasksForVariant(variant, variantSourceSets, postConfigure)
addTasksForVariant(variant, variantSourceSets, postConfigure, dummyTask)
}
} else {
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 @@ -295,7 +297,8 @@ class ProtobufPlugin implements Plugin<Project> {
private void addTasksForVariant(
Object variant,
NamedDomainObjectContainer<ProtoSourceSet> variantSourceSets,
Collection<Closure> postConfigure
Collection<Closure> postConfigure,
Provider<Task> dummyTask
) {
Boolean isTestVariant = variant instanceof TestVariant || variant instanceof UnitTestVariant
ProtoSourceSet variantSourceSet = variantSourceSets.create(variant.name)
Expand All @@ -318,7 +321,7 @@ class ProtobufPlugin implements Plugin<Project> {
}
}

setupExtractIncludeProtosTask(variantSourceSet, classPathConfig)
setupExtractIncludeProtosTask(variantSourceSet, classPathConfig, dummyTask)

// GenerateProto task, one per variant (compilation unit).
variant.sourceSets.each { SourceProvider sourceProvider ->
Expand Down Expand Up @@ -422,14 +425,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 +453,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
Loading