This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2889

filemerge: make the 'local' path match the format that 'base' and 'other' use
ClosedPublic

Authored by spectral on Mar 16 2018, 8:00 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG1e30a26a65d0: filemerge: make the 'local' path match the format that 'base' and 'other' use
Summary

If we pass a separate '$output' arg to the merge tool, we produce four files:
local, base, other, and output. In this situation, 'output' will be the
original filename, 'base' and 'other' are temporary files, and previously
'local' would be the backup file (so if 'output' was foo.txt, 'local' would be
foo.txt.orig).

This change makes it so that 'local' follows the same pattern as 'base' and
'other' - it will be a temporary file either in the
experimental.mergetempdirprefix-controlled directory with a name like
foo~local.txt, or in the normal system-wide temp dir with a name like
foo~local.RaNd0m.txt.

For the cases where the merge tool does not use an '$output' arg, 'local' is
still the destination filename, and 'base' and 'other' are unchanged.

The hope is that this is much easier for people to reason about; rather than
having a tool like Meld pop up with three panes, one of them with the filename
"foo.txt.orig", one with the filename "foo.txt", and one with
"foo~other.StuFf2.txt", we can (when the merge temp dir stuff is enabled) make
it show up as "foo~local.txt", "foo.txt" and "foo~other.txt", respectively.

This also opens the door to future customization, such as getting the
operation-provided labels and a hash prefix into the filenames (so we see
something like "foo~dest.abc123", "foo.txt", and "foo~src.d4e5f6").

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.Mar 16 2018, 8:00 PM
yuja requested changes to this revision.Mar 19 2018, 10:33 AM
yuja added a subscriber: yuja.

Not reviewed yet, but this can't be applied cleanly. Can you rebase?

This revision now requires changes to proceed.Mar 19 2018, 10:33 AM
spectral added a comment.Mar 19 2018, 2:18 PM
In D2889#46482, @yuja wrote:

Not reviewed yet, but this can't be applied cleanly. Can you rebase?

This series should rebase cleanly onto a4a95bd7 (which was accepted by pulkit separately from the rest of the series). I've rebase these three remaining ones and pushed.

spectral requested review of this revision.Mar 19 2018, 2:22 PM

Ah, there's a "Request Review" that I'd never noticed before, choosing that to hopefully get this out of "Requires Revision"

yuja added a comment.Mar 21 2018, 12:23 AM

Can't apply.

% hg up -C a4a95bd7
2 files updated, 0 files merged, 0 files removed, 0 files unresolved

% hg pimp D2889
devel-warn: accessing unregistered config item: 'phabricator.batchsize' at: /home/yuya/work/hghacks/mercurial/mercurial/ui.py:624 (configwith)
devel-warn: accessing unregistered config item: 'phabricator.url' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:124 (readurltoken)
devel-warn: accessing unregistered config item: 'phabricator.token' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:111 (readlegacytoken)
phabricator.token is deprecated - please migrate to the phabricator.auth section.
devel-warn: accessing unregistered config item: 'phabricator.curlcmd' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:162 (callconduit)
applying patch from stdin
devel-warn: accessing unregistered config item: 'phabricator.url' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:124 (readurltoken)
devel-warn: accessing unregistered config item: 'phabricator.token' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:111 (readlegacytoken)
devel-warn: accessing unregistered config item: 'phabricator.curlcmd' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:162 (callconduit)
devel-warn: accessing unregistered config item: 'phabricator.url' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:124 (readurltoken)
devel-warn: accessing unregistered config item: 'phabricator.token' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:111 (readlegacytoken)
devel-warn: accessing unregistered config item: 'phabricator.curlcmd' at: /home/yuya/work/hghacks/mercurial/contrib/phabricator.py:162 (callconduit)
patching file tests/test-merge-tools.t
Hunk #2 FAILED at 1599
1 out of 2 hunks FAILED -- saving rejects to file tests/test-merge-tools.t.rej
patching file mercurial/filemerge.py
Hunk #1 FAILED at 511
Hunk #3 FAILED at 663
2 out of 3 hunks FAILED -- saving rejects to file mercurial/filemerge.py.rej
abort: patch failed to apply
spectral updated this revision to Diff 7189.Mar 21 2018, 4:48 PM
yuja accepted this revision.Mar 22 2018, 10:34 AM
This revision is now accepted and ready to land.Mar 22 2018, 10:34 AM
Closed by commit rHG1e30a26a65d0: filemerge: make the 'local' path match the format that 'base' and 'other' use (authored by spectral). · Explain WhyMar 22 2018, 10:46 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/filemerge.py (48 lines)
Mtests/test-merge-tools.t (4 lines)

Diff 7253

mercurial/filemerge.py

