[v16.x backport] src: pass only Isolate* and env_vars to EnabledDebugList::Parse()#44070
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or 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
Collaborator
nodejs-github-bot commented Jul 31, 2022
Review requested:
|
nodejs-github-bot added c++ 
This comment was marked as outdated.
This comment was marked as outdated.
Member
targos commented Jul 31, 2022
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#43714Fixes: 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#43927Fixes: 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]>
targosforce-pushed the backport-src-pass-isolate-and-env_vars-to-16 branch from 6cd05b0 to 1073beeCompareMember
targos commented Jul 31, 2022
I removed the problematic commit from staging (19dfa47) and rebased your PR. |
This comment was marked as outdated.
This comment was marked as outdated.
Collaborator
nodejs-github-bot commented Jul 31, 2022
11 tasks
Collaborator
nodejs-github-bot commented Aug 1, 2022
Member
|
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]>
Member
targos commented Aug 1, 2022
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 freeto join this conversation on GitHub. Already have an account? Sign in to comment
20 participants
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.