-
Notifications
You must be signed in to change notification settings - Fork 207
refactor(ci): update graal release infrastructure #1966
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
Conversation
7ab27d5 to
cced4a7
Compare
This was a bit of a rabbit hole, but I just went with it. With the changes to sbt-ci-release, _somehow_ the graal native image generation was failing on windows. In order to combat that this pr makes some changes to CI and to the args we're using with Graal. I'll outline the changes below. - Migrate to `graalvm/setup-graalvm` for the graal jobs. This is necessary if we want to use the newer graal versions. When I tried with `setup-scala` I got jabba erros that what I was looking for doesn't exist. Plus `setup-scala` isn't _really_ worked on anymore, so migrating away is preferable. Plus, when using `setup-graalvm` it's easy to get things like the `native-image` command on all platforms. - Bump to 22.3.0. I tried to just bump slightly, but there is a bug in 22.1.0 on windows that I hit on oracle/graal#4502. It's fixed in the newer ones, so I just bumped up. - Use `actions/setup-java` instead of `setup-scala` for jvm tests. Same reason as above with maintenance, but also built-in sbt cache. For now we're still doing a lot on 11 (where the graal stuff is on 17) but I'm trying to not change this too much for now until I get everything green and releasing. Then I'll address that. - Move `publish-binaries-windows` into `publish-binaries`. Now that we are using `setup-graalvm` it's trivial to just keep these together, using `sbt` and just call it a day. No need for two separate ones. - Remove the windows-specific `graalVMNativeImageCommand`. Again, now that we're using `setup-graalvm` we easily have the `native-image` command on the PATH so we don't need to worry about all this extra stuff. - Change a few flags: - `--no-server` wasn't valid it seemed as we were getting a message on every run that it wasn't recognized and ignored. - `-H:EnableUrlProtocols` no just uses `--enable-url-protocols`. Apparently it's frowned upon to use the `-H` stuff since it's internal, so this was just a bit of cleaning up. We do have a couple others, but there doesn't seem to be alternatives so I left them as is. - `--allow-incomlete-classpath` This is the default now, so no need to have it. You can see a green build for all 3 os's [here in my fork](https://github.com/ckipp01/bloop/actions/runs/3787821939/jobs/6439973910). But let's see how all the other jobs go.
|
Also, i just ripped out the |
tgodzik
left a comment
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.
Wow! This great! Thanks for dealing with that. I didn't even know there was a separate grall action.
| curl -Lo coursier https://git.io/coursier-cli && chmod +x coursier && ./coursier --help | ||
| yarn --help | ||
| java -version | ||
| [[ $HYDRA_LICENSE == floating-key=* ]] && mkdir -p $HOME/.triplequote && echo "$HYDRA_LICENSE" > "$HOME/.triplequote/hydra.license" || echo "Hydra license file was not created" |
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.
A random comment, I think we are no longer running any hydra tests and potentially we could remove it. We should probably talk with the triple quote people.
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.
Huh, yea I just looked. Honestly I don't know hardly anything about Hydra. However seeing that it's a paid project I have no issues ripping all the hydra stuff out. Is it something that is needed? Something that people have requested? Who uses hydra? I'll create a follow-up issue on this.
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.
Honestly I have no idea if there are people that both use hydra and Bloop or how much it was requested 🤔
This was a bit of a rabbit hole, but I just went with it. With the changes to sbt-ci-release, somehow the graal native image generation was failing on windows. In order to combat that this pr makes some changes to CI and to the args we're using with Graal. I'll outline the changes below.
graalvm/setup-graalvmfor the graal jobs. This is necessary if we want to use the newer graal versions. When I tried withsetup-scalaI got jabba erros that what I was looking for doesn't exist. Plussetup-scalaisn't really worked on anymore, so migrating away is preferable. Plus, when usingsetup-graalvmit's easy to get things like thenative-imagecommand on all platforms.@argfileparse failure on Windows with the errorFatal error: Unsupported OptionOriginoracle/graal#4502. It's fixed in the newer ones, so I just bumped up.actions/setup-javainstead ofsetup-scalafor jvm tests. Same reason as above with maintenance, but also built-in sbt cache. For now we're still doing a lot on 11 (where the graal stuff is on 17) but I'm trying to not change this too much for now until I get everything green and releasing. Then I'll address that.publish-binaries-windowsintopublish-binaries. Now that we are usingsetup-graalvmit's trivial to just keep these together, usingsbtand just call it a day. No need for two separate ones.graalVMNativeImageCommand. Again, now that we're usingsetup-graalvmwe easily have thenative-imagecommand on the PATH so we don't need to worry about all this extra stuff.--no-serverwasn't valid it seemed as we were getting a message on every run that it wasn't recognized and ignored.-H:EnableUrlProtocolsno just uses--enable-url-protocols. Apparently it's frowned upon to use the-Hstuff since it's internal, so this was just a bit of cleaning up. We do have a couple others, but there doesn't seem to be alternatives so I left them as is.--allow-incomlete-classpathThis is the default now, so no need to have it.You can see a green build for all 3 os's here in my fork. But let's see how all the other jobs go.