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

Module does not get bundled properly into electron app #18

Closed
MasterOdin opened this issue Sep 29, 2023 · 3 comments
Closed

Module does not get bundled properly into electron app #18

MasterOdin opened this issue Sep 29, 2023 · 3 comments

Comments

@MasterOdin
Copy link

MasterOdin commented Sep 29, 2023

While upgrading some build pipeline stuff for M1, I hit an issue where this module was causing our built app to fail Gatekeeper check due to a hanging LC_RPATH for the cpu-features/build/node_gyp_bins/python3 file. Even for x86 builds, while it doesn't have that issue with LC_RPATH, it does still potentially have paths that cause the module to not load properly ever, granted gatekeeper, nor the ssh2 module doesn't seem to care.

For reference, here is the partial outputs of running otool -l over the aforementioned file, just including the load commands that center around files/dylibs.

For x86:

Load command 13
          cmd LC_LOAD_DYLIB
      cmdsize 104
         name /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 1953.1.0
compatibility version 150.0.0
Load command 14
          cmd LC_LOAD_DYLIB
      cmdsize 72
         name /usr/local/opt/gettext/lib/libintl.8.dylib (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 12.0.0
compatibility version 12.0.0
Load command 15
          cmd LC_LOAD_DYLIB
      cmdsize 56
         name /usr/lib/libSystem.B.dylib (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 1319.0.0
compatibility version 1.0.0

For arm64:

Load command 13
          cmd LC_LOAD_DYLIB
      cmdsize 104
         name /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 1953.255.0
compatibility version 150.0.0
Load command 14
          cmd LC_LOAD_DYLIB
      cmdsize 88
         name /Users/distiller/.pyenv/versions/3.11.0/lib/libpython3.11.dylib (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 3.11.0
compatibility version 3.11.0
Load command 15
          cmd LC_LOAD_DYLIB
      cmdsize 72
         name /opt/homebrew/opt/gettext/lib/libintl.8.dylib (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 12.0.0
compatibility version 12.0.0
Load command 16
          cmd LC_LOAD_DYLIB
      cmdsize 56
         name /usr/lib/libSystem.B.dylib (offset 24)
   time stamp 2 Thu Jan  1 01:00:02 1970
      current version 1319.0.0
compatibility version 1.0.0
Load command 17
          cmd LC_RPATH
      cmdsize 56
         path /Users/distiller/.pyenv/versions/3.11.0/lib (offset 12)
Load command 18
          cmd LC_RPATH
      cmdsize 32
         path /opt/homebrew/lib (offset 12)
Load command 19
          cmd LC_RPATH
      cmdsize 56
         path /Users/distiller/.pyenv/versions/3.11.0/lib (offset 12)
Load command 20
          cmd LC_RPATH
      cmdsize 32
         path /opt/homebrew/lib (offset 12)
Load command 21
          cmd LC_RPATH
      cmdsize 32
         path /opt/homebrew/lib (offset 12)

As can see, the referenced LC_LOAD_DYLIB and LC_RPATH all seem to be build machine specific, and if the files don't exist on the host, then there's issues. Ideally, cpu-features could bundle all it needs as part of its build process, but that could also just be too much for multiple versions of python onto the host.

Currently, we are avoiding this problem by removing cpu-features in a postinstall hook given that it's optional, and just hoping that it won't provide some cipher or whatever that we might have needed in ssh2 library. I'm totally fine if this issue gets deleted and it only exists purely as documentation to someone else before they waste a day on trying to figure out why their arm64 build of an electron app is failing to run.

@mscdex
Copy link
Owner

mscdex commented Sep 29, 2023

Can you elaborate on what you think is missing as part of the build process (especially since I am not really familiar with developing on macOS and thus am not sure what you are showing me)? The source code for the dependency (Google's cpu_features library) is already bundled and is what is compiled when the addon is installed.

If you're saying a process is staying alive after the build has finished, then that seems like more of a node-gyp issue than a cpu-features issue.

@MasterOdin
Copy link
Author

The cpu-features/build/node_gyp_bins/python3 file that gets setup as part of the installation process I think is a symlink to the system python / relies on system dylibs, an this dynamic linking is causing issues when the module gets bundled as we'll have the symlink, but not the stuff it dynamically links to.

I'm not well-versed enough on this library, node-gyp, and electron-builder to know where the best place to file that ticket. I'm really not sure why node-gyp is even creating that python3 file as part of the build process tbh as I'm not sure any part of this library directly relies on it?

@MasterOdin
Copy link
Author

Hah, having typed up the above reply, got inspired on some google search terms, and turns out it is a node-gyp bug (nodejs/node-gyp#2713) and has a PR already to fix it (nodejs/node-gyp#2721), and now just awaiting a new release to hopefully fix build tooling. In the meantime, instead of deleting cpu-features altogether, can instead just delete these node_gyp_bin files during the package process.

Going to close this as yeah, don't think there's anything that needs to change here.

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

2 participants