Skip to content
This repository was archived by the owner on Oct 15, 2020. It is now read-only.

Conversation

@kunalspathak
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

core, test, chakrashim

Description of change

Sync nodejs/master at 24e4488 as of 2016-08-15

Last 2 commits needs review and include following changes
c35ccd2 - Added missing API ShouldThrowOnError() on PropertyCallbackInfo.
40d6b75 - Fixed build break in src\node.cc.

With these changes, there are 2 new unit test failures

  • parallel/test-repl-definecommand.js - There is a known issue around way repl prompt is displayed in node+chakracore. Expected is > and we display .... This issue is a TODO that will be investigated later.
  • parallel/test-vm-strict-mode.js - This is a new test added along with new v8 API. Currently chakrashim implementation of executing code in vm is based on proxy and it executes code in non-strict mode because of which it fails to throw strict-mode errors. I have added CHAKRA-TODO to see what we can do to fix these scenarios.

princejwesley and others added 30 commits July 27, 2016 21:26
```js
node 🙈 ₹ git:(upstream ⚡ display-error-repl) ./node
> var 4;
var 4;
    ^
SyntaxError: Unexpected number

>
```

PR-URL: nodejs/node#7589
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
- Specify that the ‘make test’ commands are Unix/OS X specific.
- Link to BUILDING.md for other platform commands.

Fixes: nodejs/node#7646
PR-URL: nodejs/node#7783
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs/node#7849
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: JungMinu - Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit attempts to address one of the items in #4641 which is
related to src/pipe_wrap.cc and src/tcp_wrap.cc. Currently both
pipe_wrap.cc and tcp_wrap.cc contain a class that are almost
identical. This commit extracts these parts into a separate class
that both can share.

PR-URL: nodejs/node#7501
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
CTC quorum rules were not in writing. There was an informal
understanding between CTC members. Document the rules to avoid
differences in interpretation.

PR-URL: nodejs/node#7813
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Provide example activities to better distinguish Collaborator activities
from CTC member activities.

PR-URL: nodejs/node#7744
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
The test takes 50 seconds on some of the project's Windows CI
infrastructure. Reducing the test repetitions from 50 to 20 trims that
to about 20 seconds. Tests will timeout at 60 seconds, so this helps
keep the test reliable. (There was a timeout on CI today when testing an
unrelated code change.)

Refs: nodejs/node#7827 (comment)
PR-URL: nodejs/node#7886
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: JungMinu - Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
PR-URL: nodejs/node#7885
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#7878
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
For nested timers with the same timeout, we can get into a situation
where we have recreated a timer list immediately before we need to
clean up an old timer list with the same key. Fix: make sure the list
to be deleted is the same instance as the list whose reference was used
to determine that a cleanup is necessary. If it's not the same instance,
a new list with the same key has been created, and it should not be
deleted.

Fixes: nodejs/node#7722

PR-URL: nodejs/node#7827
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
Node process will no longer terminate with an assertion if the
inspector port is not available.

PR-URL: nodejs/node#7874
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Competing timers were causing a race condition and thus the test was
flaky. Instead, we check an object property on process exit.

Fixes: nodejs/node#7650
PR-URL: nodejs/node#7857
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
On every platform but `Windows`. Also, print the crash information when
using the tap reporter.

PR-URL: nodejs/node#7859
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1. `process.env['PROCESSOR_ARCHITEW6432']` ->
   `process.env.PROCESSOR_ARCHITEW6432`.

2. `path.dirname(__filename)` -> `__dirname`.

3. `path.resolve(__dirname)` -> `__dirname`.

PR-URL: nodejs/node#7758
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: nodejs/node#7915
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The inspector tests weren't closing open libuv handles properly,
making valgrind complain.  Strengthen the uv_loop_close() check
while here.

With this commit applied:

    $ valgrind ./out/Release/cctest 2>&1 | grep 'total heap' | cut -c31-
    1,017 allocs, 1,017 frees, 21,695,456 bytes allocated

PR-URL: nodejs/node#7906
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make the inspector code easier to reason about by restructuring it
to avoid manual memory allocation and copying as much as possible.

An amusing side effect is that it reduces the total amount of memory
used in the test suite.

