[PATCH 3 of 3 V3] localrepo: better error message when revlog error and file addition (issue4675)

[Jordi Gutiérrez Hermoso]

On Thu, 2015-06-04 at 14:57 -0500, Matt Mackall wrote:
> Here's what I think we ought to do:
> 
> diff -r 69609f43c752 mercurial/revlog.py
> --- a/mercurial/revlog.py	Fri May 29 13:11:52 2015 -0700
> +++ b/mercurial/revlog.py	Thu Jun 04 14:48:20 2015 -0500
> @@ -167,12 +167,6 @@
>          return index, getattr(index, 'nodemap', None), cache
>  
>      def packentry(self, entry, node, version, rev):
> -        uncompressedlength = entry[2]
> -        if uncompressedlength > _maxentrysize:
> -            raise RevlogError(
> -                _("size of %d bytes exceeds maximum revlog storage of 2GiB")
> -                % uncompressedlength)
> -
>          p = _pack(indexformatng, *entry)
>          if rev == 0:
>              p = _pack(versionformat, version) + p[4:]
> @@ -1190,6 +1184,12 @@
>          if link == nullrev:
>              raise RevlogError(_("attempted to add linkrev -1 to %s")
>                                % self.indexfile)
> +
> +        if len(text) > _maxentrysize:
> +            raise RevlogError(
> +                _("%s: size of %d bytes exceeds maximum revlog storage of 2GiB")
> +                % (self.indexfile, uncompressedlength))
> +
>          node = node or self.hash(text, p1, p2)
>          if node in self.nodemap:
>              return node
> 
> Things to note:
> - it hides the ugly traceback
> - it handles both the revlogio and revlogoldio paths
> - it handles all the revlogs
> - it hides the details from upper levels
> - it shows a filename that will probably be user-comprehensible
> - we probably won't need to think about this again for years

I like everything except exposing the revlog's file name. I disagree
that users will know what it means. Most users don't even know what a
revlog is, and without some knowledge of hg internals, they won't know
how to tell apart a manifest from a file revlog.

But I'm also tired of arguing about this. Just push your patch. It's
certainly better than what we have now, and they can always do a web
search for the error message if they need an explanation.