Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v8.9.4 proposal #17772

Closed
wants to merge 282 commits into from
Closed

v8.9.4 proposal #17772

wants to merge 282 commits into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Dec 20, 2017

Commits

verwaest and others added 30 commits December 19, 2017 19:16
Currently the template for the global object is explicitly marked as
hidden. This is unnecessary since the global object is automatically
marked as hidden by V8. Getting rid of this call gets rid of the last
use of SetHiddenPrototype.

PR-URL: #16554
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: #16863
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
`test-inspector-async-hook-setup-at-signal` is also flaky on VS2017

PR-URL: #16941
Reviewed-By: Colin Ihrig [email protected]
Reviewed-By: Gireesh Punathil [email protected]
PR-URL: #16951
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Tests a basic WebAssembly module that adds two numbers.
wasm example from the WebAssembly/wabt repo licensed
Apache 2.0.

Refs: https://github.com/WebAssembly/wabt/blob/49b7984544ddaf14d5e2f1ad9115dad7e9a2b299/demo/wat2wasm/examples.js#L27-L32
PR-URL: #16760
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Currenlty the test target will echo additional information that might
not be that useful, for example:
make doc-only
make[1]: Nothing to be done for `doc-only'.
make lint
Running JS linter...
Running C++ linter...
Total errors found: 0
make[2]: Nothing to be done for `lint-md'.
Running C++ linter on addon docs...
Total errors found: 0
make cctest

This commit suggests reducing this to:
make -s doc-only
make -s lint
Running JS linter...
Running C++ linter...
Running C++ linter on addon docs...
Total errors found: 0
make -s cctest

PR-URL: #17010
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
The motivation for this commit is to enable projects embedding Node.js
and building with --enable-static to be able to run the test suite and
linter.

Currently when building with --enable-static no node executable
will be created which means that the tests (apart from the cctest) and
linter cannot be run.

This is currently a work in progress and works on MacOS but I need to
run the CI, and manually on different environments to verify that it
works as expected.

PR-URL: #14986
Refs: #14158
Refs: #14892
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
The Cpp style guide is growing. IMHO, a Table of Contents makes
it easier to navigate.

PR-URL: #17052
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Remove commented-out code that is leftover from a refactoring.

PR-URL: #17023
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Lance Ball <[email protected]>
Make a REPL tab completion test pass in coverage mode by using
`Uin` as the common prefix of all `Uint*Array` globals instead
of `co` which could be a prefix for `console` and `coverage`,
so it doesn't expand the way it's expected to in coverage mode.

PR-URL: #17082
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Depending on the allocator, existing code leaks memory.

PR-URL: #16898
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Currently the cctest target will fail on linux when configured
--with-dtrace:

/node-v9.2.0/out/Release/obj.target/node/src/node_dtrace.o:
In function `node::DTRACE_NET_SERVER_CONNECTION(
    v8::FunctionCallbackInfo<v8::Value> const&)':
node_dtrace.cc:(.text+0x103): undefined reference to
`node_net__server__connection_semaphore'
/node-v9.2.0/out/Release/obj.target/node/src/node_dtrace.o:
In function `node::DTRACE_NET_STREAM_END(
    v8::FunctionCallbackInfo<v8::Value> const&)':
...

This is because node_dtrace_provider.o is not linked by the cctest
target.

This commit tries to fix and simplify the conditions in cctest target
so that node_dtrace.o is included for all operating systems that support
dtrace, include node_dtrace_ustack.o for all operating systems except
mac and linux, and include node_dtrace_provider.o for all operating
systems except mac.

PR-URL: #17039
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Use an std::unique_ptr for variables that are deleted
right after creation.

Since the destructor of InspectorTimer is private
but needed by the unique_ptr, define deleter_type as friend.

PR-URL: #17020
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #16923
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: #16661
Fixes: #16650
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: #16699
Fixes: #13824
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Fixes: #13471
PR-URL: #17008
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Hitesh Kanwathirtha <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Add a Support section, borrowing heavily from wp-cli project.

Move stuff about contributing to Node.js to the bottom as vastly more
users are interested in using Node.js and getting help with Node.js than
contributing to Node.js. Information still belongs, just not at the top.
(Many people will know to look in CONTRIBUTING.md anyway.)

PR-URL: #16533
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: #17108
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #16703
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
test-https-server-keep-alive-timeout relies on server timeouts and
whatnot that will be inherently unreliable on a busy host. The test
fails when run with a high `-j` value and higher `--repeat` value passed
to `tools/test.py`. Move the test to `sequential`.

PR-URL: #16775
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Use a unique file name for each benchmark. Running benchmarks
simultaneously may be a bit of an unusual use case, but there are use
cases, such as stress testing `test/parallel/test-benchmark-fs.js`.

PR-URL: #16776
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
* Add sections about first time contributions, code reviews
  and seeking consensus, waiting for approvals, testing and CI
* Move paragraphs to more suitable sections
* Update table of contents
* Document the fast-tracking process

PR-URL: #17056
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
PR-URL: #17070
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Lance Ball <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
PR-URL: #17049
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #17048
Fixes: #17043
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Although the primary use-case for the cluster module is networking, the
module provides a generic master/worker interface that could also be
used if you dont use networking at all. Currently the docs are a bit
ambiguous about this as only the primary use-case is ever mentioned,
this remark should clarify that the cluster module can also be used
without disadvantages if you dont use networking.

PR-URL: #17031
Refs: nodejs/help#970
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: #16996
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
danbev and others added 17 commits December 20, 2017 01:16
This commit removes the include of node.h which does not seem to be
used.

PR-URL: #17546
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #17522
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
PR-URL: #17553
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
autocannon appears to have trouble recognizing URLs that contain true
or false within them. Use 0 or 1 instead to represent the same.

PR-URL: #17583
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Arrow functions cannot be called with the new keyword,
convert to ES6 classes instead.

PR-URL: #17579
Refs: #17364
Reviewed-By: Rich Trott <[email protected]>
- Remove redundant `hasCrypto` checks
- Use `common.mustCall()`
- Use arrow functions
- Deduplicate HTTP & HTTPS code

PR-URL: #17562
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
PR-URL: #17572
Reviewed-By: Anatoli Papirovski <[email protected]>
Add a comment explaining the test (especailly why it forks 80 processes.
Use destructuring and an arrow function callback.

PR-URL: #17596
Reviewed-By: Khaidi Chu <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Use common/fixtures module instead of common.fixturesDir.

PR-URL: #17400
Backport-PR-URL: #17770
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Lance Ball <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Remove common.fixturesDir. All tests now use the the common/fixtures
module instead.

PR-URL: #17400
Backport-PR-URL: #17770
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Lance Ball <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Remove the practice of starting most error descriptions with "Used when"
or wordier variations.

Change errors of the form:

> Used when the type of an asynchronous resource is invalid.

...to:

> The type of an asynchronous resource was invalid.

Change errors of the form:

> The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on
> a given stream is attempted to move to a specified row without a
> specified column.

...to:

> A cursor on a given stream cannot be moved to a specified row without
> a specified column.

PR-URL: #16954
Backport-PR-URL: #17767
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
This is mostly an excuse to remind people that it's "V8" and not "v8"
when referring to the JavaScript engine.

PR-URL: #17163
Backport-PR-URL: #17765
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
PR-URL: #17163
Backport-PR-URL: #17765
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
PR-URL: #17163
Backport-PR-URL: #17765
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. openssl Issues and PRs related to the OpenSSL dependency. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. v8.x labels Dec 20, 2017
@gibfahn
Copy link
Member Author

gibfahn commented Dec 20, 2017

D'oh, wrong branch.

@gibfahn gibfahn closed this Dec 20, 2017
gibfahn added a commit that referenced this pull request Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. openssl Issues and PRs related to the OpenSSL dependency. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.