This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4035

resolve: add option to warn/abort on -m with unresolved conflict markers
ClosedPublic

Authored by spectral on Aug 1 2018, 4:49 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG6c8e3c847977: resolve: add option to warn/abort on -m with unresolved conflict markers
rHGe1fd054459fe: resolve: add option to warn/abort on -m with unresolved conflict markers
rHGbb54db4a4350: resolve: add option to warn/abort on -m with unresolved conflict markers
Summary

When a user is dropped out of Mercurial to a terminal to resolve files, we emit
messages like:

conflicts while merging file1! (edit, then use 'hg resolve --mark')
conflicts while merging file2! (edit, then use 'hg resolve --mark')

We don't mention a file name in the hint, so some users might do something like
$EDITOR file1; hg resolve --mark, see that it says "(no more unresolved
files)" and forget to deal with file2 before running the next command.

Even if we did mention a file name in the hint, it's too easy to forget it
(maybe the merge spans a couple days or something). This option lets us inform
the user that they might have missed something.

In the scenario above, the output would be something like:

warning: the following files still have conflict markers:
  file2
(no more unresolved files)

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

spectral created this revision.Aug 1 2018, 4:49 PM
pulkit added subscribers: khanchi97, pulkit.Aug 1 2018, 4:53 PM

I like the idea here. @khanchi97 is already working on adding --confirm to hg resolve, how about having something like hg resolve --mark --confirm which asks you before marking each file as resolved?

spectral added a comment.Aug 1 2018, 5:23 PM
In D4035#62542, @pulkit wrote:

I like the idea here. @khanchi97 is already working on adding --confirm to hg resolve, how about having something like hg resolve --mark --confirm which asks you before marking each file as resolved?

My primary goal is that hg resolve --mark be made safer. The exact mechanism isn't too important to me :) If this involves a --confirm mode that can be defaulted to on, preferably with an auto-confirm-if-no-markers-detected, that's fine with me. If it requires explicit opt-in on the commandline (instead of being something that can be turned on with a config), I don't think that it would accomplish the same safety improvement.

pulkit added a comment.Aug 1 2018, 5:55 PM
In D4035#62582, @spectral wrote:
In D4035#62542, @pulkit wrote:

I like the idea here. @khanchi97 is already working on adding --confirm to hg resolve, how about having something like hg resolve --mark --confirm which asks you before marking each file as resolved?

My primary goal is that hg resolve --mark be made safer. The exact mechanism isn't too important to me :) If this involves a --confirm mode that can be defaulted to on, preferably with an auto-confirm-if-no-markers-detected, that's fine with me. If it requires explicit opt-in on the commandline (instead of being something that can be turned on with a config), I don't think that it would accomplish the same safety improvement.

Yep, I agree with you. Queued this patch, many thanks! Can you add about this on https://www.mercurial-scm.org/wiki/Release4.8? We have a section for experimental features.

Closed by commit rHGbb54db4a4350: resolve: add option to warn/abort on -m with unresolved conflict markers (authored by spectral). · Explain WhyAug 1 2018, 5:57 PM
This revision was automatically updated to reflect the committed changes.
pulkit added a comment.Aug 1 2018, 6:01 PM

Also, even though this is experimental, can we document it somewhere?

spectral added a comment.Aug 1 2018, 6:07 PM
In D4035#62660, @pulkit wrote:

Also, even though this is experimental, can we document it somewhere?

Sure, any advice for where would be a good place to do that? I would have thought mercurial/help/config.txt, but there's nothing in there about experimental options right now.

I'm also fine skipping a prolonged 'experimental' phase and moving out of experimental now/soon. I generally default to starting things in experimental, but maybe I shouldn't be doing that, and just land things like this completely to start with? :)

pulkit added a comment.Aug 2 2018, 5:55 PM
In D4035#62661, @spectral wrote:
In D4035#62660, @pulkit wrote:

Also, even though this is experimental, can we document it somewhere?

Sure, any advice for where would be a good place to do that? I would have thought mercurial/help/config.txt, but there's nothing in there about experimental options right now.

I think we need 'mercurial/help/experimental.txt' or something.