Before:

    $ valgrind ./out/Release/cctest 2>&1 | grep 'total heap' | cut -c31-
    1,017 allocs, 1,017 frees, 21,695,456 allocated

After:

    $ valgrind ./out/Release/cctest 2>&1 | grep 'total heap' | cut -c31-
    869 allocs, 869 frees, 14,484,641 bytes allocated

PR-URL: nodejs/node#7906
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Don't link binaries that run on the host system against liblog, it
breaks cross-compiling for android.

Fixes: nodejs/node#7731
PR-URL: nodejs/node#7762
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Single quotes in two of the examples were throwing off the
formatting of the path documentation on the Node.js website. This
commit expands two contractions to remove the offending quotes.

PR-URL: nodejs/node#7817
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#7927
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs/node#7900
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently the build-addons target is called as part of the
test target, and it has test/addons/.buildstamp as a dependency.
When running ./configure --debug and later switching/
updating branches you can be in a situation where the config.gypi
files in the addons build directories are using an incorrect
value for default_configuration. Currently this can happen and you
will get test errors as there are a few test that depend on the
value of default_configuration to be Release.

Ben Noordhuis provided the solution for this by adding a dependency to
config.gypi, which is generated when running ./configure, and will
correct this situation.

This commit is related to #7860. Please see the dicussion in that
issue regarding the test using hard-coded paths.

PR-URL: nodejs/node#7893
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Add language identifying a request for voluntary resignation as the
typical mechanism for addressing inactive CTC members.

PR-URL: nodejs/node#7720
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#7571
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
PR-URL: nodejs/node#7570
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
closes #7366

PR-URL: nodejs/node#7390
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit allows setupMaster() to configure the stdio channels
for worker processes.

Refs: nodejs/node-v0.x-archive#5727
Refs: nodejs/node#7811
PR-URL: nodejs/node#7838
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
uv_close() is an asynchronous operation.  Calling it on a data member
inside the destructor is unsound because its memory is about to be
reclaimed but libuv is not done with it yet.

PR-URL: nodejs/node#7907
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@chromium.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
When SIMD is enabled, `util.format` couldn’t display objects
(with at least 1 key) because the formatter function got
overridden.

PR-URL: nodejs/node#7864
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Set the `SIGUSR2` handler before spawning the child process to make sure
the signal is always handled.

Fixes: nodejs/node#7767
PR-URL: nodejs/node#7854
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 and others added 23 commits August 10, 2016 23:41
Refs: nodejs/node#1771
Refs: nodejs/node#6456
Refs: nodejs/node#6773
Refs: nodejs/node#7743
PR-URL: nodejs/node#6816
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#8062
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs/node#7933
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fix parallel/test-util-sigint-watchdog by polling until the
signal has definitely been received instead of just using a timeout.

Fixes: nodejs/node#7919
PR-URL: nodejs/node#7933
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Pick up an upstream bugfix for https://crbug.com/621926 and bump V8
version to 5.1.281.80.

