-
Notifications
You must be signed in to change notification settings - Fork 336
Build electron package on Windows with Bazel #14561
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
This forces Bazel to add the cc compiler_executable to the sandbox.
… dir" This reverts commit e91757d.
# Conflicts: # project/BazelSupport.scala
farmaazon
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.
I'm curious how did we lost 3k linkes in Module.bazel.lock?
|
|
||
| // Build the installer-specific config that the Rust code expects | ||
| const installerSpecificConfig = { | ||
| publisher: 'New Byte Order sp. z o.o.', |
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 think this should be now Enso Analytics Ltd. but you may confirm with someone higher.
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.
You are right. I confirmed with James and changed it for the legacy build as well.
| //! Binary to prepare installer payload for Bazel builds. | ||
| //! | ||
| //! This binary uses `ide_ci` to create the installer payload archive (via external tar) | ||
| //! and metadata files. | ||
| //! | ||
| //! Usage: prepare_payload <unpacked_dir> <output_archive> <output_metadata> |
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'm ok with this solution, but I wonder if could we merge it with build.rs of the actual crate producing enso installer?
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 am considering this because it can potentially simplify things, but at the time, it comes at a cost:
- Providing access to the
tarexecutable for the sandboxed build script is rather awkward - Pure-Rust implementation of
compress_directory_contentsis needed to not usetarexecutable, which is not entirely awful, but I would avoid that. - build script needs to handle both codepaths, which is also undesirable
Overall, I would keep it as it is for now, because the fate of windows installer is unclear anyway.
internal/envReplacer.mjs
Outdated
| /** | ||
| * Regex pattern to match files that should have environment variable replacements applied. | ||
| * This pattern covers: | ||
| * - config.js or config-<hash>.js (GUI config files) | ||
| * - index.html (GUI entry point) | ||
| * - index.mjs or preload.mjs (Electron client entry points) | ||
| */ | ||
| const filesRegex = /config(-[0-9a-zA-Z]+)?\.js$|index\.html$|(index|preload)\.mjs$/ |
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.
Oh well, for clarity I would probably just make three strings/regex for every point, and then three ifs
to consider.
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.
Yep, it is indeed simpler. Done.
Pull Request Description
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.