fix(install): use node-gyp from homebrew npm#4994
Merged
arcanis merged 2 commits intoyarnpkg:masterfrom Jan 15, 2018
Merged
Conversation
chrmoritz
force-pushed
the
homebrew-node-gyp
branch
from
95e8976 to
7e4dd2f
Compare
chrmoritz
force-pushed
the
homebrew-node-gyp
branch
from
7e4dd2f to
79b41b0
Compare
**Summary** With this yarn will be able to discover and use the node-gyp from the homebrew installed npm on macOS instead of falling back to globally installing node-gyp every time a native addon needs to be compiled from source. Homebrew installs a clean copy of npm inside a libexec folder together with node. Previously yarn didn't look there when trying to locate node-gyp and the globally install node-gyp fallback would be used every time when building native addons with a yarn version from homebrew. This PR adds the libexec path of node-gyp from homebrew to the node-gyp search paths of yarn, making it possible to compile native addons using the homebrew npm provided node-gyp with yarn without relying on this fallback. **Test plan** This can't be tested outside a homebrew environment. A way to manually test this on macOS, by installing a test build of this PR with homebrew and trying to install a native addon with it, is provided in the PR description.
chrmoritz
force-pushed
the
homebrew-node-gyp
branch
from
c8bfb68 to
9f80e0d
Compare
Contributor
Author
|
Could someone please review this PR? (friendly ping @BYK) What else do I need to get this PR merged? |
arcanis
approved these changes
Jan 15, 2018
Member
|
It makes sense and I guess the line you removed actually test that the feature works, so I'm gonna go ahead and merge it. Thanks for your contribution! |
agoldis
added a commit
to agoldis/yarn
that referenced
this pull request
Feb 2, 2018
…readdir_files * upstream/master: (34 commits) feat(upgrade, add): Separately log added/upgraded dependencies (yarnpkg#5227) feat(publish): Publish command uses publishConfig.access in package.json (yarnpkg#5290) fix(CLI): Use process exit instead of exitCode for node < 4 (yarnpkg#5291) feat(cli): error on missing workspace directory (yarnpkg#5206) (yarnpkg#5222) feat: better error when package is not found (yarnpkg#5213) Allow scoped package as alias source (yarnpkg#5229) fix(cli): Use correct directory for upgrade-interactive (yarnpkg#5272) nohoist baseline implementation (yarnpkg#4979) 1.4.1 1.4.0 Show current version, when new version is not supplied on "yarn publish" (yarnpkg#4947) fix(install): use node-gyp from homebrew npm (yarnpkg#4994) Fix transient symlinks overriding direct ones v2 (yarnpkg#5016) fix(auth): Fixes authentication conditions and logic with registries (yarnpkg#5216) chore(package): move devDeps to appropriate place (yarnpkg#5166) fix(resolution) Eliminate "missing peerDep" warning when dep exists at root level. (yarnpkg#5088) fix(cli): improve guessing of package names that contain a dot (yarnpkg#5102) (yarnpkg#5135) feat(cli): include notice with license when generating disclaimer (yarnpkg#5072) (yarnpkg#5111) feat(cli): group by license in licenses list (yarnpkg#5074) (yarnpkg#5110) feat(cli): improve error message when file resolver can't find file (yarnpkg#5134) (yarnpkg#5145) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
With this
yarnwill be able to discover and use thenode-gypfrom the homebrew installednpmon macOS instead of falling back to globally installingnode-gypevery time a native addon needs to be compiled from source.Homebrew installs a clean copy of
npminside alibexecfolder together withnode. Previously yarn didn't look there when trying to locatenode-gypand theglobally install node-gyp-fallback would be used every time when building native addons with a yarn version from homebrew.This PR adds the
libexecpath ofnode-gypfrom homebrew to thenode-gypsearch paths of yarn, making it possible to compile native addons using the homebrew npm providednode-gypwith yarn without relying on this fallback.Test plan
This can't be tested outside a homebrew environment (with node installed by homebrew, not by a node version manager like nvm).
To manually test this on a mac with homebrew installed you can install a test build of this PR (uploaded to github releases on my fork) by installing this gist
yarnformula with:After this you can delete your global
node-gypand install any native addon (withyarn config set build-from-sourceset to skip prebuild / node-pre-gyp) and verify that no new globalnode-gypis installed. You could also look at the verbose install output to verify, that the fallback isn't used this time. You can also verify with the current homebrew version of yarn, that the global node-gyp fallback is used every time when building native addons from source until now.