-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Build and package ARM releases #3896
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
@ab77 i had no issue building direct-io for arm64 on my m1 with node-gyp, are we trying to cross-compile here ? |
Yes, cross compiling. No native ARM hardware available in this context. |
Does this build with an DMG now? I've been able to build the code but no DMG is made. I can publish a community build if a DMG is built. |
unfortunately not, see #3896 (comment) |
5077e19
to
a8b4a8b
Compare
We had the |
a8b4a8b
to
a59c9ec
Compare
a59c9ec
to
028f679
Compare
workaround by setting
Maybe because we are trying to bundle drivelist v9.2.x, which is ~ 2-4 years old and build/re-build under node 16? |
to remove the reliance on node stuff like
then it should fail on the other two platforms too |
fe1c757
to
5de9bd6
Compare
Sure, it should, but it doesn't. Maybe Windows is special. Maybe trying to build code years out of date with modern tools is the problem. Maybe both - I have no idea. I am ignoring this issue for now and hoping by the time I cycle back to it, all the dependencies are bumped up and it just builds. |
I am afraid I don't understand what that means. One problem I found is that |
e86f12b
to
5422397
Compare
5812262
to
0b8447b
Compare
package.json
Outdated
@@ -21,17 +21,18 @@ | |||
"lint-css": "prettier --write lib/**/*.css", | |||
"lint-ts": "balena-lint --fix --typescript typings lib tests scripts/clean-shrinkwrap.ts webpack.config.ts", | |||
"lint": "npm run lint-ts && npm run lint-css", | |||
"postinstall": "electron-rebuild -t prod,dev,optional", | |||
"preinstall": "bash scripts/ci/preinstall.sh", |
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.
Add cross-env
or similar instead of bash
.
On line 17 flowzone-preinstall-linux
still has electron-builder.yml
but that file is removed.
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.
Add
cross-env
or similar instead ofbash
.
https://github.com/kentcdodds/cross-env is archived and the package is not gettign any support. Do we really need to use it? My preference would be not to include depracated packages if at all possible, give we already have a ton of those. what doescross-env
give us overbash
, if bash is standardised across platforms?
On line 17
flowzone-preinstall-linux
still haselectron-builder.yml
but that file is removed.
Cheers, fixed.
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.
That is why I wrote "cross-env or similar"
however from the author:
So I'm declaring cross-env as finished. No new features will be added (and old features will not likely be removed). I don't plan to do any more than fix security/critical bugs and keep the Node.js support up-to-date.
It still looks like a safe bet.
bash is not available out of the box on many platforms, with a package like this we don't need to write if windows then cmd if linux then bash but only if not zsh, etc and so when a new contributor tries to run npm i
it will just run even if they don't have bash.
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 100% against this mate, mostly for ideological reasons of not introducing any more deprecated packages. Surely we can have a maintainer guide that says "you must install {{shell}} to work on this project".
562d564
to
c62ffbe
Compare
Change-type: minor
Superseded by #4132 |
Dependencies:
Showstoppers
Showstoppers (windows-arm64):
https://github.com/RockLakeGrass/lzma-native-Windows-ARM64/blob/42be008b8a657eae4a9506a90b96fec3e86d5582/package.json