| Submitter | via Mercurial-devel |
|---|---|
| Date | Aug. 22, 2017, 8:05 p.m. |
| Message ID | <efc4b69aecfb3b407ca3.1503432309@martinvonz.svl.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/23214/ |
| State | Superseded |
| Headers | show |
Comments
The patch seems fine. Could you move the _readmarkers function definition inside the if/else, flake8 and pyflakes are complaining: + hgext3rd/evolve/obscache.py:133:5: F811 redefinition of unused '_readmarkers' from line 115 How did you detect it? Did you call _readmarkers from another extension? Should we add an Evolve test to not be taken off-guard the next time? On Tue, 2017-08-22 at 13:05 -0700, Martin von Zweigbergk via Mercurial- devel wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1503424759 25200 > # Tue Aug 22 10:59:19 2017 -0700 > # Node ID efc4b69aecfb3b407ca362cff318044445e9672c > # Parent 95470e817c00b03fcf99e486412cc7d7f0116681 > obscache: use _readmarkers() from core with correct signature > > The copied _readmarkers() went stale with Mercurial core commit > 5d3ba4395288 (obsstore: let read marker API take a range of offsets, > 2017-06-04). At the same time, the Mercurial core version of the > function gained the desired offset argument, so we can now use that > function. > > diff --git a/hgext3rd/evolve/obscache.py > b/hgext3rd/evolve/obscache.py > --- a/hgext3rd/evolve/obscache.py > +++ b/hgext3rd/evolve/obscache.py > @@ -127,6 +127,11 @@ > % diskversion) > return diskversion, obsolete.formats[diskversion][0](data, off) > > +if obsolete._readmarkers.__code__.co_argcount > 1: > + # hg-4.3+ has the "offset" parameter, and _fm?readmarkers also > have an > + # extra "stop" parameter > + _readmarkers = obsolete._readmarkers > + > def markersfrom(obsstore, byteoffset, firstmarker): > if not firstmarker: > return list(obsstore) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Aug 23, 2017 1:36 AM, "Boris Feld" <boris.feld@octobus.net> wrote: The patch seems fine. Could you move the _readmarkers function definition inside the if/else, flake8 and pyflakes are complaining: + hgext3rd/evolve/obscache.py:133:5: F811 redefinition of unused '_readmarkers' from line 115 Ok, will do. How did you detect it? Did you call _readmarkers from another extension? Should we add an Evolve test to not be taken off-guard the next time? A user saw it crash. It's called from obscache.py, but only in rare cases, it seems. On Tue, 2017-08-22 at 13:05 -0700, Martin von Zweigbergk via Mercurial- devel wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1503424759 25200 > # Tue Aug 22 10:59:19 2017 -0700 > # Node ID efc4b69aecfb3b407ca362cff318044445e9672c > # Parent 95470e817c00b03fcf99e486412cc7d7f0116681 > obscache: use _readmarkers() from core with correct signature > > The copied _readmarkers() went stale with Mercurial core commit > 5d3ba4395288 (obsstore: let read marker API take a range of offsets, > 2017-06-04). At the same time, the Mercurial core version of the > function gained the desired offset argument, so we can now use that > function. > > diff --git a/hgext3rd/evolve/obscache.py > b/hgext3rd/evolve/obscache.py > --- a/hgext3rd/evolve/obscache.py > +++ b/hgext3rd/evolve/obscache.py > @@ -127,6 +127,11 @@ > % diskversion) > return diskversion, obsolete.formats[diskversion][0](data, off) > > +if obsolete._readmarkers.__code__.co_argcount > 1: > + # hg-4.3+ has the "offset" parameter, and _fm?readmarkers also > have an > + # extra "stop" parameter > + _readmarkers = obsolete._readmarkers > + > def markersfrom(obsstore, byteoffset, firstmarker): > if not firstmarker: > return list(obsstore) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Wed, 2017-08-23 at 06:06 -0700, Martin von Zweigbergk wrote: > On Aug 23, 2017 1:36 AM, "Boris Feld" <boris.feld@octobus.net> wrote: > The patch seems fine. Could you move the _readmarkers function > > definition inside the if/else, flake8 and pyflakes are complaining: > > > > + hgext3rd/evolve/obscache.py:133:5: F811 redefinition of unused > > '_readmarkers' from line 115 > > > Ok, will do. > > > > > How did you detect it? Did you call _readmarkers from another > > extension? Should we add an Evolve test to not be taken off-guard the > > next time? > > > A user saw it crash. It's called from obscache.py, but only in rare > cases, it seems. You are right, I just ran the tests with coverage and we don't cover this case, we will try add a test for it. > > > On Tue, 2017-08-22 at 13:05 -0700, Martin von Zweigbergk via > Mercurial- > > devel wrote: > > > # HG changeset patch > > > # User Martin von Zweigbergk <martinvonz@google.com> > > > # Date 1503424759 25200 > > > # Tue Aug 22 10:59:19 2017 -0700 > > > # Node ID efc4b69aecfb3b407ca362cff318044445e9672c > > > # Parent 95470e817c00b03fcf99e486412cc7d7f0116681 > > > obscache: use _readmarkers() from core with correct signature > > > > > > The copied _readmarkers() went stale with Mercurial core commit > > > 5d3ba4395288 (obsstore: let read marker API take a range of > offsets, > > > 2017-06-04). At the same time, the Mercurial core version of the > > > function gained the desired offset argument, so we can now use that > > > function. > > > > > > diff --git a/hgext3rd/evolve/obscache.py > > > b/hgext3rd/evolve/obscache.py > > > --- a/hgext3rd/evolve/obscache.py > > > +++ b/hgext3rd/evolve/obscache.py > > > @@ -127,6 +127,11 @@ > > > % diskversion) > > > return diskversion, obsolete.formats[diskversion][0](data, > off) > > > > > > +if obsolete._readmarkers.__code__.co_argcount > 1: > > > + # hg-4.3+ has the "offset" parameter, and _fm?readmarkers also > > > have an > > > + # extra "stop" parameter > > > + _readmarkers = obsolete._readmarkers > > > + > > > def markersfrom(obsstore, byteoffset, firstmarker): > > > if not firstmarker: > > > return list(obsstore) > > > _______________________________________________ > > > Mercurial-devel mailing list > > > Mercurial-devel@mercurial-scm.org > > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > >
Patch
diff --git a/hgext3rd/evolve/obscache.py b/hgext3rd/evolve/obscache.py --- a/hgext3rd/evolve/obscache.py +++ b/hgext3rd/evolve/obscache.py @@ -127,6 +127,11 @@ % diskversion) return diskversion, obsolete.formats[diskversion][0](data, off) +if obsolete._readmarkers.__code__.co_argcount > 1: + # hg-4.3+ has the "offset" parameter, and _fm?readmarkers also have an + # extra "stop" parameter + _readmarkers = obsolete._readmarkers + def markersfrom(obsstore, byteoffset, firstmarker): if not firstmarker: return list(obsstore)