This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8974

merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent
ClosedPublic

Authored by pulkit on Sep 1 2020, 8:13 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Commits
rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent
Summary

There are files which were deleted/not present in working copy parent but were
present on other side of merge. On merge, we might decide to keep them deleted.
We want to track such cases more closely, rather all kind of cases which results
from some kind of merging logic.

We do have ACTION_KEEP but having a dedicated action for the absent case is
more cleaner.

Initially I named the action as ACTION_KEEP_DELETED but later realized that
file can be not-present because of other reasons than deletion like rename,
hence decided to use more generic name ACTION_KEEP_ABSENT.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Sep 1 2020, 8:13 AM
pulkit retitled this revision from merge: add `ACTION_KEEP_DELETED` to represent files we want to keep deleted to merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent.Sep 2 2020, 7:53 AM
pulkit edited the summary of this revision. (Show Details)
pulkit updated this revision to Diff 22508.
marmoute requested changes to this revision.Sep 11 2020, 4:55 AM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
mercurial/merge.py
566–570

Can you split the introduction on NO_OP_ACTIONS in its own changeset so that the change coming with ACTION_KEEP_ABSENT get clearer?

This revision now requires changes to proceed.Sep 11 2020, 4:55 AM
marmoute added a comment.Sep 11 2020, 5:35 AM

