[v16.x backport] src: pass only Isolate* and env_vars to EnabledDebugList::Parse()
#44070
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
|
Review requested:
|
nodejs-github-bot
added
c++
This comment was marked as outdated.
This comment was marked as outdated.
|
Thanks. Don't worry too much if there are CI errors. I pushed a new batch of commits today and they were only tested on my mac |
This allows to support timestamps before 1970-01-01. On Windows, it's not supported due to Y2038 issue. PR-URL: nodejs#43714 Fixes: nodejs#43707 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#43881 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs#43872 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Offer additional meta-data for building custom and additional behaviour on top of parseArgs. PR-URL: nodejs#43459 Reviewed-By: Ben Coe <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#43886 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
This issue has been described in - nodejs#43860 On Windows system, git clone or git checkout on the repo turns LF line endings to CRLF in the worktree. This can happen due to many reasons like - - git config --system core.autocrlf (set to true) - git config --global core.autocrlf (set to true) - git config --local core.autocrlf (set to true) - git clone --config core.autocrlf=true ... Adding gitattributes for the shims will not convert them to CRLF line endings. Also, there is a note[1] in test/README.md which says - For the tests to run on Windows, be sure to clone Node.js source code with the `autocrlf` git config flag set to true. Reason for using build subsystem - These shims are just copied in stage_package label of vcbuild.bat Fixes: nodejs#43860 [1]: nodejs@3654cd4 PR-URL: nodejs#43879 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#43870 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
The 'test-tls-env-extra-ca-file-load.js' test assumes that NODE_EXTRA_CA_CERTS is not set. If the build environment happens to have it set, this test will fail. This change deletes that env var before running the test. PR-URL: nodejs#43858 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Co-authored-by: Rafael Gonzaga <[email protected]> PR-URL: nodejs#43835 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#43853 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
The shared libraries will now be stores in lib.target as opposed to obj.target, libnode.version.so, libnode.x (for npm backwards compat and testing), and libnode.version.x (for builds). The install will also include libnode.so link that points to libnode.version.so (this will be used by native npms for backwards compat). PR-URL: nodejs#42256 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Co-authored-by: Gaby Baghdadi <[email protected]> Co-authored-by: Wayne Zhang <[email protected]>
This change adds a new parameter `--nvd-key` to `dep_checker`, which allows the user to specify a NVD API key with which to query the National Vulnerability Database. This increases the rate at which we are allowed to query the database, which speeds up the running time of the script. PR-URL: nodejs#43909 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#43822 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: nodejs#43806 Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs#43897 (comment) PR-URL: nodejs#43913 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Fixes: nodejs#43839 PR-URL: nodejs#43953 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Mestery <[email protected]>
the returns need to be lowercase PR-URL: nodejs#43933 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mestery <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Harshitha K P <[email protected]>
PR-URL: nodejs#43927 Fixes: nodejs#43864 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Mestery <[email protected]>
PR-URL: nodejs#43857 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#43912 Refs: nodejs#39735 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#43922 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#43747 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#43779 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
Fixes: nodejs#43009 If calling `this._handle.getpeername` returns an error at the first call, its result shouldn't be cached to `this._peername`. Signed-off-by: Daeyeon Jeong [email protected] PR-URL: nodejs#43010 Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Zeyu "Alex" Yang <[email protected]>
Per the comments in nodejs#43924, almost everyone uses `git-node-v8`. I included example steps for using `git-node-v8`. I ran through both of these instructions on a clean Linux machine (I had to fudge the patch SHA of course) and they seemed to work. Refs: nodejs#43924 PR-URL: nodejs#43934 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Refs: nodejs#43929 (comment) PR-URL: nodejs#43954 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Feng Yu <[email protected]>
PR-URL: nodejs#43759 Refs: nodejs#43728 Reviewed-By: Feng Yu <[email protected]>
In the main thread, `inspector.close` is defined as `process._debugEnd`:
```
> inspector.close
[Function: _debugEnd]
```
It's not defined in worker threads:
```
> const {Worker} = require("worker_threads");
> new Worker("console.log(require(\"inspector\").close)", {eval:true})
undefined
```
(As far as I can tell this is intentional and has existed for quite some
time.)
PR-URL: nodejs#43867
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
PR-URL: nodejs#43958 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Feng Yu <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#43985 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
The function doesn't require access to the entire Environment object, so this change just passes what it needs. Addresses this TODO - https://github.com/nodejs/node/blob/71691e53d601a4cf3eab3b33cebe4cc9f50c8470/src/env.cc#L372-L374 Signed-off-by: Darshan Sen <[email protected]> PR-URL: nodejs#43668 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos
force-pushed
the
backport-src-pass-isolate-and-env_vars-to-16
branch
from
6cd05b0
to
1073bee
Compare
|
I removed the problematic commit from staging (19dfa47) and rebased your PR. |
This comment was marked as outdated.
This comment was marked as outdated.
11 tasks
|
|
joyeecheung
approved these changes
Aug 1, 2022
targos
added
the
author ready
targos
pushed a commit
that referenced
this pull request
Aug 1, 2022
The function doesn't require access to the entire Environment object, so this change just passes what it needs. Addresses this TODO - https://github.com/nodejs/node/blob/71691e53d601a4cf3eab3b33cebe4cc9f50c8470/src/env.cc#L372-L374 Signed-off-by: Darshan Sen <[email protected]> PR-URL: #43668 Backport-PR-URL: #44070 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
|
Landed in 0dc96e4 |
11 tasks
This was referenced Aug 3, 2022
guangwong
pushed a commit
to noslate-project/node
that referenced
this pull request
Oct 10, 2022
The function doesn't require access to the entire Environment object, so this change just passes what it needs. Addresses this TODO - https://github.com/nodejs/node/blob/71691e53d601a4cf3eab3b33cebe4cc9f50c8470/src/env.cc#L372-L374 Signed-off-by: Darshan Sen <[email protected]> PR-URL: nodejs/node#43668 Backport-PR-URL: nodejs/node#44070 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[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.
Backport of #43668.