Fix installation to paths with spaces

[unflxw]

When the full file system path of the folder where the package.json is located contains spaces, the AppSignal extension fails to compile when installing the AppSignal package from npm. At least on macOS, but probably not just on macOS. Repro:

mkdir "foo bar"
cd "foo bar"
npm init -y
npm i --save @appsignal/nodejs@latest
cat node_modules/@appsignal/nodejs/ext/install.report

You'll see in the install report that node-gyp at some point passes a path to clang unquoted, resulting in:

clang: error: no such file or directory: 'bar/node_modules/node-addon-api'

It's unclear at this moment whether this is an issue with node-gyp itself, or with how we configure it.

It looks to be a bug in node-gyp: #907 (comment)

[tombruijn]

This is the config we set: https://github.com/appsignal/appsignal-nodejs/blob/main/binding.gyp
Maybe something needs another set of quotes to escape it.

[tombruijn]

I can't find any place we set the current path and don't escape it.

This issue (nodejs/node-gyp#65), although closed, has activity on it indicating it's not fixed in the latest versions. Here's a possible fix in a PR: nodejs/node-gyp#2556

If it exists in node-gyp and it's not being fixed we can tell people to not use spaces in their path or switch to another tool.

[tombruijn]

I've confirmed the issue is fixed by using nodejs/node-gyp#2556, so it's not on our end. What shall we do: document, make noise on the issues/PRs, switch to another tool?

[unflxw]

Thank you for confirming it. 💯

I don't know. I don't understand why that PR hasn't been merged for over a year. Perhaps because of the comment that says that it should be fixed upstream, where it hasn't been fixed either.

I don't know what alternatives exist to node-gyp, either. Perhaps if the ecosystem at large tacitly accepts that their tooling is broken, we should too.

[tombruijn]

Perhaps because of the comment that says that it should be fixed upstream, where it hasn't been fixed either.

I'd love it if we could move it along by providing a test case or fixing it upstream, if that's their preferred solution.

The more difficult part (for me) will be a sending a PR with a reproducible test case.

I don't know what alternatives exist to node-gyp, either. Perhaps if the ecosystem at large tacitly accepts that their tooling is broken, we should too.

cmake-js #386 got suggested once, would also remove our Python dependency(?), but I have no idea how good that tool is. We might be trading in one set of quirks for another set we'd have to get burned by again.

[tombruijn]

Submitted the fix upstream (the work of @davej (oh hi 👋)) in nodejs/gyp-next#204. Let's see what happens.

[tombruijn]

Fix submitted upstream in nodejs/gyp-next#204. Closing issue, although I don't see it being merged soon.