Patchwork D12119: narrow: allow merging non-conflicting change outside of the narrow spec

login
register
mail settings
Submitter phabricator
Date Jan. 30, 2022, 8:16 p.m.
Message ID <differential-rev-PHID-DREV-xoan35piq3df6w6qrpfm-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50445/
State New
Headers show

Comments

phabricator - Jan. 30, 2022, 8:16 p.m.
marmoute created this revision.
Herald added a reviewer: durin42.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  We use the mergestate to carry information about these merge action and
  reprocess them at commit time to apply the necessary update.
  
  The dirstate itself is never affected and remains "pure", with content only in
  the narrow-spec. This file involved in such merge are therefor not listed in `hg
  status`.
  
  The current testing is based on a modification of the previous testing, that
  refused to do such merges. As a result it is a bit simple and more extensive
  testing will have to be introduced later. I am planning to do this extra
  testing.
  
  In addition, this only works for flat manifest. Support for tree manifest will
  need more work. I am not currently planning to do this work.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/commit.py
  mercurial/merge.py
  tests/test-narrow-merge.t

CHANGE DETAILS




To: marmoute, durin42, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-narrow-merge.t b/tests/test-narrow-merge.t
--- a/tests/test-narrow-merge.t
+++ b/tests/test-narrow-merge.t
@@ -83,12 +83,67 @@ 
 TODO: Can merge non-conflicting changes outside narrow spec
 
   $ hg update -q 'desc("modify inside/f1")'
+
+#if flat
+
   $ hg merge 'desc("modify outside/f1")'
-  abort: merge affects file 'outside/f1' outside narrow, which is not yet supported (flat !)
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+
+status should be clean
+
+  $ hg status
+  ? inside/f1.orig
+
+file out of the spec should still not be in the dirstate at all
+
+  $ hg debugdirstate | grep outside/f1
+  [1]
+
+Commit that merge
+
+  $ hg ci -m 'merge from outside to inside'
+
+status should be clean
+
+  $ hg status
+  ? inside/f1.orig
+
+file out of the spec should not be in the mergestate anymore
+
+  $ hg debugmergestate | grep outside/f1
+  [1]
+
+file out of the spec should still not be in the dirstate at all
+
+  $ hg debugdirstate | grep outside/f1
+  [1]
+
+The filenode used should come from p2
+
+  $ hg manifest --debug --rev . | grep outside/f1
+  83cd11431a3b2aff8a3995e5f27bcf33cdb5be98 644   outside/f1
+  $ hg manifest --debug --rev 'p1(.)' | grep outside/f1
+  c6b956c48be2cd4fa94be16002aba311143806fa 644   outside/f1
+  $ hg manifest --debug --rev 'p2(.)' | grep outside/f1
+  83cd11431a3b2aff8a3995e5f27bcf33cdb5be98 644   outside/f1
+
+
+remove the commit to get in the previous situation again
+
+  $ hg debugstrip -r .
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  saved backup bundle to $TESTTMP/narrow/.hg/strip-backup/48eb25338b19-a1bb8350-backup.hg
+
+#else
+
+  $ hg merge 'desc("modify outside/f1")'
   abort: merge affects file 'outside/' outside narrow, which is not yet supported (tree !)
   (merging in the other direction may work)
   [255]
 
+#endif
+
   $ hg update -q 'desc("modify outside/f1")'
   $ hg merge 'desc("modify inside/f1")'
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -518,13 +518,20 @@ 
             mresult.removefile(f)  # just updating, ignore changes outside clone
         elif action[0].no_op:
             mresult.removefile(f)  # merge does not affect file
-        elif action[0].narrow_safe:  # TODO: handle these cases
-            msg = _(
-                b'merge affects file \'%s\' outside narrow, '
-                b'which is not yet supported'
-            )
-            hint = _(b'merging in the other direction may work')
-            raise error.Abort(msg % f, hint=hint)
+        elif action[0].narrow_safe:
+            if not f.endswith(b'/'):
+                mresult.removefile(f)  # merge won't affect on-disk files
+
+                mresult.addcommitinfo(
+                    f, b'outside-narrow-merge-action', action[0].changes
+                )
+            else:  # TODO: handle the tree case
+                msg = _(
+                    b'merge affects file \'%s\' outside narrow, '
+                    b'which is not yet supported'
+                )
+                hint = _(b'merging in the other direction may work')
+                raise error.Abort(msg % f, hint=hint)
         else:
             msg = _(b'conflict in file \'%s\' is outside narrow clone')
             raise error.StateError(msg % f)
