This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12171

filemerge: put temporary files in single temp dir by default
ClosedPublic

Authored by martinvonz on Feb 11 2022, 7:54 PM.

Details

Reviewers
marmoute
Alphare
Group Reviewers
hg-reviewers
Commits
rHGb70c9697ab41: filemerge: put temporary files in single temp dir by default
rHG75e091b18e57: filemerge: put temporary files in single temp dir by default
Summary

The feature introduced in D2888 seems like a pure improvement to
me. It makes the names' of temporary file easier to read. Let's have
it always enabled.

I also removed the config option for the path prefix because it
doesn't seem useful. I asked Kyle (the author of the feature) about it
and he couldn't think of a reason to keep it. I suspect it was just
that we to have a config to turn it on/off while it was experimental,
so it might as well be a configurable prefix then.

Diff Detail

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

Event Timeline

martinvonz created this revision.Feb 11 2022, 7:54 PM
Alphare added subscribers: spectral, Alphare.Feb 14 2022, 6:03 AM

Honestly unless @spectral can tell us why we should keep the config around, I don't see why we shouldn't remove it.

martinvonz added a comment.Feb 14 2022, 11:30 AM
In D12171#186644, @Alphare wrote:

Honestly unless @spectral can tell us why we should keep the config around, I don't see why we shouldn't remove it.

I talked to him about it before I sent the patch. He couldn't think of a reason to keep it. Want me to remove it in this patch? I'm fine with removing it in this patch, in a separate patch, or not at all.

martinvonz updated this revision to Diff 32181.Feb 14 2022, 7:59 PM
marmoute accepted this revision.Feb 15 2022, 5:12 AM
Alphare requested changes to this revision.Feb 15 2022, 7:52 AM

Yeah, let's just remove it completely in this revision. Thanks

This revision now requires changes to proceed.Feb 15 2022, 7:52 AM
martinvonz edited the summary of this revision. (Show Details)Feb 15 2022, 11:59 AM
martinvonz updated this revision to Diff 32218.
martinvonz added a comment.Feb 15 2022, 12:00 PM
In D12171#186763, @Alphare wrote:

Yeah, let's just remove it completely in this revision. Thanks

Done.

martinvonz added a commit: rHG75e091b18e57: filemerge: put temporary files in single temp dir by default.Feb 15 2022, 2:50 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG75e091b18e57: filemerge: put temporary files in single temp dir by default (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.
martinvonz updated this revision to Diff 32224.Feb 15 2022, 3:17 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/configitems.py (5 lines)
Mmercurial/filemerge.py (31 lines)
Mtests/test-merge-tools.t (49 lines)

Diff 32224

mercurial/configitems.py

# kept/undeleted them) and creates new filenodes for them # kept/undeleted them) and creates new filenodes for them
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'merge-track-salvaged', b'merge-track-salvaged',
default=False, default=False,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'mergetempdirprefix',
default=None,
)
coreconfigitem(
b'experimental',
b'mmapindexthreshold', b'mmapindexthreshold',
default=None, default=None,
) )
coreconfigitem( coreconfigitem(
b'experimental', b'experimental',
b'narrow', b'narrow',
default=False, default=False,
) )

mercurial/filemerge.py

