Patchwork D10830: merge: make applyupdates() not mutate mresult argument

login
register
mail settings
Submitter phabricator
Date June 1, 2021, 10:46 p.m.
Message ID <differential-rev-PHID-DREV-67kqooqvoduxteoaz63b-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49150/
State Superseded
Headers show

Comments

phabricator - June 1, 2021, 10:46 p.m.
martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  We have an extension at work that overrides `merge.applyupdates()` to
  make it skip some writes and instead instruct the virtual filesystem
  we use to get a different version. That override doesn't work
  correctly when doing `hg co -m` and there's a modified file in the
  dirstate that's deleted in the destination. That's because
  `applyupdates()` mutates its `mresult` argument and our extension had
  passed in a modified copied of `mresult` to the overridden function,
  which resulted in the mutation not having any effect. This patch fixes
  that by letting the caller (i.e. `merge._update()`) update `mresult`
  with the extra actions instead. Besides fixing our internal extension,
  that seems cleaner to me anyway (better to not mutate `mresult` only
  in some cases and we can skip some of the logic if we're not going to
  update the dirstate anyway).

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/merge.py

CHANGE DETAILS




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

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1724,20 +1724,13 @@ 
     removed += msremoved
 
     extraactions = ms.actions()
-    if extraactions:
-        for k, acts in pycompat.iteritems(extraactions):
-            for a in acts:
-                mresult.addfile(a[0], k, *a[1:])
-            if k == mergestatemod.ACTION_GET and wantfiledata:
-                # no filedata until mergestate is updated to provide it
-                for a in acts:
-                    getfiledata[a[0]] = None
 
     progress.complete()
-    assert len(getfiledata) == (
-        mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0
+    return (
+        updateresult(updated, merged, removed, unresolved),
+        getfiledata,
+        extraactions,
     )
-    return updateresult(updated, merged, removed, unresolved), getfiledata
 
 
 def _advertisefsmonitor(repo, num_gets, p1node):
@@ -2122,7 +2115,7 @@ 
         )
 
         wantfiledata = updatedirstate and not branchmerge
-        stats, getfiledata = applyupdates(
+        stats, getfiledata, extraactions = applyupdates(
             repo,
             mresult,
             wc,
@@ -2133,6 +2126,18 @@ 
         )
 
         if updatedirstate:
+            if extraactions:
+                for k, acts in pycompat.iteritems(extraactions):
+                    for a in acts:
+                        mresult.addfile(a[0], k, *a[1:])
+                    if k == mergestatemod.ACTION_GET and wantfiledata:
+                        # no filedata until mergestate is updated to provide it
+                        for a in acts:
+                            getfiledata[a[0]] = None
+
+            assert len(getfiledata) == (
+                mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0
+            )
             with repo.dirstate.parentchange():
                 repo.setparents(fp1, fp2)
                 mergestatemod.recordupdates(