This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8988

merge: store commitinfo if mergetool resolved a dc or cd conflict
ClosedPublic

Authored by pulkit on Sep 5 2020, 3:10 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHG4c8a93ec6908: merge: store commitinfo if these is a dc or cd conflict
Summary

delete-changed or changed-delete conflicts can be resolved by mergetool, either
if some tool is passed and using or by user choose something on prompt.

If the user decides to keep the changed side, on commit we just reuse the parent
filenode. This is mostly fine unless we are in a distributed environment and
people are doing criss-cross merges.

Since, we don't have recursive merges or any other way of describing the end
result of the merge was an explicit choice and it should be differentiated from
it's ancestors, merge algo during criss-cross merges fails to take in account
the explicit choice made by user and end up with a what-can-be-said-wrong-merge.

The solution which we are trying to fix this is by creating a filenode on commit
instead of reusing the parent filenode. This helps differentiate between
pre-merged filenode and post-merge filenode and kind of tells about the choice
user made.

To implement creating new filenode functionality, we store info about these
files in mergestate so that we can read them on commit and force create a new
filenode.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Sep 5 2020, 3:10 AM
pulkit updated this revision to Diff 22587.Sep 9 2020, 9:44 AM
marmoute requested changes to this revision.Sep 11 2020, 5:12 AM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
mercurial/merge.py
1735

We should get something more descriptive here. like:

Instead of:

merge code:  "sudo make a new file node"
commit code: "okay, I guess"

More

merge code:  "This file might have been deleted by the merge"
commit code: "ho ho ho, I'll use a new filenode to convey that !"
This revision now requires changes to proceed.Sep 11 2020, 5:12 AM
marmoute added inline comments.Sep 11 2020, 5:17 AM
tests/test-merge-criss-cross.t
530

We should introduce the hg debugmergstate in a previous changeset so that we know what change this one introduce. Should we not ?

pulkit added a commit: rHG4c8a93ec6908: merge: store commitinfo if these is a dc or cd conflict.Sep 28 2020, 7:20 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG4c8a93ec6908: merge: store commitinfo if these is a dc or cd conflict (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (21 lines)
Mtests/test-merge-criss-cross.t (4 lines)

Diff 22672

mercurial/merge.py

repo, wctx, mctx, wctx.ancestor(mctx), overwrite, labels repo, wctx, mctx, wctx.ancestor(mctx), overwrite, labels
) )
continue continue
wctx[f].audit() wctx[f].audit()
complete, r = ms.preresolve(f, wctx) complete, r = ms.preresolve(f, wctx)
if not complete: if not complete:
numupdates += 1 numupdates += 1
tocomplete.append((f, args, msg)) tocomplete.append((f, args, msg))
else:
action = mresult.getfile(f)[0]
if action in (
mergestatemod.ACTION_DELETED_CHANGED,
mergestatemod.ACTION_CHANGED_DELETED,
):
# the merge was conflicting and either the mergetool
# choosed by the user or the user themselves on the prompt
# made a choice.
# If the changed file was kept, at commit we end up using
# the same filelog. This is not completely correct behavior
# as the new file post merge also represents that a
# conflicting change-delete merge was resolved in it's
# favor. Hence we will like to create a new filenode for
# that. Let's store this in mergestate extras
repo.ui.debug(
b"file %s had %s conflicts and mergetool resolved it, storing info in commitinfo\n"
% (f, action)
)
ms.addcommitinfo(f, {b'MERGE_DELETE_CANDIDATE': b'yes'})
marmouteUnsubmitted
Not Done

We should get something more descriptive here. like:

Instead of:

merge code:  "sudo make a new file node"
commit code: "okay, I guess"

More