(I wonder if we don't use "missing" more than "absent" through the code. However quick grepping is not very conclusive and "missing" could be confused with the hg status state)

pulkit added a commit: rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent.Sep 16 2020, 7:49 AM
This revision was not accepted when it landed; it landed in state Needs Revision.
Closed by commit rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (15 lines)
Mmercurial/mergestate.py (8 lines)

Diff 22650

mercurial/merge.py

class mergeresult(object): class mergeresult(object):
''''An object representing result of merging manifests. ''''An object representing result of merging manifests.
It has information about what actions need to be performed on dirstate It has information about what actions need to be performed on dirstate
mapping of divergent renames and other such cases. ''' mapping of divergent renames and other such cases. '''
NO_OP_ACTIONS = (mergestatemod.ACTION_KEEP,) NO_OP_ACTIONS = (
mergestatemod.ACTION_KEEP,
mergestatemod.ACTION_KEEP_ABSENT,
)
marmouteUnsubmitted
Not Done

Can you split the introduction on NO_OP_ACTIONS in its own changeset so that the change coming with ACTION_KEEP_ABSENT get clearer?

marmoute: Can you split the introduction on NO_OP_ACTIONS in its own changeset so that the change coming…
def __init__(self): def __init__(self):
""" """
filemapping: dict of filename as keys and action related info as values filemapping: dict of filename as keys and action related info as values
diverge: mapping of source name -> list of dest name for diverge: mapping of source name -> list of dest name for
divergent renames divergent renames
renamedelete: mapping of source name -> list of destinations for files renamedelete: mapping of source name -> list of destinations for files
deleted on one side and renamed on other. deleted on one side and renamed on other.
commitinfo: dict containing data which should be used on commit commitinfo: dict containing data which should be used on commit
repo.ui.note(_(b" %s: consensus for %s\n") % (f, m)) repo.ui.note(_(b" %s: consensus for %s\n") % (f, m))
mresult.addfile(f, *l[0]) mresult.addfile(f, *l[0])
continue continue
# If keep is an option, just do it. # If keep is an option, just do it.
if mergestatemod.ACTION_KEEP in bids: if mergestatemod.ACTION_KEEP in bids:
repo.ui.note(_(b" %s: picking 'keep' action\n") % f) repo.ui.note(_(b" %s: picking 'keep' action\n") % f)
mresult.addfile(f, *bids[mergestatemod.ACTION_KEEP][0]) mresult.addfile(f, *bids[mergestatemod.ACTION_KEEP][0])
continue continue
# If keep absent is an option, just do that
if mergestatemod.ACTION_KEEP_ABSENT in bids:
repo.ui.note(_(b" %s: picking 'keep absent' action\n") % f)
mresult.addfile(f, *bids[mergestatemod.ACTION_KEEP_ABSENT][0])
continue
# If there are gets and they all agree [how could they not?], do it. # If there are gets and they all agree [how could they not?], do it.
if mergestatemod.ACTION_GET in bids: if mergestatemod.ACTION_GET in bids:
ga0 = bids[mergestatemod.ACTION_GET][0] ga0 = bids[mergestatemod.ACTION_GET][0]
if all(a == ga0 for a in bids[mergestatemod.ACTION_GET][1:]): if all(a == ga0 for a in bids[mergestatemod.ACTION_GET][1:]):
repo.ui.note(_(b" %s: picking 'get' action\n") % f) repo.ui.note(_(b" %s: picking 'get' action\n") % f)
mresult.addfile(f, *ga0) mresult.addfile(f, *ga0)
continue continue
# TODO: Consider other simple actions such as mode changes # TODO: Consider other simple actions such as mode changes
progress.increment(item=f) progress.increment(item=f)
# keep (noop, just log it) # keep (noop, just log it)
for f, args, msg in mresult.getactions( for f, args, msg in mresult.getactions(
(mergestatemod.ACTION_KEEP,), sort=True (mergestatemod.ACTION_KEEP,), sort=True
): ):
repo.ui.debug(b" %s: %s -> k\n" % (f, msg)) repo.ui.debug(b" %s: %s -> k\n" % (f, msg))
# no progress # no progress
for f, args, msg in mresult.getactions(
(mergestatemod.ACTION_KEEP_ABSENT,), sort=True
):
repo.ui.debug(b" %s: %s -> ka\n" % (f, msg))
# no progress
# directory rename, move local # directory rename, move local
for f, args, msg in mresult.getactions( for f, args, msg in mresult.getactions(
(mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,), sort=True (mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,), sort=True
): ):
repo.ui.debug(b" %s: %s -> dm\n" % (f, msg)) repo.ui.debug(b" %s: %s -> dm\n" % (f, msg))
progress.increment(item=f) progress.increment(item=f)
f0, flags = args f0, flags = args

mercurial/mergestate.py

ACTION_ADD_MODIFIED = b'am' ACTION_ADD_MODIFIED = b'am'
ACTION_CREATED = b'c' ACTION_CREATED = b'c'
ACTION_DELETED_CHANGED = b'dc' ACTION_DELETED_CHANGED = b'dc'
ACTION_CHANGED_DELETED = b'cd' ACTION_CHANGED_DELETED = b'cd'
ACTION_MERGE = b'm' ACTION_MERGE = b'm'
ACTION_LOCAL_DIR_RENAME_GET = b'dg' ACTION_LOCAL_DIR_RENAME_GET = b'dg'
ACTION_DIR_RENAME_MOVE_LOCAL = b'dm' ACTION_DIR_RENAME_MOVE_LOCAL = b'dm'
ACTION_KEEP = b'k' ACTION_KEEP = b'k'
# the file was absent on local side before merge and we should
# keep it absent (absent means file not present, it can be a result
# of file deletion, rename etc.)
ACTION_KEEP_ABSENT = b'ka'
ACTION_EXEC = b'e' ACTION_EXEC = b'e'
ACTION_CREATED_MERGE = b'cm' ACTION_CREATED_MERGE = b'cm'
class mergestate(object): class mergestate(object):
'''track 3-way merge state of individual files '''track 3-way merge state of individual files
The merge state is stored on disk when needed. Two files are used: one with The merge state is stored on disk when needed. Two files are used: one with
# exec change # exec change
for f, args, msg in actions.get(ACTION_EXEC, []): for f, args, msg in actions.get(ACTION_EXEC, []):
repo.dirstate.normallookup(f) repo.dirstate.normallookup(f)
# keep # keep
for f, args, msg in actions.get(ACTION_KEEP, []): for f, args, msg in actions.get(ACTION_KEEP, []):
pass pass
# keep deleted
for f, args, msg in actions.get(ACTION_KEEP_ABSENT, []):
pass
# get # get
for f, args, msg in actions.get(ACTION_GET, []): for f, args, msg in actions.get(ACTION_GET, []):
if branchmerge: if branchmerge:
repo.dirstate.otherparent(f) repo.dirstate.otherparent(f)
else: else:
parentfiledata = getfiledata[f] if getfiledata else None parentfiledata = getfiledata[f] if getfiledata else None
repo.dirstate.normal(f, parentfiledata=parentfiledata) repo.dirstate.normal(f, parentfiledata=parentfiledata)