I'm also fine skipping a prolonged 'experimental' phase and moving out of experimental now/soon. I generally default to starting things in experimental, but maybe I shouldn't be doing that, and just land things like this completely to start with? :)

As Augie also mentioned this looks appropriate as non-experimental, lets move this out of experimental and hence document that.

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (17 lines)
Mmercurial/configitems.py (3 lines)
Mmercurial/filemerge.py (7 lines)
Mtests/test-resolve.t (49 lines)

Diff 9751

mercurial/commands.py

debugcommands as debugcommandsmod, debugcommands as debugcommandsmod,
destutil, destutil,
dirstateguard, dirstateguard,
discovery, discovery,
encoding, encoding,
error, error,
exchange, exchange,
extensions, extensions,
filemerge,
formatter, formatter,
graphmod, graphmod,
hbisect, hbisect,
help, help,
hg, hg,
logcmdutil, logcmdutil,
merge as mergemod, merge as mergemod,
obsolete, obsolete,
return 1 return 1
m = scmutil.match(wctx, pats, opts) m = scmutil.match(wctx, pats, opts)
ret = 0 ret = 0
didwork = False didwork = False
runconclude = False runconclude = False
tocomplete = [] tocomplete = []
hasconflictmarkers = []
if mark:
markcheck = ui.config('experimental', 'resolve.mark-check')
for f in ms: for f in ms:
if not m(f): if not m(f):
continue continue
didwork = True didwork = True
# don't let driver-resolved files be marked, and run the conclude # don't let driver-resolved files be marked, and run the conclude
# step if asked to resolve # step if asked to resolve
elif unmark: elif unmark:
ms.mark(f, mergemod.MERGE_RECORD_UNRESOLVED_PATH) ms.mark(f, mergemod.MERGE_RECORD_UNRESOLVED_PATH)
elif ms[f] == mergemod.MERGE_RECORD_UNRESOLVED_PATH: elif ms[f] == mergemod.MERGE_RECORD_UNRESOLVED_PATH:
ui.warn(_('%s: path conflict must be resolved manually\n') ui.warn(_('%s: path conflict must be resolved manually\n')
% f) % f)
continue continue
if mark: if mark:
if markcheck:
with repo.wvfs(f) as fobj:
fdata = fobj.read()
if filemerge.hasconflictmarkers(fdata) and \
ms[f] != mergemod.MERGE_RECORD_RESOLVED:
hasconflictmarkers.append(f)
ms.mark(f, mergemod.MERGE_RECORD_RESOLVED) ms.mark(f, mergemod.MERGE_RECORD_RESOLVED)
elif unmark: elif unmark:
ms.mark(f, mergemod.MERGE_RECORD_UNRESOLVED) ms.mark(f, mergemod.MERGE_RECORD_UNRESOLVED)
else: else:
# backup pre-resolve (merge uses .orig for its own purposes) # backup pre-resolve (merge uses .orig for its own purposes)
a = repo.wjoin(f) a = repo.wjoin(f)
try: try:
util.copyfile(a, a + ".resolve") util.copyfile(a, a + ".resolve")
if complete: if complete:
try: try:
util.rename(a + ".resolve", util.rename(a + ".resolve",
scmutil.origpath(ui, repo, a)) scmutil.origpath(ui, repo, a))
except OSError as inst: except OSError as inst:
if inst.errno != errno.ENOENT: if inst.errno != errno.ENOENT:
raise raise
if hasconflictmarkers:
ui.warn(_('warning: the following files still have conflict '
'markers:\n ') + '\n '.join(hasconflictmarkers) + '\n')
if markcheck == 'abort' and not all:
raise error.Abort(_('conflict markers detected'),
hint=_('use --all to mark anyway'))
for f in tocomplete: for f in tocomplete:
try: try:
# resolve file # resolve file
overrides = {('ui', 'forcemerge'): opts.get('tool', '')} overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
with ui.configoverride(overrides, 'resolve'): with ui.configoverride(overrides, 'resolve'):
r = ms.resolve(f, wctx) r = ms.resolve(f, wctx)
if r: if r:
ret = 1 ret = 1

