-
Notifications
You must be signed in to change notification settings - Fork 326
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
Introduce ENSO_LAUNCHER env var to configure behavior of buildEngineDistribution #12035
Conversation
The Engine Checks CI job uses I believe this PR is ready to be merged.
|
So that native image is actually built.
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.
- please remove CHANGELOG entry - this is not end user visible change
- I would avoid setting
ENSO_LAUNCHER
when default value is needed in the CI scripts
Co-authored-by: Jaroslav Tulach <[email protected]>
This reverts commit a1797c2.
d65aa9b
to
318c786
Compare
Seq("--no-fallback", "--no-server") ++ | ||
Seq("-march=compatibility") ++ | ||
initializeAtBuildtimeOptions ++ | ||
initializeAtRuntimeOptions ++ | ||
buildMemoryLimitOptions ++ | ||
runtimeMemoryOptions ++ | ||
additionalOptions ++ | ||
additionalOpts.value ++ |
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.
additionalOptions
.. additionalOpts
.. additionalFoo
. This is getting confusing.
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 bit. One is a parameter to the method, and the other one is a value from the task. I don't know now how to merge these two, with minimal changes.
Looks like the CI is green:
It would be better to split/restructure the CI jobs in the future, but as far as I can say, things seem to work as they used to. Good job. |
```bash | ||
$ ENSO_LAUNCHER=native sbt buildEngineDistribution | ||
``` | ||
|
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.
Also check line 283:
Espresso support works also with
[native image support](#engine-runner-configuration). Just make sure
`ENSO_JAVA=espresso` is specified when building the `runner` executable:
```bash
enso$ rm ./built-distribution/enso-engine-*/enso-*/bin/enso
enso$ ENSO_JAVA=espresso sbt --java-home /graalvm
sbt> engine-runner/buildNativeImage
Closes #12014
Pull Request Description
Introduce
ENSO_LAUNCHER
env var that configures if and how the native image ofengine-runner
is built (More specifically, howsbt buildEngineDistribution
command behaves):native
: NI is built optimized without assertions and without debugging symbolsdebugnative
: NI is built not-optimized with assertions enabled and with debugging symbols.shell
(the default value): NI is not built -buildEngineDistribution
continues to behave as on develop.Docs in native-image.md
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.