Sheriffing/How To/Landing patches: Difference between revisions

Jump to navigation Jump to search

Sheriffing/How To/Landing patches (view source)

Revision as of 15:24, 10 June 2019

7,138 bytes removed ,  10 June 2019
Replace instructions with updated ones detailing #Check-in Needed using Phabricator and Lando.
(Replace instructions with updated ones detailing #Check-in Needed using Phabricator and Lando.)
Line 3: Line 3:
Usually, patches should be landed after the merges and after the nightly builds started running. Of course, it can also be done when requested.
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 =
= Phabricator queries for revisions requiring check-in =
* 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]
* There is a saved Phabricator differential query that may be used to find revisions requiring check-in.
** 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.
** Visit https://phabricator.services.mozilla.com/differential/ and click the '''#Check-in Needed''' link in the left menu’s list of queries.
* 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]
** 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.
[[File:Check-in needed phabricator query menu.png|frame|center|Location of the menu link for '''#Check-in Needed''' Query]]


= How to land check-in needed patches =
= How to land '''#Check-in Needed''' revisions =
* <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
The only supported method for landing check-in needed patches from Phabricator is Lando.
** [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.
== Landing revisions via Lando (to [https://hg.mozilla.org/integration/autoland integration/autoland]) ==
* Apply the patch to the appropriate repository using <code>hg import</code> (preferred) or <code>patch</code>.
=== Steps: ===
** For almost all patches, the appropriate repository is mozilla-inbound.
* Access the revision and click '''View Stack in Lando''' at the top right of the page:
* 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.
[[File:Check-in needed phabricator view stack in lando.png|frame|center|Location of the '''View Stack in Lando''' link]]
* 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:
* Verify that the reviewers who have accepted the revision have proper authority.
* '''Automatically''', from <span style="color:#14866d">'''Lando'''</span> – and the patches will land on '''AUTOLAND'''
** Look at the reviewers column in the table showing the stack of revisions and identify the reviewers who have accepted each revision.
* '''Manually''', from the <span style="color:#14866d">'''console'''</span> – and the patches will land on '''INBOUND'''
** 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|frame|center|Location of reviewers in Lando]]


== Landing patches via Lando - Autoland ==
* Click the '''Preview Landing''' button in the bottom right corner of the page.
[[File:Check-in needed lando click preview landing.png|frame|center|Location of the '''Preview Landing''' button]]


'''Steps:'''
* 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.
# Access the bug and go directly to the comment with the phabricator patch '''positively reviewed''', and open the given link:
[[File:Check-in needed lando inspect commit messages.png|frame|center|Location of each commit message's first line.]]
[[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]
* 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.
** '''Only the "Has Previously landed." warning is acceptable to acknowledge''' and continue landing. If there are any other warnings present landing should halt.
[[File:Check-in needed lando warnings list.png|frame|center|A list of warnings in the Landing Preview]]


<span style="color:#b32425">'''Note:  If you cannot land the patch from GUI:'''</span>
=== If you cannot land the patch from the GUI: ===
# '''Comment''' on the bug that the patch could not be applied.
* Remove the '''#Check-in Needed''' project tag.
# '''Needinfo''' the developer
** To do this access the revision and click '''Edit Revision''' at the top right of the page.
# '''Remove''' the checkin-needed tag
[[File:Check-in needed phabricator edit revision.png|frame|center|Location of the '''Edit Revision''' link]]
 
* Comment on the revision that it could not be checked-in, providing the reason.
== Landing patches from the console - Inbound ==
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 qfinish -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>
# '''hg pull --rebase inbound'''
# '''hg push -r . inbound'''
 
=== Deleting a patch from the tip ===
If you encounter any errors when applying a patch, you will have to:
* '''delete''' the patch from the queue
* '''needinfo''' the dev
* '''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>
* '''hg update -C'''
* '''hg qpop'''
* '''hg qdelete <span style="color:#14866d"><patch_name></span>'''
 
== Relanding a revision ==
 
If requested, one or more revisions can be relanded after, for example, being backed out.
 
# '''hg graft -er <span style="color:#14866d"><revision number> OR <revision>::<revision></span>'''
# '''hg import <span style="color:#14866d"><revision link></span>'''
# '''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 ==
 
If there are other patches, except the zip file, do the following:
# '''hg qimport''' all the patches <span style="color:#b32425">UP TO</span> the zip file
# '''hg push -a''' <span style="color:#14866d">// if there are multiple patches</span>
# '''hg qfinish -a'''
# '''hg histedit''' <span style="color:#14866d">// if you need to edit the reviewers</span>
# '''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 ==
Landing Patches (rebasing your revision to the right tree in case you forgot to update to inbound before importing patches)
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.
What to do in order to rebase to the right tree:
# '''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>
[[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.
Smacleod
Confirmed users
47

edits

Retrieved from "https://wiki.mozilla.org/Special:MobileDiff/1213536"

Navigation menu