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

Fix GraalVM distribution download on MacOS #7364

Merged
merged 6 commits into from
Jul 23, 2023

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Jul 21, 2023

Pull Request Description

Follow-up of recent GraalVM update #7176 that fixes downloading of GraalVM for Mac - instead of "darwin", the releases are now named "macos"

Important Notes

Also re-enables the JDK/GraalVM version check as onLoad hook to the sbt process. We used to have that check a long time ago. Provides errors like this one if the sbt is run with a different JVM version:

[error] GraalVM version mismatch - you are running Oracle GraalVM 20.0.1+9.1 but GraalVM 17.0.7 is expected.
[error] GraalVM version check failed.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@Akirathan Akirathan self-assigned this Jul 21, 2023
@Akirathan Akirathan added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 21, 2023
Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised we haven't seen some failures before this fix.

@Akirathan
Copy link
Member Author

I am surprised we haven't seen some failures before this fix.

I should have asked for QA on MacOS. I have only asked for QA on Windows. That is why I have not spotted this error. Until this is solved, I believe that also nightly builds for MacOS will be failing.

Copy link
Contributor

@mwu-tow mwu-tow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks build in another place, see the run results:

/Users/runner/work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-macos-amd64/enso-2023.2.1-dev/bin/enso: No such file or directory

@Akirathan
Copy link
Member Author

This breaks build in another place, see the run results:

/Users/runner/work/enso/enso/built-distribution/enso-engine-2023.2.1-dev-macos-amd64/enso-2023.2.1-dev/bin/enso: No such file or directory

I think I have just discovered the problems with the IDE building. It seems that I have accidentally broken ./run backend build and ./run ide build, both for Linux and Mac. How is it possible that it passed all the tests? Do we not do ./run ide build on the CI tests?

@Akirathan Akirathan force-pushed the wip/akirathan/fix-macos-graalvm-download branch from e980f62 to dea650e Compare July 21, 2023 16:26
@Akirathan Akirathan added the p-highest Should be completed ASAP label Jul 21, 2023
@Akirathan Akirathan added the CI: Ready to merge This PR is eligible for automatic merge label Jul 21, 2023
@mergify mergify bot merged commit efb39ad into develop Jul 23, 2023
@mergify mergify bot deleted the wip/akirathan/fix-macos-graalvm-download branch July 23, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge p-highest Should be completed ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants