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

Enable install and build using node-chakracore #1777

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 55 additions & 12 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,45 +1,77 @@
language: node_js

dist: trusty
compiler: gcc
sudo: false

env:
global:
# https://github.com/jasongin/nvs/blob/master/doc/CI.md
- NVS_VERSION=1.3.0
- SKIP_SASS_BINARY_DOWNLOAD_FOR_CI=true

jobs:
include:
- stage: test
node_js: "node"
env:
global:
- NODEJS_VERSION: node/6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the latest node current release i.e. v8.1.2

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure what is picked by default when you specify node_js value to node. I will update it to 8 and nvs should pick the right version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default the node is the latest stable, currently 8.1.2. We'd want the equivalent to node/latest

os: linux
before_script: npm run lint || exit 1;
after_success: npm run-script coverage;
- stage: platform-test
node_js: "node"
env:
global:
- NODEJS_VERSION: node/8
os: osx
- stage: platform-test
node_js: "7"
env:
global:
- NODEJS_VERSION: node/7
os: linux
- stage: platform-test
node_js: "7"
env:
global:
- NODEJS_VERSION: node/7
os: osx
- stage: platform-test
node_js: "lts/boron"
env:
global:
- NODEJS_VERSION: node/boron
os: linux
- stage: platform-test
node_js: "lts/boron"
env:
global:
- NODEJS_VERSION: node/boron
os: osx
- stage: platform-test
node_js: "lts/argon"
env:
global:
- NODEJS_VERSION: node/argon
os: linux
- stage: platform-test
node_js: "lts/argon"
env:
global:
- NODEJS_VERSION: node/argon
os: osx
- stage: platform-test
node_js: "0.12"
env:
global:
- NODEJS_VERSION: node/0.12
os: linux
- stage: platform-test
env:
global:
- NODEJS_VERSION: node/0.10
os: linux
- stage: platform-test
node_js: "0.10"
env:
global:
- NODEJS_VERSION: chakracore/latest
os: osx
- stage: platform-test
env:
global:
- NODEJS_VERSION: chakracore/latest
os: linux

addons:
Expand All @@ -51,14 +83,25 @@ addons:
- g++-4.7

before_install:
# Install NVS.
- git clone --branch v$NVS_VERSION --depth 1 https://github.com/jasongin/nvs ~/.nvs
- . ~/.nvs/nvs.sh
- nvs --version

# Install selected version of Node.js using NVS
- nvs add $NODEJS_VERSION
- nvs use $NODEJS_VERSION
- node --version
- npm --version

- npm config set python `which python`
- if [ $TRAVIS_OS_NAME == "linux" ]; then
export CC="gcc-4.7";
export CXX="g++-4.7";
export LINK="gcc-4.7";
export LINKXX="g++-4.7";
fi
- nvm --version

- node --version
- npm --version
- gcc --version
Expand Down
37 changes: 23 additions & 14 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,44 +32,53 @@
- '%AppData%\npm-cache'

environment:
# https://github.com/jasongin/nvs/blob/master/doc/CI.md
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth looking at https://github.com/appveyor/ci/blob/master/scripts/nodejs-utils.psm1 to get "native" support for Chakra on Appveyor

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is definitely an option (and likewise in nvm and hence node-chakracore), but at this time we just preferred using nvs since it added support for CI recently.

NVS_VERSION: 1.3.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you bump this to 1.4.0 it looks like the rate limit issues should go away https://github.com/jasongin/nvs/releases/tag/v1.4.0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, forgot to update it.

SKIP_SASS_BINARY_DOWNLOAD_FOR_CI: true
matrix:
- nodejs_version: 0.10
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 0.12
- NODEJS_VERSION: node/0.10
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 1.0
- NODEJS_VERSION: node/0.12
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 1
- NODEJS_VERSION: iojs/1
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 2
- NODEJS_VERSION: iojs/2
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 3
- NODEJS_VERSION: iojs/3
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 4
- NODEJS_VERSION: node/4
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 5
- NODEJS_VERSION: node/5
GYP_MSVS_VERSION: 2013
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2013
- nodejs_version: 6
- NODEJS_VERSION: node/6
GYP_MSVS_VERSION: 2015
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- nodejs_version: 7
- NODEJS_VERSION: node/7
GYP_MSVS_VERSION: 2015
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- nodejs_version: 8
- NODEJS_VERSION: node/8
GYP_MSVS_VERSION: 2015
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- NODEJS_VERSION: chakracore/latest
GYP_MSVS_VERSION: 2015
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015

