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

Can't run bundles relying on Argon2 with Electron Builder 24 and up on x64 macOS #374

Closed
3 tasks done
Nantris opened this issue Apr 6, 2023 · 12 comments
Closed
3 tasks done

Comments

@Nantris
Copy link

Nantris commented Apr 6, 2023

Important: I'm not sure this is an Argon2 issue, it may be an Electron-Builder issue

Before creating an issue, please be sure to:

  • Checkout to the latest version, including submodules
  • Try to find an isolated way to reproduce the behavior
  • Fill in all the blanks in the most specific way you can

Steps to reproduce

  1. Bundle argon2 with the latest Electron Builder
  2. The resulting executable runs on macOS ARM64, but not x64

Expected behaviour

"Just works"

Actual behaviour

[2023-04-05 20:03:28.432] [error] Error: dlopen(/var/folders/y5/pp8xq1qd1f78jth_11nsxndh0000gn/T/.com.electron.ourApp.NKD44Y, 0x0001): tried: '/var/folders/y5/pp8xq1qd1f78jth_11nsxndh0000gn/T/.com.electron.ourApp.NKD44Y' (mach-o file, but is an incompatible architecture (have (arm64), need (x86_64))), '/private/var/folders/y5/pp8xq1qd1f78jth_11nsxndh0000gn/T/.com.electron.ourApp.NKD44Y' (mach-o file, but is an incompatible architecture (have (arm64), need (x86_64)))
    at process.func [as dlopen] (node:electron/js2c/asar_bundle:2:1822)
    at Module._extensions..node (node:internal/modules/cjs/loader:1259:18)
    at Object.func [as .node] (node:electron/js2c/asar_bundle:2:2049)
    at Module.load (node:internal/modules/cjs/loader:1044:32)
    at Module._load (node:internal/modules/cjs/loader:885:12)
    at f._load (node:electron/js2c/asar_bundle:2:13330)
    at Module.require (node:internal/modules/cjs/loader:1068:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/Volumes/ourApp 3.0.0-universal 1/ourApp.app/Contents/Resources/app.asar/node_modules/argon2/argon2.js:6:25)
    at Module._compile (node:internal/modules/cjs/loader:1174:14)

Environment

Operating system: macOS 12.6.4

Node version: 16.18.0

Compiler version: ???

Additional information:

I decided to create this issue report after proving that other native dependencies (like Sharp) do not suffer this issue with Electron Builder 24. I'm not sure where exactly the issue lies. I suspect with Electron-Builder, but we're sort of at an impasse there. Do you have any ideas what the cause could be @ranisalt? Relevant issue: electron-userland/electron-builder#7512

electron-builder@24.x implements @electron/rebuild which didn't used to be used in the build process - I wonder if this issue is relevant? electron/rebuild#1055

@ranisalt
Copy link
Owner

ranisalt commented Apr 6, 2023

You are building it on ARM64 and trying to run on x86_64. This will not work, currently it only installs the version for the current architecture when you add argon2, so you can't directly bundle it.

@Nantris
Copy link
Author

Nantris commented Apr 6, 2023

Thanks for your reply @ranisalt!

Actually it's the reverse - I'm on x64 trying to bundle for both x64 and ARM64 - but aren't there prebuilt binaries for this purpose? I guess you're saying only one of the prebuilts will be installed?

