MozillaWiki

Sheriffing/How To/Landing patches: Difference between revisions

< Sheriffing
(added instructions for manually landing a patch from he command line)
 
(5 intermediate revisions by 4 users not shown)
Line 1: Line 1:
{{Sheriffing How To|Landing check-needed patches}}
{{Sheriffing How To|Landing check-needed patches}}
Sheriffs have the responsibility for landing checkin-needed patches on behalf of others who do not have the needed commit access to do so themselves or those who do not have the ability to watch their pushes afterwards.<br />
Usually, patches should be landed after the merges and after the nightly builds started running. Of course, it can also be done when requested.


= Bugzilla queries for patches requiring check-in =
= Manually landing a patch =
* Only patches for mozilla-central: [https://bugzilla.mozilla.org/buglist.cgi?v4=checkin%3F&f1=OP&v6=Calendar%20Chat%20Instantbird%20MailNews%20SeaMonkey%20Thunderbird%20NSS%20NSPR&o3=substring&v3=checkin-needed&o6=nowordssubstr&o2=substring&f4=flagtypes.name&query_format=advanced&j1=OR&f3=status_whiteboard&o4=substring&f2=keywords&f5=CP&f6=product&v2=checkin-needed&list_id=13972647 Bugs with checkin-needed or checkin? requests set excluding non-core components in comm-central and NSS and NSPR]
A patch can be landed manually if
** This is now available as a shared saved search for anyone with editbugs permissions on bugzilla. Go [https://bugzilla.mozilla.org/userprefs.cgi?tab=saved-searches here] and search for "checkin-needed-core" to find it in the list, and you can add it to Bugzilla's footer.
* the repository in which it shall land is not supported by Lando or
* Also including patches for other trees, e.g. comm-central: [https://bugzilla.mozilla.org/buglist.cgi?order=Bug%20Number;resolution=---;resolution=FIXED;field0-0-0=keywords;type0-0-0=anywordssubstr;value0-0-0=checkin-needed;field0-0-1=status_whiteboard;type0-0-1=substring;value0-0-1=checkin-needed;field0-0-2=flagtypes.name;type0-0-2=equals;value0-0-2=checkin%3F; Bugs with checkin-needed or checkin? requests set in all components]
* the repository is closed for landing patches with Lando and the patch needs to land now.


= How to land check-in needed patches =
Apply the patch from Phabricator:
* <u><b>Verify that the patch has proper review before doing anything else</b></u>. We have to check if that the person who reviewed the patch has the authority to grant the permission to get the code changes added. The list of allowed reviewers broken down by module can be found at
** [https://wiki.mozilla.org/Modules Module information]
** If the module can't be identified, it's possible to check if a person is a Mozilla employee at https://phonebook.mozilla.org/ and land it if that is true.


* Saving the attachment from the bug.
<code>moz-phab patch D<number> --apply-to . --skip-dependencies</code>
* Apply the patch to the appropriate repository using <code>hg import</code> (preferred) or <code>patch</code>.
** For almost all patches, the appropriate repository is mozilla-inbound.
* If the patch does not apply cleanly, you can try to [https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/extensions.html#core-extensions-to-perform-history-rewriting rebase]. If that doesn't solve the issue, your best course of action is to remove the checkin-needed request and ask the developer to post an updated patch for check-in. If you understand the code very well or the conflicts are extremely trivial, you can try to resolve the conflicts yourself, but note that '''YOU''' are now on the hook for this patch too.
* Once the patch applies cleanly, verify that the commit information is correct:
** Author (use the information from their Bugzilla account if needed)
** Bug number
** Commit message (keeping in mind that the commit message should be a brief description of what the patch is <u>doing</u>)
*** Format should be something like "Bug 123456 - Add a null check to XYZ to avoid a crash. r=somebody"
* If a patch is missing commit information, remind the patch author to include it for future patches with a link to the [https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F Mercurial FAQ page] that describes how to set it.


The patches can be landed in 2 ways:
<code>--skip-dependencies</code> ensures only the selected patch gets applied. This prevents accidentally applying more patches in the patch stack (the ones between the revision used as base for development and the one mentioned in the command). If a patch stack shall land, drop this argument but still check only the intended patches get applied.
'''Automatically''', from <span style="color:#14866d">'''mozreview'''</span> – and the patches will land on '''AUTOLAND'''
'''Manually''', from the <span style="color:#14866d">'''console'''</span> – and the patches will land on '''INBOUND'''


== Landing patches via Lando - Autoland ==
The commit message has to be updated to add the granted reviews:


'''Steps:'''
<code>hg commit --amend</code>
# Access the bug and go directly to the comment with the phabricator patch '''positively reviewed''', and open the given link:
[[File:Phabricator review.png|frame|center]]
# On the right side of the page, under ''Edit Related Objects'', click on the '''View in Lando''' option:
[[File:Phabricator lando link.png|900px|center]]
# To land this patch use the bottom right corner button. If the patch has not yet been landed, it will appear as ''“Land DXXXX”'' otherwise, it will appear as ''“Re-land”''
[[File:Lando green button.png|900px|center]]


For more in-depth info regarding Phabricator, go to the [https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#quick-start Mozilla Phabricator User Guide]
In case the commit messages of a patch stack shall be edited:


<span style="color:#b32425">'''Note:  If you cannot land the patch from GUI:'''</span>
<code>hg histedit</code>
# '''Comment''' on the bug that the patch could not be applied.
# '''Needinfo''' the developer
# '''Remove''' the checkin-needed tag


== Landing patches from the console - Inbound ==
Select <code>m</code> for <code>mess</code> and save to get prompted for the commit message for each commit.
These are the steps that you should follow:
# '''cd mozilla-unified'''
# '''hg pull inbound'''
# '''hg update inbound'''
# '''hg qimport bz://<bugnumber>''' <span style="color:#14866d">''//to import the patches from the chosen bug''</span>
# '''hg qseries'''  <span style="color:#14866d">''//check the patches in the queue''</span>
# '''hg qpush''' <span style="color:#14866d">''//apply patch. Use hg qpush -a if there are multiple patches''</span>
# '''hg log''' <span style="color:#14866d">''//check the commit message''</span>
# If in hg log, the name or e-mail of the reviewer are '''not correct''', use '''hg qref –u “Name <email>”'''
# '''hg qref -e''' <span style="color:#14866d">//to add the reviewer and/ or edit the commit message</span>
# '''hg finish -a '''
# '''hg push -r . inbound'''


<span style="color:#b32425">'''Note: If you get the following error when pushing to inbound:  "abort: push creates new remote head ..." run:'''</span>
= Phabricator queries for revisions requiring check-in =
# '''hg pull --rebase inbound'''
People only get the permission to land changes after repeated contributions. Until they can land them, the reviewers are expected to land their changes. Sheriffs should not land such patches. The documentation for the workflow below is kept for potential future use.
# '''hg push -r . inbound'''


=== Deleting a patch from the tip ===
* There is a saved Phabricator differential query that may be used to find revisions requiring check-in.
If you encounter any errors when applying a patch, you will have to:
** Visit https://phabricator.services.mozilla.com/differential/ and click the '''#Check-in Needed''' link in the left menu’s list of queries.
* '''delete''' the patch from the queue
** It is important to follow the link in this menu rather than use a direct link as if the query is modified the URL will be changed.
* '''needinfo''' the dev
[[File:Check-in needed phabricator query menu.png|203px|Location of the menu link for '''#Check-in Needed''' Query]]
* '''delete''' the checkin-needed tag
* '''leave a comment''' on the bug with the issue that stopped you from landing the bug.
And furthermore to :


* '''hg purge''' <span style="color:#14866d">''(--only if there are .rej files)''</span>
= How to land '''#Check-in Needed''' revisions =
* '''hg update -C'''
The only supported method for landing check-in needed patches from Phabricator is Lando.
* '''hg qpop'''
* '''hg qdelete <span style="color:#14866d"><patch_name></span>'''


== Relanding a revision ==
== Landing revisions via Lando (to [https://hg.mozilla.org/integration/autoland integration/autoland]) ==
=== Steps: ===
* Access the revision and click '''View Stack in Lando''' at the top right of the page:
[[File:Check-in needed phabricator view stack in lando.png|779px|Location of the '''View Stack in Lando''' link]]


If requested, one or more revisions can be relanded after, for example, being backed out.
* Verify that the reviewers who have accepted the revision have proper authority.
** Look at the reviewers column in the table showing the stack of revisions and identify the reviewers who have accepted each revision.
** Verify these reviewers are [[Modules/All|Module Owners/Peers]] and have the authority to review code for Firefox.
[[File:Check-in needed lando verify reviewers.png|754px|Location of reviewers in Lando]]


# '''hg graft -er <span style="color:#14866d"><revision number> OR <revision>::<revision></span>'''
* Click the '''Preview Landing''' button in the bottom right corner of the page.
# '''hg import <span style="color:#14866d"><revision link></span>'''
[[File:Check-in needed lando click preview landing.png|790px|Location of the '''Preview Landing''' button]]
# '''hg commit --amend''' <span style="color:#14866d">//to update the commit message</span>'
# '''hg push -r . <span style="color:#14866d"><tree></span>'''


== Landing bugs with multiple patches, out of which one is a zip file ==
* Quickly look over each commit message’s first line in the landing preview to ensure it is formatted correctly. Note that the preview pane scrolls vertically to list all of the commits.
[[File:Check-in needed lando inspect commit messages.png|922px|Location of each commit message's first line.]]


If there are other patches, except the zip file, do the following:
* Check the landing warnings.
# '''hg qimport''' all the patches <span style="color:#b32425">UP TO</span> the zip file
** The bottom of the landing preview might contain a list of warnings that block landing unless acknowledged.
# '''hg push -a''' <span style="color:#14866d">// if there are multiple patches</span>
** Warnings should not be acknowledged without consideration.
# '''hg qfinish -a'''
** {{warning|Only the "Has Previously landed" warning is acceptable to acknowledge and continue landing. If there are any other warnings present landing should halt.}}
# '''hg histedit''' <span style="color:#14866d">// if you need to edit the reviewers</span>
[[File:Check-in needed lando warnings list.png|407px|A list of warnings in the Landing Preview]]
# '''hg finish -a'''
Then:
# Go to the bug in your VM, '''download''' the zip file, then '''unzip''' it <span style="color:#14866d">''(in your home directory to be easier)''</span>
# Go to your console and run h'''g qimport ~/bug.'''<span style="color:#14866d">'''patch'''</span> <span style="color:#14866d">''// bug.patch is the file from the zip; ~/ = home = path where you extracted the file''</span>
# '''hg qpush'''
# '''hg qref -e''' <span style="color:#14866d">// to edit reviewer when it's just one patch</span>
# '''hg finish -a'''
# '''hg qimport bz:<span style="color:#14866d">//bug</span>''' <span style="color:#14866d">// import the rest of the missing patches, the ones that are not zip or other formats</span>
# '''hg qpush -a'''
# '''hg qfinish -a'''
# '''hg histedit''' <span style="color:#14866d">// edit reviewers if necessary</span>
# '''hg push -r . inbound'''


== What to do in case you forget to update to the right branch before importing patches ==
=== If you cannot land the patch from the GUI: ===
Landing Patches (rebasing your revision to the right tree in case you forgot to update to inbound before importing patches)
* Remove the '''#Check-in Needed''' project tag.
Example: Let's say you are on the top of Autoland and you want to land some patches on Inbound, but you forgot to update to inbound, so you already imported a few patches and you realize you are on Autoland instead of inbound.
** To do this access the revision and click '''Edit Revision''' at the top right of the page.
What to do in order to rebase to the right tree:
[[File:Check-in needed phabricator edit revision.png|779px|Location of the '''Edit Revision''' link]]
# '''hg log -fr.''' <span style="color:#14866d">// to force show the log and check how many revisions you have created - how many patches you have imported. Also you will need this later for the revision.</span>
* Comment on the revision that it could not be checked-in, providing the reason.
[[File:Landing paches1.png|frame|center]]
# '''hg pull inbound'''  
# '''hg rebase -s <span style="color:#14866d"><revision/changeset number of the bottom most patch></span> -d inbound'''
<span style="color:#b32425">Note:
revision number of the bottom most patch = in the case above is 19212c6d7051, we only need the second string of characters from the changeset.
-s = source;  -d = destination </span> Good to know: When running this command, hg will '''move(cut)''' all the patches from Autoland to Inbound, starting from the source (e.g:19212c6d7051) to the last patch. (top most patch).  
[[File:Landing paches2.png|frame|center]]
# '''hg log''' <span style="color:#14866d">''// to check if the commands had the desired effect'' </span>
# '''hg update tip''' <span style="color:#14866d">''// this command will activate the rebase, will make sure that the changesets that were imported from the other tree are going to be in your tip for this specific tree. In this case inbound.'' </span>
[[File:Tip.png|frame|center]]
# '''hg out -r . inbound''' <span style="color:#14866d">''// check to see that the two patches rebased to inbound and are active in the tip'' <span>
# Once the patches are where they should, you can go ahead and push.
# '''hg push -r . inbound'''
 
Same steps should be followed for any other branch, just replace "inbound" with the right one.

Latest revision as of 15:18, 13 May 2022


Manually landing a patch

A patch can be landed manually if

  • the repository in which it shall land is not supported by Lando or
  • the repository is closed for landing patches with Lando and the patch needs to land now.

Apply the patch from Phabricator:

moz-phab patch D<number> --apply-to . --skip-dependencies

--skip-dependencies ensures only the selected patch gets applied. This prevents accidentally applying more patches in the patch stack (the ones between the revision used as base for development and the one mentioned in the command). If a patch stack shall land, drop this argument but still check only the intended patches get applied.

The commit message has to be updated to add the granted reviews:

hg commit --amend

In case the commit messages of a patch stack shall be edited:

hg histedit

Select m for mess and save to get prompted for the commit message for each commit.

Phabricator queries for revisions requiring check-in

People only get the permission to land changes after repeated contributions. Until they can land them, the reviewers are expected to land their changes. Sheriffs should not land such patches. The documentation for the workflow below is kept for potential future use.

  • There is a saved Phabricator differential query that may be used to find revisions requiring check-in.

 

How to land #Check-in Needed revisions

The only supported method for landing check-in needed patches from Phabricator is Lando.

Landing revisions via Lando (to integration/autoland)

Steps:

  • Access the revision and click View Stack in Lando at the top right of the page:

 

  • Verify that the reviewers who have accepted the revision have proper authority.
    • Look at the reviewers column in the table showing the stack of revisions and identify the reviewers who have accepted each revision.
    • Verify these reviewers are Module Owners/Peers and have the authority to review code for Firefox.

 

  • Click the Preview Landing button in the bottom right corner of the page.

 

  • Quickly look over each commit message’s first line in the landing preview to ensure it is formatted correctly. Note that the preview pane scrolls vertically to list all of the commits.

 

  • Check the landing warnings.
    • The bottom of the landing preview might contain a list of warnings that block landing unless acknowledged.
    • Warnings should not be acknowledged without consideration.
    •  Warning: Only the "Has Previously landed" warning is acceptable to acknowledge and continue landing. If there are any other warnings present landing should halt.

 

If you cannot land the patch from the GUI:

  • Remove the #Check-in Needed project tag.
    • To do this access the revision and click Edit Revision at the top right of the page.

 

  • Comment on the revision that it could not be checked-in, providing the reason.
Retrieved from "https://wiki.allizom.org/index.php?title=Sheriffing/How_To/Landing_patches&oldid=1242390"