[v8.x backport] build: make lint-ci work properly on Linux make #21261
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds the optional options argument to `http.createServer()`. It contains two options: the `IncomingMessage` and `ServerReponse` option. Backport-PR-URL: nodejs#20456 PR-URL: nodejs#15752 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
This adds the Http1IncomingMessage and Http1ServerReponse options to http2.createServer(). Backport-PR-URL: nodejs#20456 PR-URL: nodejs#15752 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
Add optional Http2ServerRequest and Http2ServerResponse options to createServer and createSecureServer. Allows custom req & res classes that extend the default ones to be used without overriding the prototype. Backport-PR-URL: nodejs#20456 PR-URL: nodejs#15560 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Backport-PR-URL: nodejs#20456 PR-URL: nodejs#18872 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
When configure with --debug-http2 --debug-nghttp2 the following
compilation error is generated:
DEBUG_HTTP2SESSION2(this, "fatal error receiving data: %d", ret);
^
../src/node_http2.cc:1690:27:
error: invalid use of 'this' outside of a non-static member function
1 errors generated.
OnStreamReadImpl is static and I think the intention was to pass in the
session variable here.
PR-URL: nodejs#20815
Refs: nodejs#20806
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
by using package-lock.json PR-URL: nodejs#16945 Fixes: nodejs#16628 Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: James M Snell <[email protected]>
The error check doesn't matter because a failure would be ignored as part of the loop condition. PR-URL: nodejs#16950 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
Allow the user to specify the filepath for the trace_events log file using a template string. Backport-PR-URL: nodejs#19145 PR-URL: nodejs#18480 Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Before these changes, only V8 added postmortem metadata to Node's binary, limiting the possibilities for debugger's developers to add some features that rely on investigating Node's internal structures. These changes are first steps towards empowering debug tools to navigate Node's internal structures. One example of what can be achieved with this is shown at nodejs/llnode#122 (a command which prints information about handles and requests on the queue for a core dump file). Node postmortem metadata are prefixed with nodedbg_. This also adds tests to validate if all postmortem metadata are calculated correctly, plus some documentation on what is postmortem metadata and a few care to be taken to avoid breaking it. Ref: nodejs/llnode#122 Ref: nodejs/post-mortem#46 Backport-PR-URL: nodejs#19176 PR-URL: nodejs#14901 Refs: nodejs/post-mortem#46 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Backport-PR-URL: nodejs#19176 PR-URL: nodejs#18530 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
Redefining private breaks any private inheritance in the included files. We can simply declare GenDebugSymbols() as friends in related classes to gain the access that we need. Backport-PR-URL: nodejs#19176 PR-URL: nodejs#18653 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Add a errno -> [error code, uv error message] map to the uv binding so the error message can be assembled in the JS layer. Backport-PR-URL: nodejs#19191 PR-URL: nodejs#17338 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Reimplement uv.errname() as internal/util.getSystemErrorName() to avoid the memory leaks caused by unknown error codes and avoid calling into C++ for the error names. Also expose it as a public API for external use. Backport-PR-URL: nodejs#19191 PR-URL: nodejs#18186 Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Backport-PR-URL: nodejs#19191 PR-URL: nodejs#18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
Backport-PR-URL: nodejs#19191 PR-URL: nodejs#18546 Reviewed-By: James M Snell <[email protected]>
This commit moves error creation helpers scattered around under lib/ into lib/internal/errors.js in the hope of being clearer about the differences of errors that we throw into the user land. - Move util._errnoException and util._exceptionWithHostPort into internal/errors.js and simplify their logic so it's clearer what the properties these helpers create. - Move the errnoException helper in dns.js to internal/errors.js into internal/errors.js and rename it to dnsException. Simplify it's logic so it no longer calls errnoException and skips the unnecessary argument checks. Backport-PR-URL: nodejs#19191 PR-URL: nodejs#18546 Reviewed-By: James M Snell <[email protected]>
A error message should always be non-enumerable. This makes sure that is true for dns errors as well. It also adds another check in `common.expectsError` to make sure no other regressions are introduced going forward. Fixes nodejs#19716 Backport-PR-URL: nodejs#19191 PR-URL: nodejs#19719 Fixes: nodejs#19716 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>
Currently this test will overwrite the clientOpts object with the port,
instead of setting the port property on the clientOpts object which
looks like the original intent.
Doing this the test fails reporting that the fake-cnnic-root-cert has
expired. This is indeed true:
$ openssl x509 -in test/fixtures/keys/fake-cnnic-root-cert.pem \
-text -noout
Certificate:
...
Validity
Not Before: Jun 9 17:15:16 2015 GMT
Not After : Mar 29 17:15:16 2018 GMT
This commit sets the errorCode to CERT_HAS_EXPIRED. I tried updating the
certificate using test/fixtures/keys/Makefile but then no error is
thrown and I'm currently looking into this.
Backport-PR-URL: nodejs#20776
PR-URL: nodejs#19767
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
I looks like this test has not worked as expected since commit 2bc7841 ("test: use random ports where possible"). The test in that commit checked for `CERT_REVOKED` which was returned by CheckWhitelistedServerCert. CheckWhitelistedServerCert was later removed in commit 6ee4228 ("src: drop CNNIC+StartCom certificate whitelisting"). I'm suggesting that this test case be removed as I don't think it is valid anymore. Backport-PR-URL: nodejs#20776 PR-URL: nodejs#19767 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
I've not been able to find any reason for calling BIO_set_shutdown(bio, 1). This is done by default for the following versions of OpenSSL: https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/ crypto/bio/bio_lib.c#L26 https://github.com/openssl/openssl/blob/OpenSSL_1_0_1/ crypto/bio/bio_lib.c#L90 https://github.com/openssl/openssl/blob/OpenSSL_1_0_2/ crypto/bio/bio_lib.c#L88 https://github.com/openssl/openssl/blob/OpenSSL_1_0_0/ crypto/bio/bio_lib.c#L90 This commit removes the call and the comment. PR-URL: nodejs#17542 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: nodejs#17587 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
`tls.Socket` does not exist, and the deprecation message should refer to `tls.TLSSocket` (like the documentation for the deprecation message already does). PR-URL: nodejs#17561 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs#17432 Fixes: nodejs#17430 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#17558 Fixes: nodejs#17540 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]>
Percent-encoded additional characters in fragment state with new FRAGMENT_ENCODE_SET lookup table. The fragment percent-encode set includes the C0 control percent-encode set and code points U+0020, U+0022, U+003C, U+003E, and U+0060. PR-URL: nodejs#17627 Fixes: nodejs#17540 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit adds a test to validate postmortem debugging metadata. When this test runs, it can check for the presence of metadata constants used by tools such as llnode and mdb and report if any have accidentally been removed. PR-URL: nodejs#17685 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Is safer to use a `process.binding(config)` defined boolean, than to regex on `process.execArgv`. Also, this better falls in line with the conventions of checking flags passed to the executable. PR-URL: nodejs#17814 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Use an interval to keep the event loop open so the test does not exit before receiving all signals fom asynchronous `exec()` calls. PR-URL: nodejs#17827 Fixes: nodejs#14070 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
PR-URL: nodejs#17939 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#17939 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: James M Snell <[email protected]>
Since these are executing JS code, and in particular parts of that code may be provided by userland, handle such exceptions in C++. Refs: nodejs#17938 (comment) PR-URL: nodejs#18028 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
This should make these function calls a lot more intuitive for people who are more accustomed to Node’s EventEmitter API. PR-URL: nodejs#17701 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
I mistakenly introduced user defined variables using uppercase characters, reading the gyp documentation they state: "Predefined variables. By convention, these are named with CAPITAL_LETTERS. Predefined variables are set automatically by GYP" and also "By convention, user-defined variables are named with lowercase_letters." This commit renames the user defined variables to lowercase to follow the above mentioned convention. Backport-PR-URL: nodejs#20797 PR-URL: nodejs#16238 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Currently the cctest target depend on the node_core_target_name target. But it is the node_lib_target_name target that compiles the sources now which means that if a source file in src is updated the cctest executable will not be re-linked against it, but will remain unchanged. The code will still be compiled, just not linked which means that if you are debugging you'll not see the changes and also a warning will be displayed about this issue. This commit changes the cctest target to depend on node_lib_target_name. Backport-PR-URL: nodejs#20797 PR-URL: nodejs#18576 Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: Yihong Wang <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Use template helpers instead of `#define`s to generate the raw C callbacks that are passed to the HTTP parser library. A nice effect of this is that it is more obvious what parameters the `Parser` methods take. PR-URL: nodejs#18133 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
Instead of providing a separate class for keeping the parser alive during its own call back, just delay a possible `.close()` call until the stack has cleared completely. PR-URL: nodejs#18135 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Original commit message:
Introduce ScriptOrModule and HostDefinedOptions
This patch introduces a new container type ScriptOrModule which
provides the name and the host defined options of the script/module.
This patch also introduces a new PrimitivesArray that can hold
Primitive values, which the embedder can use to store metadata.
The HostDefinedOptions is passed to V8 through the ScriptOrigin, and
passed back to the embedder through HostImportModuleDynamically for
module loading.
Bug: v8:5785, v8:6658, v8:6683
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I56c26fc9a680b273ac0a6691e5ad75f15b8dc80a
Reviewed-on: https://chromium-review.googlesource.com/622158
Reviewed-by: Adam Klein <[email protected]>
Reviewed-by: Georg Neis <[email protected]>
Commit-Queue: Sathya Gunasekaran <[email protected]>
Cr-Commit-Position: refs/heads/master@{nodejs#47724}
Backport-PR-URL: nodejs#17823
PR-URL: nodejs#16889
Refs: v8/v8@dbfe4a4
Refs: nodejs#15713
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
This is an initial implementation to support dynamic import in both scripts and modules. It's off by default since support for dynamic import is still flagged in V8. Without setting the V8 flag, this code won't be executed. This initial version does not support importing into vm contexts. Backport-PR-URL: nodejs#17823 PR-URL: nodejs#15713 Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
currently if you want to use dynamic import you must use both the `--experimental-modules` and the `--harmony-dynamic-imports` flags. Chrome is currently shipping dynamic import unflagged, the flag only remains in V8 to guard embedders who have not set the appropriate callback from throwing an unhandled rejection when the feature is used. As such it is reasonable to enable the flag by default for `--experimental-modules` Backport-PR-URL: nodejs#17823 PR-URL: nodejs#18387 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: nodejs#7843 PR-URL: nodejs#16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#18445 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]>
In `Writable.prototype.end()`, `state.ending` is true after calling `endWritable()` and it doesn't reset to false. In `Writable.prototype.uncork()`, `state.finished` must be false if `state.bufferedRequest` is true. PR-URL: nodejs#18145 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
`state.corkedRequestsFree` of a writable stream is always not null. PR-URL: nodejs#18145 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Take n-api out of experimental as per: nodejs/TSC#501 Backport-PR-URL: nodejs#21083 PR-URL: nodejs#19262 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs#19746 Reviewed-By: Joyee Cheung <[email protected]>
nodejs-github-bot
added
build
richardlau
approved these changes
Jun 11, 2018
|
Customary full CI run: https://ci.nodejs.org/job/node-test-pull-request/15398/ |
|
Optimistic Rebuild: https://ci.nodejs.org/job/node-test-pull-request/15441/ |
|
Single Windows CI failure was a platform glitch, but IMHO we have enough coverage from other platforms. |
refack
approved these changes
Jun 13, 2018
MylesBorins
force-pushed
the
v8.x-staging
branch
from
084ef60
to
859dc64
Compare
|
landed in f493001 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original PR: #19746
/cc @nodejs/build @rvagg @MylesBorins