Is there any way around this so we can provide users with an universal Electron installer that covers both x64 and ARM64? Or otherwise, if we offer separate downloads, is there a way to create an ARM64 bundle on an x64 machine? (Actually that seems to be what we're already (accidentally) doing.

Or is there some way this could be accommodated in the future? This issue doesn't affect other native dependencies we use, (eg. Sharp.)

@Nantris
Copy link
Author

Nantris commented Apr 6, 2023

I guess the issue is that argon2 doesn't use prebuild and prebuild-install so then @electron/rebuild tries to build it from source?

How can I integrate this with prebuild?

If your module uses prebuild for creating prebuilt binaries, it also uses prebuild-install to download them. If this is the case, then electron-rebuild will run prebuild-install to download the correct binaries from the project's GitHub Releases instead of rebuilding them. -- https://github.com/electron/rebuild#how-can-i-integrate-this-with-prebuild

I'm guessing I'm misunderstanding something because it runs fine in development, but it seems to have the wrong version in the bundled app. Or maybe electron-builder (or one of its dependencies) is doing something wrong to get the ARM64 binary mixed up with the x64?

@ranisalt
Copy link
Owner

ranisalt commented Apr 6, 2023

I don't know how electron-builder works, but it needs to download argon2 during runtime, not during build-time. So, if you are building and packaging your app, and that gets sent to another system, it will not run.

I guess you're saying only one of the prebuilts will be installed?

That is the case. Instead, I actually want to use prebuildify to ship all binaries in the package, skipping the download step. It means some ~150 extra KBs on the package, but that's less than the dependencies needed to figure out what file to download anyway.

From your error log, though:

have (arm64), need (x86_64))

You are packaging the arm64 binary, but running the resulting executable on x86_64?

@Nantris
Copy link
Author

Nantris commented Apr 6, 2023

Hey @ranisalt, I just a found a few minutes ago that the issue is actually in @electron/rebuild and submitted a PR - I apologize for the wild goose chase.

Relevant PR: electron/rebuild#1076.

I think prebuildify sounds like a good solution though - I think you also mentioned it in #364, although not by name and it sounds like it may reduce the shipped size, although I suspect the binaries are not very compressible so maybe it won't. In any event, developer time is worth more than a few KBs, right? I support the prebuildify idea!

Thanks again for all your great work.

@Nantris Nantris closed this as completed Apr 6, 2023
@Nantris Nantris reopened this Apr 7, 2023
@Nantris
Copy link
Author

Nantris commented Apr 7, 2023

I think the @electron/rebuild issue is only one of the issues here.

You are packaging the arm64 binary, but running the resulting executable on x86_64?

That appears to be the case. Definitely running on x86_64 and getting the error about the binary being for arm64 - so I guess we need to get @electron/rebuild fixed, but also probably need the hypothetical prebuildify update you mentioned. Any idea about a timeline?

For now we're going back to just packaging x86_64 for all users. I swear 20% of all development time is just jumping through hoops for Apple.

@Nantris
Copy link
Author

Nantris commented Aug 2, 2023

@ranisalt nice work with the new release!

I wonder if you have any updates/thoughts about supporting Electron universal builds?

I'm working on confirming a potential solution to this issue for the sharp package, but I don't think the same approach will work with argon2 because (if for no other reason) it seems that the required files occupy the same paths for x64 vs arm64, whereas with sharp they are relegated to their own directories.

Thank you again for continuing to build and support this great project!

@ranisalt
Copy link
Owner

ranisalt commented Aug 3, 2023

@slapbox hey, I'm still looking to just bundle all architectures in the package as we've talked about before, but it's not as trivial as I expected. I was working on the node-gyp-build branch, now it's slightly out of sync with master, so I'll try to rebase that, but it's promising.

@Nantris
Copy link
Author

Nantris commented Aug 3, 2023

Thank you again so much for everything you do @ranisalt!

@ranisalt
Copy link
Owner

@slapbox you can experiment using the alpha version by installing argon2@next which contains prebuilt binaries included in the package, no extra steps needed.

Note that it is also ESM-only but it shouldn't be a problem

@mloiseleur
Copy link

I experimented argon2@next for an arm64 build on bitwarden cli.
It did not work because this project uses pkg. There are has been a lot of discussion in this issue on pkg repo : it does not support ESM.
FTM, I found a workaround by overwriting local amd64 ELF file with one from arm64 archive in github release.
I have no clue if getting back to commonjs would fix this issue for this use case.

@ranisalt
Copy link
Owner

Should be fixed with v0.40.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants