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

GenerateProtoTask is not cache relocateable between OS because of alternativePaths #457

Closed
runningcode opened this issue Jan 6, 2021 · 6 comments

Comments

@runningcode
Copy link
Contributor

The GenerateProtoTask adds an OS specific binary to the alternativePaths input. This path is OS specific meaning that the cache cannot be re-used between linux, macOS and windows.
GenerateProtoTask

The tasks output should be the same no matter what kind of protoc dependency it is using so one solution would be to remove this as an input to the task.

@ejona86
Copy link
Collaborator

ejona86 commented Jan 6, 2021

The output would be different with different versions of protoc, so I don't think simply removing the input is appropriate.

@runningcode
Copy link
Contributor Author

By different versions do you mean that the output is different depending on the OS? Or just the version number of protoc? If it is just the version number of protoc perhaps we can just make the version number of protoc an input?

@ejona86
Copy link
Collaborator

ejona86 commented Jan 6, 2021

Well, it would seem the group ID and artifact ID are also appropriate. So "exclude the classifier" would seem the more conservative approach.

Note that the change you're suggesting seems to mean alternative paths could no longer be file-based, but we'll still need a file-based input if the user specifies a path directly.

How does the dependency cache normally handle things like different javac versions? It just assumes that all "Java 8" versions behave the same? And I guess for things like Groovy/Kotlin you pin to a specific version?

@ejona86
Copy link
Collaborator

ejona86 commented Jan 6, 2021

Also, if we stop using file-based input -SNAPSHOT versions may stop working correctly.

@mvitaly
Copy link

mvitaly commented Mar 12, 2022

I'm hitting this issue too.

How about adding a new property to the task that will be used for caching (will not have os classifier or os extension) and ignore the alternativePaths from cache key?

clayburn added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 uncacheable 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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 27, 2022
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 added a commit to clayburn/protobuf-gradle-plugin that referenced this issue Apr 28, 2022
This change will remove the OS from the input properties of the
GenerateProtoTask when `protoc` is defined as a resolvable artifact. The
expectation is that same versions of protoc will produce same outputs
regardless of the OS on which it was run.

This is done by removing the "protoc" key from the `alternativePaths`
property when present and not a SNAPSHOT and adding a protocArtifactGav
property as an input that only considers the group, artifact ID, and
version of the resolvable `protoc`.

See google#457.
clayburn added a commit to clayburn/protobuf-gradle-plugin that referenced this issue May 3, 2022
This change will remove the OS from the input properties of the
GenerateProtoTask when `protoc` or a plugin is defined as a resolvable
artifact. The expectation is that same versions of `protoc` or plugins
will produce same outputs regardless of the OS on which they are run.

This is done by creating marking `alternativePaths` as Internal for
backwards compatibility and creating two new properties:
`releaseArtifacts` and `snapshotArtifacts`. `releaseArtifacts` will
contain "$groupId:$artifact:$version" for each non-snapshot dependency,
effectively ignoring OS for these dependencies. `snapshotArtifacts` will
contain each snapshot artifact in a `FileCollection` since the
snapshots cannot be matched solely by their maven coordinates.

See google#457.
clayburn added a commit to clayburn/protobuf-gradle-plugin that referenced this issue May 3, 2022
This change will remove the OS from the input properties of the
GenerateProtoTask when `protoc` or a plugin is defined as a resolvable
artifact. The expectation is that same versions of `protoc` or plugins
will produce same outputs regardless of the OS on which they are run.

This is done by creating marking `alternativePaths` as Internal for
backwards compatibility and creating two new properties:
`releaseArtifacts` and `snapshotArtifacts`. `releaseArtifacts` will
contain "$groupId:$artifact:$version" for each non-snapshot dependency,
effectively ignoring OS for these dependencies. `snapshotArtifacts` will
contain each snapshot artifact in a `FileCollection` since the
snapshots cannot be matched solely by their maven coordinates.

See google#457.
@ejona86
Copy link
Collaborator

ejona86 commented Jun 6, 2022

Fixed by #560

@ejona86 ejona86 closed this as completed Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants