Submitter | Matt Mackall |
---|---|
Date | June 4, 2015, 7:57 p.m. |
Message ID | <1433447820.3128.14.camel@selenic.com> |
Download | mbox | patch |
Permalink | /patch/9487/ |
State | Not Applicable |
Headers | show |
Comments
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.
Patch
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