tools: run tick processor without forking

[matthewloring]

Using the tick processor no longer creates temporary files or spawns a
child process.

/cc @bnoordhuis

[JungMinu]

maybe const nm

[matthewloring]

I've converted as many of the vars in the polyfill and processor to const as I could. Unfortunately, the lack of strict mode prevents the use of let (not sure why const is allowed outside of strict now that I think about it).

[evanlucas]

Hm, if we are not in strict mode, shouldn't we use var instead. Sloppy mode const is totally different IIRC

[matthewloring]

The script appears to behave correctly using either const or var and I am unfamiliar with how things operate outside of strict mode. Happy to switch back to var if that is preferred.

[JungMinu]

Thank you for your update, CI : https://ci.nodejs.org/job/node-test-pull-request/973/

[bnoordhuis]

Maybe use .spawnSync() here? You can pass it arguments as an array and it will take care of shell escaping them.

[matthewloring]

When I use spawnSync, I get the error Error: spawnSync foo() { nm "$@" | (c++filt -p -i || cat) }; foo ENOENT. I'm not sure exactly how spawnSync and execSync differ but they're not handling this command the same way.

[bnoordhuis]

.execSync() executes the command as /bin/sh -c '<cmd> <args>' (cmd.exe /s /c "<cmd> <args>" on Windows) whereas .spawnSync() simply calls execve(). Guess I was wrong saying it takes care of shell escaping because it doesn't need to escape anything.

You could wrap that bit of shell script in a sh -c or bash -c below. Longer term, it might be nice to move normalizeExecArgs() from lib/child_process.js to lib/internal/child_process.js so you can use it here. It's possible some more work needs to be done on the internal module system to make that work, though.

I'll leave it up to you if you want to change this line.

[matthewloring]

I don't believe wrapping it in sh -c or bash -c will work in this instance because the shell script defines a function and then calls it on arguments that will be provided later at the call site. This is the snippet in question:

foo() { nm "$@" | (c++filt -p -i || cat) }; foo

Later it will be invoked as:

foo() { nm "$@" | (c++filt -p -i || cat) }; foo -n -f /usr/lib/system/example.dylib

Is it possible to perform that wrapping you described in this case?

[bnoordhuis]

sh -c 'foo() { ... }; foo $@' -- should work, if I understand you right.

[matthewloring]

I have this working now. Will spawning /bin/sh work correctly on windows or does that need to be special cased?

[bnoordhuis]

On Windows there is no /bin/sh, you'd use cmd.exe /s /c, but I don't think you actually need to spawn a shell. The OS X case is special because you're executing shell script, on other platforms nm is invoked directly.

[matthewloring]

I can handle the OS X case specially here if that is preferable. What is the motivation of changing execSync to spawnSync?

[bnoordhuis]

That you won't have to worry about escaping spaces in paths, for example.

[matthewloring]

Ok, that makes sense, I've added the special casing here and it's working on my test machine (OS X).

[bnoordhuis]

LGTM with a suggestion.

[jasnell]

LGTM

[bnoordhuis]

LGTM. CI: https://ci.nodejs.org/job/node-test-pull-request/1030/

[ofrobots]

Thanks. Landed on master in 3e2a2e6.

[MylesBorins]

@jasnell should this go to LTS?

[MylesBorins]

relies on #4021