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

Handle spaces in parent directory name #43

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Conversation

jhayes-dev
Copy link
Contributor

@jhayes-dev jhayes-dev commented Feb 21, 2023

If there is a space in the parent directory name farmhash will not compile. If fails with:

CXX(target) Release/obj.target/farmhash/src/upstream/farmhash.o
clang: error: no such file or directory: 'space/node_modules/node-addon-api'
make: *** [Release/obj.target/farmhash/src/upstream/farmhash.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: make failed with exit code: 2
gyp ERR! stack at ChildProcess.onExit (ROOT/src/framfash-with space/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack at ChildProcess.emit (node:events:513:28)
gyp ERR! stack at Process.ChildProcess._handle.onexit (node:internal/child_process:293:12)
gyp ERR! System Darwin 21.5.0
gyp ERR! command "ROOT/.nvm/versions/node/v16.18.0/bin/node" "ROOT/src/framfash-with space/node_modules/.bin/node-gyp" "rebuild"
gyp ERR! cwd ROOT/src/framfash-with space
gyp ERR! node -v v16.18.0
gyp ERR! node-gyp -v v6.1.0

The generated build/farmhash.target.mk does not quote the include path.

This change uses a relative path which is handled correctly.

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left a comment inline as there's one small change required to use include_dir in a scalar rather than array context, otherwise this is good to go.

binding.gyp Outdated Show resolved Hide resolved
@lovell lovell merged commit cd692d3 into lovell:main Apr 14, 2023
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

Successfully merging this pull request may close these issues.

2 participants