Please ignore this msg #1
Merged
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
It's likely that anyone using `process.report.getReport()` will be processing the return value thereafter (e.g., filtering fields or redacting secrets). This change eliminates boilerplate by calling `JSON.parse()` on the return value. Also modified the `validateContent()` and `validate()` test helpers in `test/common/report.js` to be somewhat more obvious and helpful. Of note, a report failing validation will now be easier (though still not _easy_) to read when prepended to the stack trace. - Refs: nodejs/diagnostics#315 PR-URL: #28630 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #28611 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
NodeMainInstance::Create will now returrn an instance of NodeMainInstance in a unique_ptr. PR-URL: #28577 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #28282 Fixes: #28114 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Any use of --debug, --debug=, --debug-brk, or --debug-brk= now triggers an error. That means we can eliminate their aliases with --inspect counterparts and simplify the code. PR-URL: #28615 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
The deprecations documentation links to the GitHub issue tracker in several places. This commit makes the text around those links consistent. PR-URL: #28617 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This commit defines a named constant instead of using a mix of 2 ** 16 and 0x10000 throughout the code. PR-URL: #28638 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
This commit removes an IIFE in the readline SIGCONT handler that was previously being used to bind `this`. PR-URL: #28639 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
The non-ICU-based isFullWidthCodePoint() can be simplified to a single `return` statement. This commit removes the extra branching logic. PR-URL: #28640 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: #28246 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This commit adds an optional callback to clearScreenDown(), which is passed to the stream's write() method. It also exposes the return value of write(). PR-URL: #28641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Resolving a path against root with `path.relative()` should not include a trailing slash. Fixes: #28549 PR-URL: #28556 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]>
This code branch only makes sense when i === length. Otherwise it'll already be handled. PR-URL: #28556 Fixes: #28549 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Weijia Wang <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]>
Everything under process.report is experimental. This commit adds the missing stability index entries. PR-URL: #28653 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
The various TSFN APIs are marked as stable, but the TSFN heading itself is still marked as experimental. PR-URL: #28643 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
This is an approach to address the issue linked below. Previously, when `.write()` and `.flush()` calls to a zlib stream were interleaved synchronously (i.e. without waiting for these operations to finish), multiple flush calls would have been coalesced into a single flushing operation. This patch changes behaviour so that each `.flush()` all corresponds to one flushing operation on the underlying zlib resource, and the order of operations is as if the `.flush()` call were a `.write()` call. One test had to be removed because it specifically tested the previous behaviour. As a drive-by fix, this also makes sure that all flush callbacks are called. Previously, that was not the case. Fixes: #28478 PR-URL: #28520 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Otherwise there’s a memory leak left by the context when the Isolate tears down without having run the weak callback. PR-URL: #28631 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Includes support for bigint syntax so we can remove the acorn-bigint plugin. PR-URL: #28649 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]>
This helps the JS engine have a better understanding of the memory situation in HTTP/2-heavy applications, and avoids situations that behave like memory leaks due to previous underestimation of memory usage which is tied to JS objects. Refs: #28088 (comment) PR-URL: #28645 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
This adds the coverage directory to the .gitignore file. PR-URL: #28626 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Masashi Hirano <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Some libraries do not exist on IBM i (OS400). Commit 417c18e introduces these missing libraries. Need to differentiate `AIX` and `OS400`(IBM i). PR-URL: #28607 Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Some pty tests persistently hung on the AIX CI buildbots. Fix that by adding a helper script that properly sets up the pty before spawning the script under test. On investigation I discovered that the test runner hung when it tried to close the slave pty's file descriptor, probably due to a bug in AIX's pty implementation. I could reproduce it with a short C program. The test runner also leaked file descriptors to the child process. I couldn't convince python's `subprocess.Popen()` to do what I wanted it to do so I opted to move the logic to a helper script that can do fork/setsid/etc. without having to worry about stomping on state in tools/test.py. In the process I also uncovered some bugs in the pty module of the python distro that ships with macOS 10.14, leading me to reimplement a sizable chunk of the functionality of that module. And last but not least, of course there are differences between ptys on different platforms and the helper script has to paper over that. Of course. Really, this commit took me longer to put together than I care to admit. Caveat emptor: this commit takes the hacky ^D feeding to the slave out of tools/test.py and puts it in the *.in input files. You can also feed other control characters to tests, like ^C or ^Z, simply by inserting them into the corresponding input file. I think that's nice. Fixes: nodejs/build#1820 Fixes: #28489 PR-URL: #28600 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Now we are using `pipes` and `pipesCount` in Readable state and the `pipes` value can be a stream or an array of streams. This change reducing them into one `pipes` value, which is an array of streams. PR-URL: #28583 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
A few #defines in src/node.h had inconsistent spacing and tabbing. This commit changes the spacing to be the same style as the rest of the project. PR-URL: #28547 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Support for VTune profiling was added in commit a881b53 from November 2015 but has since bitrotted. Remove it. Fixes: #28310 Refs: #3785 PR-URL: #28522 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
This allows embedders to run `node::options_parser::Parse` for a `node::DebugOptions`. PR-URL: #28543 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This test imposes a limit on the average bytes of space per chunk for network traffic. However this number depends on VM implementation details, and upcoming changes to V8's array buffer management require a small bump to this limit in this test. PR-URL: #28492 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #20077 Fixes: #20107 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #28548 Refs: #445 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #28585 Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #29029 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
... before trying to valueOf them PR-URL: #29029 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #28974 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
* Use `console.error()` for error or stderr output. * Unify comment style. * Unify link format. * Correct link URL. * Fix some typos. PR-URL: #29024 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #28956 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anto Aravinth <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: #28989 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: #29019 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: #27700 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Inline and simplify onwritedrain. Also remove comment that seems to be outdated/invalid. PR-URL: #29037 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: #29018 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
One skipped test remains, it creates very large Buffer objects, triggering the AIX OOM to kill node and its parent processes. See: nodejs/build#1849 (comment) PR-URL: #29054 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Richard Lau <[email protected]>
The first argument to lookupService() should be an IP address, and is named "address" in the documentation. This commit updates the code to match the documentation and provide less confusing errors. PR-URL: #29040 Fixes: #29039 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Tests can leave processes running blocking the tmpdir. This does not yet prevent tests from doing that, but prevents failures on subsequent tests. PR-URL: #28858 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #28858 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #28858 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Many benchmarks use test/common/tmpdir. This changes 3 benchmarks that use NODE_TMPDIR to also use test/common/tmpdir. This is necessary in preparation for the next commit that changes tmpdir to delete tmpdir.path when the Node.js process exits. Thus, if multiple benchmarks are run sequentially, the ones that use tmpdir will remove the directory and the ones changed here would fail because it does not exist. This happens when running test/benchmark. Note: to explicitly select a directory for tmpdir, use NODE_TEST_DIR. PR-URL: #28858 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #28858 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Notable changes: - UV_FS_O_FILEMAP has been added for faster access to memory mapped files on Windows. - uv_fs_mkdir() now returns UV_EINVAL for invalid filenames on Windows. It previously returned UV_ENOENT. - The uv_fs_statfs() API has been added. - The uv_os_environ() and uv_os_free_environ() APIs have been added. Fixes: #28599 Fixes: #28945 Fixes: #29008 PR-URL: #29070 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jiawen Geng <[email protected]>
This commit simplifies the diagnostic report's code for listing environment variables by using uv_os_environ() instead of platform specific code. PR-URL: #28963 Reviewed-By: Saúl Ibarra Corretgé <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
`socketOnDrain()` performs the exact same check and doesn't return anything. PR-URL: #29022 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Rich Trott <[email protected]>
In the other .cc files in the project, includes are in alphabetical order, with local files first, and libraries after. However, inspector_profiler.cc has a library declared in the middle of the import order, and v8 is the second to last being imported, instead of the last. So I reordered the imports and testing showed no side effects; everything passed. PR-URL: #29073 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]>
PR-URL: #28949 Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Jan Krems <[email protected]>
The description for the --link-module configuration option is as
follows:
$ ./configure --help | grep -A 5 'link-module'
--link-module=LINKED_MODULE
Path to a JS file to be bundled in the binary as a
builtin. This module will be referenced by path
without extension; e.g. /root/x/y.js will be
referenced via require('root/x/y'). Can be used
multiple times
This lead me to think that it was possible to specify a file like this:
$ ./configure --link-module=something.js
$ NODE_DEBUG=mkcodecache make -j8
This will lead to a compilation error as an entry in the source_ map in
node_javascript.cc will end up having an empty string as its key:
source_.emplace("", UnionBytes{_raw, 105});
This will then be used by CodeCacheBuilder when it iterates over the
module ids, which will lead to the following compilation errors:
/node/out/Release/obj/gen/node_code_cache.cc:12:23: warning:
ISO C++17 does not allow a decomposition group to be
empty [-Wempty-decomposition]
static const uint8_t [] = {
^
/node/out/Release/obj/gen/node_code_cache.cc:12:22: warning:
decomposition declarations are a C++17 extension [-Wc++17-extensions]
static const uint8_t [] = {
^~
/node/out/Release/obj/gen/node_code_cache.cc:12:1: error:
decomposition declaration cannot be declared 'static'
static const uint8_t [] = {
^~~~~~
/node/out/Release/obj/gen/node_code_cache.cc:12:22: error:
decomposition declaration cannot be declared with type 'const uint8_t'
(aka 'const unsigned char'); declared type must be 'auto' or
reference to 'auto'
static const uint8_t [] = {
^
/node/out/Release/obj/gen/node_code_cache.cc:12:22: error:
excess elements in scalar initializer
static const uint8_t [] = {
^
/node/out/Release/obj/gen/node_code_cache.cc:660:7: error:
expected expression
,
^
/node/out/Release/obj/gen/node_code_cache.cc:661:24: error:
no matching function for call to 'arraysize'
static_cast<int>(arraysize()), policy
^~~~~~~~~
../src/util.h:667:18: note: candidate function template not viable:
requires 1 argument, but 0 were provided
constexpr size_t arraysize(const T (&)[N]) {
^
2 warnings and 5 errors generated.
This commit suggests that passing a single file be allowed by modifying
tools/js2c.py.
PR-URL: #28443
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
`conn.destroyed` is guaranteed to be `false` because a previous `if` statement already handles the case where `conn && conn.destroyed` evaluates to `true` returning `false` in that case. PR-URL: #29078 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Clarifies that creating multiple async iterators from the same stream can lead to event listener leak. PR-URL: #28997 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: #29075 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #29077 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: #29076 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
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.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes