This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1806

filemerge: fix backing up an in-memory file to a custom location
ClosedPublic

Authored by phillco on Jan 4 2018, 2:36 PM.

Details

Reviewers
durham
Group Reviewers
hg-reviewers
Commits
rHGc0439e11af16: filemerge: fix backing up an in-memory file to a custom location
Summary

If the user specifies a ui.origbackuppath, we used to always copy the file
there, but if the source file is in memory we must write it instead of copying.

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

phillco created this revision.Jan 4 2018, 2:36 PM
phillco edited the summary of this revision. (Show Details)Jan 4 2018, 2:37 PM
durham accepted this revision.Jan 4 2018, 3:17 PM
durham added a subscriber: durham.

Accepting, since it seems more correct than before.

Should we even be calling _makebackup in the case of an inmemory merge? Like, maybe the makebackup should be conditional based on if the source file context is actually a workingctx?

mercurial/filemerge.py
623

I know you're just documenting the parameter that already existed, but might be nice to explain why this is (if you happen to know why right now).

643

s/wherever/whatever/

phillco added a comment.Jan 4 2018, 3:18 PM

Should we even be calling _makebackup in the case of an inmemory merge? Like, maybe the makebackup should be conditional based on if the source file context is actually a workingctx?

The merge process itself uses the backup, it's not just for the user.

mercurial/filemerge.py
623

I think I can

phillco edited the summary of this revision. (Show Details)Jan 5 2018, 12:35 AM
phillco updated this revision to Diff 4697.
phillco updated this revision to Diff 4699.Jan 5 2018, 12:38 AM
Closed by commit rHGc0439e11af16: filemerge: fix backing up an in-memory file to a custom location (authored by phillco). · Explain WhyJan 18 2018, 8:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/filemerge.py (17 lines)

Diff 4870

mercurial/filemerge.py

def _makebackup(repo, ui, wctx, fcd, premerge): def _makebackup(repo, ui, wctx, fcd, premerge):
"""Makes and returns a filectx-like object for ``fcd``'s backup file. """Makes and returns a filectx-like object for ``fcd``'s backup file.
In addition to preserving the user's pre-existing modifications to `fcd` In addition to preserving the user's pre-existing modifications to `fcd`
(if any), the backup is used to undo certain premerges, confirm whether a (if any), the backup is used to undo certain premerges, confirm whether a
merge changed anything, and determine what line endings the new file should merge changed anything, and determine what line endings the new file should
have. have.
Backups only need to be written once (right before the premerge) since their
content doesn't change afterwards.
durhamUnsubmitted
Not Done

I know you're just documenting the parameter that already existed, but might be nice to explain why this is (if you happen to know why right now).

durham: I know you're just documenting the parameter that already existed, but might be nice to explain…
phillcoAuthorUnsubmitted
Not Done

I think I can

phillco: I think I can
""" """
if fcd.isabsent(): if fcd.isabsent():
return None return None
# TODO: Break this import cycle somehow. (filectx -> ctx -> fileset -> # TODO: Break this import cycle somehow. (filectx -> ctx -> fileset ->
# merge -> filemerge). (I suspect the fileset import is the weakest link) # merge -> filemerge). (I suspect the fileset import is the weakest link)
from . import context from . import context
a = _workingpath(repo, fcd) a = _workingpath(repo, fcd)
back = scmutil.origpath(ui, repo, a) back = scmutil.origpath(ui, repo, a)
inworkingdir = (back.startswith(repo.wvfs.base) and not inworkingdir = (back.startswith(repo.wvfs.base) and not
back.startswith(repo.vfs.base)) back.startswith(repo.vfs.base))
if isinstance(fcd, context.overlayworkingfilectx) and inworkingdir: if isinstance(fcd, context.overlayworkingfilectx) and inworkingdir:
# If the backup file is to be in the working directory, and we're # If the backup file is to be in the working directory, and we're
# merging in-memory, we must redirect the backup to the memory context # merging in-memory, we must redirect the backup to the memory context
# so we don't disturb the working directory. # so we don't disturb the working directory.
relpath = back[len(repo.wvfs.base) + 1:] relpath = back[len(repo.wvfs.base) + 1:]
wctx[relpath].write(fcd.data(), fcd.flags()) wctx[relpath].write(fcd.data(), fcd.flags())
return wctx[relpath] return wctx[relpath]
else: else:
# Otherwise, write to wherever the user specified the backups should go.
#
# A arbitraryfilectx is returned, so we can run the same functions on
# the backup context regardless of where it lives.
if premerge: if premerge:
# Otherwise, write to wherever path the user specified the backups
durhamUnsubmitted
Not Done

s/wherever/whatever/

durham: s/wherever/whatever/
# should go. We still need to switch based on whether the source is
# in-memory so we can use the fast path of ``util.copy`` if both are
# on disk.
if isinstance(fcd, context.overlayworkingfilectx):
util.writefile(back, fcd.data())
else:
util.copyfile(a, back) util.copyfile(a, back)
# A arbitraryfilectx is returned, so we can run the same functions on
# the backup context regardless of where it lives.
return context.arbitraryfilectx(back, repo=repo) return context.arbitraryfilectx(back, repo=repo)
def _maketempfiles(repo, fco, fca): def _maketempfiles(repo, fco, fca):
"""Writes out `fco` and `fca` as temporary files, so an external merge """Writes out `fco` and `fca` as temporary files, so an external merge
tool may use them. tool may use them.
""" """
def temp(prefix, ctx): def temp(prefix, ctx):
fullbase, ext = os.path.splitext(ctx.path()) fullbase, ext = os.path.splitext(ctx.path())