From 85c517fcd7e9f832bfe5e9858b538cba537e6a4d Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Thu, 21 Jul 2022 16:22:31 +0000 Subject: [PATCH 1/4] doc: recommend git-node-v8 Per the comments in #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: https://github.com/nodejs/node/pull/43924 --- doc/contributing/maintaining-V8.md | 82 ++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 11 deletions(-) diff --git a/doc/contributing/maintaining-V8.md b/doc/contributing/maintaining-V8.md index fde030b2e95712..bf0c134285a599 100644 --- a/doc/contributing/maintaining-V8.md +++ b/doc/contributing/maintaining-V8.md @@ -208,6 +208,62 @@ backport the fix: Abandoned V8 branches are supported in the Node.js repository. The fix needs to be cherry-picked in the Node.js repository and V8-CI must test the change. +As an example for how to backport changes, consider the bug [RegExp show +inconsistent result with other browsers](https://crbug.com/v8/5199). From the +bug we can see that it was merged by V8 into 5.2 and 5.3, and not into V8 5.1 +(since it was already abandoned). Since Node.js `v6.x` uses V8 5.1, the fix +needed to be backported. + +#### Backporting with `git-node` (recommended) + +You can use [`git-node`][] to help you backport patches. This removes +some manual steps and is recommended. + +Here are the steps for the bug mentioned above: + +1. Install `git-node` by installing [`node-core-utils`][]. +2. Install the prerequisites for [`git-node-v8`][]. +3. Find the commit hash linked-to in the issue (in this case a51f429). +4. Checkout a branch off the appropriate _vY.x-staging_ branch (e.g. + _v6.x-staging_ to fix an issue in V8 5.1). +5. Run `git node v8 backport a51f429`. +6. If there are conflicts, `git-node` will wait for you to resolve them. + `git-node` will prompt you to resolve them in another terminal and then + enter `RESOLVED`: + +```console +$ git node v8 backport a51f429 +✔ Update local V8 clone +❯ V8 commit backport + ✔ Get current V8 version + ✔ Generate patches + ❯ Apply and commit patches to deps/v8 + ❯ Commit a51f429772d1 + ⠏ Apply patch + ◼ Increment embedder version number + ◼ Commit patch + +? Resolve merge conflicts and enter 'RESOLVED' ‣ +``` + +7. After you resolve conflicts (or if there are no conflicts), the + output should look like this: + +```console +$ git node v8 backport a51f429 +✔ Update local V8 clone +✔ V8 commit backport +``` + +8. Open a PR against the v6.x-staging branch in the Node.js repository. + Launch the normal and [V8 CI][] using the Node.js CI system. We only + needed to backport to v6.x as the other LTS branches weren't affected + by this bug. + +See [`git-node-v8-backport`][] for more documentation and additional options. + +#### Backporting manually (not recommended) + * For each abandoned V8 branch corresponding to an LTS branch that is affected by the bug: * Checkout a branch off the appropriate _vY.x-staging_ branch (e.g. @@ -224,14 +280,7 @@ to be cherry-picked in the Node.js repository and V8-CI must test the change. `tools/make-v8.sh` to reconstruct a git tree in the `deps/v8` directory to run V8 tests.[^2] -The [`git-node`][] tool can be used to simplify this task. Run -`git node v8 backport ` to cherry-pick a commit. - -An example for workflow how to cherry-pick consider the bug -[RegExp show inconsistent result with other browsers](https://crbug.com/v8/5199). -From the bug we can see that it was merged by V8 into 5.2 and 5.3, and not into -V8 5.1 (since it was already abandoned). Since Node.js `v6.x` uses V8 5.1, the -fix needed to be cherry-picked. To cherry-pick, here's an example workflow: +Here are the steps for the bug mentioned above: * Download and apply the commit linked-to in the issue (in this case a51f429): @@ -322,6 +371,16 @@ compute the diff between these tags on the V8 repository, and then apply that patch on the copy of V8 in Node.js. This should preserve the patches/backports that Node.js may be floating (or else cause a merge conflict). +#### Applying minor updates with `git-node` (recommended) + +1. Install [`git-node`][] by installing [`node-core-utils`][]. +2. Install the prerequisites for [`git-node-v8`][]. +3. Run `git node v8 minor` to apply a minor update. + +See [`git-node-v8-minor`][] for more documentation and additional options. + +#### Applying minor updates manually (not recommended) + The rough outline of the process is: ```bash @@ -340,9 +399,6 @@ curl -L https://github.com/v8/v8/compare/${V8_OLD_VERSION}...${V8_NEW_VERSION}.p V8 also keeps tags of the form _5.4-lkgr_ which point to the _Last Known Good Revision_ from the 5.4 branch that can be useful in the update process above. -The [`git-node`][] tool can be used to simplify this task. Run `git node v8 minor` -to apply a minor update. - ### Major updates We upgrade the version of V8 in Node.js `main` whenever a V8 release goes stable @@ -427,4 +483,8 @@ This would require some tooling to: [V8MergingPatching]: https://v8.dev/docs/merge-patch [V8TemplateMergeRequest]: https://bugs.chromium.org/p/v8/issues/entry?template=Node.js%20merge%20request [V8TemplateUpstreamBug]: https://bugs.chromium.org/p/v8/issues/entry?template=Node.js%20upstream%20bug +[`git-node-v8-backport`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-v8-backport-sha +[`git-node-v8-minor`]: https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-v8-minor +[`git-node-v8`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md#git-node-v8 [`git-node`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md#git-node-v8 +[`node-core-utils`]: https://github.com/nodejs/node-core-utils#Install From b88ea178c8777977a6ce7fa88bbe5e3747d8afaf Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Thu, 21 Jul 2022 18:16:45 +0000 Subject: [PATCH 2/4] suggestions --- doc/contributing/maintaining-V8.md | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/contributing/maintaining-V8.md b/doc/contributing/maintaining-V8.md index bf0c134285a599..c44d5c257f931b 100644 --- a/doc/contributing/maintaining-V8.md +++ b/doc/contributing/maintaining-V8.md @@ -208,11 +208,11 @@ backport the fix: Abandoned V8 branches are supported in the Node.js repository. The fix needs to be cherry-picked in the Node.js repository and V8-CI must test the change. -As an example for how to backport changes, consider the bug [RegExp show -inconsistent result with other browsers](https://crbug.com/v8/5199). From the -bug we can see that it was merged by V8 into 5.2 and 5.3, and not into V8 5.1 -(since it was already abandoned). Since Node.js `v6.x` uses V8 5.1, the fix -needed to be backported. +As an example for how to backport changes, consider the bug +[RegExp show inconsistent result with other browsers](https://crbug.com/v8/5199). +From the bug we can see that it was merged by V8 into 5.2 and 5.3, and not into +V8 5.1 (since it was already abandoned). Since Node.js `v6.x` uses V8 5.1, the +fix needed to be backported. #### Backporting with `git-node` (recommended) @@ -227,9 +227,7 @@ Here are the steps for the bug mentioned above: 4. Checkout a branch off the appropriate _vY.x-staging_ branch (e.g. _v6.x-staging_ to fix an issue in V8 5.1). 5. Run `git node v8 backport a51f429`. -6. If there are conflicts, `git-node` will wait for you to resolve them. - `git-node` will prompt you to resolve them in another terminal and then - enter `RESOLVED`: +6. If there are conflicts, `git-node` will wait for you to resolve them: ```console $ git node v8 backport a51f429 @@ -245,6 +243,9 @@ $ git node v8 backport a51f429 ? Resolve merge conflicts and enter 'RESOLVED' ‣ ``` +Resolve conflicts by opening another terminal and staging resolutions +to the merge conflicts. Once you fixed the conflicts, return to +`git-node` and enter `RESOLVED`. 7. After you resolve conflicts (or if there are no conflicts), the output should look like this: @@ -262,7 +263,7 @@ $ git node v8 backport a51f429 See [`git-node-v8-backport`][] for more documentation and additional options. -#### Backporting manually (not recommended) +#### Backporting manually * For each abandoned V8 branch corresponding to an LTS branch that is affected by the bug: @@ -379,7 +380,7 @@ that Node.js may be floating (or else cause a merge conflict). See [`git-node-v8-minor`][] for more documentation and additional options. -#### Applying minor updates manually (not recommended) +#### Applying minor updates manually The rough outline of the process is: From 5b61c66aa0fbe7a115b318a305b77875d7896a3e Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Thu, 21 Jul 2022 18:31:02 +0000 Subject: [PATCH 3/4] oops; forgot format --- doc/contributing/maintaining-V8.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/contributing/maintaining-V8.md b/doc/contributing/maintaining-V8.md index c44d5c257f931b..558ed8137ac451 100644 --- a/doc/contributing/maintaining-V8.md +++ b/doc/contributing/maintaining-V8.md @@ -243,6 +243,7 @@ $ git node v8 backport a51f429 ? Resolve merge conflicts and enter 'RESOLVED' ‣ ``` + Resolve conflicts by opening another terminal and staging resolutions to the merge conflicts. Once you fixed the conflicts, return to `git-node` and enter `RESOLVED`. From ec65e9ec4f858737f4353f8a96de08ac9f58e969 Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Sun, 24 Jul 2022 13:01:58 -0700 Subject: [PATCH 4/4] Update doc/contributing/maintaining-V8.md Co-authored-by: Antoine du Hamel --- doc/contributing/maintaining-V8.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/contributing/maintaining-V8.md b/doc/contributing/maintaining-V8.md index 558ed8137ac451..e52526181177cc 100644 --- a/doc/contributing/maintaining-V8.md +++ b/doc/contributing/maintaining-V8.md @@ -244,9 +244,9 @@ $ git node v8 backport a51f429 ? Resolve merge conflicts and enter 'RESOLVED' ‣ ``` -Resolve conflicts by opening another terminal and staging resolutions -to the merge conflicts. Once you fixed the conflicts, return to -`git-node` and enter `RESOLVED`. +Resolve conflicts, stage the files (you may need to open another terminal or use +a GUI git client), then return to the terminal running `git-node`, type +`RESOLVED`, and hit Enter. 7. After you resolve conflicts (or if there are no conflicts), the output should look like this: