Sheriffing/How To/Merge Conflicts

From MozillaWiki
Jump to: navigation, search

During merges, sheriffs might run into merge conflicts if code in the same line or near each other has changed in the same file both on the repository to which the merge is done and from which the changesets got pulled.

Resolving merge conflicts without the domain knowledge of the original patch author is prone to error. Sheriffs should not to resolve merge conflicts except for trivial cases or they possess the required domain knowledge. In almost all cases, it is safer to simply backout the changeset causing the merge conflict.

Example: Backout of conflicting changeset from integration branch

  1. mozilla-central was on revision c2fe4b3b1b93
  2. Then autoland got "merged" up to revision 597025d8888f to central. Technically it got updated because there were no changes on central since the last merge, so autoland already contained all the changes from central.
  3. The attempted merge of the revisions up to a84fff04d938 from inbound to central failed:
    ~/mozilla/mozilla-unified$ hg merge -r a84fff04d938 
    merging dom/base/nsRange.cpp
    merging mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
    merging mobile/android/base/java/org/mozilla/gecko/webapps/WebAppActivity.java
    merging mobile/android/base/moz.build
    merging taskcluster/scripts/misc/build-sccache.sh
    merging toolkit/library/gtest/rust/Cargo.lock
    merging toolkit/library/rust/Cargo.lock
    merging widget/android/GeneratedJNINatives.h
    merging widget/android/GeneratedJNIWrappers.cpp
    merging widget/android/GeneratedJNIWrappers.h
    warning: conflicts while merging mobile/android/base/moz.build! (edit, then use 'hg resolve --mark')
    warning: conflicts while merging widget/android/GeneratedJNINatives.h! (edit, then use 'hg resolve --mark')
    warning: conflicts while merging widget/android/GeneratedJNIWrappers.cpp! (edit, then use 'hg resolve --mark')
    warning: conflicts while merging widget/android/GeneratedJNIWrappers.h! (edit, then use 'hg resolve --mark')
    282 files updated, 6 files merged, 0 files removed, 4 files unresolved
    use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
    These files with the conflicts got changed on both inbound and central (which got the change from the previous merge from autoland.
  4. Let's abandon the merge and back out the conflicting changes to these files from the integration repository (inbound!). Update the working repository to the inbound revision which shall be merged: hg update -C a84fff04d938
  5. Now the bugs with the changesets which cause the merge conflicts have to be identified. On hg.mozilla.org, open the repository to which shall be merged (in this case: mozilla-central).
  6. Click on 'Files' in the header and navigate to the folder which contains the file with the merge conflict. This is mobile/android/base/ in this case.
  7. Click on the revisions link next to the file name, in this case moz.build.
  8. Now you can see the recent changes to the file. The latest changes got pushed 2017-11-07 10:28 +0000 and are from bug 1413698. The bug contains information what changesets belong to that push. You only need that information if you want to fix the merge conflict by manually merging the code.
  9. Do the same thing for mozilla-inbound, this time it's mandatory. Copying the url of the tab with the revision history, opening a new tab and pasting the url while replacing "/mozilla-central/" with "/integration/mozilla-inbound/" and the revision (here "597025d8888fa91b9418231f33e65424d384d83f") with "tip" is often a useful shortcut (but will even show changes which landed after the merge candidate you picked; if you don't want that, use the revision of the merge candidate). This got this changelog.
  10. There is one recent change pushed 2017-11-07 01:43 +0000 from bug 1413362.
  11. Find the changesets which got pushed for that bug in comment 38.
  12. Do a backout for these changesets:
    hg oops -esr 653c66220a5f::8b15dfaeecaa
  13. The working directory is now in a state which allows merging with central. Run:
    hg merge central
  14. Then:
    hg commit
  15. Push the backout and the merge to central:
    hg push -r tip central
  16. Update the bug about the backout.
  17. Merge back from central as usual.
  18. Get rid of the files left over from the merge conflicts with the original state of the files. They are in the same folder like the files for which the merge failed, but got an additional .orig file extension. To achieve this, run:
    hg purge
    If you the Mercurial purge extension hasn't been enabled yet, open the .hgrc file in your home directory and add the following line below [extensions]:
    purge =

