This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6469

bookmarks: use correct store for "ambiguity check"
ClosedPublic

Authored by martinvonz on Jun 1 2019, 1:48 AM.

Details

Reviewers
marmoute
pulkit
Group Reviewers
hg-reviewers
Commits
rHGd532292eff22: bookmarks: use correct store for "ambiguity check"
Summary

I still don't quite know what the check does, but I clearly got it
wrong in 526750cdd02d (bookmarks: keep bookmarks in .hg/store if new
config set, 2019-05-15). Just compare with the strings we use in
@repofilecache and @storecache. These bugs were then copied to the
stable branch in c2b83c957621 (localrepo: grab mixedrepostorecache
class from 526750cdd02d, 2019-05-20) and 2338bdea4474 (bookmark: also
make bookmark cache depends of the changelog, 2019-05-20). As a
result, test-wireproto-exchangev2.t is flaky on both branches. This
patch fixes that.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Jun 1 2019, 1:48 AM
martinvonz added a comment.Jun 1 2019, 1:48 AM

This is meant for the stable branch

marmoute requested changes to this revision.Jun 5 2019, 7:17 AM
marmoute added a subscriber: marmoute.

Can we go fully explicit ? Having both plain and store as the possible value?

This revision now requires changes to proceed.Jun 5 2019, 7:17 AM
martinvonz requested review of this revision.Jun 5 2019, 9:03 AM
In D6469#94049, @marmoute wrote:

Can we go fully explicit ? Having both plain and store as the possible value?

I'd prefer to avoid that on the stable branch. It would require more changes (to the various vfsmap instances). I wanted to do it on the default branch, though.

marmoute added a comment.Jun 5 2019, 11:30 AM

Ah, I see. The move from ('plain', '') to ('', 'plain') is matching the key used for the vfsmap?
If so, go ahead with this patch on stable.

pulkit accepted this revision.Jun 5 2019, 12:19 PM
Closed by commit rHGd532292eff22: bookmarks: use correct store for "ambiguity check" (authored by martinvonz). · Explain WhyJun 5 2019, 3:05 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/localrepo.py (8 lines)

Diff 15317

mercurial/localrepo.py

# scmutil.filecache only uses the path for passing back into our # scmutil.filecache only uses the path for passing back into our
# join(), so we can safely pass a list of paths and locations # join(), so we can safely pass a list of paths and locations
super(mixedrepostorecache, self).__init__(*pathsandlocations) super(mixedrepostorecache, self).__init__(*pathsandlocations)
for path, location in pathsandlocations: for path, location in pathsandlocations:
_cachedfiles.update(pathsandlocations) _cachedfiles.update(pathsandlocations)
def join(self, obj, fnameandlocation): def join(self, obj, fnameandlocation):
fname, location = fnameandlocation fname, location = fnameandlocation
if location == '': if location == 'plain':
return obj.vfs.join(fname) return obj.vfs.join(fname)
else: else:
if location != 'store': if location != '':
raise error.ProgrammingError('unexpected location: %s' % raise error.ProgrammingError('unexpected location: %s' %
location) location)
return obj.sjoin(fname) return obj.sjoin(fname)
def isfilecached(repo, name): def isfilecached(repo, name):
"""check if a repo has already cached "name" filecache-ed property """check if a repo has already cached "name" filecache-ed property
This returns (cachedobj-or-None, iscached) tuple. This returns (cachedobj-or-None, iscached) tuple.
example calling `repo.filtered("served")` will return a repoview using example calling `repo.filtered("served")` will return a repoview using
the "served" view, regardless of the initial view used by `repo`. the "served" view, regardless of the initial view used by `repo`.
In other word, there is always only one level of `repoview` "filtering". In other word, there is always only one level of `repoview` "filtering".
""" """
cls = repoview.newtype(self.unfiltered().__class__) cls = repoview.newtype(self.unfiltered().__class__)
return cls(self, name, visibilityexceptions) return cls(self, name, visibilityexceptions)
@mixedrepostorecache(('bookmarks', ''), ('bookmarks.current', ''), @mixedrepostorecache(('bookmarks', 'plain'), ('bookmarks.current', 'plain'),
('00changelog.i', 'store')) ('00changelog.i', ''))
def _bookmarks(self): def _bookmarks(self):
return bookmarks.bmstore(self) return bookmarks.bmstore(self)
@property @property
def _activebookmark(self): def _activebookmark(self):
return self._bookmarks.active return self._bookmarks.active
# _phasesets depend on changelog. what we need is to call # _phasesets depend on changelog. what we need is to call