Skip to content

Commit

Permalink
merge: abort merge when there is a conflict with subtree copy
Browse files Browse the repository at this point in the history
Summary:
This is mainly for subtree copying to an exiting directory case. For example:

* B created a subtree copy from `foo` to `foo-stable` based on an ancestor commit.
* C (from another user) added a new file to `foo-stable` and landed the commit first
* when rebasing B onto C, there is a conflict for the `foo-stable` directory. We don't
  want to merge `foo-stable/new_file` into B automatically, this might conflict with B's
  intended state.

```
C add foo-stable/new_file (remote/master)
|
|  B subtree copy foo to foo-stable
| /
A
:
```

This should be an uncommon use case, so as the first step, we will just abort the rebase
for the commit, and ask users to re-create the the subtree copy by checking out to a
commit after C. If users need the change of C, users can do a `subtree graft` or
`subtree merge` to merge it to the `foo-stable` directory. Later, we will handle this
process automatically.

Reviewed By: quark-zju

Differential Revision: D64901698

fbshipit-source-id: 65d1f310196b27c78959e2adbf5d1253f0e346a5
  • Loading branch information
zzl0 authored and facebook-github-bot committed Oct 27, 2024
1 parent 73b4928 commit 123f3f7
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 2 deletions.
25 changes: 24 additions & 1 deletion eden/scm/sapling/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from .i18n import _
from .node import addednodeid, bin, hex, nullhex, nullid, wdirhex
from .pycompat import encodeutf8
from .utils import subtreeutil

# merge action types
ACTION_MERGE = "m"
Expand Down Expand Up @@ -992,6 +993,13 @@ def handle_file_on_other_side(f, diff, reverse_copies):
for k, v in copy.items():
reverse_copies[v].append(k)

subtree_copy_info = subtreeutil.get_branch_info(repo, p2)
subtree_copy_dests = (
[br["to_path"] for br in subtree_copy_info["branches"]]
if subtree_copy_info
else []
)

actions = {}
# (n1, fl1) = "local" (m1)
# (n2, fl2) = "remote" (m2)
Expand All @@ -1002,7 +1010,22 @@ def handle_file_on_other_side(f, diff, reverse_copies):
f2 = m2.ungraftedpath(f1) or f1
fa = ma.ungraftedpath(f1) or f1

if n1 and n2: # file exists on both local and remote side
na = ma.get(fa) # na is None when fa does not exist in ma
fla = ma.flags(fa) # fla is '' when fa does not exist in ma

subtree_copy_dest = subtreeutil.find_enclosing_dest(f1, subtree_copy_dests)
if subtree_copy_dest and (n1 != na or fl1 != fla):
hint = _("use '@prog@ subtree copy' to re-create the directory branch")
if extra_hint := repo.ui.config("subtree", "copy-conflict-hint"):
hint = f"{hint}. {extra_hint}"
raise error.Abort(
_(
"subtree copy dest path '%s' of '%s' has been updated on the other side"
)
% (subtree_copy_dest, p2),
hint=hint,
)
elif n1 and n2: # file exists on both local and remote side
if fa not in ma:
fa = copy.get(f1, None)
if fa is not None:
Expand Down
20 changes: 19 additions & 1 deletion eden/scm/sapling/utils/subtreeutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import json
from operator import itemgetter

from .. import error, node, util
from .. import error, node, pathutil, util
from ..i18n import _

# todo: remove the 'test_' prefix when this feature is stable
Expand Down Expand Up @@ -122,3 +122,21 @@ def validate_path_overlap(from_paths, to_paths):
_("overlapping --from-path '%s' and --to-path '%s'")
% (from_path, to_path)
)


def find_enclosing_dest(target_path, paths):
"""Find the path that contains the target path.
>>> is_in_subtree_copy_dest("a/b/c", ["a/b"])
'a/b'
>>> is_in_subtree_copy_dest("a/b/c", ["a/b/c"])
'a/b/c'
>>> is_in_subtree_copy_dest("a/b/c", ["a/b", "e/f"])
'a/b'
>>> is_in_subtree_copy_dest("a/b/c", ["a/b/c/d", "e/f"])
"""
target_dir = pathutil.dirname(target_path)
for path in paths:
if target_dir.startswith(path) or path == target_path:
return path
return None
17 changes: 17 additions & 0 deletions eden/scm/tests/test-subtree-rebase.t
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,20 @@ test rebase subtree copy commit and keep the subtree copy metadata
rebasing 53a64b86e21e "subtree copy foo to foo2"
$ hg dbsh -c 'print(repo["."].extra())'
{'branch': 'default', 'rebase_source': '53a64b86e21e09413fee85ae45ae94218c365e87', 'test_subtree_copy': '{"branches":[{"from_commit":"b4cb27eee4e2633aae0d62de87523007d1b5bfdd","from_path":"foo","to_path":"foo2"}],"v":1}'}

test rebase subtree copy commit fails if the to-path is updated on the dest side
$ newclientrepo
$ drawdag <<'EOS'
> B C # B/foo2/x = 1a\n2\n3\n
> |/ # C/foo/x = 1\n2\n3a\n
> A # A/foo/x = 1\n2\n3\n
> EOS
$ hg go -q $C
$ hg subtree copy -r $A --from-path foo --to-path foo2 -m "subtree copy foo to foo2"
copying foo to foo2
$ setconfig subtree.copy-conflict-hint="Please see https://abc.com/sapling-subtree-copy-conflict for more help"
$ hg rebase -r . -d $B
rebasing 53a64b86e21e "subtree copy foo to foo2"
abort: subtree copy dest path 'foo2' of '53a64b86e21e' has been updated on the other side
(use 'hg subtree copy' to re-create the directory branch. Please see https://abc.com/sapling-subtree-copy-conflict for more help)
[255]

0 comments on commit 123f3f7

Please sign in to comment.