[PATCH 1 of 3 RFC] changelog: handle obsolete and secret changesets more gracefully
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Thu Aug 15 18:43:22 CDT 2013
On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
> # Date 1375817670 -7200
> # Tue Aug 06 21:34:30 2013 +0200
> # Node ID 02c24ad6f4d2d954da11d128280978e25aaa48fd
> # Parent df2155ebf502d14f7ef5276df806a35fc06dabd5
> changelog: handle obsolete and secret changesets more gracefully
repoview: introduce specialized exception for filtered lookup
>
> We now issue a 'changeset is hidden' message when the user attempts to
> access a hidden (i.e. secret or obsolete) changeset.
"hidden" sounds wrong we have multiple level of filtering. A more generic message stating that "changeset is filtered" may be a simpler first step.
You probably want to add a hint toward the "--hidden" flag at some point too (not in this changeset)
> Please notice that this introduces a slight information leak, as
> knowledge of a 'secret' hash may now be used to test whether it's
> present in a given repository.
I do not mind much information leak of the secret changeset, but I know some people do. Sensible code could double check the phases of the phases before printing information.
> diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
> --- a/mercurial/branchmap.py
> +++ b/mercurial/branchmap.py
> @@ -7,7 +7,7 @@
>
> from node import bin, hex, nullid, nullrev
> import encoding
> -import util, repoview
> +import error, util, repoview
>
> def _filename(repo):
> """name of a branchcache file for a given repo or repoview"""
> @@ -121,7 +121,7 @@ class branchcache(dict):
> try:
> return ((self.tipnode == repo.changelog.node(self.tiprev))
> and (self.filteredhash == self._hashfiltered(repo)))
> - except IndexError:
> + except (IndexError, error.FilteredLookupError):
Your new exception -must- be a super class of the original exception. Then code that cares about the distinction may detect it and all other code can keep working unchanged.
> return False
>
> def copy(self):
> diff --git a/mercurial/changelog.py b/mercurial/changelog.py
> --- a/mercurial/changelog.py
> +++ b/mercurial/changelog.py
> @@ -5,7 +5,7 @@
> # This software may be used and distributed according to the terms of the
> # GNU General Public License version 2 or any later version.
>
> -from node import bin, hex, nullid
> +from node import bin, hex, nullid, short
> from i18n import _
> import util, error, revlog, encoding
>
> @@ -159,12 +159,15 @@ class changelog(revlog.revlog):
> self.rev(self.node(0))
> return self._nodecache
>
> - def hasnode(self, node):
> + def hasnode(self, node, hidden=False):
> """filtered version of revlog.hasnode"""
> + if hidden:
> + return super(changelog, self).hasnode(node)
> +
Disabling filtering should be done using an unfiltered version instead. (1) we have more that the "visible" view. (2) we can't add such argument everywhere.
> try:
> i = self.rev(node)
> return i not in self.filteredrevs
> - except KeyError:
> + except (KeyError, error.FilteredLookupError):
Same as above. raise a super class of KeyError
> return False
>
> def headrevs(self):
> @@ -183,31 +186,36 @@ class changelog(revlog.revlog):
> """filtered version of revlog.rev"""
> r = super(changelog, self).rev(node)
> if r in self.filteredrevs:
> - raise error.LookupError(hex(node), self.indexfile, _('no node'))
> + msg = _('changeset %s is hidden') % short(node)
> + raise error.FilteredLookupError(r, msg)
(forging the message here is probably useless. useful information is probably <node> + <filtername>)
> return r
>
> def node(self, rev):
> """filtered version of revlog.node"""
> if rev in self.filteredrevs:
> - raise IndexError(rev)
> + msg = _('revision %d is hidden') % rev
> + raise error.FilteredLookupError(rev, msg)
This should be a different exception (and an IndexError subclass)
> return super(changelog, self).node(rev)
>
> def linkrev(self, rev):
> """filtered version of revlog.linkrev"""
> if rev in self.filteredrevs:
> - raise IndexError(rev)
> + msg = _('revision %d is hidden') % rev
> + raise error.FilteredLookupError(rev, msg)
idem
> return super(changelog, self).linkrev(rev)
>
> def parentrevs(self, rev):
> """filtered version of revlog.parentrevs"""
> if rev in self.filteredrevs:
> - raise IndexError(rev)
> + msg = _('revision %d is hidden') % rev
> + raise error.FilteredLookupError(rev, msg)
idem ;-)
> return super(changelog, self).parentrevs(rev)
>
> def flags(self, rev):
> """filtered version of revlog.flags"""
> if rev in self.filteredrevs:
> - raise IndexError(rev)
> + msg = _('revision %d is hidden') % rev
> + raise error.FilteredLookupError(rev, msg)
idem ;-p
> return super(changelog, self).flags(rev)
>
> def delayupdate(self):
> diff --git a/mercurial/error.py b/mercurial/error.py
> --- a/mercurial/error.py
> +++ b/mercurial/error.py
> @@ -59,6 +59,14 @@ class RepoError(Exception):
> class RepoLookupError(RepoError):
> pass
>
> +class FilteredLookupError(RepoLookupError):
> + """Exception raised on accessing filtered (i.e. hidden) changesets."""
not necessarily "hidden"
> + def __init__(self, rev, message):
> + self.rev = rev
> + RepoLookupError.__init__(self, message)
See above comment about message and argument. You can probably forge a default message to be passed to RepoLookupError.
>
> class CapabilityError(RepoError):
> pass
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -279,7 +279,7 @@ class revlog(object):
> self.rev(self.node(0))
> return self._nodecache
>
> - def hasnode(self, node):
> + def hasnode(self, node, hidden=False):
> try:
> self.rev(node)
> return True
> @@ -752,7 +752,7 @@ class revlog(object):
> def _partialmatch(self, id):
> try:
> n = self.index.partialmatch(id)
> - if n and self.hasnode(n):
> + if n and self.hasnode(n, hidden=True):
> return n
> return None
> except RevlogError:
> diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
> --- a/tests/test-hgweb-commands.t
> +++ b/tests/test-hgweb-commands.t
> @@ -1390,7 +1390,7 @@ proper status for filtered revision
> Content-Type: text/plain; charset=ascii\r (esc)
> \r (esc)
>
> - error: unknown revision '5'
> + error: revision 5 is hidden
>
>
>
> @@ -1404,7 +1404,7 @@ proper status for filtered revision
> Content-Type: text/plain; charset=ascii\r (esc)
> \r (esc)
>
> - error: unknown revision '4'
> + error: revision 4 is hidden
>
> filtered '0' changeset
>
> diff --git a/tests/test-log.t b/tests/test-log.t
> --- a/tests/test-log.t
> +++ b/tests/test-log.t
> @@ -1226,6 +1226,15 @@ enable obsolete to test hidden feature
> $ hg log -r a
> abort: unknown revision 'a'!
> [255]
> + $ hg log -r a7
> + abort: unknown revision 'a7'!
> + [255]
> + $ hg log -r a76
> + abort: unknown revision 'a76'!
> + [255]
> + $ hg log -r a765
> + abort: changeset a765632148dc is hidden!
> + [255]
>
> test that parent prevent a changeset to be hidden
>
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -211,7 +211,7 @@ check that various commands work well wi
> abort: unknown revision '6'!
> [255]
> $ hg log -r 4
> - abort: unknown revision '4'!
> + abort: revision 4 is hidden!
> [255]
>
> Check that public changeset are not accounted as obsolete:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list