Build: Add v8 specific platform builds

[nschonni]

Idea courtesy of node-fibers.
When installing, check to see if there is a compatible binary version for the current version of v8. v8 3.6 = Node 0.6, 3.11 = Node 0.8, 3.14 = Node 0.10. The test suite is run as well, so that any underlying libsass breaks will be rebuilt.

Closes gh-146

---

I added the console.log('Please consider contributing the release binary to node-sass for npm distribution.'); line, but you'll have to figure out a method for accepting binaries to include (DropBox, etc...). Alternately you could create a node-sass-binary repo for accepting PRs.

[andrew]

Looks good, I'll have a play around with this over the next few days.

Thanks!

[nschonni]

I excluded the binaries from the repo, but maybe you do want to include them. I just figured it would be nicer for the git repo size.

[andrew]

I think we do want to include them, but obviously they will need to be renamed.

[kennethormandy]

Hey, I’m just curious how this worked on your end, @nschonni. I added the binaries back in and renamed them correctly. With your additions, Node-sass finds them successfully, but they still seem to rebuild anyway:

`darwin-x64-v8-3.14` exists; testing
Problem with the binary; manual build incoming
Please consider contributing the release binary to node-sass for npm distribution.

Do the binaries themselves need to be updated with this change? They still get rebuilt even if I use the binary Node-sass itself just built. We’re really looking forward to this feature so we can add Node-sass to Harp. I’m happy to test it out more if that would help at all—it’s also entirely possibly I’m doing something wrong here. Thanks for your work on this!

[nschonni]

The check now runs the full test suite to see if the binary is OK before trying to rebuild. If there are any bugs in the build, it should probably check again at the end.

[nschonni]

I think there may be an unrelated rgba() bug that might be causing the rebuild

[andrew]

What platform is this on? I'm not getting any failures on mac os x or travis.

[nschonni]

I rebased over the latest to include the CLI test fixes that were added in after

[kennethormandy]

I’m using OS X 10.8.4 with Node 0.10.18. Thanks for the update, all the tests are passing, including the new ones. The same thing is still happening as before, but maybe I’m misunderstanding what should be happening?

I expected when I run npm install, the build shouldn’t happen because there is already a binary there. If it does need to rebuild, if I run npm install again, it definitely shouldn’t need to rebuild because the correct binary was just created.

What actually happens is the binary is found, but there is a Problem with the binary; manual build incoming every time I run npm install.

[nschonni]

You're right, there is a problem with the mocha test snifffing that it is doing right now

[kennethormandy]

Is there anything information I could provide or testing I could do that would help resolve that?

[nschonni]

The spawning and testing of the mocha return here https://github.com/andrew/node-sass/pull/148/files#L1R34 needs to be fixed.

[kennethormandy]

There is undoubtably a cleaner way to do this, but I believe I got it working here.

You can npm install kennethormandy/node-sass#node-gyp-build-like-fibers and it shouldn’t rebuild the binary now.

[nschonni]

OK, I think I found a fix courtesy of http://ronderksen.nl/category/mocha-js/.
Could someone else try it out?

[kennethormandy]

So the goal would be to run all the tests, and test.js in the root was just a work-around?

[nschonni]

Yeah, I'm not sure what the one in the root is used for.

[kennethormandy]

@nschonni Good find, that post worked for me. https://github.com/kennethormandy/node-sass/blob/node-gyp-build-like-fibers/build.js

[nschonni]

I did push a working version to this PR, but I did not include all the tests since the CLI test consistently fail for me on Windows.

[kennethormandy]

I totally missed that, thanks. Like you said, your version worked on Windows 8, whereas mine didn’t. I got this issue on Windows 7 for both, though.

C:\Users\IEUser\Documents\GitHub\node-sass [node-gyp-build-like-fibers]> npm ins
tall

> [email protected] install C:\Users\IEUser\Documents\GitHub\node-sass
> node build.js


C:\Users\IEUser\Documents\GitHub\node-sass>node "C:\Program Files\nodejs\node_mo
dules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild

