Error with webpack plugin when cross compiling - rebuild uses the wrong architecture

seanmacisaac commented 1 year ago

Pre-flight checklist

[X] I have read the contribution documentation for this project.
[X] I agree to follow the code of conduct that this project uses.
[X] I have searched the issue tracker for a bug that matches the one I want to file, without success.

Electron Forge version

6.0.4

Electron version

v22.0.0

Operating system

macOS 12.6

Last known working Electron Forge version

6.0.0-beta.66

Expected behavior

By passing in the appropriate arguments we can build an arm64 MacOS electron client using webpack-plugin.

In moving from 6.0.0-beta.66 to 6.0.4 the only thing we changed about our config was the plugins section using the {name, config} object instead of array in specifying the plugin.

Actual behavior

The correct electron binary is used, but the native modules are x64 so we're getting mach-o file, but is an incompatible architecture (have (x86_64), need (arm64e)) when it is trying to load the modules.

Steps to reproduce

I haven't made a minimal full reproduction, but I can try if you'd like. I did however read a bunch of code to make sure it wasn't just some config mistake on my part and did some local debugging and know what is going on.

I added some logging of the parameters at https://github.com/electron/forge/blob/main/packages/plugin/webpack/src/WebpackPlugin.ts#L155 and it looks like no config is being passed in. I get a task, then I get this config: 'darwin', platform: 'arm64', arch: undefined

This makes sense because in https://github.com/electron/forge/blob/main/packages/api/core/src/util/plugin-interface.ts it looks like if __hookName is defined it passes the task, but not the config, and if not it passes the config, but not the task. That field is populated since the WebpackPlugin is using namedHookWithTaskFn which defines it. Given that listrCompatibleRebuildHook at https://github.com/electron/forge/blob/main/packages/utils/core-utils/src/rebuild.ts#L7 requires both I'm not sure what the right fix here would be.

Additional information

No response

MarshallOfSound commented 1 year ago

Please ensure you are using the same version of all forge dependencies. i.e. everything in your package.json should be pinned to exactly 6.0.4, not a range

seanmacisaac commented 1 year ago

I can confirm that all of my packages were on exactly 6.0.4 when I had the issue, and rolling them all back to 6.0.0-beta.66 resolved it.

seanmacisaac commented 1 year ago

I've made a minimal reproduction of the issue at https://github.com/WonderInventions/forge_cross_arch_rebuild_error

The base project was created by running npm init electron-app@latest my-app -- --template=webpack-typescript

I then edited the package.json to force version 6.0.4 as they were ^6.0.4. I ran npm install keytar to get a native module in the project, and added these three lines of code just before the create window:

findPassword('forge_cross_arch_rebuild_error').then((pass) => {
  console.log(`Got pass from keytar: ${pass}`);
});

On my M1 I then built for both arm64 and x64. The arm64 version runs just fine, the x64 version produces the error described.

You can also see the included keytar.node is of the wrong architecture

➜  my-app git:(main) file ./out/my-app-darwin-x64/my-app.app/Contents/MacOS/my-app
./out/my-app-darwin-x64/my-app.app/Contents/MacOS/my-app: Mach-O 64-bit executable x86_64
➜  my-app git:(main) file ./out/my-app-darwin-x64/my-app.app/Contents/Resources/app/.webpack/main/native_modules/build/Release/keytar.node
./out/my-app-darwin-x64/my-app.app/Contents/Resources/app/.webpack/main/native_modules/build/Release/keytar.node: Mach-O 64-bit bundle arm64

seanmacisaac commented 1 year ago

This morning I added a branch to my minimal reproduction repository that uses forge 6.0.0 and the cross compile does work there.

https://github.com/WonderInventions/forge_cross_arch_rebuild_error/tree/works_on_forge_6_0_0

nathanlesage commented 1 year ago

I have the same issue and found where it emanates:

In the file core/src/api/start.ts, which defines the task "Prepare Native dependencies", electron-rebuild is called and – as the documentation states – the architecture used cannot be overridden by the forge config. So far so good.

But look how the architecture is being defined on line 37:

const arch = process.env.npm_config_arch || process.arch;

In other words: electron-rebuild currently uses either the environment variable npm_config_arch OR the process arch, but it never uses the architecture as defined in your package.json (e.g., via a CLI flag, --arch=x64). In other words, unless one defines npm_config_arch manually, electron-rebuild will always fall back to the platform's architecture, making cross-compilation, e.g., on an Intel platform for ARM, impossible.

I have double-checked and confirmed that, by setting npm_config_arch explicitly in my build targets, it recompiles the modules for the correct architecture.

I'm on Forge v6.0.4 as well.

seanmacisaac commented 1 year ago

This was fixed with https://github.com/electron/forge/pull/3126, published in 6.0.5, closing.

electron / forge