Original commit message for 588e15c:
    Fixes a bug in cmpw.

    The opcodes for 'cmpw r/m16, r16' and 'cmpw r16, r/m16' were
    swapped, causing a few issues when less than/greater than
    comparison were performed.

    Adds a regression test.

    BUG=621926

    Committed: https://crrev.com/efa7095e3e360fbadbe909d831ac11b268ca26b0
    Review-Url: https://codereview.chromium.org/2103713003
    Cr-Original-Commit-Position: refs/heads/master@{#37339}
    Cr-Commit-Position: refs/heads/master@{#37345}

Original commit message for c0d4bb8:
    Fixes a wrong use of Operand in a test.

    Operand(reg) -> reg
    Operand(reg, 0) -> [reg]

    BUG=

    Review-Url: https://codereview.chromium.org/2111503002
    Cr-Commit-Position: refs/heads/master@{#37370}

PR-URL: nodejs/node#8038
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
We now have adequate AIX hardware to add AIX to
the regular regression runs.

However, there are a couple of failing tests even
though AIX was green at one point.  This PR
marks those tests as flaky so that we can add AIX
so that we can spot any new regressions without making
the builds RED

The tests are being worked under the following PRs

- being worked under nodejs/node#7564
test-async-wrap-post-did-throw
test-async-wrap-throw-from-callback
test-crypto-random

- being worked under nodejs/node#7973
test-stdio-closed

- covered by nodejs/node#3796
test-debug-signal-cluster

PR-URL: nodejs/node#8065
Reviewed-By: joaocgreis - João Reis <reis@janeasystems.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Pick up latest from [1] corresponding to the Blink commit 62cd277.

[1]: pavelfeldman/v8_inspector@e6b8355

PR-URL: nodejs/node#8014
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
When node is running with --inspect flag, default console.log,
console.warn and other methods call inspector console methods in
addition to current behaviour (dump formatted message to stderr and
stdout). Inspector console methods forward message to DevTools and
show up in DevTools Console with DevTools formatters. Inspector
console methods not present on Node console will be added into it.

Only own methods on global.console object will be changed while in a
debugging session. User are still able to redefine it, use
console.Console or change original methods on Console.prototype.

PR-URL: nodejs/node#7988
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
This reverts parts of nodejs/node@b488b19
restoring javascript implementation of realpath and realpathSync.

Fixes: nodejs/node#7175
Fixes: nodejs/node#6861
Fixes: nodejs/node#7294
Fixes: nodejs/node#7192
Fixes: nodejs/node#7044
Fixes: nodejs/node#6624
Fixes: nodejs/node#6978
PR-URL: nodejs/node#7899
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The benchmarks included also work for the previous JS
implementation of fs.realpath(). In case the new implementation of
realpath() needs to be reverted, we want these changes to stick around.

PR-URL: nodejs/node#7899
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
I noticed some typos and the lack of {} following an if.

PR-URL: nodejs/node#7961
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Node documentation recommends using process.exitCode = x and returning
as a way to exit.

PR-URL: nodejs/node#7961
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Added the option of using --format csv when outputting data.

Fixes: nodejs/node#7890
PR-URL: nodejs/node#7961
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Visual Studio 2013 is no longer supported.  Update the Windows build
prerequisites.

Refs: nodejs/node#7484
PR-URL: nodejs/node#8049
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
`POST_STATUS_TO_PR` previously did not work.

Now it works.

Update the onboarding documentation accordingly.

PR-URL: nodejs/node#8059
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs/node#8060
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#8061
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#8088
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
`\n` is not enough for Linux with some custom stream
add carriage returns to ensure that the output is displayed correctly
using `\r\n` should not be a problem, even on non-Windows platforms.

Fixes: nodejs/node#7954
PR-URL: nodejs/node#8028
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/8068.patch
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
There are currently 17 instances of the spread operator and all of them
have no space between the operator and the subsequent argument. This
change enables a lint rule to enforce that same style on any future uses
of the spread operator.

Refs: https://github.com/nodejs/node/pull/6573/files/7a1b47f329f2e6481ef8f54951570197fd98378d#r74479351
PR-URL: nodejs/node#8073
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
To output exception in DevTools console method exceptionThrown should
be called on uncaught exception on V8Inspector object. Additionally
we need to wait disconnect to provide user way to inspect exception.

PR-URL: nodejs/node#8043
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
querystring subsystem docs referred to `obj.hashOwnProperty()`
which is non-existent. Corrected to `obj.hasOwnProperty()`.

PR-URL: nodejs/node#8107
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@kunalspathak
Copy link
Member Author

FYI - @nodejs/node-chakracore

@agarwal-sandeep , please review.

@agarwal-sandeep
Copy link
Contributor

Last 2 commits looks good to me and reset is a simple merge of node. Signoff.

Sync nodejs/master at 24e4488 as of 2016-08-15

PR-URL: nodejs#108
Reviewed-By: Sandeep Agarwal <agarwal.sandeep@microsoft.com>
PR-URL: nodejs#108
Reviewed-By: Sandeep Agarwal <agarwal.sandeep@microsoft.com>
`StartInspector` should return `bool` but there was signature
mismatch if not building for v8 platform i.e.
`!NODE_USE_V8_PLATFORM`.

I have submitted PR nodejs/node#8114 to fix this in
node repo.

PR-URL: nodejs#108
Reviewed-By: Sandeep Agarwal <agarwal.sandeep@microsoft.com>
@kunalspathak kunalspathak merged commit a606f84 into nodejs:chakracore-master Aug 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.