gyp ERR! configure error
gyp ERR! stack Error: Can't find Python executable "python", you can set the PYT
HON env variable.
gyp ERR! stack     at failNoPython (C:\Program Files\nodejs\node_modules\npm\nod
e_modules\node-gyp\lib\configure.js:118:14)
gyp ERR! stack     at C:\Program Files\nodejs\node_modules\npm\node_modules\node
-gyp\lib\configure.js:81:11
gyp ERR! stack     at Object.oncomplete (fs.js:107:15)
gyp ERR! System Windows_NT 6.1.7601
gyp ERR! command "node" "C:\\Program Files\\nodejs\\node_modules\\npm\\node_modu
les\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd C:\Users\IEUser\Documents\GitHub\node-sass
gyp ERR! node -v v0.10.18
gyp ERR! node-gyp -v v0.10.9
gyp ERR! not ok
Build failed
npm ERR! weird error 1
npm ERR! not ok code 0

That said, I actually can’t install the latest version of Node-sass on Windows 7 (v0.6.4 seems to work better) so maybe that issue isn’t here. npm install node-sass on Windows 7 SP1 with Node v0.10.17:

module.js:340
    throw err;
          ^
Error: Cannot find module 'mocha'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (C:\Users\IEUser\Documents\GitHub\node_modules\node-sa
ss\build.js:5:10)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
npm ERR! weird error 8
npm ERR! not ok code 0

[nschonni]

1. Python is required for node-gyp installation. This requirement will hopefully be skipped if the binaries are included when the NPM package is published
2. Good call on mocha that will need to be moved from the devDependencies to dependencies now

[kennethormandy]

Ever since you included the binaries (thanks!), I imagine it would work exactly the same on npm. If I npm install kennethormandy/node-sass#patch-1 (which is your version with the Mocha dependency) then I still hit the Python requirement, rather than just getting the win32-ia32-v8-3.14 binary.

I don’t know if that’s helpful, but it seems something with node-gyp needs to be deferred until the build is triggered. Maybe that means we have to use a different method to check if the binary exists.

[nschonni]

@kennethormandy Thanks! I've moved Mocha and bumped the dependency. I did run into a file locking issue once during the rebuild, but haven't been able to reproduce it again. I think @andrew may add back in the binaries again, or just include them during the NPM publish.

[kennethormandy]

I haven’t tried it yet myself, but it seems like #160 should help out here?

[nschonni]

Whoops, merge conflicts now. I'll rebase and see if it goes green on Windows

[nschonni]

I've rebased, but I still need to re-check on Windows

[kennethormandy]

I’m hitting the Python issue on Windows 7 & 8 now. So at least they’re doing the same thing, that seems positive.

Entirely likely I’m misunderstanding, but since the binaries are already there (I’ve cloned your repo and am just running npm install) I didn’t think them being included on npm will fix this. Actually installing node-gyp is the problem, correct? Are you using it for anything before the rebuild? Could it be delayed until someone actually needs to rebuild?

I also had updated this and hit the Xcode license agreement warning, which I don’t think should have happened since we it shouldn’t have needed to rebuild.

Agreeing to the Xcode/iOS license requires admin privileges, please re-run as root via sudo.
gyp: Error 69 running xcodebuild
gyp ERR! configure error
gyp ERR! stack Error: `gyp` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onCpExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/configure.js:424:16)
gyp ERR! stack     at ChildProcess.EventEmitter.emit (events.js:98:17)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (child_process.js:789:12)
gyp ERR! System Darwin 12.4.0
gyp ERR! command "node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"

Once I agreed, I’m hitting this error on OS X, which I’ve seen before. I believe I got around it here, but I could be mis-remembering. Something in test.js had an extra line break or something weird like that? And it seems like it was causing this to happen. Anyway, this is the error with npm install on OS X.

  CXX(target) Release/obj.target/binding/binding.o
In file included from ../binding.cpp:7:
../sass_context_wrapper.h:1:10: fatal error: 'libsass/sass_interface.h' file not found
#include "libsass/sass_interface.h"
         ^
1 error generated.
make: *** [Release/obj.target/binding/binding.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/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:789:12)
gyp ERR! System Darwin 12.4.0
gyp ERR! command "node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"

[LaurentGoderre]

Did you do node-gyp configure?

[LaurentGoderre]

Btw, because you need visual studio on Windows, it would be preferable having the bindings in the repo to not force everyone to rebuild.

[andrew]

Prepublish hook sounds good

[LaurentGoderre]

👍

[nschonni]

@andrew I made a very lazy prepublish hook that assumes you have cloned node-sass-binaries on your machine 😄
Also update the note about contributing any rebuilds to the node-sass-binaries repo.

[andrew]

How is that going to work on travis? Do we need to add a bit of setup script to .travis.yml as well?

