Generate native-image binaries for google-java-format #1057
Conversation
|
@vorburger can I get your review of this? |
And include them in release artifacts. I tested this on my fork of the repo, and have a demo here: https://github.com/cushon/google-java-format/releases When downloading the artifacts from the releases page they don't have the executable bit set, and my Mac blocks them because they aren't signed. That can be worked around with the following, but isn't ideal: ``` chmod a+rx google-java-format-darwin sudo xattr -r -d com.apple.quarantine google-java-format-darwin ``` Progress towards google#868
.github/workflows/release.yml
Outdated
| - name: "Native" | ||
| run: mvn -Pnative -DskipTests package -pl core -am | ||
| - name: "Move outputs" | ||
| run: cp core/target/google-java-format google-java-format-${{ matrix.os == 'ubuntu-latest' && 'linux' || 'darwin' }} |
There was a problem hiding this comment.
@cushon I don't understand this expression... and it (now) won't work for Windows?
There was a problem hiding this comment.
It returns the string 'linux' or 'darwin' depending on the current OS, so the platform-specific outputs get uploaded with unique names. The syntax is documented here: https://docs.github.com/en/actions/learn-github-actions/expressions
I agree we'll want to update this for windows, I'm not sure what the cleanest way to do that is yet. I looked around at a few different ways to upload these artifacts to the release and didn't find anything very appealing.
There was a problem hiding this comment.
Oh, I think I just got it - the idea is that this will evaluate to the following, right?
google-java-format-linuxgoogle-java-format-darwin
How about we use slightly different suffixes for the released native binaries? Inspired by e.g. uname -a output, I would suggest:
google-java-format_linux-x86_64google-java-format_darwin-arm64(or something like that - what exactly doesuname -aoutput on your Mac?)google-java-format_win-x64(I think that's what that's commonly referred to - but I'm not 100% sure; but I would NOT use "x86_64" à la Linux for this binary, because MS seems to use x86=32bit VS x64=64bit - apparently.)
There was a problem hiding this comment.
Those suffixes LGTM, thanks. (My uname output includes Darwin and arm64.)
There was a problem hiding this comment.
I agree we'll want to update this for windows, I'm not sure what the cleanest way to do that is yet. I looked around at a few different ways to upload these artifacts to the release and didn't find anything very appealing.
Simplest could be to duplicate this (and the "gh release upload") lines x3, with an "if" for penguins, apples, and wins?
There was a problem hiding this comment.
google-java-format_win-x64(I think that's what that's commonly referred to - but I'm not 100% sure; but I would NOT use "x86_64" à la Linux for this binary, because MS seems to use x86=32bit VS x64=64bit - apparently.)
Just noticed on https://github.com/bazelbuild/bazel/releases that they use "*windows_x86-64" so perhaps we can go for that here as well after all.
There was a problem hiding this comment.
Thanks! I found a different way to express that mapping, and updated to use the suggested names.
Co-authored-by: Michael Vorburger <[email protected]>
.github/workflows/release.yml
Outdated
| @@ -87,6 +87,8 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest, windows-latest] | |||
| env: | |||
| SUFFIX: ${{fromJson('{"ubuntu-latest":"linux-x86_64", "macos-latest":"darwin-arm64", "windows-latest":"windows_x86-64"}')[matrix.os]}} | |||
There was a problem hiding this comment.
Fancy!! That's a really neat trick - TIL; Thank You!
Co-authored-by: Michael Vorburger <[email protected]>
And include them in release artifacts.
I tested this on my fork of the repo, and have a demo here: https://github.com/cushon/google-java-format/releases
When downloading the artifacts from the releases page they don't have the executable bit set, and my Mac blocks them because they aren't signed. That can be worked around with the following, but isn't ideal:
Progress towards #868, builds on @vorburger's work in #1048.