This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4287

overlayworkingctx: fix exception in metadata-only inmemory merges (issue5960)
ClosedPublic

Authored by spectral on Aug 15 2018, 8:52 PM.

Details

Reviewers
phillco
pulkit
Group Reviewers
hg-reviewers
Commits
rHG95bd19f60957: overlayworkingctx: fix exception in metadata-only inmemory merges (issue5960)
Summary

If there was a metadata-only mutation, such as +x or -x on a file, we would
create a cache entry with None for data, and this would cause problems later on
when some code tried to run fctx.data() or similar, and was expecting a string.

My original fix for this involved passing data=self._wrappedctx[path].data() in
setflags(), but this version seems slightly better - this way, if we ever call
write() and then call setflags(), we don't destroy the data that we wrote that's
in the cache. I haven't verified that other fields aren't destroyed, such as
date or flags :)

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 15 2018, 8:52 PM
spectral updated this revision to Diff 10365.Aug 15 2018, 9:01 PM
phillco accepted this revision.Aug 15 2018, 9:39 PM

lgtm

pulkit accepted this revision.Aug 16 2018, 9:17 AM
pulkit added a subscriber: pulkit.

Thanks @phillco for the review.

Closed by commit rHG95bd19f60957: overlayworkingctx: fix exception in metadata-only inmemory merges (issue5960) (authored by spectral). · Explain WhyAug 16 2018, 9:25 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/context.py (7 lines)
Mtests/test-rebase-inmemory.t (28 lines)

Diff 10385

mercurial/context.py

# Path not in the underlying manifest (created). # Path not in the underlying manifest (created).
continue continue
for path in keys: for path in keys:
del self._cache[path] del self._cache[path]
return keys return keys
def _markdirty(self, path, exists, data=None, date=None, flags=''): def _markdirty(self, path, exists, data=None, date=None, flags=''):
# data not provided, let's see if we already have some; if not, let's
# grab it from our underlying context, so that we always have data if
# the file is marked as existing.
if exists and data is None:
oldentry = self._cache.get(path) or {}
data = oldentry.get('data') or self._wrappedctx[path].data()
self._cache[path] = { self._cache[path] = {
'exists': exists, 'exists': exists,
'data': data, 'data': data,
'date': date, 'date': date,
'flags': flags, 'flags': flags,
'copied': None, 'copied': None,
} }

tests/test-rebase-inmemory.t

|/ c |/ c
| |
o 1:d2ae7f538514 test o 1:d2ae7f538514 test
| b | b
| |
o 0:cb9a9f314b8b test o 0:cb9a9f314b8b test
a a
#if execbit
Test a metadata-only in-memory merge
$ cd $TESTTMP
$ hg init no_exception
$ cd no_exception
# Produce the following graph:
# o 'add +x to foo.txt'
# | o r1 (adds bar.txt, just for something to rebase to)
# |/
# o r0 (adds foo.txt, no +x)
$ echo hi > foo.txt
$ hg ci -qAm r0
$ echo hi > bar.txt
$ hg ci -qAm r1
$ hg co -qr ".^"
$ chmod +x foo.txt
$ hg ci -qAm 'add +x to foo.txt'
issue5960: this was raising an AttributeError exception
$ hg rebase -r . -d 1
rebasing 2:539b93e77479 "add +x to foo.txt" (tip)
saved backup bundle to $TESTTMP/no_exception/.hg/strip-backup/*.hg (glob)
$ hg diff -c tip
diff --git a/foo.txt b/foo.txt
old mode 100644
new mode 100755
#endif