mercurial/configitems.py

) )
coreconfigitem('push', 'pushvars.server', coreconfigitem('push', 'pushvars.server',
default=False, default=False,
) )
coreconfigitem('storage', 'revlog.optimize-delta-parent-choice', coreconfigitem('storage', 'revlog.optimize-delta-parent-choice',
default=True, default=True,
alias=[('format', 'aggressivemergedeltas')], alias=[('format', 'aggressivemergedeltas')],
) )
coreconfigitem('experimental', 'resolve.mark-check',
default=None,
)
coreconfigitem('server', 'bookmarks-pushkey-compat', coreconfigitem('server', 'bookmarks-pushkey-compat',
default=True, default=True,
) )
coreconfigitem('server', 'bundle1', coreconfigitem('server', 'bundle1',
default=True, default=True,
) )
coreconfigitem('server', 'bundle1gd', coreconfigitem('server', 'bundle1gd',
default=None, default=None,

mercurial/filemerge.py

if action == 'prompt': if action == 'prompt':
msg = _('continue merge operation (yn)?' '$$ &Yes $$ &No') msg = _('continue merge operation (yn)?' '$$ &Yes $$ &No')
if ui.promptchoice(msg, 0) == 1: if ui.promptchoice(msg, 0) == 1:
_haltmerge() _haltmerge()
if action == 'halt': if action == 'halt':
_haltmerge() _haltmerge()
# default action is 'continue', in which case we neither prompt nor halt # default action is 'continue', in which case we neither prompt nor halt
def hasconflictmarkers(data):
return bool(re.search("^(<<<<<<< .*|=======|>>>>>>> .*)$", data,
re.MULTILINE))
def _check(repo, r, ui, tool, fcd, files): def _check(repo, r, ui, tool, fcd, files):
fd = fcd.path() fd = fcd.path()
unused, unused, unused, back = files unused, unused, unused, back = files
if not r and (_toolbool(ui, tool, "checkconflicts") or if not r and (_toolbool(ui, tool, "checkconflicts") or
'conflicts' in _toollist(ui, tool, "check")): 'conflicts' in _toollist(ui, tool, "check")):
if re.search("^(<<<<<<< .*|=======|>>>>>>> .*)$", fcd.data(), if hasconflictmarkers(fcd.data()):
re.MULTILINE):
r = 1 r = 1
checked = False checked = False
if 'prompt' in _toollist(ui, tool, "check"): if 'prompt' in _toollist(ui, tool, "check"):
checked = True checked = True
if ui.promptchoice(_("was merge of '%s' successful (yn)?" if ui.promptchoice(_("was merge of '%s' successful (yn)?"
"$$ &Yes $$ &No") % fd, 1): "$$ &Yes $$ &No") % fd, 1):
r = 1 r = 1

tests/test-resolve.t

$ mkdir .hg/merge $ mkdir .hg/merge
$ touch .hg/merge/state $ touch .hg/merge/state
resolve -l should be empty resolve -l should be empty
$ hg resolve -l $ hg resolve -l
resolve -m can be configured to look for remaining conflict markers
$ hg up -qC 2
$ hg merge -q --tool=internal:merge 1
warning: conflicts while merging file1! (edit, then use 'hg resolve --mark')
warning: conflicts while merging file2! (edit, then use 'hg resolve --mark')
[1]
$ hg resolve -l
U file1
U file2
$ echo 'remove markers' > file1
$ hg --config experimental.resolve.mark-check=abort resolve -m
warning: the following files still have conflict markers:
file2
abort: conflict markers detected
(use --all to mark anyway)
[255]
$ hg resolve -l
U file1
U file2
Try with --all from the hint
$ hg --config experimental.resolve.mark-check=abort resolve -m --all
warning: the following files still have conflict markers:
file2
(no more unresolved files)
$ hg resolve -l
R file1
R file2
$ hg resolve --unmark
$ hg resolve -l
U file1
U file2
$ hg --config experimental.resolve.mark-check=warn resolve -m
warning: the following files still have conflict markers:
file2
(no more unresolved files)
$ hg resolve -l
R file1
R file2
If the file is already marked as resolved, we don't warn about it
$ hg resolve --unmark file1
$ hg resolve -l
U file1
R file2
$ hg --config experimental.resolve.mark-check=warn resolve -m
(no more unresolved files)
$ hg resolve -l
R file1
R file2
$ cd .. $ cd ..