This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1747

repoview: add visibilityexception argument to filterrevs() and related fns
ClosedPublic

Authored by pulkit on Dec 22 2017, 12:56 PM.

Download Raw Diff

Details

Reviewers

yuja

Group Reviewers

hg-reviewers

Commits

rHG3c9c05a38d78: repoview: add visibilityexception argument to filterrevs() and related fns

Summary

After this patch, filterrevs() can take an optional argument
visibilityexceptions which is a set of revs which should be exception to
being hidden. The visibilityexceptions will be passed to the function computing
hidden revisions for that filtername and are considered there while calculating
the set of hidden revs.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

pulkit created this revision.Dec 22 2017, 12:56 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptDec 22 2017, 12:56 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

pulkit added a child revision: D1732: revsetlang: add utility function to return hash like symbols from the tree.Dec 22 2017, 12:59 PM

yuja requested changes to this revision.Dec 25 2017, 7:54 AM

yuja added a subscriber: yuja.

yuja added inline comments.

mercurial/repoview.py
160	Since revisions are dynamically unhidden, we can't cache the result to the (unfiltered) repo per `filterename`. Perhaps we could apply `visibilityexceptions` at repoview.changelog(). revs = filterrevs(unfi, self.filtername) revs = revs - self._visibilityexceptions

This revision now requires changes to proceed.Dec 25 2017, 7:54 AM

Since revisions are dynamically unhidden, we can't cache the result
to the (unfiltered) repo per filterename.

FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?

In D1747#29916, @yuja wrote:

Since revisions are dynamically unhidden, we can't cache the result
to the (unfiltered) repo per filterename.

FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?

Yep I agree, but I wanted be on the safe side to prevent unknown bugs by using the same filtername.

mercurial/repoview.py
160	One reason to have this considered in computehidden() is we want this to be considered in `_revealancestors()` otherwise things which needs parent of hidden revision will throw an error like `export`. I will make it not cache the result if visibilityexceptions is present.

pulkit updated this revision to Diff 4607.Dec 25 2017, 10:08 AM

yuja accepted this revision.Dec 26 2017, 8:42 AM

This revision is now accepted and ready to land.Dec 26 2017, 8:42 AM

FWIW, if the cache don't include "unhidden" revisions, maybe we don't
need new filter names like 'visible-hidden"?

Yep I agree, but I wanted be on the safe side to prevent unknown bugs by using the same filtername.

Actually we have to use new filtername to separate branch/tags caches until
we can disable these caches when revisions are dynamically pinned.

Queued the series, thanks.

Closed by commit rHG3c9c05a38d78: repoview: add visibilityexception argument to filterrevs() and related fns (authored by pulkit). · Explain WhyDec 26 2017, 9:11 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/repoview.py (22 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	4582		Dec 22 2017, 12:56 PM	★	★
Diff 2	4607		Dec 25 2017, 10:08 AM	★	★
Diff 3	4616	rHG3c9c05a38d78a54c21cb0f397099a3dafd69547f	Dec 22 2017, 11:49 AM	★	★

Status	Author	Revision
Closed	pulkit	D1735 commands: use the new API to access hidden changesets in various commands
Closed	pulkit	D1734 repoview: add a new filtername for accessing hidden commits
Closed	pulkit	D1733 scmutil: add utility fn to return repo object with user passed revs unhidden
Closed	pulkit	D1732 revsetlang: add utility function to return hash like symbols from the tree
Abandoned	pulkit	D1731 repoview: introduce function to add visibility exceptions
Abandoned	pulkit	D1730 repoview: store visibility exceptions for the filter in filtertable
Closed	pulkit	D1747 repoview: add visibilityexception argument to filterrevs() and related fns
Closed	pulkit	D1746 repoview: add visibilityexceptions as an optional argument to repo.filtered()

Diff 4607

mercurial/repoview.py

	"""			"""
	stack = list(revs)			stack = list(revs)
	while stack:			while stack:
	for p in pfunc(stack.pop()):			for p in pfunc(stack.pop()):
	if p != nullrev and p in hidden:			if p != nullrev and p in hidden:
	hidden.remove(p)			hidden.remove(p)
	stack.append(p)			stack.append(p)

	def computehidden(repo):			def computehidden(repo, visibilityexceptions=None):
	"""compute the set of hidden revision to filter			"""compute the set of hidden revision to filter

	During most operation hidden should be filtered."""			During most operation hidden should be filtered."""
	assert not repo.changelog.filteredrevs			assert not repo.changelog.filteredrevs

	hidden = hideablerevs(repo)			hidden = hideablerevs(repo)
	if hidden:			if hidden:
	hidden = set(hidden - pinnedrevs(repo))			hidden = set(hidden - pinnedrevs(repo))
				if visibilityexceptions:
				hidden -= visibilityexceptions
	pfunc = repo.changelog.parentrevs			pfunc = repo.changelog.parentrevs
	mutablephases = (phases.draft, phases.secret)			mutablephases = (phases.draft, phases.secret)
	mutable = repo._phasecache.getrevset(repo, mutablephases)			mutable = repo._phasecache.getrevset(repo, mutablephases)

	visible = mutable - hidden			visible = mutable - hidden
	_revealancestors(pfunc, hidden, visible)			_revealancestors(pfunc, hidden, visible)
	return frozenset(hidden)			return frozenset(hidden)

	def computeunserved(repo):			def computeunserved(repo, visibilityexceptions=None):
	"""compute the set of revision that should be filtered when used a server			"""compute the set of revision that should be filtered when used a server

	Secret and hidden changeset should not pretend to be here."""			Secret and hidden changeset should not pretend to be here."""
	assert not repo.changelog.filteredrevs			assert not repo.changelog.filteredrevs
	# fast path in simple case to avoid impact of non optimised code			# fast path in simple case to avoid impact of non optimised code
	hiddens = filterrevs(repo, 'visible')			hiddens = filterrevs(repo, 'visible')
	if phases.hassecret(repo):			if phases.hassecret(repo):
	cl = repo.changelog			cl = repo.changelog
	secret = phases.secret			secret = phases.secret
	getphase = repo._phasecache.phase			getphase = repo._phasecache.phase
	first = min(cl.rev(n) for n in repo._phasecache.phaseroots[secret])			first = min(cl.rev(n) for n in repo._phasecache.phaseroots[secret])
	revs = cl.revs(start=first)			revs = cl.revs(start=first)
	secrets = set(r for r in revs if getphase(repo, r) >= secret)			secrets = set(r for r in revs if getphase(repo, r) >= secret)
	return frozenset(hiddens \| secrets)			return frozenset(hiddens \| secrets)
	else:			else:
	return hiddens			return hiddens

	def computemutable(repo):			def computemutable(repo, visibilityexceptions=None):
	assert not repo.changelog.filteredrevs			assert not repo.changelog.filteredrevs
	# fast check to avoid revset call on huge repo			# fast check to avoid revset call on huge repo
	if any(repo._phasecache.phaseroots[1:]):			if any(repo._phasecache.phaseroots[1:]):
	getphase = repo._phasecache.phase			getphase = repo._phasecache.phase
	maymutable = filterrevs(repo, 'base')			maymutable = filterrevs(repo, 'base')
	return frozenset(r for r in maymutable if getphase(repo, r))			return frozenset(r for r in maymutable if getphase(repo, r))
	return frozenset()			return frozenset()

	def computeimpactable(repo):			def computeimpactable(repo, visibilityexceptions=None):
	"""Everything impactable by mutable revision			"""Everything impactable by mutable revision

	The immutable filter still have some chance to get invalidated. This will			The immutable filter still have some chance to get invalidated. This will
	happen when:			happen when:

	- you garbage collect hidden changeset,			- you garbage collect hidden changeset,
	- public phase is moved backward,			- public phase is moved backward,
	- something is changed in the filtering (this could be fixed)			- something is changed in the filtering (this could be fixed)
	# mercurial.branchmap.subsettable			# mercurial.branchmap.subsettable
	# Otherwise your filter will have to recompute all its branches cache			# Otherwise your filter will have to recompute all its branches cache
	# from scratch (very slow).			# from scratch (very slow).
	filtertable = {'visible': computehidden,			filtertable = {'visible': computehidden,
	'served': computeunserved,			'served': computeunserved,
	'immutable': computemutable,			'immutable': computemutable,
	'base': computeimpactable}			'base': computeimpactable}

	def filterrevs(repo, filtername):			def filterrevs(repo, filtername, visibilityexceptions=None):
	"""returns set of filtered revision for this filter name"""			"""returns set of filtered revision for this filter name

				visibilityexceptions is a set of revs which must are exceptions for
				hidden-state and must be visible. They are dynamic and hence we should not
				cache it's result"""
	if filtername not in repo.filteredrevcache:			if filtername not in repo.filteredrevcache:
	func = filtertable[filtername]			func = filtertable[filtername]
				if visibilityexceptions:
				return func(repo.unfiltered, visibilityexceptions)
	repo.filteredrevcache[filtername] = func(repo.unfiltered())			repo.filteredrevcache[filtername] = func(repo.unfiltered())
				yujaUnsubmitted Not Done Since revisions are dynamically unhidden, we can't cache the result to the (unfiltered) repo per `filterename`. Perhaps we could apply `visibilityexceptions` at repoview.changelog(). revs = filterrevs(unfi, self.filtername) revs = revs - self._visibilityexceptions yuja: Since revisions are dynamically unhidden, we can't cache the result to the (unfiltered) repo…
				pulkitAuthorUnsubmitted Not Done One reason to have this considered in computehidden() is we want this to be considered in `_revealancestors()` otherwise things which needs parent of hidden revision will throw an error like `export`. I will make it not cache the result if visibilityexceptions is present. pulkit: One reason to have this considered in computehidden() is we want this to be considered in…
	return repo.filteredrevcache[filtername]			return repo.filteredrevcache[filtername]

	class repoview(object):			class repoview(object):
	"""Provide a read/write view of a repo through a filtered changelog			"""Provide a read/write view of a repo through a filtered changelog

	This object is used to access a filtered version of a repository without			This object is used to access a filtered version of a repository without
	altering the original repository object itself. We can not alter the			altering the original repository object itself. We can not alter the
	original object for two main reasons:			original object for two main reasons:
	# some cache may be implemented later			# some cache may be implemented later
	unfi = self._unfilteredrepo			unfi = self._unfilteredrepo
	unfichangelog = unfi.changelog			unfichangelog = unfi.changelog
	# bypass call to changelog.method			# bypass call to changelog.method
	unfiindex = unfichangelog.index			unfiindex = unfichangelog.index
	unfilen = len(unfiindex) - 1			unfilen = len(unfiindex) - 1
	unfinode = unfiindex[unfilen - 1][7]			unfinode = unfiindex[unfilen - 1][7]

	revs = filterrevs(unfi, self.filtername)			revs = filterrevs(unfi, self.filtername, self._visibilityexceptions)
	cl = self._clcache			cl = self._clcache
	newkey = (unfilen, unfinode, hash(revs), unfichangelog._delayed)			newkey = (unfilen, unfinode, hash(revs), unfichangelog._delayed)
	# if cl.index is not unfiindex, unfi.changelog would be			# if cl.index is not unfiindex, unfi.changelog would be
	# recreated, and our clcache refers to garbage object			# recreated, and our clcache refers to garbage object
	if (cl is not None and			if (cl is not None and
	(cl.index is not unfiindex or newkey != self._clcachekey)):			(cl.index is not unfiindex or newkey != self._clcachekey)):
	cl = None			cl = None
	# could have been made None by the previous if			# could have been made None by the previous if