diff --git a/mercurial/commit.py b/mercurial/commit.py
--- a/mercurial/commit.py
+++ b/mercurial/commit.py
@@ -134,7 +134,13 @@ 
     for s in salvaged:
         files.mark_salvaged(s)
 
-    if ctx.manifestnode():
+    narrow_files = {}
+    if not ctx.repo().narrowmatch().always():
+        for f, e in ms.allextras().items():
+            action = e.get(b'outside-narrow-merge-action')
+            if action is not None:
+                narrow_files[f] = action
+    if ctx.manifestnode() and not narrow_files:
         # reuse an existing manifest revision
         repo.ui.debug(b'reusing known manifest\n')
         mn = ctx.manifestnode()
@@ -142,11 +148,11 @@ 
         if writechangesetcopy:
             files.update_added(ctx.filesadded())
             files.update_removed(ctx.filesremoved())
-    elif not ctx.files():
+    elif not ctx.files() and not narrow_files:
         repo.ui.debug(b'reusing manifest from p1 (no file change)\n')
         mn = p1.manifestnode()
     else:
-        mn = _process_files(tr, ctx, ms, files, error=error)
+        mn = _process_files(tr, ctx, ms, files, narrow_files, error=error)
 
     if origctx and origctx.manifestnode() == mn:
         origfiles = origctx.files()
@@ -177,7 +183,7 @@ 
     return salvaged
 
 
-def _process_files(tr, ctx, ms, files, error=False):
+def _process_files(tr, ctx, ms, files, narrow_files=None, error=False):
     repo = ctx.repo()
     p1 = ctx.p1()
     p2 = ctx.p2()
@@ -198,8 +204,33 @@ 
     linkrev = len(repo)
     repo.ui.note(_(b"committing files:\n"))
     uipathfn = scmutil.getuipathfn(repo)
-    for f in sorted(ctx.modified() + ctx.added()):
+    all_files = ctx.modified() + ctx.added()
+    all_files.extend(narrow_files.keys())
+    all_files.sort()
+    for f in all_files:
         repo.ui.note(uipathfn(f) + b"\n")
+        if f in narrow_files:
+            narrow_action = narrow_files.get(f)
+            if narrow_action == mergestate.CHANGE_REMOVED:
+                files.mark_removed(f)
+                removed.append(f)
+            elif narrow_action == mergestate.CHANGE_REMOVED:
+                files.mark_added(f)
+                added.append(f)
+                m[f] = m2[f]
+                flags = m2ctx.find(f)[1] or b''
+                m.setflag(f, flags)
+            elif narrow_action == mergestate.CHANGE_MODIFIED:
+                files.mark_touched(f)
+                added.append(f)
+                m[f] = m2[f]
+                flags = m2ctx.find(f)[1] or b''
+                m.setflag(f, flags)
+            else:
+                msg = _(b"corrupted mergestate, unkown narrow action: %b")
+                hint = _(b"restart the merge")
+                raise error.Abort(msg, hint=hint)
+            continue
         try:
             fctx = ctx[f]
             if fctx is None:
@@ -239,7 +270,17 @@ 
             if not rf(f):
                 files.mark_removed(f)
 
-    mn = _commit_manifest(tr, linkrev, ctx, mctx, m, files.touched, added, drop)
+    mn = _commit_manifest(
+        tr,
+        linkrev,
+        ctx,
+        mctx,
+        m,
+        files.touched,
+        added,
+        drop,
+        bool(narrow_files),
+    )
 
     return mn
 
@@ -409,7 +450,17 @@ 
     return fnode, touched
 
 
-def _commit_manifest(tr, linkrev, ctx, mctx, manifest, files, added, drop):
+def _commit_manifest(
+    tr,
+    linkrev,
+    ctx,
+    mctx,
+    manifest,
+    files,
+    added,
+    drop,
+    has_some_narrow_action=False,
+):
     """make a new manifest entry (or reuse a new one)
 
     given an initialised manifest context and precomputed list of
@@ -451,6 +502,10 @@ 
         # at this point is merges, and we already error out in the
         # case where the merge has files outside of the narrowspec,
         # so this is safe.
+        if has_some_narrow_action:
+            match = None
+        else:
+            match = repo.narrowmatch()
         mn = mctx.write(
             tr,
             linkrev,
@@ -458,7 +513,7 @@ 
             p2.manifestnode(),
             added,
             drop,
-            match=repo.narrowmatch(),
+            match=match,
         )
     else:
         repo.ui.debug(