Repository navigation
node_gyp_bins causes build failures in downstream packages #2713
Description
Activity
I am working on an electron project and running into this issue too. Though this only occurs when I use electron-packager (via electorn-forge) with asar enabled. Any temporary workaround?
I'm also hitting this issue in an Electron project, it's breaking my build completely due to this dangling symlink.
As an alternative to moving this path, could this symlink just be cleaned up after the build is completed? I can see how this is useful during the build process, but it doesn't seem like there's any good reason for it to persist afterwards.
Another report of problems caused by this issue from stack overflow: https://stackoverflow.com/questions/73216989/electron-forge-throws-python3-8-links-out-of-the-package-error-when-makin
From 2016: electron-userland/electron-builder#675 --
file links out of the packagehttps://www.npmjs.com/package/asar-dev
Purpose of this package is to offer a solution to the issue electron-userland/electron-builder#675.
Error: /Users/user1/myapp/node_modules/myapp-lib: file links out of the packageCan we please see the
command typedand thefull error log?In my case this I am building an Electron app and using Electron Builder, but the failure doesn't come from there at all, it comes from
codesign- Apple's code signing tool from XCode - so although that bug above is a similar issue I don't think the cause is directly related @cclauss. It's just another example of symlink + bundling issues.In my case, the specific command that finally fails for me is:
codesign --verify --deep --strict --verbose=2 \ /Users/runner/work/httptoolkit-desktop/httptoolkit-desktop/dist/mac/HTTP Toolkit.appYou can see the full error log in my CI build here: https://github.com/httptoolkit/httptoolkit-desktop/runs/7883734525?check_suite_focus=true.
In this case, my
.appfolder includes an separate subcomponent, whose build includes annpm installthat builds native modules, and whose built output (e.g. the tarballs here) is just directly copied into my codebase.The
node_modulesfolder for this subcomponent ends up including a[...]/build/node_gyp_bins/python3link to/usr/bin/python3, which (AFAICT) is created by node-gyp during thenpm installstep.It's impossible to codesign any app for Mac that contains symlinks outside the codebase like this, so these links completely break my build. I think Apple is acting reasonably in rejecting this, I agree that a distributable app generally shouldn't contain absolute links to external locations on the filesystem. Even if they're weren't correct though, Apple is not likely to change this behaviour any time soon.
This will affect any
node_modulefolder that eventually gets code signed like this, and it'll also affect many other tools that attempt to bundle a directory including node_modules, because they will all (sensibly) refuse to bundle/usr/bin/pythoninto the app itself.For now I've fixed this by manually scanning for all
node_gyp_binsdirectories and deleting them during my install process. That solves the issue, but it would be much better if node-gyp cleaned up after itself instead.I would recommend reading thru https://github.com/txoof/codesign (which is written in Python) and then opening an issue on that repo if you still do not have a resolution.
Reacted by Aleksei KuzminI would recommend reading thru https://github.com/txoof/codesign (which is written in Python) and then opening an issue on that repo if you still do not have a resolution.
@cclauss I'm not using that package, I'm using Apple's own codesign binary. There's more info here: https://developer.apple.com/library/archive/documentation/Security/Conceptual/CodeSigningGuide/Procedures/Procedures.html#//apple_ref/doc/uid/TP40005929-CH4-SW3.
To be clear: Codesign is working correctly in this case. It is rejecting bad input from node-gyp. I want this error - I do not want to codesign my /usr/bin/python binary. Similarly, people who are bundling node_modules usually do want that to fail if they accidentally include symlinks to other parts of the system outside their codebase.
This is not a problem with codesign or anything else. Node-gyp is now leaving behind absolute symlinks in node_modules. This will cause problems with many tools, but particularly for people building any kind of distributable applications. Node-gyp should not leave absolute symlinks like this in node_modules after the build is completed.
Is there a specific reason that leaving these links after the build completes is useful? Would there be any downside to removing them? I'm happy to open a PR to do so and fix this issue, if that would be accepted.
Well explained. I now understand. I think your pull request would be the right path forward. Let's try to remove the absolute symlinks in node_modules and see where that takes us.
Ok, great, I'll do that now
Reacted by Christian ClaussCreate a PR to fix this here: #2721
Not quite as simple as described above, but the same end result. Since the symlinks were created in
configure, deleting them at the end of builds would've meant that repeated builds after a singleconfigurecould have different behaviour (or might fail entirely) which seemed bad.Instead, once we calculate the link path in
configurewe now just store it in the config file, and then the symlink exists only during the build - created at the start using the config value, and deleted straight after.so add a whitelist to ignore links out of the package ??
1 remaining item
Would love to see a fix for this.
Hello. Thanks for the good work on node-gyp. I wanted the check in on this.
I was finding that the rush package manager's
rush deploycommand (reasonably) does not like the symlink out of node_modules because it is non-hermetic (pointing out of node-modules). Similar in spirit to the codesign rule discussed above. Would love to see a fix (like #2721) to avoid leaving symlinks pointing out of node_modules.When you want a pull request to be merged, please give it a positive review as @DeeDeeG has done at the top right of this page. Every checkmark ✔️ that project maintainers see there gives them confidence that the proposed changes have been looked at and have been deemed both useful and safe to merge into the codebase. Lots of "what is the ETA?" comments are easier for maintainers to ignore than ✔️✔️✔️✔️✔️ from several different reviewers.
Anyone can review a pull request on GitHub. To do so here:
- Scroll to the top of the pull request page.
- Click the
Files changedtab. - Click the
Review changesbutton. - Click
Approve(or one of the other options) and make comments only if they have not already been stated in the PR. - Click
Submit reviewso that your ✔️ can be added to the list.
Reacted by Nipunn KoorapatiWhen you want a pull request to be merged, please give it a positive review as @DeeDeeG has done at the top right of this page. Every checkmark ✔️ that project maintainers see there gives them confidence that the proposed changes have been looked at and have been deemed both useful and safe to merge into the codebase. Lots of "what is the ETA?" comments are easier for maintainers to ignore than ✔️✔️✔️✔️✔️ from several different reviewers.
Sounds good! Happy to help! I know not all maintainers like this, but good to know here.
I know not all maintainers like this
I have never seen that. If they want to turn off the code review capability for non-maintainers then they are free to do so.
Reacted by Nipunn Koorapati- added 4 commits that reference this issue
on Dec 18, 2023
Hey folks! I'm a maintainer of
electron-packagerandelectron-rebuild. We're seeing an issue with rebuilding native modules using node-gyp, that we think we introduced by this change: b9ddcd5 .When trying to rebuild a native module using Electron and node-gyp 9.0.0+, rebuilding will fail for older versions of python or folks using a python version manager with this error:
The link to the specific issue, with more details, is here: electron/rebuild#1024
We can handle this error inside electron-packager by ignoring the directory, but would it be possible to move this symlink directory outside of
node_modulesand into something like a tmp dir?I wasn't sure if the directory being in node_modules may cause other issues with other downstream users/packages, and thought we'd err on the side of reporting it 🙂 If there's a technical reason why it needs to be included in node_modules, please feel free to close this.
Thanks for your time!