_(b'warning: %s cannot merge change/delete conflict for %s\n') _(b'warning: %s cannot merge change/delete conflict for %s\n')
% (tool, uipathfn(fcd.path())) % (tool, uipathfn(fcd.path()))
) )
return False, 1, None return False, 1, None
localpath = _workingpath(repo, fcd) localpath = _workingpath(repo, fcd)
args = _toolstr(repo.ui, tool, b"args") args = _toolstr(repo.ui, tool, b"args")
with _maketempfiles( with _maketempfiles(
repo, fco, fca, repo.wvfs.join(backup.path()), b"$output" in args fco, fca, repo.wvfs.join(backup.path()), b"$output" in args
) as temppaths: ) as temppaths:
basepath, otherpath, localoutputpath = temppaths basepath, otherpath, localoutputpath = temppaths
outpath = b"" outpath = b""
def format_label(input): def format_label(input):
if input.label_detail: if input.label_detail:
return b'%s: %s' % (input.label, input.label_detail) return b'%s: %s' % (input.label, input.label_detail)
else: else:
a = _workingpath(repo, fcd) a = _workingpath(repo, fcd)
util.copyfile(a, backup) util.copyfile(a, backup)
# 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(backup, repo=repo) return context.arbitraryfilectx(backup, repo=repo)
@contextlib.contextmanager @contextlib.contextmanager
def _maketempfiles(repo, fco, fca, localpath, uselocalpath): def _maketempfiles(fco, fca, localpath, uselocalpath):
"""Writes out `fco` and `fca` as temporary files, and (if uselocalpath) """Writes out `fco` and `fca` as temporary files, and (if uselocalpath)
copies `localpath` to another temporary file, so an external merge tool may copies `localpath` to another temporary file, so an external merge tool may
use them. use them.
""" """
tmproot = None tmproot = pycompat.mkdtemp(prefix=b'hgmerge-')
tmprootprefix = repo.ui.config(b'experimental', b'mergetempdirprefix')
if tmprootprefix:
tmproot = pycompat.mkdtemp(prefix=tmprootprefix)
def maketempfrompath(prefix, path): def maketempfrompath(prefix, path):
fullbase, ext = os.path.splitext(path) fullbase, ext = os.path.splitext(path)
pre = b"%s~%s" % (os.path.basename(fullbase), prefix) pre = b"%s~%s" % (os.path.basename(fullbase), prefix)
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, "wb") f = open(name, "wb")
else:
fd, name = pycompat.mkstemp(prefix=pre + b'.', suffix=ext)
f = os.fdopen(fd, "wb")
return f, name return f, name
def tempfromcontext(prefix, ctx): def tempfromcontext(prefix, ctx):
f, name = maketempfrompath(prefix, ctx.path()) f, name = maketempfrompath(prefix, ctx.path())
data = ctx.decodeddata() data = ctx.decodeddata()
f.write(data) f.write(data)
f.close() f.close()
return name return name
f, d = maketempfrompath(b"local", d) f, d = maketempfrompath(b"local", d)
with open(localpath, b'rb') as src: with open(localpath, b'rb') as src:
f.write(src.read()) f.write(src.read())
f.close() f.close()
try: try:
yield b, c, d yield b, c, d
finally: finally:
if tmproot:
shutil.rmtree(tmproot) shutil.rmtree(tmproot)
else:
util.unlink(b)
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(repo, wctx, mynode, orig, fcd, fco, fca, labels=None): def filemerge(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
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

> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \ > --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \
> --config ui.mergemarkertemplate='uitmpl {rev}' \ > --config ui.mergemarkertemplate='uitmpl {rev}' \
> --config ui.mergemarkers=detailed \ > --config ui.mergemarkers=detailed \
> merge -r 2 > merge -r 2
merging f merging f
arg: "ll:working copy" arg: "ll:working copy"
arg: "lo:" arg: "lo:"
arg: "merge rev" arg: "merge rev"
arg: "lb:common ancestor: */f~base.*" (glob) arg: "lb:common ancestor: */f~base" (glob)
0 files updated, 1 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
$ rm -f 'printargs_merge_tool'
Same test with experimental.mergetempdirprefix set:
$ beforemerge
[merge-tools]
false.whatever=
true.priority=1
true.executable=cat
# hg update -C 1
$ cat <<EOF > printargs_merge_tool
> while test \$# -gt 0; do echo arg: \""\$1"\"; shift; done
> EOF
$ hg --config experimental.mergetempdirprefix=$TESTTMP/hgmerge. \
> --config merge-tools.true.executable='sh' \
> --config merge-tools.true.args='./printargs_merge_tool ll:$labellocal lo: $labelother lb:$labelbase": "$base' \
> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \
> --config ui.mergemarkertemplate='uitmpl {rev}' \
> --config ui.mergemarkers=detailed \
> merge -r 2
merging f
arg: "ll:working copy"
arg: "lo:"
arg: "merge rev"
arg: "lb:common ancestor: */hgmerge.*/f~base" (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)
$ rm -f 'printargs_merge_tool' $ rm -f 'printargs_merge_tool'
Merge using a tool that supports labellocal, labelother, and labelbase, checking Merge using a tool that supports labellocal, labelother, and labelbase, checking
that they're quoted properly as well. This is using 'detailed' mergemarkers, that they're quoted properly as well. This is using 'detailed' mergemarkers,
even though ui.mergemarkers is 'basic', and using the tool's even though ui.mergemarkers is 'basic', and using the tool's
mergemarkertemplate: mergemarkertemplate:
> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \ > --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \
> --config ui.mergemarkertemplate='uitmpl {rev}' \ > --config ui.mergemarkertemplate='uitmpl {rev}' \
> --config ui.mergemarkers=basic \ > --config ui.mergemarkers=basic \
> merge -r 2 > merge -r 2
merging f merging f
arg: "ll:working copy: tooltmpl ef83787e2614" arg: "ll:working copy: tooltmpl ef83787e2614"
arg: "lo:" arg: "lo:"
arg: "merge rev: tooltmpl 0185f4e0cf02" arg: "merge rev: tooltmpl 0185f4e0cf02"
arg: "lb:common ancestor: */f~base.*" (glob) arg: "lb:common ancestor: */f~base" (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)
$ rm -f 'printargs_merge_tool' $ rm -f 'printargs_merge_tool'
The merge tool still gets labellocal and labelother as 'basic' even when The merge tool still gets labellocal and labelother as 'basic' even when
premerge=keep is used and has 'detailed' markers: premerge=keep is used and has 'detailed' markers:
$ beforemerge $ beforemerge
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.* */f~local.*.txt */f~other.*.txt $TESTTMP/repo/f.txt (glob) */hgmerge-*/f~base */hgmerge-*/f~local.txt */hgmerge-*/f~other.txt */repo/f.txt (glob)
0 files updated, 1 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
Verify naming of temporary files and that extension is preserved
(experimental.mergetempdirprefix version):
$ hg update -q -C 1
$ hg mv f f.txt
$ hg ci -qm "f.txt"
warning: commit already existed in the repository!
$ hg update -q -C 2
$ hg merge -y -r tip --tool echo \
> --config merge-tools.echo.args='$base $local $other $output' \
> --config experimental.mergetempdirprefix=$TESTTMP/hgmerge.
merging f and f.txt to f.txt
$TESTTMP/hgmerge.*/f~base $TESTTMP/hgmerge.*/f~local.txt $TESTTMP/hgmerge.*/f~other.txt $TESTTMP/repo/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)
Binary files capability checking Binary files capability checking
$ hg update -q -C 0 $ hg update -q -C 0
$ "$PYTHON" <<EOF $ "$PYTHON" <<EOF
> with open('b', 'wb') as fp: > with open('b', 'wb') as fp: