-
Notifications
You must be signed in to change notification settings - Fork 333
Building native engine distribution with Bazel #14351
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
base: develop
Are you sure you want to change the base?
Conversation
This forces Bazel to add the cc compiler_executable to the sandbox.
… dir" This reverts commit e91757d.
# Conflicts: # project/BazelSupport.scala
| args = [ | ||
| "buildEngineDistribution", | ||
| ], | ||
| env = NATIVE_IMAGE_ENV, |
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.
I don't think we want fast,-ls by default, do we?
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.
@Akirathan please help 🙏🏻
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.
As @JaroslavTulach mentioned in #14351 (comment), I have originally introduced fast,-ls on purpose, so that it will build faster. For release NI build, however, the ENSO_LAUNCHER env variable needs to be native. Alternatively, just unset the ENSO_LAUNCHER env var. That will ensure that the NI will be build for production.
| ), | ||
| "_linux_constraint": attr.label( | ||
| default = Label("@platforms//os:linux"), | ||
| ), |
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.
missing macos/darwin? if linux & macos are treated in the same way then maybe just rename the constraint.
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.
Linux is treated differently, because it requires zlib during native build. Mac does not need any special actions, so it is treated as “default”.
| exit 1 | ||
| fi | ||
| $PWD/$binary_path --version | ||
| $PWD/$binary_path --version | grep Substrate > /dev/null |
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.
Why not use something like file --mime-encoding rather than this weird version check?
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.
Fair, I guess I will change it.
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.
file --mime-encoding sounds easier on Linux and MacOS. But note that it is possible that the file command is not present in the sandboxed Bazel environment. Moreover, this whole _ensure_native_enso_impl was meant just as a sanity check.
BUILD.bazel
Outdated
|
|
||
| NATIVE_IMAGE_ENV = dict( | ||
| ENV, | ||
| ENSO_LAUNCHER = "native,fast,-ls", |
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.
| ENSO_LAUNCHER = "native,fast,-ls", | |
| ENSO_LAUNCHER = "native", |
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.
Production build should run just with native:
- in response to https://github.com/enso-org/enso/pull/14351/files#r2577514787
- more info at
enso/docs/infrastructure/native-image.md
Line 212 in 73e8fc2
- `native`: `buildEngineDistribution` command builds native image in _release
Alternatively one should completely remove ENSO_LAUNCHER definition.
- it is set to proper
nativevalue automatically in thesbtscript: Line 22 in 73e8fc2
"native"
Akirathan
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, what a black bazel magic. But it works! Congratulations. I can see that the latest Run bazel Engine build correctly builds native image, and executes few tests with it. I have also tried to download the uploaded artifact, and everything seems fine.
Pull Request Description
Another step towards fully-functional Electron package in Bazel.
bazel build //:sbt_build_native_engine_distributionis working on Mac and Linux and produces practically identical distribution comparing to a regular sbt run. CI action is modified to check native distribution on Linux.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.