merge code:  "This file might have been deleted by the merge"
commit code: "ho ho ho, I'll use a new filenode to convey that !"
marmoute: We should get something more descriptive here. like: Instead of: ``` merge code: "sudo make…
# merge # merge
for f, args, msg in tocomplete: for f, args, msg in tocomplete:
repo.ui.debug(b" %s: %s -> m (merge)\n" % (f, msg)) repo.ui.debug(b" %s: %s -> m (merge)\n" % (f, msg))
progress.increment(item=f, total=numupdates) progress.increment(item=f, total=numupdates)
ms.resolve(f, wctx) ms.resolve(f, wctx)
finally: finally:

tests/test-merge-criss-cross.t

Do all the merge combination (from the deleted or the update side × keeping and deleting the file Do all the merge combination (from the deleted or the update side × keeping and deleting the file
$ hg update 'desc("delete-the-file")' $ hg update 'desc("delete-the-file")'
0 files updated, 0 files merged, 0 files removed, 0 files unresolved 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg merge 'desc("updating-both-file")' -t :local $ hg merge 'desc("updating-both-file")' -t :local
1 files updated, 1 files merged, 0 files removed, 0 files unresolved 1 files updated, 1 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit) (branch merge, don't forget to commit)
$ hg debugmergestate $ hg debugmergestate
marmouteUnsubmitted
Not Done

We should introduce the hg debugmergstate in a previous changeset so that we know what change this one introduce. Should we not ?

marmoute: We should introduce the `hg debugmergstate` in a previous changeset so that we know what change…
local (working copy): 7801bc9b9899de5e304bd162cafde9b78e10ab9b local (working copy): 7801bc9b9899de5e304bd162cafde9b78e10ab9b
other (merge rev): 9b610631ab29024c5f44af7d2c19658ef8f8f071 other (merge rev): 9b610631ab29024c5f44af7d2c19658ef8f8f071
file: the-file (state "r") file: the-file (state "r")
local path: the-file (hash 0000000000000000000000000000000000000000, flags "") local path: the-file (hash 0000000000000000000000000000000000000000, flags "")
ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a) ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a)
other path: the-file (node 59e363a07dc876278f0e41756236f30213b6b460) other path: the-file (node 59e363a07dc876278f0e41756236f30213b6b460)
extra: MERGE_DELETE_CANDIDATE = yes
extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589 extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589
extra: other-file (filenode-source = other) extra: other-file (filenode-source = other)
$ hg ci -m "merge-deleting-the-file-from-deleted" $ hg ci -m "merge-deleting-the-file-from-deleted"
$ hg manifest $ hg manifest
other-file other-file
$ hg debugrevlogindex the-file $ hg debugrevlogindex the-file
rev linkrev nodeid p1 p2 rev linkrev nodeid p1 p2
0 0 4b69178b9bda 000000000000 000000000000 0 0 4b69178b9bda 000000000000 000000000000
1 1 59e363a07dc8 4b69178b9bda 000000000000 1 1 59e363a07dc8 4b69178b9bda 000000000000
$ hg update 'desc("updating-both-file")' $ hg update 'desc("updating-both-file")'
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg merge 'desc("delete-the-file")' -t :other $ hg merge 'desc("delete-the-file")' -t :other
0 files updated, 0 files merged, 1 files removed, 0 files unresolved 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
(branch merge, don't forget to commit) (branch merge, don't forget to commit)
$ hg debugmergestate $ hg debugmergestate
local (working copy): 9b610631ab29024c5f44af7d2c19658ef8f8f071 local (working copy): 9b610631ab29024c5f44af7d2c19658ef8f8f071
other (merge rev): 7801bc9b9899de5e304bd162cafde9b78e10ab9b other (merge rev): 7801bc9b9899de5e304bd162cafde9b78e10ab9b
file: the-file (state "r") file: the-file (state "r")
local path: the-file (hash 6d2e02da5a9fe0691363dc6b573845fa271eaa35, flags "") local path: the-file (hash 6d2e02da5a9fe0691363dc6b573845fa271eaa35, flags "")
ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a) ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a)
other path: the-file (node 0000000000000000000000000000000000000000) other path: the-file (node 0000000000000000000000000000000000000000)
extra: MERGE_DELETE_CANDIDATE = yes
extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589 extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589
$ hg ci -m "merge-deleting-the-file-from-updated" $ hg ci -m "merge-deleting-the-file-from-updated"
created new head created new head
$ hg manifest $ hg manifest
other-file other-file
$ hg debugrevlogindex the-file $ hg debugrevlogindex the-file
rev linkrev nodeid p1 p2 rev linkrev nodeid p1 p2
0 0 4b69178b9bda 000000000000 000000000000 0 0 4b69178b9bda 000000000000 000000000000
1 1 59e363a07dc8 4b69178b9bda 000000000000 1 1 59e363a07dc8 4b69178b9bda 000000000000
$ hg update 'desc("delete-the-file")' $ hg update 'desc("delete-the-file")'
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg merge 'desc("updating-both-file")' -t :other $ hg merge 'desc("updating-both-file")' -t :other
1 files updated, 1 files merged, 0 files removed, 0 files unresolved 1 files updated, 1 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit) (branch merge, don't forget to commit)
$ hg debugmergestate $ hg debugmergestate
local (working copy): 7801bc9b9899de5e304bd162cafde9b78e10ab9b local (working copy): 7801bc9b9899de5e304bd162cafde9b78e10ab9b
other (merge rev): 9b610631ab29024c5f44af7d2c19658ef8f8f071 other (merge rev): 9b610631ab29024c5f44af7d2c19658ef8f8f071
file: the-file (state "r") file: the-file (state "r")
local path: the-file (hash 0000000000000000000000000000000000000000, flags "") local path: the-file (hash 0000000000000000000000000000000000000000, flags "")
ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a) ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a)
other path: the-file (node 59e363a07dc876278f0e41756236f30213b6b460) other path: the-file (node 59e363a07dc876278f0e41756236f30213b6b460)
extra: MERGE_DELETE_CANDIDATE = yes
extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589 extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589
extra: other-file (filenode-source = other) extra: other-file (filenode-source = other)
$ hg ci -m "merge-keeping-the-file-from-deleted" $ hg ci -m "merge-keeping-the-file-from-deleted"
created new head created new head
$ hg manifest $ hg manifest
other-file other-file
the-file the-file
(branch merge, don't forget to commit) (branch merge, don't forget to commit)
$ hg debugmergestate $ hg debugmergestate
local (working copy): 9b610631ab29024c5f44af7d2c19658ef8f8f071 local (working copy): 9b610631ab29024c5f44af7d2c19658ef8f8f071
other (merge rev): 7801bc9b9899de5e304bd162cafde9b78e10ab9b other (merge rev): 7801bc9b9899de5e304bd162cafde9b78e10ab9b
file: the-file (state "r") file: the-file (state "r")
local path: the-file (hash 6d2e02da5a9fe0691363dc6b573845fa271eaa35, flags "") local path: the-file (hash 6d2e02da5a9fe0691363dc6b573845fa271eaa35, flags "")
ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a) ancestor path: the-file (node 4b69178b9bdae28b651393b46e631427a72f217a)
other path: the-file (node 0000000000000000000000000000000000000000) other path: the-file (node 0000000000000000000000000000000000000000)
extra: MERGE_DELETE_CANDIDATE = yes
extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589 extra: ancestorlinknode = 955800955977bd6c103836ee3e437276e940a589
$ hg ci -m "merge-keeping-the-file-from-updated" $ hg ci -m "merge-keeping-the-file-from-updated"
created new head created new head
$ hg manifest $ hg manifest
other-file other-file
the-file the-file
XXX: This should create a new filenode because user explicitly decided to keep XXX: This should create a new filenode because user explicitly decided to keep