Example: code added and modified, conflict fixed

  1. mozilla-inbound revision f60ce40b73c519a0d3b0b2a490de9b2475f84ed7 should be merged to mozilla-central revision 46a5fc19bd7aed9fc92946215ee7f83eea279f8d.
  2. This failed:
    ~/mozilla/mozilla-unified$ hg merge -r f60ce40b73c519a0d3b0b2a490de9b2475f84ed7
    merging devtools/client/preferences/devtools.js
    merging devtools/client/shared/components/splitter/SplitBox.js
    merging dom/html/HTMLMediaElement.cpp
    merging dom/media/tests/mochitest/head.js
    merging dom/media/tests/mochitest/mochitest.ini
    warning: conflicts while merging devtools/client/shared/components/splitter/SplitBox.js! (edit, then use 'hg resolve --mark')
    289 files updated, 4 files merged, 4 files removed, 1 files unresolved
    use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
  3. Changelogs for those revisions: mozilla-central, mozilla-inbound
  4. There is only one recent changeset for that file on each tree, the other ones are much older: mozilla-central, mozilla-inbound
  5. The merge conflict in the file is shown like this:
      onMove(x, y) {
        const node = ReactDOM.findDOMNode(this);
    
        let size;
        let { endPanelControl, vert } = this.state;
    
    <<<<<<< working copy
        if (this.state.vert) {
          // Use the document owning the SplitBox to detect rtl. The global document might be
          // the one bound to the toolbox shared BrowserRequire, which is irrelevant here.
          const doc = node.ownerDocument;
    
    =======
        if (vert) {
    >>>>>>> merge rev
          // Switch the control flag in case of RTL. Note that RTL
          // has impact on vertical splitter only.
          if (doc.dir === "rtl") {
            endPanelControl = !endPanelControl;
          }
  6. The changes on mozilla-central are these:
       onMove(x, y) {
         const node = ReactDOM.findDOMNode(this);
     
         let size;
         let { endPanelControl } = this.props;
     
         if (this.state.vert) {
    +      // Use the document owning the SplitBox to detect rtl. The global document might be
    +      // the one bound to the toolbox shared BrowserRequire, which is irrelevant here.
    +      const doc = node.ownerDocument;
    +
           // Switch the control flag in case of RTL. Note that RTL
           // has impact on vertical splitter only.
    -      if (document.dir === "rtl") {
    +      if (doc.dir === "rtl") {
             endPanelControl = !endPanelControl;
           }
  7. The changes on mozilla-inbound at the conflicting position are those:
        * Adjust size of the controlled panel. Depending on the current
        * orientation we either remember the width or height of
        * the splitter box.
        */
       onMove(x, y) {
         const node = ReactDOM.findDOMNode(this);
     
         let size;
    -    let { endPanelControl } = this.props;
    +    let { endPanelControl, vert } = this.state;
     
    -    if (this.state.vert) {
    +    if (vert) {
           // Switch the control flag in case of RTL. Note that RTL
           // has impact on vertical splitter only.
           if (document.dir === "rtl") {
             endPanelControl = !endPanelControl;
           }
     
           size = endPanelControl ?
             (node.offsetLeft + node.offsetWidth) - x :
  8. Observations:
    • The mozilla-inbound code changes affect the code before the one changed on mozilla-central.
    • On mozilla-inbound, lines got only modified, while on mozilla-central also new lines got added. This is the reason for the different number of lines between the conflict markers (<<<<<<< and >>>>>>>) and the separator for the conflicting code (=======).
    • The element used in the new code added on mozilla-central node.ownerDocument didn't get modified by the mozilla-inbound patch.
    • The patches don't share a modified line.
    • The conflict can be resolved: The new states have to be taken into account. This means, the lines which got added remain and for the line starting with if, the changed version if (vert) { gets used instead of if (this.state.vert) {.
  9. The end result is this:
        if (vert) {
          // Use the document owning the SplitBox to detect rtl. The global document might be
          // the one bound to the toolbox shared BrowserRequire, which is irrelevant here.
          const doc = node.ownerDocument;
    
    

    With surrounding code:

      onMove(x, y) {
        const node = ReactDOM.findDOMNode(this);
    
        let size;
        let { endPanelControl, vert } = this.state;
    
        if (vert) {
          // Use the document owning the SplitBox to detect rtl. The global document might be
          // the one bound to the toolbox shared BrowserRequire, which is irrelevant here.
          const doc = node.ownerDocument;
    
          // Switch the control flag in case of RTL. Note that RTL
          // has impact on vertical splitter only.
          if (doc.dir === "rtl") {
            endPanelControl = !endPanelControl;
          }

Example: deleted file causing merge conflict

  1. mozilla-inbound revision 4b0e963b050b63441999b49a23cc03b6e4406cd5 should be merged to mozilla-central revision 8229360171458d5e2a36fd562e9ba1da6af9b6ba.
  2. This failed:
    ~/mozilla/mozilla-unified$ hg merge -r 4b0e963b050b
    other [merge rev] changed dom/interfaces/xul/nsIDOMXULTextboxElement.idl which local [working copy] deleted
    use (c)hanged version, leave (d)eleted, or leave (u)nresolved?
  3. This means the file got deleted on the tree to which shall be merged (mozilla-central) while there was a change to it on the tree which gets merged (mozilla-inbound).
  4. The options are:
    1. use (c)hanged version: File gets restored and committed with the changes from the merged tree. This makes sense if the file e.g. contains some list whose content dropped to zero and the file got deleted, but a new item got added on the tree which got merged (which then still requires that list).
    2. leave (d)eleted: File remains deleted and the changes to it from the merge are discarded. This option makes sense if the code calling the deleted file doesn't call code a different position which needs the same changes.
    3. leave (u)unresolved: You will fix the issue manually, e.g. because Mercurial won't be able to fix it. If the file got renamed or moved, the changes have to be ported to the new location.
  5. The search at https://hg.mozilla.org/mozilla-central/ finds ef26c00223691ade20d344707e30220580d664aa as the changeset which removed the file.
    For mozilla-inbound, 67be83ace31210b4ce904d2e928ffe87618a9b15 is found as changeset modifying the file.
    --- a/dom/interfaces/xul/nsIDOMXULTextboxElement.idl
    +++ b/dom/interfaces/xul/nsIDOMXULTextboxElement.idl
    @@ -1,15 +1,16 @@
     /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
     /* This Source Code Form is subject to the terms of the Mozilla Public
      * License, v. 2.0. If a copy of the MPL was not distributed with this
      * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
     
     #include "nsIDOMXULLabeledControlEl.idl"
    -interface nsIDOMHTMLInputElement;
    +
    +interface nsIDOMNode;
     
     [scriptable, uuid(7edd8215-5155-4845-a02f-dc2c08645cb9)]
     interface nsIDOMXULTextBoxElement : nsIDOMXULControlElement
     {
       // inputField may be any type of editable field, such as an
       // HTML <input type="text"> or <textarea>
       readonly attribute nsIDOMNode inputField;
    
  6. Because there is no indication that the change to that file affects other files, option "d" to keep the file deleted is the correct choice.
  7. Complete the merge by committing and pushing.