Closed mderijcke closed 9 years ago
Thanks for the pull request! Did you test this on a Windows machine?
I'm setting up a Windows environment to test it with right now, will report back.
Sorry for the super late response. I got derailed by different subjects and forgot to report back.
I am a Linux developer and to my surprise could not get the build environment working on Windows, so I could not concretely test it.
But Microsoft indicates they use Sleep, and not sleep, and you previously already had an ifdef for it but it only triggered the Unix variant (used by Mac OS X, Linux and *BSD) on Mac OS X, leaving the Linux and BSD compilers confused when they encounter Sleep (which again, is a Windows only function).
TL;DR: we don't actually need to do a test compilation for such a minor, but important change.
Without this patch on Linux: (which worked before you commited 4117669
)
mitchell@ubuntu-pc:~$ npm install node-core-audio [...] > node-core-audio@0.4.1 install /home/mitchell/node_modules/node-core-audio > node-gyp rebuild [...] ../NodeCoreAudio/AudioEngine.cpp:406:11: error: ‘Sleep’ was not declared in this scope Sleep(1); ^ make: *\ [Release/obj.target/NodeCoreAudio/NodeCoreAudio/AudioEngine.o] Error 1 make: Leaving directory
/home/mitchell/node_modules/node-core-audio/build' gyp ERR! build error gyp ERR! stack Error:
make` failed with exit code: 2 gyp ERR! stack at ChildProcess.onExit (/usr/share/node-gyp/lib/build.js:267:23) gyp ERR! stack at ChildProcess.EventEmitter.emit (events.js:98:17) gyp ERR! stack at Process.ChildProcess._handle.onexit (child_process.js:797:12) gyp ERR! System Linux 3.13.0-43-generic gyp ERR! command "nodejs" "/usr/bin/node-gyp" "rebuild" gyp ERR! cwd /home/mitchell/node_modules/node-core-audio gyp ERR! node -v v0.10.25 gyp ERR! node-gyp -v v0.10.10 gyp ERR! not ok npm WARN This failure might be due to the use of legacy binary "node" npm WARN For further explanations, please read /usr/share/doc/nodejs/README.Debiannpm ERR! weird error 1 npm ERR! not ok code 0
It's important to note that Windows' Sleep is not the rule, it's the exception. Our if statements should follow that. Your initial ifdef was correct.
unistd.h is required for sleep string.h is required for memcpy
Only Windows appears to have sleep with a capital S, all other (unix based) operation systems appear to use the lower case name.