Patchwork D12154: filemerge: move check for identical sides out of filemerge()

login
register
mail settings
Submitter phabricator
Date Feb. 8, 2022, 9:28 p.m.
Message ID <differential-rev-PHID-DREV-mo4mzevahrne72mua465-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50489/
State New
Headers show

Comments

phabricator - Feb. 8, 2022, 9:28 p.m.
martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  `filemerge.filemerge()` returns `None` if no merge was necessary
  because the two sides were identical. I don't think it should be that
  function's responsibility to handle that case; we should ideally not
  even call `filemerge.filemerge()` if the two inputs identical. This
  patch therefore moves the check out to the caller (`mergestate.py`).
  
  The largefiles test changed because we now notice that the two sides
  made the same change, so we don't consider it a merge. Also note that
  the new message better matches the line above it in the test output.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D12154

AFFECTED FILES
  mercurial/filemerge.py
  mercurial/mergestate.py
  tests/test-largefiles-misc.t

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-largefiles-misc.t b/tests/test-largefiles-misc.t
--- a/tests/test-largefiles-misc.t
+++ b/tests/test-largefiles-misc.t
@@ -962,7 +962,7 @@ 
   what do you want to do? l
   getting changed largefiles
   1 largefiles updated, 0 removed
-  0 files updated, 4 files merged, 0 files removed, 0 files unresolved
+  1 files updated, 3 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
   $ cat f-different
   1
diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -421,6 +421,14 @@ 
             self._restore_backup(wctx[dfile], localkey, flags)
         else:
             wctx[dfile].remove(ignoremissing=True)
+
+        if not fco.cmp(fcd):  # files identical?
+            # If return value of merge is None, then there are no real conflict
+            del self._state[dfile]
+            self._results[dfile] = None, None
+            self._dirty = True
+            return None
+
         merge_ret, deleted = filemerge.filemerge(
             self._repo,
             wctx,
@@ -431,12 +439,6 @@ 
             fca,
             labels=self._labels,
         )
-        if merge_ret is None:
-            # If return value of merge is None, then there are no real conflict
-            del self._state[dfile]
-            self._results[dfile] = None, None
-            self._dirty = True
-            return None
 
         if not merge_ret:
             self.mark(dfile, MERGE_RECORD_RESOLVED)
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -986,10 +986,6 @@ 
 
     Returns whether the merge is complete, the return value of the merge, and
     a boolean indicating whether the file was deleted from disk."""
-
-    if not fco.cmp(fcd):  # files identical?
-        return None, False
-
     ui = repo.ui
     fd = fcd.path()
     uipathfn = scmutil.getuipathfn(repo)