-
Notifications
You must be signed in to change notification settings - Fork 184
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
gRPC gradle plugin rework #983
Conversation
3e72295
to
0cb7da2
Compare
File scriptExecutableFile | ||
try { | ||
if (org.gradle.internal.os.OperatingSystem.current().isWindows()) { | ||
scriptExecutableFile = File.createTempFile(scriptNamePrefix, ".bat") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generation of temp files is not ideal ... but a reliable way to make sure we use the jar file that is actually resolved for each build. I'm open to alternatives @idelpivnitskiy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how grpc-java solves this issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they don't have to deal with this because they write their plugin in c++ and publish a single executable for each platform.
0cb7da2
to
bf58a81
Compare
...gradle-plugin/src/main/groovy/io/servicetalk/grpc/gradle/plugin/ServiceTalkGrpcPlugin.groovy
Outdated
Show resolved
Hide resolved
...gradle-plugin/src/main/groovy/io/servicetalk/grpc/gradle/plugin/ServiceTalkGrpcPlugin.groovy
Outdated
Show resolved
Hide resolved
File scriptExecutableFile | ||
try { | ||
if (org.gradle.internal.os.OperatingSystem.current().isWindows()) { | ||
scriptExecutableFile = File.createTempFile(scriptNamePrefix, ".bat") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how grpc-java solves this issue?
build failure attributed to #744 |
@servicetalk-bot test this please |
Motivation: 0ebe48e divided the servicetalk-grpc-gradle plugin into two files: 1. an executable script 2. an uber jar with the plugin logic The executable script assumed the uber jar would be co-located in the same directory as the uber jar, but that isn't the case in gradle caches. This means the plugin may fail to execute outside of the maven m2 repository structure. Modifications: - Instead of publishing a static script for each platform which assumes a co-located uber jar, dynamically generate the executable script depending upon where the uber jar is resolved from for the local build. Result: servicetalk-grpc-gradle works with gradle cache directory structure and local development.
30b4f2c
to
5cceef5
Compare
another failure attributed to #744 |
Motivation:
0ebe48e divided the
servicetalk-grpc-gradle plugin into two files:
The executable script assumed the uber jar would be co-located in the
same directory as the uber jar, but that isn't the case in gradle
caches. This means the plugin may fail to execute outside of the maven
m2 repository structure.
Modifications:
a co-located uber jar, dynamically generate the executable script
depending upon where the uber jar is resolved from for the local build.
Result:
servicetalk-grpc-gradle works with gradle cache directory structure and
local development.