def _xmerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None): def _xmerge(repo, mynode, orig, fcd, fco, fca, toolconf, files, labels=None):
tool, toolpath, binary, symlink = toolconf tool, toolpath, binary, symlink = toolconf
if fcd.isabsent() or fco.isabsent(): if fcd.isabsent() or fco.isabsent():
repo.ui.warn(_('warning: %s cannot merge change/delete conflict ' repo.ui.warn(_('warning: %s cannot merge change/delete conflict '
'for %s\n') % (tool, fcd.path())) 'for %s\n') % (tool, fcd.path()))
return False, 1, None return False, 1, None
unused, unused, unused, back = files unused, unused, unused, back = files
localpath = _workingpath(repo, fcd) localpath = _workingpath(repo, fcd)
with _maketempfiles(repo, fco, fca) as temppaths: args = _toolstr(repo.ui, tool, "args")
basepath, otherpath = temppaths
with _maketempfiles(repo, fco, fca, repo.wvfs.join(back.path()),
"$output" in args) as temppaths:
basepath, otherpath, localoutputpath = temppaths
outpath = "" outpath = ""
mylabel, otherlabel = labels[:2] mylabel, otherlabel = labels[:2]
if len(labels) >= 3: if len(labels) >= 3:
baselabel = labels[2] baselabel = labels[2]
else: else:
baselabel = 'base' baselabel = 'base'
env = {'HG_FILE': fcd.path(), env = {'HG_FILE': fcd.path(),
'HG_MY_NODE': short(mynode), 'HG_MY_NODE': short(mynode),
'HG_OTHER_NODE': short(fco.changectx().node()), 'HG_OTHER_NODE': short(fco.changectx().node()),
'HG_BASE_NODE': short(fca.changectx().node()), 'HG_BASE_NODE': short(fca.changectx().node()),
'HG_MY_ISLINK': 'l' in fcd.flags(), 'HG_MY_ISLINK': 'l' in fcd.flags(),
'HG_OTHER_ISLINK': 'l' in fco.flags(), 'HG_OTHER_ISLINK': 'l' in fco.flags(),
'HG_BASE_ISLINK': 'l' in fca.flags(), 'HG_BASE_ISLINK': 'l' in fca.flags(),
'HG_MY_LABEL': mylabel, 'HG_MY_LABEL': mylabel,
'HG_OTHER_LABEL': otherlabel, 'HG_OTHER_LABEL': otherlabel,
'HG_BASE_LABEL': baselabel, 'HG_BASE_LABEL': baselabel,
} }
ui = repo.ui ui = repo.ui
args = _toolstr(ui, tool, "args")
if "$output" in args: if "$output" in args:
# read input from backup, write to original # read input from backup, write to original
outpath = localpath outpath = localpath
localpath = repo.wvfs.join(back.path()) localpath = localoutputpath
replace = {'local': localpath, 'base': basepath, 'other': otherpath, replace = {'local': localpath, 'base': basepath, 'other': otherpath,
'output': outpath, 'labellocal': mylabel, 'output': outpath, 'labellocal': mylabel,
'labelother': otherlabel, 'labelbase': baselabel} 'labelother': otherlabel, 'labelbase': baselabel}
args = util.interpolate(br'\$', replace, args, args = util.interpolate(br'\$', replace, args,
lambda s: util.shellquote(util.localpath(s))) lambda s: util.shellquote(util.localpath(s)))
cmd = toolpath + ' ' + args cmd = toolpath + ' ' + args
if _toolbool(ui, tool, "gui"): if _toolbool(ui, tool, "gui"):
repo.ui.status(_('running merge tool %s for file %s\n') % repo.ui.status(_('running merge tool %s for file %s\n') %
util.writefile(back, fcd.data()) util.writefile(back, fcd.data())
else: else:
util.copyfile(a, back) util.copyfile(a, back)
# A arbitraryfilectx is returned, so we can run the same functions on # A arbitraryfilectx is returned, so we can run the same functions on
# the backup context regardless of where it lives. # the backup context regardless of where it lives.
return context.arbitraryfilectx(back, repo=repo) return context.arbitraryfilectx(back, repo=repo)
@contextlib.contextmanager @contextlib.contextmanager
def _maketempfiles(repo, fco, fca): def _maketempfiles(repo, fco, fca, localpath, uselocalpath):
"""Writes out `fco` and `fca` as temporary files, so an external merge """Writes out `fco` and `fca` as temporary files, and (if uselocalpath)
tool may use them. copies `localpath` to another temporary file, so an external merge tool may
use them.
""" """
tmproot = None tmproot = None
tmprootprefix = repo.ui.config('experimental', 'mergetempdirprefix') tmprootprefix = repo.ui.config('experimental', 'mergetempdirprefix')
if tmprootprefix: if tmprootprefix:
tmproot = tempfile.mkdtemp(prefix=tmprootprefix) tmproot = tempfile.mkdtemp(prefix=tmprootprefix)
def temp(prefix, ctx): def maketempfrompath(prefix, path):
fullbase, ext = os.path.splitext(ctx.path()) fullbase, ext = os.path.splitext(path)
pre = "%s~%s" % (os.path.basename(fullbase), prefix) pre = "%s~%s" % (os.path.basename(fullbase), prefix)
if tmproot: if tmproot:
name = os.path.join(tmproot, pre) name = os.path.join(tmproot, pre)
if ext: if ext:
name += ext name += ext
f = open(name, r"wb") f = open(name, r"wb")
else: else:
(fd, name) = tempfile.mkstemp(prefix=pre + '.', suffix=ext) fd, name = tempfile.mkstemp(prefix=pre + '.', suffix=ext)
f = os.fdopen(fd, r"wb") f = os.fdopen(fd, r"wb")
return f, name
def tempfromcontext(prefix, ctx):
f, name = maketempfrompath(prefix, ctx.path())
data = repo.wwritedata(ctx.path(), ctx.data()) data = repo.wwritedata(ctx.path(), ctx.data())
f.write(data) f.write(data)
f.close() f.close()
return name return name
b = temp("base", fca) b = tempfromcontext("base", fca)
c = temp("other", fco) c = tempfromcontext("other", fco)
d = localpath
if uselocalpath:
# We start off with this being the backup filename, so remove the .orig
# to make syntax-highlighting more likely.
if d.endswith('.orig'):
d, _ = os.path.splitext(d)
f, d = maketempfrompath("local", d)
with open(localpath, 'rb') as src:
f.write(src.read())
f.close()
try: try:
yield b, c yield b, c, d
finally: finally:
if tmproot: if tmproot:
shutil.rmtree(tmproot) shutil.rmtree(tmproot)
else: else:
util.unlink(b) util.unlink(b)
util.unlink(c) util.unlink(c)
# if not uselocalpath, d is the 'orig'/backup file which we
# shouldn't delete.
if d and uselocalpath:
util.unlink(d)
def _filemerge(premerge, repo, wctx, mynode, orig, fcd, fco, fca, labels=None): def _filemerge(premerge, repo, wctx, mynode, orig, fcd, fco, fca, labels=None):
"""perform a 3-way merge in the working directory """perform a 3-way merge in the working directory
premerge = whether this is a premerge premerge = whether this is a premerge
mynode = parent node before merge mynode = parent node before merge
orig = original local filename before merge orig = original local filename before merge
fco = other file context fco = other file context

tests/test-merge-tools.t

Verify naming of temporary files and that extension is preserved: Verify naming of temporary files and that extension is preserved:
$ hg update -q -C 1 $ hg update -q -C 1
$ hg mv f f.txt $ hg mv f f.txt
$ hg ci -qm "f.txt" $ hg ci -qm "f.txt"
$ hg update -q -C 2 $ hg update -q -C 2
$ hg merge -y -r tip --tool echo --config merge-tools.echo.args='$base $local $other $output' $ hg merge -y -r tip --tool echo --config merge-tools.echo.args='$base $local $other $output'
merging f and f.txt to f.txt merging f and f.txt to f.txt
*/f~base.* $TESTTMP/f.txt.orig */f~other.*.txt $TESTTMP/f.txt (glob) */f~base.* */f~local.*.txt */f~other.*.txt $TESTTMP/f.txt (glob)
0 files updated, 1 files merged, 0 files removed, 0 files unresolved 0 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)
Verify naming of temporary files and that extension is preserved Verify naming of temporary files and that extension is preserved
(experimental.mergetempdirprefix version): (experimental.mergetempdirprefix version):
$ hg update -q -C 1 $ hg update -q -C 1
$ hg mv f f.txt $ hg mv f f.txt
$ hg ci -qm "f.txt" $ hg ci -qm "f.txt"
$ hg update -q -C 2 $ hg update -q -C 2
$ hg merge -y -r tip --tool echo \ $ hg merge -y -r tip --tool echo \
> --config merge-tools.echo.args='$base $local $other $output' \ > --config merge-tools.echo.args='$base $local $other $output' \
> --config experimental.mergetempdirprefix=$TESTTMP/hgmerge. > --config experimental.mergetempdirprefix=$TESTTMP/hgmerge.
merging f and f.txt to f.txt merging f and f.txt to f.txt
$TESTTMP/hgmerge.*/f~base $TESTTMP/f.txt.orig $TESTTMP/hgmerge.*/f~other.txt $TESTTMP/f.txt (glob) $TESTTMP/hgmerge.*/f~base $TESTTMP/hgmerge.*/f~local.txt $TESTTMP/hgmerge.*/f~other.txt $TESTTMP/f.txt (glob)
0 files updated, 1 files merged, 0 files removed, 0 files unresolved 0 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)
Check that debugpicktool examines which merge tool is chosen for Check that debugpicktool examines which merge tool is chosen for
specified file as expected specified file as expected
$ beforemerge $ beforemerge
[merge-tools] [merge-tools]