-
Notifications
You must be signed in to change notification settings - Fork 322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update merge handling #763
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
--- | ||
title: 'Merging & patching' | ||
description: 'This document explains how to merge V8 patches to the master branch.' | ||
description: 'This document explains how to merge V8 patches to a release branch.' | ||
--- | ||
If you have a patch to the `master` branch (e.g. an important bug fix) that needs to be merged into one of the production V8 branches, read on. | ||
If you have a patch to the `main` branch (e.g. an important bug fix) that needs to be merged into one of the release V8 branches (refs/branch-heads/12.5), read on. | ||
|
||
The following examples use a branched 2.4 version of V8. Substitute `2.4` with your version number. Read the documentation on [V8’s release process](/docs/release-process) and [V8’s version numbering](/docs/version-numbers) for more information. | ||
The following examples use a branched 12.3 version of V8. Substitute `12.3` with your version number. Read the documentation on [V8’s version numbering](/docs/version-numbers) for more information. | ||
|
||
An associated issue on Chromium’s or V8’s issue tracker is mandatory if a patch is merged. This helps with keeping track of merges. You can use [a template](https://code.google.com/p/v8/issues/entry?template=Merge%20request) to create a merge request issue. | ||
An associated issue on V8’s issue tracker is **mandatory** if a patch is merged. This helps with keeping track of merges. | ||
|
||
## What qualifies a merge candidate? | ||
|
||
|
@@ -22,98 +22,61 @@ More information can be found on the [relevant Chromium page](https://chromium.g | |
|
||
## The merge process | ||
|
||
The merge process in the Chromium and V8 tracker is driven by labels in the form of: | ||
The merge process in the V8 tracker is driven by Attributes in the form of: | ||
|
||
``` | ||
Merge-[Status]-[Branch] | ||
Merge-Request: 123 | ||
``` | ||
|
||
The currently important labels for V8 are: | ||
once reviewed, this will be adjusted during the review to: | ||
|
||
1. `Merge-Request-{Branch}` initiates the process, and means that this fix should be merged into `{Branch}`. `{Branch}` is the name/number of the V8 branch e.g. `7.2` for M72. | ||
1. `Merge-Review-{Branch}` means the merge is not approved yet for `{Branch}` e.g. because Canary coverage is missing. | ||
1. `Merge-Approved-{Branch}` means that the Chrome TPMs have signed off on the merge. | ||
1. When the merge is done, the `Merge-Approved-{Branch}` label is replaced with `Merge-Merged-{Branch}`. | ||
``` | ||
Merge: Approved-123 | ||
or | ||
Merge: Rejected-123 | ||
``` | ||
|
||
After the CL landed, this will be adjusted one more time to: | ||
|
||
``` | ||
Merge: Merged-123 | ||
``` | ||
|
||
## How to check if a commit was already merged/reverted/has Canary coverage | ||
|
||
Use `mergeinfo.py` to get all the commits which are connected to the `$COMMIT_HASH` according to Git. | ||
Use [chromiumdash](https://chromiumdash.appspot.com/commit/) to verify if the relevant CL has Canary coverage. | ||
|
||
```bash | ||
tools/release/mergeinfo.py $COMMIT_HASH | ||
``` | ||
|
||
If it tells you `Is on Canary: No Canary coverage` you should not merge yet because the fix was not yet deployed on a Canary build. A good rule of the thumb is to wait at least 3 days after the fix has landed until the merge is conducted. | ||
On top the **Releases** section should show a Canary. | ||
|
||
## How to create the merge CL | ||
|
||
### Option 1: Using [gerrit](https://chromium-review.googlesource.com/) | ||
### Option 1: Using [gerrit](https://chromium-review.googlesource.com/) - Recommended | ||
|
||
Note that this option only works if the patch applies cleanly on the release branch. | ||
|
||
1. Open the CL you want to back-merge. | ||
1. Select "Cherry pick" from the extended menu (three vertical dots in the upper right corner). | ||
1. Enter "refs/branch-heads/*X.X*" as destination branch (replace *X.X* by the proper branch). | ||
1. Enter "refs/branch-heads/*XX.X*" as destination branch (replace *XX.X* by the proper branch). | ||
1. Modify the commit message: | ||
1. Prefix the title with "Merged: ". | ||
1. Remove lines from the footer that correspond to the original CL ("Change-Id", "Reviewed-on", "Reviewed-by", "Commit-Queue", "Cr-Commit-Position"). Definitely keep the "(cherry picked from commit XXX)" line, as this is needed by some tools to relate merges to original CLs. | ||
1. In case of merge conflict, please also go ahead and create the CL. To resolve conflicts (if any) - either using the gerrit UI or you can easily pull the patch locally by using the "download patch" command from the menu (three vertical dots in the upper right corner). | ||
1. In case of merge conflict, please also go ahead and create the CL. To resolve conflicts (if any) - either using the gerrit UI or you can easily pull the patch locally by using the "download patch" command from the menu (three vertical dots in the upper right corner). | ||
1. Send out for review. | ||
|
||
### Option 2: Using the automated script | ||
|
||
Let’s assume you’re merging revision af3cf11 to branch 2.4 (please specify full git hashes - abbreviations are used here for simplicity). | ||
Let’s assume you’re merging revision af3cf11 to branch 12.2 (please specify full git hashes - abbreviations are used here for simplicity). | ||
|
||
```bash | ||
tools/release/merge_to_branch.py --branch 2.4 af3cf11 | ||
``` | ||
|
||
Run the script with `-h` to display its help message, which includes more options (e.g. you can specify a file containing your patch, or you can reverse a patch, specify a custom commit message, or resume a merging process you’ve canceled before). Note that the script will use a temporary checkout of V8 - it won’t touch your work space. You can also merge more than one revision at once; just list them all. | ||
|
||
```bash | ||
tools/release/merge_to_branch.py --branch 2.4 af3cf11 cf33f1b sf3cf09 | ||
https://source.chromium.org/chromium/chromium/src/+/main:v8/tools/release/merge_to_branch_gerrit.py --branch 12.3 -r af3cf11 | ||
``` | ||
|
||
|
||
### After landing: Observe the [branch waterfall](https://ci.chromium.org/p/v8) | ||
|
||
If one of the builders is not green after handling your patch, revert the merge immediately. A bot (`AutoTagBot`) takes care of the correct versioning after a 10-minute wait. | ||
|
||
## Patching a version used on Canary/Dev | ||
|
||
In case you need to patch a Canary/Dev version (which should not happen often), follow these instructions. Googlers: please check out the [internal site](http://g3doc/company/teams/v8/patching_a_version) before creating the CL. | ||
|
||
### Step 1: Merge to roll branch | ||
|
||
Example version used is `5.7.433`. | ||
|
||
```bash | ||
tools/release/roll_merge.py --branch 5.7.433 af3cf11 | ||
``` | ||
|
||
### Step 2: Make Chromium aware of the fix | ||
|
||
Example Chromium branch used is `2978`: | ||
|
||
```bash | ||
git checkout chromium/2978 | ||
git merge 5.7.433.1 | ||
git push | ||
``` | ||
|
||
### Step 3: The end | ||
|
||
Chrome/Chromium should pick up the change when they build automatically. | ||
|
||
## FAQ | ||
|
||
### I get an error during merge that is related to tagging. What should I do? | ||
|
||
When two people are merging at the same time a race condition can happen in the merge scripts. If this is the case, contact <[email protected]> and <[email protected]>. | ||
|
||
### Is there a TL;DR? | ||
In case you need to patch a Canary/Dev version (which should not happen often), cc vahl@ or machenbach@ on the issue. Googlers: please check out the [internal site](http://g3doc/company/teams/v8/patching_a_version) before creating the CL. | ||
|
||
1. [Create an issue on the issue tracker](https://bugs.chromium.org/p/v8/issues/entry?template=Merge%20request). | ||
1. Check status of the fix with `tools/release/mergeinfo.py` | ||
1. Add `Merge-Request-{Branch}` to the issue. | ||
1. Wait until somebody adds `Merge-Approved-{Branch}`. | ||
1. [Merge](#step-1-run-the-script). |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually scripted this process as https://source.chromium.org/chromium/chromium/src/+/main:v8/tools/release/merge_to_branch_gerrit.py, maybe we want to recommend that instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG, does it make sense to replace Option 2 below with the new script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sg