This is an archive of the discontinued Mercurial Phabricator instance.

Differential D776

merge: add merge action 'p' to record path conflicts during update
ClosedPublic

Authored by mbthomas on Sep 22 2017, 5:28 AM.

Details

Reviewers
ryanmce
Group Reviewers
hg-reviewers
Commits
rHG81aebcc73beb: merge: add merge action 'p' to record path conflicts during update
rHG337512875503: merge: add merge action 'p' to record path conflicts during update
Summary

Add a new merge action to record path conflicts. A status message is
printed, and the path conflict is added to the merge state.

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

mbthomas created this revision.Sep 22 2017, 5:28 AM
ryanmce accepted this revision.Sep 25 2017, 3:35 PM
ryanmce added a subscriber: ryanmce.

There's some opportunities here but most are definitely outside the scope of this patch. One inline to consider but do what makes sense to you.

mercurial/merge.py
1254–1263

I've seen this trick used in the mercurial codebase to fit more on a single line:

s = repo.ui.status
...
s(_("%s: path conflict - a file or link has the same name as a directory\n") % f)
...
s(_("resolve manually then use 'hg resolve --mark %s'\n") % f)
...

It may help to tighten up the code a bit here.

1717

future refactoring opportunity! -- this is incredibly ugly, but it's not your fault.

mercurial/sparse.py
490

Another future refactoring opportunity.

mbthomas updated this revision to Diff 2212.Oct 1 2017, 5:32 AM
mbthomas marked an inline comment as done.Oct 1 2017, 6:38 AM
mbthomas updated this revision to Diff 2353.Oct 2 2017, 5:15 PM
ryanmce accepted this revision.Oct 5 2017, 10:51 AM

lgtm

Closed by commit rHG337512875503: merge: add merge action 'p' to record path conflicts during update (authored by mbthomas). · Explain WhyOct 9 2017, 9:25 PM
This revision was automatically updated to reflect the committed changes.
mbolin added a subscriber: mbolin.Oct 31 2017, 12:56 PM

What test verifies this behavior? Do we need a new one?

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (22 lines)
Mmercurial/sparse.py (3 lines)

Diff 2547

mercurial/merge.py

# remove renamed files after safely stored # remove renamed files after safely stored
for f in moves: for f in moves:
if wctx[f].lexists(): if wctx[f].lexists():
repo.ui.debug("removing %s\n" % f) repo.ui.debug("removing %s\n" % f)
wctx[f].audit() wctx[f].audit()
wctx[f].remove() wctx[f].remove()
numupdates = sum(len(l) for m, l in actions.items() if m != 'k') numupdates = sum(len(l) for m, l in actions.items() if m != 'k')
z = 0
if [a for a in actions['r'] if a[0] == '.hgsubstate']: if [a for a in actions['r'] if a[0] == '.hgsubstate']:
subrepo.submerge(repo, wctx, mctx, wctx, overwrite, labels) subrepo.submerge(repo, wctx, mctx, wctx, overwrite, labels)
# remove in parallel (must come first) # record path conflicts
z = 0 for f, args, msg in actions['p']:
f1, fo = args
s = repo.ui.status
s(_("%s: path conflict - a file or link has the same name as a "
"directory\n") % f)
if fo == 'l':
s(_("the local file has been renamed to %s\n") % f1)
else:
s(_("the remote file has been renamed to %s\n") % f1)
s(_("resolve manually then use 'hg resolve --mark %s'\n") % f)
ms.addpath(f, f1, fo)
z += 1
ryanmceUnsubmitted
Done

I've seen this trick used in the mercurial codebase to fit more on a single line:

s = repo.ui.status
...
s(_("%s: path conflict - a file or link has the same name as a directory\n") % f)
...
s(_("resolve manually then use 'hg resolve --mark %s'\n") % f)
...

It may help to tighten up the code a bit here.

ryanmce: I've seen this trick used in the mercurial codebase to fit more on a single line: ``` s = repo.
progress(_updating, z, item=f, total=numupdates, unit=_files)
# remove in parallel (must come before getting)
prog = worker.worker(repo.ui, 0.001, batchremove, (repo, wctx), prog = worker.worker(repo.ui, 0.001, batchremove, (repo, wctx),
actions['r']) actions['r'])
for i, item in prog: for i, item in prog:
z += i z += i
progress(_updating, z, item=item, total=numupdates, unit=_files) progress(_updating, z, item=item, total=numupdates, unit=_files)
removed = len(actions['r']) removed = len(actions['r'])
# We should flush before forking into worker processes, since those workers # We should flush before forking into worker processes, since those workers
_("other%(o)s changed %(f)s which local%(l)s deleted\n" _("other%(o)s changed %(f)s which local%(l)s deleted\n"
"use (c)hanged version or leave (d)eleted?" "use (c)hanged version or leave (d)eleted?"
"$$ &Changed $$ &Deleted") % prompts, 0) == 0: "$$ &Changed $$ &Deleted") % prompts, 0) == 0:
actionbyfile[f] = ('g', (flags, False), "prompt recreating") actionbyfile[f] = ('g', (flags, False), "prompt recreating")
else: else:
del actionbyfile[f] del actionbyfile[f]
# Convert to dictionary-of-lists format # Convert to dictionary-of-lists format
actions = dict((m, []) for m in 'a am f g cd dc r dm dg m e k'.split()) actions = dict((m, [])
for m in 'a am f g cd dc r dm dg m e k p'.split())
ryanmceUnsubmitted
Not Done

future refactoring opportunity! -- this is incredibly ugly, but it's not your fault.

ryanmce: future refactoring opportunity! -- this is incredibly ugly, but it's not your fault.
for f, (m, args, msg) in actionbyfile.iteritems(): for f, (m, args, msg) in actionbyfile.iteritems():
if m not in actions: if m not in actions:
actions[m] = [] actions[m] = []
actions[m].append((f, args, msg)) actions[m].append((f, args, msg))
if not util.fscasesensitive(repo.path): if not util.fscasesensitive(repo.path):
# check collision between files only in p2 for clean update # check collision between files only in p2 for clean update
if (not branchmerge and if (not branchmerge and

mercurial/sparse.py

for file, state in dirstate.iteritems(): for file, state in dirstate.iteritems():
if not file in files: if not file in files:
old = origsparsematch(file) old = origsparsematch(file)
new = sparsematch(file) new = sparsematch(file)
if old and not new: if old and not new:
dropped.append(file) dropped.append(file)
# Apply changes to disk # Apply changes to disk
typeactions = dict((m, []) for m in 'a f g am cd dc r dm dg m e k'.split()) typeactions = dict((m, [])
for m in 'a f g am cd dc r dm dg m e k p'.split())
ryanmceUnsubmitted
Not Done

Another future refactoring opportunity.

ryanmce: Another future refactoring opportunity.
for f, (m, args, msg) in actions.iteritems(): for f, (m, args, msg) in actions.iteritems():
if m not in typeactions: if m not in typeactions:
typeactions[m] = [] typeactions[m] = []
typeactions[m].append((f, args, msg)) typeactions[m].append((f, args, msg))
mergemod.applyupdates(repo, typeactions, repo[None], repo['.'], False) mergemod.applyupdates(repo, typeactions, repo[None], repo['.'], False)
# Fix dirstate # Fix dirstate