google / google/protobuf-gradle-plugin

Update ExecutableLocator to be compatible with the Gradle Provider API

Open
#727 1 comment 4 reactions 0 assignees View on GitHub
Dominant language
Groovy
Stars
1.8k
Forks
288
PR merge metrics
No merged PRs in 30d

Description

I would like to use Protobuf Plugin for Gradle to generate some TypeScript.

I am using [Gradle Node Plugin](https://github.com/node-gradle/gradle-node-plugin) to install [ts-protoc-gen](https://github.com/improbable-eng/ts-protoc-gen).

I've tried to set the path of the ts-protoc-gen executable, however, I get an error

```
FAILURE: Build failed with an exception.
* What went wrong:
Could not determine the dependencies of task ':generateProto'.
> Not yet resolved
* Try:
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.
* Exception is:
org.gradle.api.internal.tasks.TaskDependencyResolveException: Could not determine the dependencies of task ':generateProto'.
at org.gradle.api.internal.tasks.CachingTaskDependencyResolveContext.getDependencies(CachingTaskDependencyResolveContext.java:68)
at org.gradle.execution.plan.TaskDependencyResolver.resolveDependenciesFor(TaskDependencyResolver.java:49)
...
Caused by: java.lang.IllegalStateException: Not yet resolved
at com.google.protobuf.gradle.Preconditions.checkState(Preconditions.java:45)
at com.google.protobuf.gradle.ExecutableLocator.getSimplifiedArtifactName(ExecutableLocator.groovy:97)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at com.google.protobuf.gradle.GenerateProtoTask$_getReleaseExecutableLocators_closure7.doCall(GenerateProtoTask.groovy:354)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at com.google.protobuf.gradle.GenerateProtoTask.getReleaseExecutableLocators(GenerateProtoTask.groovy:354)
at com.google.protobuf.gradle.GenerateProtoTask.access$0(GenerateProtoTask.groovy)
at com.google.protobuf.gradle.GenerateProtoTask$_getExecutables_closure6.doCall(GenerateProtoTask.groovy:346)
at com.google.protobuf.gradle.GenerateProtoTask$_getExecutables_closure6.call(GenerateProtoTask.groovy)
at org.gradle.api.internal.provider.DefaultProvider.calculateOwnValue(DefaultProvider.java:72)
at org.gradle.api.internal.provider.AbstractMinimalProvider.calculateOwnPresentValue(AbstractMinimalProvider.java:73)
at org.gradle.api.internal.provider.AbstractMinimalProvider.get(AbstractMinimalProvider.java:93)
at org.gradle.api.internal.provider.ProviderResolutionStrategy$2.resolve(ProviderResolutionStrategy.java:33)
at org.gradle.api.internal.file.collections.ProviderBackedFileCollection.visitDependencies(ProviderBackedFileCollection.java:57)
at org.gradle.api.internal.tasks.CachingTaskDependencyResolveContext$TaskGraphImpl.getNodeValues(CachingTaskDependencyResolveContext.java:103)
at org.gradle.internal.graph.CachingDirectedGraphWalker$GraphWithEmptyEdges.getNodeValues(CachingDirectedGraphWalker.java:213)
at org.gradle.internal.graph.CachingDirectedGraphWalker.doSearch(CachingDirectedGraphWalker.java:121)
at org.gradle.internal.graph.CachingDirectedGraphWalker.findValues(CachingDirectedGraphWalker.java:73)
at org.gradle.api.internal.tasks.CachingTaskDependencyResolveContext.getDependencies(CachingTaskDependencyResolveContext.java:66)
... 149 more
BUILD FAILED in 2s
```

I think this is caused by Protobuf Plugin for Gradle trying to evaluate the path in the [configuration phase](https://docs.gradle.org/current/userguide/build_lifecycle.html#configuration), before the Npm install task has run.

### Example

Here is a very brief example showing how I am currently registering ts-protoc-gen with Protobuf Plugin for Gradle.

```kotlin
// build.gradle.kts
import org.gradle.internal.os.OperatingSystem

plugins {
id("com.google.protobuf") version "0.9.4"
id("com.github.node-gradle.node") version "5.0.0"
}

val npmCleanInstall by tasks.registering(NpmTask::class) {
group = project.name
args.addAll("clean-install")
}

tasks.generateProto {
dependsOn(npmCleanInstall)
}

protobuf {
plugins {
register("ts") {
val protocGenTsPath = node.nodeProjectDir
.map { it.dir("node_modules/ts-protoc-gen/bin/protoc-gen-ts") }
.map { protocGenTs ->
val ext = (if (OperatingSystem.current().isWindows) ".cmd" else "")
protocGenTs.asFile.invariantSeparatorsPath + ext
}

path = protocGenTsPath.get()
}
}
}
```

`ExecutableLocator.path` only accepts a String, so unfortunately this breaks [Gradle lazy configuration](https://docs.gradle.org/current/userguide/lazy_configuration.html)

### Proposal

Change the type of `ExecutableLocator.path` from String to [RegularFileProperty](https://docs.gradle.org/current/kotlin-dsl/gradle/org.gradle.api.file/-regular-file-property/index.html?query=interface%20RegularFileProperty%20:%20FileSystemLocationProperty%3CT%3E). This would allow for the path to be set lazily.

https://github.com/google/protobuf-gradle-plugin/blob/ff4fe684478b0de763f62e6f89ea5d5bae882425/src/main/groovy/com/google/protobuf/gradle/ExecutableLocator.groovy#L48

```java
private RegularFileProperty path
```

This change would be backwards compatible - `getPath()`/`setPath()` could still exist (but should probably be `@Deprecated`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.