install:
- ps: Install-Product node $env:nodejs_version $env:platform
# Install NVS
- git clone --branch v%NVS_VERSION% --depth 1 https://github.com/jasongin/nvs %LOCALAPPDATA%\nvs
- set PATH=%LOCALAPPDATA%\nvs;%PATH%
- nvs --version
# Install selected version of Node.js using NVS
- nvs add %NODEJS_VERSION%
- nvs use %NODEJS_VERSION%

- node --version
- npm --version
- npm install
Expand Down
14 changes: 10 additions & 4 deletions lib/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,16 @@ function getBinaryName() {
}

binaryName = [
platform, '-',
process.arch, '-',
process.versions.modules
].join('');
platform,
process.arch,
process.versions.modules,
];

if (process.jsEngine === 'chakracore') {
binaryName = binaryName.concat(process.jsEngine);
}

binaryName = binaryName.join('-');
}

return [binaryName, 'binding.node'].join('_');
Expand Down
56 changes: 48 additions & 8 deletions scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,20 @@ function afterBuild(options) {
*/

function build(options) {
var args = [require.resolve(path.join('node-gyp', 'bin', 'node-gyp.js')), 'rebuild', '--verbose'].concat(
['libsass_ext', 'libsass_cflags', 'libsass_ldflags', 'libsass_library'].map(function(subject) {
return ['--', subject, '=', process.env[subject.toUpperCase()] || ''].join('');
})).concat(options.args);

console.log('Building:', [process.execPath].concat(args).join(' '));

var proc = spawn(process.execPath, args, {
var nodeGyp = resolveNodeGyp();
var nodeGypArgs = nodeGyp.args.concat([
'rebuild',
'--verbose',
'--libsass_ext=' + (process.env['LIBSASS_EXT'] || ''),
'--libsass_cflags=' + (process.env['LIBSASS_CFLAGS'] || ''),
'--libsass_ldflags=' + (process.env['LIBSASS_LDFLAGS'] || ''),
'--libsass_library=' + (process.env['LIBSASS_LIBRARY'] || ''),
])
.concat(options.args);

console.log(['Building:', nodeGyp.exeName].concat(nodeGypArgs).join(' '));

var proc = spawn(nodeGyp.exeName, nodeGypArgs, {
stdio: [0, 1, 2]
});

Expand All @@ -83,6 +89,40 @@ function build(options) {
});
}

/**
* Resolve node-gyp command to invoke
*
* @api private
*/
function resolveNodeGyp() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this now that the version is bumped to 3.6?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No and that's why i removed my changes in 5021dff . I just kept the resolution of node-gyp in separate function for easy readability.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 that makes sense

if (process.jsEngine === 'chakracore') {
// For node-chakracore, check if node-gyp is in the path.
// If yes, use it instead of using node-gyp directly from
// node_modules because the one in node_modules is not
// compatible with node-chakracore.
var nodePath = path.dirname(process.execPath);
var nodeGypName = process.platform === 'win32' ? 'node-gyp.cmd' : 'node-gyp';
var globalNodeGypBin = process.env.Path.split(';').filter(function(envPath) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct for non-Windows?

what is the difference between chakraized node-gyp and the normal one?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct for non-Windows?

good catch @saper . I have updated it with appropriate delim.

what is the difference between chakraized node-gyp and the normal one?

See nodejs/node-gyp#873 for more details and the description of my first commit.

In short, the changes are to add engine specific libs and headers while compiling native modules.

Since node-gyp PR has landed, we might not need this. The reason I still want to have this around is we could have a case of having a stale non-chakracore supported node-gyp in module path which will break building node-sass with node-chakracore as it breaks today.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have node-gyp as a dependency in our package.json to get around these exact issues. You could bump that dependency to the minimum safe version.

return envPath.startsWith(nodePath) &&
envPath.endsWith('node-gyp-bin') &&
fs.existsSync(path.resolve(envPath, nodeGypName));
});

if (globalNodeGypBin.length !== 0) {
return { exeName: 'node-gyp', args: [] };
}

console.error('node-gyp incompatible with node-chakracore! Please use node-gyp installed with node-chakracore.');
process.exit(1);
}

var localNodeGypBin = require.resolve(path.join('node-gyp', 'bin', 'node-gyp.js'));

return {
exeName: process.execPath,
args: [localNodeGypBin],
};
}
/**
* Parse arguments
*
Expand Down