Improve caching of protoc in GenerateProtoTask
#1
Conversation
clayburn
force-pushed
the
457-generateProtoTask-protoc-caching
branch
2 times, most recently
from
35df827 to
aa107a8
Compare
src/main/groovy/com/google/protobuf/gradle/GenerateProtoTask.groovy
Outdated
Show resolved
Hide resolved
clayburn
force-pushed
the
457-generateProtoTask-protoc-caching
branch
3 times, most recently
from
c1e0b45 to
07361fd
Compare
| } | ||
|
|
||
| outputs.upToDateWhen { | ||
| !provider.getOrElse("").endsWith("-SNAPSHOT") |
There was a problem hiding this comment.
Couldn't decide if it was worth trying to cache on snapshots. A typical plugin consumer isn't going to utilize them.
There was a problem hiding this comment.
this eagerly resolves the provider when this function is called in order to add an output upToDate condition. i don't think this is a good practice.
clayburn
force-pushed
the
457-generateProtoTask-protoc-caching
branch
from
07361fd to
4bdd52b
Compare
This change will remove the OS from the input properties of the `GenerateProtoTask`. The expectation is that same versions of `protoc` will produce same outputs regardless of the OS on which it was run. If a protoc `artifact` is defined with GAV parameters, the classifier and extension will not be considered part of the input. An exception is made for snapshots, which will always be considered out of date and be re-run. If a protoc `path` is defined, then the path will be exposed as a File Provider with absolute path normalization since there is no good way to tell the version of `protoc` without running `protoc --version`. See google#457.
clayburn
force-pushed
the
457-generateProtoTask-protoc-caching
branch
from
4bdd52b to
8a7e414
Compare
| @InputFile | ||
| @PathSensitive(PathSensitivity.ABSOLUTE) | ||
| @Optional | ||
| Provider<File> getProtocPath() { |
There was a problem hiding this comment.
what's the case where this absolute path is relevant to the output? the previous input had PathSensitivity.NONE
| } | ||
|
|
||
| outputs.upToDateWhen { | ||
| !provider.getOrElse("").endsWith("-SNAPSHOT") |
There was a problem hiding this comment.
this eagerly resolves the provider when this function is called in order to add an output upToDate condition. i don't think this is a good practice.
| @PathSensitive(PathSensitivity.NONE) | ||
| @Input | ||
| @Optional | ||
| Provider<String> getProtocArtifact() { |
There was a problem hiding this comment.
this returns a single string as an input but the previous alternativePaths property could return a collection of files. do you know what the case is with multiple alternativePaths?
This change will remove the OS from the input properties of the
GenerateProtoTask. The expectation is that same versions ofprotocwill produce same outputs regardless of the OS on which it was run.
If a protoc
artifactis defined with GAV parameters, the classifierand extension will not be considered part of the input. An exception is
made for snapshots, which will always be considered out of date and be
re-run.
If a protoc
pathis defined, then the path will be exposed as a FileProvider with absolute path normalization since there is no good way to
tell the version of
protocwithout runningprotoc --version.See google#457.