[nschonni]

Not sure why it is trying to run the prepublish task as part of npm install, but I'm looking into it

[nschonni]

Ah, from the NPM docs https://npmjs.org/doc/files/package.json.html

The prepublish script will be run before publishing, so that users can consume the functionality without requiring them to compile it themselves. In dev mode (ie, locally running npm install), it'll run this script as well, so that you can test it easily.

Guess it does need a guard of some kind 😦

[andrew]

Can you move the prepublish code into it's own script then we can check and maybe even curl the specific binary directly from github rather than needing the whole repo locally?

[nschonni]

Done, I think the curl idea is good, but it can be tackled separately.

[andrew]

Looks good, is there anything stopping me from merging this?

[kennethormandy]

As much as I’m looking forward to this being merged, I think you should just be able to install Node and npm install node-sass on a fresh install of Windows or OS X without a rebuild.

I don’t think that will be the case with Windows. It’s still possible I’m doing something wrong, but I think way the binaries are being checked makes Python a requirement for installing this on Windows.

If someone who doesn’t have Python on Windows has tested this successfully, then I would be happy to see this merged, as it’s probably me messing up. But so far I haven’t been able to do it successfully myself.

@andrew Never mind. I manually git clone’d the binaries repo into bin/ and it worked without Python on Windows 8 (which is what I assume will happen once it’s published) so I am happy if you are. Sorry about that.

Edit again

I did find one issue. Something is wrong with one of Mocha’s dependencies’ package.json on Windows 7, but the good news is when I switch mocha to a much older version, the tests work and it doesn’t rebuild, so I think this would be a relatively small hurdle.

[nschonni]

npm install node-sass on a fresh install of Windows or OS X without a rebuild.

@kennethormandy the NPM install will include the binaries when it is published. Python is only a requirement if the tests fail and a rebuild is required.

I did find one issue. Something is wrong with one of Mocha’s dependencies’ package.json on Windows 7, but the good news is when I switch mocha to a much older version, the tests work and it doesn’t rebuild, so I think this would be a relatively small hurdle.

Can you give specific steps to reproduce the issue?

---

Looks good, is there anything stopping me from merging this?

@andrew I think I'm finally finished 😄 depending on whether @kennethormandy's problem can be reproduced, I think it is good to go.

[kennethormandy]

@nschonni Yep, I submitted a pull request against yours which solves the problem, but I’m not sure if it’s the best way.

Commander v0.6.1’s package.json prevents the install on Windows 7, but any later version doesn’t, so I added a npm-shrinkwrap file. I’m not sure how much of an inconvenience that is to maintain, though.

[andrew]

Let's get it merged in now and we can start kicking the tyres before shipping it.

Thanks @nschonni, excellent work!

[andrew]

I just pushed out v0.7.0-alpha and installed it and it all worked great, you can get it with:

npm install [email protected]

If someone can give that a try on windows and see if it works that would be very handy, then we can 🚢

[kennethormandy]

@andrew This is great, it worked exactly as I would have expected on Windows 8 and OS X.

On Windows 7, I’m still getting and unexpected token in the package.json error for commander v0.6.1, which mocha is using. Using any higher version of commander seemed to fix the problem, so I added shrinkwrap file: nschonni#2. This fixes the problem on Windows 7, but there might be a better way to solve it that I’m unfamiliar with.

[nschonni]

@kennethormandy do you mind creating a new issue for the commander issue and maybe including the console output (or in a Gist if it's long)?

[andrew]

@kennethormandy I guess we wouldn't be able to get @visionmedia to upgrade commander in mocha, but I've sent a pull request anyway: mochajs/mocha#1010

The only thing with shrinkwrap is that it locks down all the dependency versions, meaning we'll never get any bug fix releases unless we manually update the shrinkwrap.

Can you try pointing mocha to my fork and see if that helps on windows 7:

  "dependencies": {
    "mkdirp": "0.3.x",
    "colors": "0.6.0-1",
    "optimist": "0.6.x",
    "node-watch": "0.3.x",
    "mocha": "git://github.com/andrew/mocha.git"
  }

[kennethormandy]

@andrew Ugh, sorry. Good news, this was totally my fault. It was just a caching issue with npm. npm install [email protected] works on Windows 7 & 8.

Thanks for the work on this everyone!

[andrew]

🤘

[andrew]

Just published this as v0.7.0: https://github.com/andrew/node-sass/releases/tag/v0.7.0