From 4aff0563aa75f64adc6f6d4ef0965b3a14617d2b Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Tue, 25 Apr 2017 17:36:50 -0400 Subject: [PATCH 1/7] build: reduce one level of spawning in node_gyp `configure` will now call `node_gyp` as a module instead of forking makes it easier to debug PR-URL: https://github.com/nodejs/node/pull/12653 Reviewed-By: Ben Noordhuis Reviewed-By: Gibson Fahnestock --- configure | 5 +++-- tools/gyp_node.py | 18 ++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/configure b/configure index 5a6de084836..9d9bc9bc17d 100755 --- a/configure +++ b/configure @@ -40,6 +40,7 @@ import nodedownload # imports in tools/ sys.path.insert(0, os.path.join(root_dir, 'tools')) import getmoduleversion +from gyp_node import run_gyp # parse our options parser = optparse.OptionParser() @@ -1380,7 +1381,7 @@ config = '\n'.join(map('='.join, config.iteritems())) + '\n' write('config.mk', do_not_edit + config) -gyp_args = [sys.executable, 'tools/gyp_node.py', '--no-parallel'] +gyp_args = ['--no-parallel'] if options.use_xcode: gyp_args += ['-f', 'xcode'] @@ -1399,4 +1400,4 @@ gyp_args += args if warn.warned: warn('warnings were emitted in the configure phase') -sys.exit(subprocess.call(gyp_args)) +run_gyp(gyp_args) diff --git a/tools/gyp_node.py b/tools/gyp_node.py index 8de046aae25..b37cc7c5f04 100755 --- a/tools/gyp_node.py +++ b/tools/gyp_node.py @@ -13,14 +13,6 @@ output_dir = os.path.join(os.path.abspath(node_root), 'out') def run_gyp(args): - rc = gyp.main(args) - if rc != 0: - print 'Error running GYP' - sys.exit(rc) - -if __name__ == '__main__': - args = sys.argv[1:] - # GYP bug. # On msvs it will crash if it gets an absolute path. # On Mac/make it will crash if it doesn't get an absolute path. @@ -63,5 +55,11 @@ def run_gyp(args): args.append('-Dlinux_use_bundled_gold=0') args.append('-Dlinux_use_gold_flags=0') - gyp_args = list(args) - run_gyp(gyp_args) + rc = gyp.main(args) + if rc != 0: + print 'Error running GYP' + sys.exit(rc) + + +if __name__ == '__main__': + run_gyp(sys.argv[1:]) From 80355271c324d2a5515768c1d58976d4483f650c Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Wed, 26 Apr 2017 09:27:46 -0400 Subject: [PATCH 2/7] build: simplify `if` in setting of arg_paths PR-URL: https://github.com/nodejs/node/pull/12653 Reviewed-By: Ben Noordhuis Reviewed-By: Gibson Fahnestock --- tools/gyp_node.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tools/gyp_node.py b/tools/gyp_node.py index b37cc7c5f04..043053c3daa 100755 --- a/tools/gyp_node.py +++ b/tools/gyp_node.py @@ -16,16 +16,11 @@ def run_gyp(args): # GYP bug. # On msvs it will crash if it gets an absolute path. # On Mac/make it will crash if it doesn't get an absolute path. - if sys.platform == 'win32': - args.append(os.path.join(node_root, 'node.gyp')) - common_fn = os.path.join(node_root, 'common.gypi') - options_fn = os.path.join(node_root, 'config.gypi') - options_fips_fn = os.path.join(node_root, 'config_fips.gypi') - else: - args.append(os.path.join(os.path.abspath(node_root), 'node.gyp')) - common_fn = os.path.join(os.path.abspath(node_root), 'common.gypi') - options_fn = os.path.join(os.path.abspath(node_root), 'config.gypi') - options_fips_fn = os.path.join(os.path.abspath(node_root), 'config_fips.gypi') + a_path = node_root if sys.platform == 'win32' else os.path.abspath(node_root) + args.append(os.path.join(a_path, 'node.gyp')) + common_fn = os.path.join(a_path, 'common.gypi') + options_fn = os.path.join(a_path, 'config.gypi') + options_fips_fn = os.path.join(a_path, 'config_fips.gypi') if os.path.exists(common_fn): args.extend(['-I', common_fn]) From 53dae8383309b8d1a7ccdf81bbddd5e4e594b48d Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 12 May 2017 12:47:47 -0700 Subject: [PATCH 3/7] src: fix --abort_on_uncaught_exception arg parsing Fix c0bde73f, which inadvertently introduced a use of strcmp() without correctly comparing its return to zero. Caught by coverity: >>> CID 169223: Integer handling issues (CONSTANT_EXPRESSION_RESULT) >>> The "or" condition "strcmp(arg, "--abort-on-uncaught-exception") || strcmp(arg, "--abort_on_uncaught_exception")" will always be true because "arg" cannot be equal to two different values at the same time, so it must be not equal to at least one of them. 3909 } else if (strcmp(arg, "--abort-on-uncaught-exception") || 3910 strcmp(arg, "--abort_on_uncaught_exception")) { 3911 abort_on_uncaught_exception = true; 3912 // Also a V8 option. Pass through as-is. 3913 new_v8_argv[new_v8_argc] = arg; 3914 new_v8_argc += 1; PR-URL: https://github.com/nodejs/node/pull/13004 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: Gibson Fahnestock Reviewed-By: Richard Lau --- src/node.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index f943d75a575..8ad742b0c1f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3906,8 +3906,8 @@ static void ParseArgs(int* argc, } else if (strcmp(arg, "--") == 0) { index += 1; break; - } else if (strcmp(arg, "--abort-on-uncaught-exception") || - strcmp(arg, "--abort_on_uncaught_exception")) { + } else if (strcmp(arg, "--abort-on-uncaught-exception") == 0 || + strcmp(arg, "--abort_on_uncaught_exception") == 0) { abort_on_uncaught_exception = true; // Also a V8 option. Pass through as-is. new_v8_argv[new_v8_argc] = arg; From f2ba06db92ea561e23f93e85ba29404abfe74b1a Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 12 May 2017 09:06:57 -0700 Subject: [PATCH 4/7] benchmark: remove redundant timers benchmark The immediate.js benchmark with `type` set to `depth` measures the same thing as set-immediate-depth.js. Remove the redundancy.` PR-URL: https://github.com/nodejs/node/pull/13009 Reviewed-By: Joyee Cheung Reviewed-By: Refael Ackermann --- benchmark/timers/set-immediate-depth.js | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 benchmark/timers/set-immediate-depth.js diff --git a/benchmark/timers/set-immediate-depth.js b/benchmark/timers/set-immediate-depth.js deleted file mode 100644 index 12b9cdc7e6f..00000000000 --- a/benchmark/timers/set-immediate-depth.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; - -const common = require('../common.js'); -const bench = common.createBenchmark(main, { - millions: [10] -}); - -function main(conf) { - const N = +conf.millions * 1e6; - let n = N; - - process.on('exit', function() { - bench.end(N / 1e6); - }); - - bench.start(); - setImmediate(onNextTick); - function onNextTick() { - if (--n) - setImmediate(onNextTick); - } -} From ad7b98baa84172d1c6de1ed0a06be6aad9f6f3db Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 15 May 2017 20:24:43 +0200 Subject: [PATCH 5/7] build: don't print directory for GNUMake Currently when running make targets the directory is printed on some operating systems (Linux for example): $ make lint make[1]: Entering directory '/work/node' Running JS linter... ./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js,.md \ benchmark doc lib test tools make[1]: Leaving directory '/work/node' make[1]: Entering directory '/work/node' Running C++ linter... On other operating systems the directory is not printed. This commit suggests adding a flag to make this consistent for GNUMake by not printing the directory. PR-URL: https://github.com/nodejs/node/pull/13042 Reviewed-By: Gibson Fahnestock --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 9034037049b..a97c85fa714 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,7 @@ LOGLEVEL ?= silent OSTYPE := $(shell uname -s | tr '[A-Z]' '[a-z]') COVTESTS ?= test GTEST_FILTER ?= "*" +GNUMAKEFLAGS += --no-print-directory ifdef JOBS PARALLEL_ARGS = -j $(JOBS) From 5debcceafcdd73035d840f53deb931925691a3ab Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 15 May 2017 20:39:52 +0200 Subject: [PATCH 6/7] test: add hasCrypto to tls-wrap-event-emmiter Currently when building --without-ssl this test will report the following error: internal/util.js:82 throw new Error('Node.js is not compiled with openssl crypto support'); This commit adds a check for crypto and skips this test if node was built without ssl support. PR-URL: https://github.com/nodejs/node/pull/13041 Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca --- test/parallel/test-tls-wrap-event-emmiter.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-tls-wrap-event-emmiter.js b/test/parallel/test-tls-wrap-event-emmiter.js index b468304249e..417b21804a4 100644 --- a/test/parallel/test-tls-wrap-event-emmiter.js +++ b/test/parallel/test-tls-wrap-event-emmiter.js @@ -5,7 +5,11 @@ * Test checks if we get exception instead of runtime error */ -require('../common'); +const common = require('../common'); +if (!common.hasCrypto) { + common.skip('missing crypto'); + return; +} const assert = require('assert'); const TlsSocket = require('tls').TLSSocket; From 6342988053652e8ea1e3cd52e36d02926c2dea44 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 15 May 2017 12:58:21 +0800 Subject: [PATCH 7/7] build: clean up napi build in test-addons-clean PR-URL: https://github.com/nodejs/node/pull/13034 Ref: https://github.com/nodejs/node/issues/13031 Reviewed-By: Daniel Bevenius Reviewed-By: Gibson Fahnestock Reviewed-By: Refael Ackermann Reviewed-By: Michael Dawson Reviewed-By: Rajaram Gaunker --- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index a97c85fa714..94e6ff455cc 100644 --- a/Makefile +++ b/Makefile @@ -405,13 +405,18 @@ test-npm-publish: $(NODE_EXE) test-addons-napi: test-build-addons-napi $(PYTHON) tools/test.py --mode=release addons-napi +test-addons-napi-clean: + $(RM) -r test/addons-napi/*/build + $(RM) test/addons-napi/.buildstamp + test-addons: test-build test-addons-napi $(PYTHON) tools/test.py --mode=release addons test-addons-clean: - $(RM) -rf test/addons/??_*/ - $(RM) -rf test/addons/*/build + $(RM) -r test/addons/??_*/ + $(RM) -r test/addons/*/build $(RM) test/addons/.buildstamp test/addons/.docbuildstamp + $(MAKE) test-addons-napi-clean test-timers: $(MAKE) --directory=tools faketime @@ -977,6 +982,7 @@ endif test-addons \ test-addons-clean \ test-addons-napi \ + test-addons-napi-clean \ test-all \ test-ci \ test-ci-js \