Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Commit

Permalink
Revamp build steps
Browse files Browse the repository at this point in the history
* Instead of providing the binaries with the package, it'll only download the
needed binary for the users system.
* Will only build if the tests fail.
* Move `build.js` into scripts since it's not part of the actual API.
* Move binaries to `vendor` folder to keep it separate from the rest of the code.

Fixes #504.
  • Loading branch information
kevva committed Nov 4, 2014
1 parent 014537a commit d4b8f6b
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 49 deletions.
3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
*.log
.DS_Store
.sass-cache
bin
!bin/node-sass
build
lib-cov
node_modules
vendor
2 changes: 1 addition & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function getBinding() {
var candidates = [
path.join(__dirname, '..', 'build', 'Release', 'binding.node'),
path.join(__dirname, '..', 'build', 'Debug', 'binding.node'),
path.join(__dirname, '..', 'bin', name, 'binding.node')
path.join(__dirname, '..', 'vendor', name, 'binding.node')
];

var candidate = candidates.filter(fs.existsSync)[0];
Expand Down
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
"gypfile": true,
"scripts": {
"coverage": "node scripts/coverage.js",
"install": "node lib/build.js",
"prepublish": "node scripts/prepublish",
"install": "node scripts/install.js",
"postinstall": "node scripts/build.js",
"pretest": "node_modules/.bin/jshint bin lib test",
"test": "node_modules/.bin/mocha test"
},
Expand All @@ -44,14 +44,15 @@
"dependencies": {
"chalk": "^0.5.1",
"cross-spawn": "^0.2.3",
"download": "^3.1.2",
"download-status": "^2.1.0",
"get-stdin": "^3.0.0",
"meow": "^2.0.0",
"mkdirp": "^0.5.0",
"mocha": "^2.0.1",
"nan": "^1.3.0",
"node-watch": "^0.3.4",
"object-assign": "^1.0.0",
"shelljs": "^0.3.0"
"object-assign": "^1.0.0"
},
"devDependencies": {
"coveralls": "^2.11.1",
Expand Down
18 changes: 9 additions & 9 deletions lib/build.js → scripts/build.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env node
var fs = require('fs'),
path = require('path'),
spawn = require('child_process').spawn,
mkdir = require('mkdirp'),
Mocha = require('mocha');

/**
Expand All @@ -14,24 +14,24 @@ var fs = require('fs'),
function afterBuild(options) {
var folder = options.debug ? 'Debug' : 'Release';
var target = path.join(__dirname, '..', 'build', folder, 'binding.node');
var install = path.join(__dirname, '..', 'bin', options.bin, 'binding.node');
var install = path.join(__dirname, '..', 'vendor', options.bin, 'binding.node');

fs.mkdir(path.join(__dirname, '..', 'bin', options.bin), function (err) {
mkdir(path.join(__dirname, '..', 'vendor', options.bin), function (err) {
if (err && err.code !== 'EEXIST') {
console.error(err.message);
process.exit(1);
return;
}

fs.stat(target, function (err) {
if (err) {
console.error('Build succeeded but target not found');
process.exit(1);
return;
}

fs.rename(target, install, function (err) {
if (err) {
console.error(err.message);
process.exit(1);
return;
}

console.log('Installed in `' + install + '`');
Expand Down Expand Up @@ -61,11 +61,11 @@ function build(options) {
'You need at least 1.1.5 (I think) and preferably 1.1.30.'
].join(' '));

process.exit(code);
return;
}

console.error('Build failed');
process.exit(code);
return;
}

afterBuild(options);
Expand Down Expand Up @@ -120,7 +120,7 @@ function testBinary(options) {
}

if (!process.env.SKIP_NODE_SASS_TESTS) {
fs.stat(path.join(__dirname, '..', 'bin', options.bin, 'binding.node'), function (err) {
fs.stat(path.join(__dirname, '..', 'vendor', options.bin, 'binding.node'), function (err) {
if (err) {
return build(options);
}
Expand Down
71 changes: 71 additions & 0 deletions scripts/install.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
var fs = require('fs'),
path = require('path'),
Download = require('download'),
status = require('download-status');

/**
* Check if binaries exists
*
* @api private
*/

function exists() {
var v8 = 'v8-' + /[0-9]+\.[0-9]+/.exec(process.versions.v8)[0];
var name = process.platform + '-' + process.arch + '-' + v8;

fs.exists(path.join(__dirname, '..', 'vendor', name), function (exists) {
if (exists) {
return;
}

fetch(name);
});
}

/**
* Fetch binaries
*
* @param {String} name
* @api private
*/

function fetch(name) {
var download = new Download({
extract: true,
mode: '777',
strip: 1
});

var url = [
'https://github.com/sass/node-sass-binaries/raw/master/',
name + '/binding.node'
].join('');

download.get(url);
download.dest(path.join(__dirname, '..', 'vendor', name));
download.use(status());

download.run(function(err) {
if (err) {
console.error(err.message);
return;
}

console.log('Binary installed in ' + download.dest());
});
}

/**
* Skip if CI
*/

if (process.env.CI || process.env.APPVEYOR) {

This comment has been minimized.

Copy link
@localjo

localjo Nov 6, 2014

@kevva Skipping downloading binaries on CI builds is causing my CircleCI builds to fail for projects that depend on node-sass. Curious as to the reasoning for this, and if there's a recommended workaround?

This comment has been minimized.

Copy link
@j15e

j15e Nov 12, 2014

Same issue here.. my node projects are not building on CircelCI because of this.

Could you rely on an other env variable or add an other env flag to NOT skip downloading binaries even on CI?

@josiahsprague I am setting my dependencies step manually like this with CI="":

# circle.yml
dependencies:
  override:
    # CI="" is a temporary fix for node-sass
    # @see https://github.com/sass/node-sass/commit/d4b8f6b49008c46c1740b34bf3a8975beb8eada9
    - 'CI="" npm install'

But I am still getting an error with the binary because CircleCi uses ubuntu 12.04 which does not have GCC 4.8 :

Error: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.18' not found (required by /home/ubuntu/didacte/frontend/node_modules/broccoli-sass/node_modules/node-sass/vendor/linux-x64-v8-3.14/binding.node)

So maybe we should build from source on CirleCi, but it is not working either because it also seems to requires GCC 4.8.

This comment has been minimized.

Copy link
@j15e

j15e Nov 12, 2014

@josiahsprague working solution :

dependencies:
  pre:
    # Gcc 4.8 required for node-sass
    - sudo add-apt-repository ppa:ubuntu-toolchain-r/test -y
    - sudo apt-get update
    - sudo apt-get install -y g++-4.8
    # CI="" is a temporary fix for node-sass
    # @see https://github.com/sass/node-sass/commit/d4b8f6b49008c46c1740b34bf3a8975beb8eada9
    - 'CI="" npm install'

This comment has been minimized.

Copy link
@localjo

localjo Nov 12, 2014

This is similar to what I did, and it worked for CircleCI, but I'm still having issues with missing binaries on local Vagrant instances. For now I'm just manually reinstalling gulp-sass after each npm install of my project and that seems to work. Haven't had time to look into what the exact issue is.

console.log('Skipping downloading binaries on CI builds');
return;
}

/**
* Run
*/

exists();
33 changes: 0 additions & 33 deletions scripts/prepublish

This file was deleted.

0 comments on commit d4b8f6b

Please sign in to comment.