Patchwork [3,of,3] dispatch: only check compatibility against major and minor versions

login
register
mail settings
Submitter Gregory Szorc
Date Jan. 16, 2015, 4:36 a.m.
Message ID <28e3fdafed22396c1096.1421382980@3.1.168.192.in-addr.arpa>
Download mbox | patch
Permalink /patch/7486/
State Accepted
Headers show

Comments

Gregory Szorc - Jan. 16, 2015, 4:36 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1421382963 28800
#      Thu Jan 15 20:36:03 2015 -0800
# Node ID 28e3fdafed22396c10964fa72c869c0e8c81ae05
# Parent  d9c6f710438d71fb5b4114f0639d27f75d8cf1b9
dispatch: only check compatibility against major and minor versions

Extensions can declare compatibility with Mercurial versions. If an
error occurs, Mercurial will attempt to pin blame on an extension that
isn't marked as compatible.

While all bets are off when it comes to the internal API, my experience
has shown that a monthly/patch release of Mercurial has never broken any
of the extensions I've written. I think that expecting extensions to
declare compatibility with every patch release of Mercurial is asking a
bit much and adds little to no value.

This patch changes the blame logic from exact version matching to only
match on the major and minor Mercurial versions. This means that
extensions only need to mark themselves as compatible with the major,
quarterly releases and not the monthly ones in order to stay current and
avoid what is almost certainly unfair blame. This will mean less work
for extension authors and almost certainly fewer false positives in the
blame attribution.
Ryan McElroy - Jan. 16, 2015, 6:52 a.m.
On 1/15/2015 8:36 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1421382963 28800
> #      Thu Jan 15 20:36:03 2015 -0800
> # Node ID 28e3fdafed22396c10964fa72c869c0e8c81ae05
> # Parent  d9c6f710438d71fb5b4114f0639d27f75d8cf1b9
> dispatch: only check compatibility against major and minor versions
>
> Extensions can declare compatibility with Mercurial versions. If an
> error occurs, Mercurial will attempt to pin blame on an extension that
> isn't marked as compatible.
>
> While all bets are off when it comes to the internal API, my experience
> has shown that a monthly/patch release of Mercurial has never broken any
> of the extensions I've written. I think that expecting extensions to
> declare compatibility with every patch release of Mercurial is asking a
> bit much and adds little to no value.
>
> This patch changes the blame logic from exact version matching to only
> match on the major and minor Mercurial versions. This means that
> extensions only need to mark themselves as compatible with the major,
> quarterly releases and not the monthly ones in order to stay current and
> avoid what is almost certainly unfair blame. This will mean less work
> for extension authors and almost certainly fewer false positives in the
> blame attribution.
>
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -282,14 +282,21 @@ def _runcatch(req):
>               if not testedwith.strip():
>                   # We found an untested extension. It's likely the culprit.
>                   worst = name, 'unknown', report
>                   break
> -            if compare not in testedwith.split() and testedwith != 'internal':
> -                tested = [tuplever(v) for v in testedwith.split()]
> -                lower = [t for t in tested if t < ct]
> -                nearest = max(lower or tested)
> -                if worst[0] is None or nearest < worst[1]:
> -                    worst = name, nearest, report
> +
> +            # Never blame on extensions bundled with Mercurial.
> +            if testedwith == 'internal':
> +                continue
> +
> +            tested = [tuplever(t) for t in testedwith.split()]
> +            if ct in tested:
> +                continue
> +
> +            lower = [t for t in tested if t < ct]
> +            nearest = max(lower or tested)
> +            if worst[0] is None or nearest < worst[1]:
> +                worst = name, nearest, report
>           if worst[0] is not None:
>               name, testedwith, report = worst
>               if not isinstance(testedwith, str):
>                   testedwith = '.'.join([str(c) for c in testedwith])
> @@ -314,9 +321,12 @@ def _runcatch(req):
>       return -1
>   
>   def tuplever(v):
>       try:
> -        return tuple([int(i) for i in v.split('.')])
> +        # Assertion: tuplever is only used for extension compatibility
> +        # checking. Otherwise, the discarding of extra version fields is
> +        # incorrect.
> +        return tuple([int(i) for i in v.split('.')[0:2]])
>       except ValueError:
>           return tuple()
>   
>   def aliasargs(fn, givenargs):
> diff --git a/tests/test-extension.t b/tests/test-extension.t
> --- a/tests/test-extension.t
> +++ b/tests/test-extension.t
> @@ -857,9 +857,9 @@ Broken disabled extension and command:
>     (try "hg help --keyword foo")
>     [255]
>   
>     $ cat > throw.py <<EOF
> -  > from mercurial import cmdutil, commands
> +  > from mercurial import cmdutil, commands, util
>     > cmdtable = {}
>     > command = cmdutil.command(cmdtable)
>     > class Bogon(Exception): pass
>     > @command('throw', [], 'hg throw', norepo=True)
> @@ -909,9 +909,9 @@ If the extensions declare outdated versi
>     $ rm -f throw.pyc throw.pyo
>     $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
>     >   throw 2>&1 | egrep '^\*\*'
>     ** Unknown exception encountered with possibly-broken third-party extension older
> -  ** which supports versions 1.9.3 of Mercurial.
> +  ** which supports versions 1.9 of Mercurial.
>     ** Please disable older and try your action again.
>     ** If that fixes the bug please report it to the extension author.
>     ** Python * (glob)
>     ** Mercurial Distributed SCM (version 2.2)
> @@ -922,9 +922,9 @@ One extension only tested with older, on
>     $ rm -f older.pyc older.pyo
>     $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
>     >   throw 2>&1 | egrep '^\*\*'
>     ** Unknown exception encountered with possibly-broken third-party extension older
> -  ** which supports versions 1.9.3 of Mercurial.
> +  ** which supports versions 1.9 of Mercurial.
>     ** Please disable older and try your action again.
>     ** If that fixes the bug please report it to the extension author.
>     ** Python * (glob)
>     ** Mercurial Distributed SCM (version 2.1)
> @@ -935,9 +935,9 @@ Older extension is tested with current v
>     $ rm -f older.pyc older.pyo
>     $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
>     >   throw 2>&1 | egrep '^\*\*'
>     ** Unknown exception encountered with possibly-broken third-party extension throw
> -  ** which supports versions 2.1.1 of Mercurial.
> +  ** which supports versions 2.1 of Mercurial.
>     ** Please disable throw and try your action again.
>     ** If that fixes the bug please report it to https://urldefense.proofpoint.com/v1/url?u=http://example.com/bts&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=zxRJZ6melt%2FqLtQ%2Bw2Gaeg%3D%3D%0A&m=qoS%2F9eZdZYNoIzL0gpp6tWE%2BFpkmZjZRAV%2F%2BmKm2V5M%3D%0A&s=500584f65f60a10bafb67431f6b711c16907bd92e7a0bf50f1d9cf7f7172ca35
>     ** Python * (glob)
>     ** Mercurial Distributed SCM (version 1.9.3)
> @@ -953,8 +953,19 @@ Declare the version as supporting this h
>     ** Python * (glob)
>     ** Mercurial Distributed SCM (*) (glob)
>     ** Extensions loaded: throw
>   
> +Patch version is ignored during compatibility check
> +  $ echo "testedwith = '3.2'" >> throw.py
> +  $ echo "util.version = lambda:'3.2.2'" >> throw.py
> +  $ rm -f throw.pyc throw.pyo
> +  $ hg --config extensions.throw=throw.py throw 2>&1 | egrep '^\*\*'
> +  ** unknown exception encountered, please report by visiting
> +  ** http://mercurial.selenic.com/wiki/BugTracker
> +  ** Python * (glob)
> +  ** Mercurial Distributed SCM (*) (glob)
> +  ** Extensions loaded: throw
> +
>   Test version number support in 'hg version':
>     $ echo '__version__ = (1, 2, 3)' >> throw.py
>     $ rm -f throw.pyc throw.pyo
>     $ hg version -v
>

I'm happy enough with this series (I'm not for or against this third 
patch the the other two seem strictly positive). I might argue against 
this third patch on the basis that major revisions don't seem to have 
any semantic meaning in mercurial (as I understand it -- but I'm new 
here -- hg goes from 2.9 to 3.0 on a schedule, not on a set of 
features). However, I could argue for it in that it reduces the overhead 
of maintaining extensions to every ten freezes instead of every freeze, 
which is probably a net win for the world overall.

That being said, the whole blaming an extension based on declared 
compatibility is totally broken. I'd much rather see us try to guess 
where the break is based on any extension being in the stack trace, 
otherwise not assigning blame to any particular extension. In my 
experience at FB, this code *always* blames crecord, which is never 
actually at fault.

I'll probably take a crack at doing something like this eventually if 
nobody else ever does, but it's not very high on any priority list.

Another thing that I think would be good is to write the stack trace to 
a file (and mention this file in the crash report) but never expose the 
user to a stack trace. For most users, a stack trace is pretty scary 
looking and a nice "something went wrong" message with instructions on 
how to report it without the stack trace would be a lot more 
approachable. But now I'm just rambling :-)

~Ryan
Gregory Szorc - Jan. 16, 2015, 7:38 a.m.
On Thu, Jan 15, 2015 at 10:52 PM, Ryan McElroy <rm@fb.com> wrote:

> On 1/15/2015 8:36 PM, Gregory Szorc wrote:
>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1421382963 28800
>> #      Thu Jan 15 20:36:03 2015 -0800
>> # Node ID 28e3fdafed22396c10964fa72c869c0e8c81ae05
>> # Parent  d9c6f710438d71fb5b4114f0639d27f75d8cf1b9
>> dispatch: only check compatibility against major and minor versions
>>
>> Extensions can declare compatibility with Mercurial versions. If an
>> error occurs, Mercurial will attempt to pin blame on an extension that
>> isn't marked as compatible.
>>
>> While all bets are off when it comes to the internal API, my experience
>> has shown that a monthly/patch release of Mercurial has never broken any
>> of the extensions I've written. I think that expecting extensions to
>> declare compatibility with every patch release of Mercurial is asking a
>> bit much and adds little to no value.
>>
>> This patch changes the blame logic from exact version matching to only
>> match on the major and minor Mercurial versions. This means that
>> extensions only need to mark themselves as compatible with the major,
>> quarterly releases and not the monthly ones in order to stay current and
>> avoid what is almost certainly unfair blame. This will mean less work
>> for extension authors and almost certainly fewer false positives in the
>> blame attribution.
>>
>> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
>> --- a/mercurial/dispatch.py
>> +++ b/mercurial/dispatch.py
>> @@ -282,14 +282,21 @@ def _runcatch(req):
>>               if not testedwith.strip():
>>                   # We found an untested extension. It's likely the
>> culprit.
>>                   worst = name, 'unknown', report
>>                   break
>> -            if compare not in testedwith.split() and testedwith !=
>> 'internal':
>> -                tested = [tuplever(v) for v in testedwith.split()]
>> -                lower = [t for t in tested if t < ct]
>> -                nearest = max(lower or tested)
>> -                if worst[0] is None or nearest < worst[1]:
>> -                    worst = name, nearest, report
>> +
>> +            # Never blame on extensions bundled with Mercurial.
>> +            if testedwith == 'internal':
>> +                continue
>> +
>> +            tested = [tuplever(t) for t in testedwith.split()]
>> +            if ct in tested:
>> +                continue
>> +
>> +            lower = [t for t in tested if t < ct]
>> +            nearest = max(lower or tested)
>> +            if worst[0] is None or nearest < worst[1]:
>> +                worst = name, nearest, report
>>           if worst[0] is not None:
>>               name, testedwith, report = worst
>>               if not isinstance(testedwith, str):
>>                   testedwith = '.'.join([str(c) for c in testedwith])
>> @@ -314,9 +321,12 @@ def _runcatch(req):
>>       return -1
>>     def tuplever(v):
>>       try:
>> -        return tuple([int(i) for i in v.split('.')])
>> +        # Assertion: tuplever is only used for extension compatibility
>> +        # checking. Otherwise, the discarding of extra version fields is
>> +        # incorrect.
>> +        return tuple([int(i) for i in v.split('.')[0:2]])
>>       except ValueError:
>>           return tuple()
>>     def aliasargs(fn, givenargs):
>> diff --git a/tests/test-extension.t b/tests/test-extension.t
>> --- a/tests/test-extension.t
>> +++ b/tests/test-extension.t
>> @@ -857,9 +857,9 @@ Broken disabled extension and command:
>>     (try "hg help --keyword foo")
>>     [255]
>>       $ cat > throw.py <<EOF
>> -  > from mercurial import cmdutil, commands
>> +  > from mercurial import cmdutil, commands, util
>>     > cmdtable = {}
>>     > command = cmdutil.command(cmdtable)
>>     > class Bogon(Exception): pass
>>     > @command('throw', [], 'hg throw', norepo=True)
>> @@ -909,9 +909,9 @@ If the extensions declare outdated versi
>>     $ rm -f throw.pyc throw.pyo
>>     $ hg --config extensions.throw=throw.py --config
>> extensions.older=older.py \
>>     >   throw 2>&1 | egrep '^\*\*'
>>     ** Unknown exception encountered with possibly-broken third-party
>> extension older
>> -  ** which supports versions 1.9.3 of Mercurial.
>> +  ** which supports versions 1.9 of Mercurial.
>>     ** Please disable older and try your action again.
>>     ** If that fixes the bug please report it to the extension author.
>>     ** Python * (glob)
>>     ** Mercurial Distributed SCM (version 2.2)
>> @@ -922,9 +922,9 @@ One extension only tested with older, on
>>     $ rm -f older.pyc older.pyo
>>     $ hg --config extensions.throw=throw.py --config
>> extensions.older=older.py \
>>     >   throw 2>&1 | egrep '^\*\*'
>>     ** Unknown exception encountered with possibly-broken third-party
>> extension older
>> -  ** which supports versions 1.9.3 of Mercurial.
>> +  ** which supports versions 1.9 of Mercurial.
>>     ** Please disable older and try your action again.
>>     ** If that fixes the bug please report it to the extension author.
>>     ** Python * (glob)
>>     ** Mercurial Distributed SCM (version 2.1)
>> @@ -935,9 +935,9 @@ Older extension is tested with current v
>>     $ rm -f older.pyc older.pyo
>>     $ hg --config extensions.throw=throw.py --config
>> extensions.older=older.py \
>>     >   throw 2>&1 | egrep '^\*\*'
>>     ** Unknown exception encountered with possibly-broken third-party
>> extension throw
>> -  ** which supports versions 2.1.1 of Mercurial.
>> +  ** which supports versions 2.1 of Mercurial.
>>     ** Please disable throw and try your action again.
>>     ** If that fixes the bug please report it to
>> https://urldefense.proofpoint.com/v1/url?u=http://example.com/bts&k=
>> ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=zxRJZ6melt%2FqLtQ%
>> 2Bw2Gaeg%3D%3D%0A&m=qoS%2F9eZdZYNoIzL0gpp6tWE%
>> 2BFpkmZjZRAV%2F%2BmKm2V5M%3D%0A&s=500584f65f60a10bafb67431f6b711
>> c16907bd92e7a0bf50f1d9cf7f7172ca35
>>     ** Python * (glob)
>>     ** Mercurial Distributed SCM (version 1.9.3)
>> @@ -953,8 +953,19 @@ Declare the version as supporting this h
>>     ** Python * (glob)
>>     ** Mercurial Distributed SCM (*) (glob)
>>     ** Extensions loaded: throw
>>   +Patch version is ignored during compatibility check
>> +  $ echo "testedwith = '3.2'" >> throw.py
>> +  $ echo "util.version = lambda:'3.2.2'" >> throw.py
>> +  $ rm -f throw.pyc throw.pyo
>> +  $ hg --config extensions.throw=throw.py throw 2>&1 | egrep '^\*\*'
>> +  ** unknown exception encountered, please report by visiting
>> +  ** http://mercurial.selenic.com/wiki/BugTracker
>> +  ** Python * (glob)
>> +  ** Mercurial Distributed SCM (*) (glob)
>> +  ** Extensions loaded: throw
>> +
>>   Test version number support in 'hg version':
>>     $ echo '__version__ = (1, 2, 3)' >> throw.py
>>     $ rm -f throw.pyc throw.pyo
>>     $ hg version -v
>>
>>
> I'm happy enough with this series (I'm not for or against this third patch
> the the other two seem strictly positive). I might argue against this third
> patch on the basis that major revisions don't seem to have any semantic
> meaning in mercurial (as I understand it -- but I'm new here -- hg goes
> from 2.9 to 3.0 on a schedule, not on a set of features). However, I could
> argue for it in that it reduces the overhead of maintaining extensions to
> every ten freezes instead of every freeze, which is probably a net win for
> the world overall.
>

Unless I did something wrong, I reduced the warn threshold to every 3rd
freeze, not ten. We should be looking at X.Y versions.


> That being said, the whole blaming an extension based on declared
> compatibility is totally broken. I'd much rather see us try to guess where
> the break is based on any extension being in the stack trace, otherwise not
> assigning blame to any particular extension. In my experience at FB, this
> code *always* blames crecord, which is never actually at fault.
>
> I'll probably take a crack at doing something like this eventually if
> nobody else ever does, but it's not very high on any priority list.
>
> Another thing that I think would be good is to write the stack trace to a
> file (and mention this file in the crash report) but never expose the user
> to a stack trace. For most users, a stack trace is pretty scary looking and
> a nice "something went wrong" message with instructions on how to report it
> without the stack trace would be a lot more approachable. But now I'm just
> rambling :-)
>

I completely agree about blaming being almost completely busted. I, too,
wish it were based on stacks. But even that isn't perfect. For example, you
can get a TypeError at a call site when passing an unknown argument into a
function in an extension that doesn't accept that argument. Of course, you
can also get a TypeError inside a function. How do you attribute which file
is to blame? I argue that in the current world of relying on
monkeypatching, you can't. One of the benefits of a more well-defined
extensibility API (such as the events proposal I sent a few months back) is
that call sites into extensions are more clearly defined. So, if an error
occurs, we can look at the module/file being called into an attribute it to
an extension. That's better but still not perfect. You still have a general
class of errors around "extension manipulated something wrong and we didn't
find out about it until later." Oy.

I like your idea of writing stacks to files! I'd love to see that in 3.4,
even if it's behind a feature flag.
Pierre-Yves David - Jan. 16, 2015, 8:06 a.m.
On 01/15/2015 10:52 PM, Ryan McElroy wrote:
> I'm happy enough with this series (I'm not for or against this third
> patch the the other two seem strictly positive).

I've pushed the two first on the clowncopter

> I might argue against
> this third patch on the basis that major revisions don't seem to have
> any semantic meaning in mercurial (as I understand it -- but I'm new
> here -- hg goes from 2.9 to 3.0 on a schedule, not on a set of
> features). However, I could argue for it in that it reduces the overhead
> of maintaining extensions to every ten freezes instead of every freeze,
> which is probably a net win for the world overall.

I think you are getting this wrong. The changes move from declaring 
compatibility from "1.9.3" to "1.9".

I'm not sure what I think about it. I certainly already did change that 
break BC between bug fixe release (when bug is super bad).

But I kind of agree that updating for each version is too tedious to 
ever happen (I do not do it).

I'll let the nigh rest over it.
Pierre-Yves David - Jan. 16, 2015, 8:10 a.m.
On 01/15/2015 11:38 PM, Gregory Szorc wrote:
> I like your idea of writing stacks to files! I'd love to see that in
> 3.4, even if it's behind a feature flag.

I think this is a super good idea too.
Augie Fackler - Jan. 16, 2015, 4:07 p.m.
On Thu, Jan 15, 2015 at 11:38:28PM -0800, Gregory Szorc wrote:
> On Thu, Jan 15, 2015 at 10:52 PM, Ryan McElroy <rm@fb.com> wrote:
>
> > On 1/15/2015 8:36 PM, Gregory Szorc wrote:
> >>   Test version number support in 'hg version':
> >>     $ echo '__version__ = (1, 2, 3)' >> throw.py
> >>     $ rm -f throw.pyc throw.pyo
> >>     $ hg version -v
> >>
> >>
> > I'm happy enough with this series (I'm not for or against this third patch
> > the the other two seem strictly positive). I might argue against this third
> > patch on the basis that major revisions don't seem to have any semantic
> > meaning in mercurial (as I understand it -- but I'm new here -- hg goes
> > from 2.9 to 3.0 on a schedule, not on a set of features). However, I could
> > argue for it in that it reduces the overhead of maintaining extensions to
> > every ten freezes instead of every freeze, which is probably a net win for
> > the world overall.
> >
>
> Unless I did something wrong, I reduced the warn threshold to every 3rd
> freeze, not ten. We should be looking at X.Y versions.
>
>
> > That being said, the whole blaming an extension based on declared
> > compatibility is totally broken. I'd much rather see us try to guess where
> > the break is based on any extension being in the stack trace, otherwise not
> > assigning blame to any particular extension. In my experience at FB, this
> > code *always* blames crecord, which is never actually at fault.
> >
> > I'll probably take a crack at doing something like this eventually if
> > nobody else ever does, but it's not very high on any priority list.
> >
> > Another thing that I think would be good is to write the stack trace to a
> > file (and mention this file in the crash report) but never expose the user
> > to a stack trace. For most users, a stack trace is pretty scary looking and
> > a nice "something went wrong" message with instructions on how to report it
> > without the stack trace would be a lot more approachable. But now I'm just
> > rambling :-)
> >
>
> I completely agree about blaming being almost completely busted. I, too,
> wish it were based on stacks. But even that isn't perfect. For example, you
> can get a TypeError at a call site when passing an unknown argument into a
> function in an extension that doesn't accept that argument. Of course, you
> can also get a TypeError inside a function. How do you attribute which file
> is to blame? I argue that in the current world of relying on
> monkeypatching, you can't. One of the benefits of a more well-defined
> extensibility API (such as the events proposal I sent a few months back) is
> that call sites into extensions are more clearly defined. So, if an error
> occurs, we can look at the module/file being called into an attribute it to
> an extension. That's better but still not perfect. You still have a general
> class of errors around "extension manipulated something wrong and we didn't
> find out about it until later." Oy.

I think we could probably do significantly better by recording a list
of what extension wrapped what things in extensions.wrap*, and then
pattern matching the bottommost stack frame against and see if we can
point a more accurate finger.

>
> I like your idea of writing stacks to files! I'd love to see that in 3.4,
> even if it's behind a feature flag.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Jan. 16, 2015, 4:09 p.m.
On Fri, Jan 16, 2015 at 12:06:41AM -0800, Pierre-Yves David wrote:
>
>
> On 01/15/2015 10:52 PM, Ryan McElroy wrote:
> >I'm happy enough with this series (I'm not for or against this third
> >patch the the other two seem strictly positive).
>
> I've pushed the two first on the clowncopter
>
> >I might argue against
> >this third patch on the basis that major revisions don't seem to have
> >any semantic meaning in mercurial (as I understand it -- but I'm new
> >here -- hg goes from 2.9 to 3.0 on a schedule, not on a set of
> >features). However, I could argue for it in that it reduces the overhead
> >of maintaining extensions to every ten freezes instead of every freeze,
> >which is probably a net win for the world overall.
>
> I think you are getting this wrong. The changes move from declaring
> compatibility from "1.9.3" to "1.9".
>
> I'm not sure what I think about it. I certainly already did change that
> break BC between bug fixe release (when bug is super bad).

This appears to be the exception rather than the rule - we typically
only do prep work in hg-git and hgsubversion for the major releases,
and assume we're good for the next few months.

>
> But I kind of agree that updating for each version is too tedious to ever
> happen (I do not do it).

Yup.

>
> I'll let the nigh rest over it.

I'm going to queue these.

>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Jan. 16, 2015, 4:09 p.m.
On Thu, Jan 15, 2015 at 08:36:20PM -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1421382963 28800
> #      Thu Jan 15 20:36:03 2015 -0800
> # Node ID 28e3fdafed22396c10964fa72c869c0e8c81ae05
> # Parent  d9c6f710438d71fb5b4114f0639d27f75d8cf1b9
> dispatch: only check compatibility against major and minor versions

I think this is a strict improvement. I'm going to queue all of this, and mark this one (bc)

>
> Extensions can declare compatibility with Mercurial versions. If an
> error occurs, Mercurial will attempt to pin blame on an extension that
> isn't marked as compatible.
>
> While all bets are off when it comes to the internal API, my experience
> has shown that a monthly/patch release of Mercurial has never broken any
> of the extensions I've written. I think that expecting extensions to
> declare compatibility with every patch release of Mercurial is asking a
> bit much and adds little to no value.
>
> This patch changes the blame logic from exact version matching to only
> match on the major and minor Mercurial versions. This means that
> extensions only need to mark themselves as compatible with the major,
> quarterly releases and not the monthly ones in order to stay current and
> avoid what is almost certainly unfair blame. This will mean less work
> for extension authors and almost certainly fewer false positives in the
> blame attribution.
>
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -282,14 +282,21 @@ def _runcatch(req):
>              if not testedwith.strip():
>                  # We found an untested extension. It's likely the culprit.
>                  worst = name, 'unknown', report
>                  break
> -            if compare not in testedwith.split() and testedwith != 'internal':
> -                tested = [tuplever(v) for v in testedwith.split()]
> -                lower = [t for t in tested if t < ct]
> -                nearest = max(lower or tested)
> -                if worst[0] is None or nearest < worst[1]:
> -                    worst = name, nearest, report
> +
> +            # Never blame on extensions bundled with Mercurial.
> +            if testedwith == 'internal':
> +                continue
> +
> +            tested = [tuplever(t) for t in testedwith.split()]
> +            if ct in tested:
> +                continue
> +
> +            lower = [t for t in tested if t < ct]
> +            nearest = max(lower or tested)
> +            if worst[0] is None or nearest < worst[1]:
> +                worst = name, nearest, report
>          if worst[0] is not None:
>              name, testedwith, report = worst
>              if not isinstance(testedwith, str):
>                  testedwith = '.'.join([str(c) for c in testedwith])
> @@ -314,9 +321,12 @@ def _runcatch(req):
>      return -1
>
>  def tuplever(v):
>      try:
> -        return tuple([int(i) for i in v.split('.')])
> +        # Assertion: tuplever is only used for extension compatibility
> +        # checking. Otherwise, the discarding of extra version fields is
> +        # incorrect.
> +        return tuple([int(i) for i in v.split('.')[0:2]])
>      except ValueError:
>          return tuple()
>
>  def aliasargs(fn, givenargs):
> diff --git a/tests/test-extension.t b/tests/test-extension.t
> --- a/tests/test-extension.t
> +++ b/tests/test-extension.t
> @@ -857,9 +857,9 @@ Broken disabled extension and command:
>    (try "hg help --keyword foo")
>    [255]
>
>    $ cat > throw.py <<EOF
> -  > from mercurial import cmdutil, commands
> +  > from mercurial import cmdutil, commands, util
>    > cmdtable = {}
>    > command = cmdutil.command(cmdtable)
>    > class Bogon(Exception): pass
>    > @command('throw', [], 'hg throw', norepo=True)
> @@ -909,9 +909,9 @@ If the extensions declare outdated versi
>    $ rm -f throw.pyc throw.pyo
>    $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
>    >   throw 2>&1 | egrep '^\*\*'
>    ** Unknown exception encountered with possibly-broken third-party extension older
> -  ** which supports versions 1.9.3 of Mercurial.
> +  ** which supports versions 1.9 of Mercurial.
>    ** Please disable older and try your action again.
>    ** If that fixes the bug please report it to the extension author.
>    ** Python * (glob)
>    ** Mercurial Distributed SCM (version 2.2)
> @@ -922,9 +922,9 @@ One extension only tested with older, on
>    $ rm -f older.pyc older.pyo
>    $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
>    >   throw 2>&1 | egrep '^\*\*'
>    ** Unknown exception encountered with possibly-broken third-party extension older
> -  ** which supports versions 1.9.3 of Mercurial.
> +  ** which supports versions 1.9 of Mercurial.
>    ** Please disable older and try your action again.
>    ** If that fixes the bug please report it to the extension author.
>    ** Python * (glob)
>    ** Mercurial Distributed SCM (version 2.1)
> @@ -935,9 +935,9 @@ Older extension is tested with current v
>    $ rm -f older.pyc older.pyo
>    $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
>    >   throw 2>&1 | egrep '^\*\*'
>    ** Unknown exception encountered with possibly-broken third-party extension throw
> -  ** which supports versions 2.1.1 of Mercurial.
> +  ** which supports versions 2.1 of Mercurial.
>    ** Please disable throw and try your action again.
>    ** If that fixes the bug please report it to http://example.com/bts
>    ** Python * (glob)
>    ** Mercurial Distributed SCM (version 1.9.3)
> @@ -953,8 +953,19 @@ Declare the version as supporting this h
>    ** Python * (glob)
>    ** Mercurial Distributed SCM (*) (glob)
>    ** Extensions loaded: throw
>
> +Patch version is ignored during compatibility check
> +  $ echo "testedwith = '3.2'" >> throw.py
> +  $ echo "util.version = lambda:'3.2.2'" >> throw.py
> +  $ rm -f throw.pyc throw.pyo
> +  $ hg --config extensions.throw=throw.py throw 2>&1 | egrep '^\*\*'
> +  ** unknown exception encountered, please report by visiting
> +  ** http://mercurial.selenic.com/wiki/BugTracker
> +  ** Python * (glob)
> +  ** Mercurial Distributed SCM (*) (glob)
> +  ** Extensions loaded: throw
> +
>  Test version number support in 'hg version':
>    $ echo '__version__ = (1, 2, 3)' >> throw.py
>    $ rm -f throw.pyc throw.pyo
>    $ hg version -v
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -282,14 +282,21 @@  def _runcatch(req):
             if not testedwith.strip():
                 # We found an untested extension. It's likely the culprit.
                 worst = name, 'unknown', report
                 break
-            if compare not in testedwith.split() and testedwith != 'internal':
-                tested = [tuplever(v) for v in testedwith.split()]
-                lower = [t for t in tested if t < ct]
-                nearest = max(lower or tested)
-                if worst[0] is None or nearest < worst[1]:
-                    worst = name, nearest, report
+
+            # Never blame on extensions bundled with Mercurial.
+            if testedwith == 'internal':
+                continue
+
+            tested = [tuplever(t) for t in testedwith.split()]
+            if ct in tested:
+                continue
+
+            lower = [t for t in tested if t < ct]
+            nearest = max(lower or tested)
+            if worst[0] is None or nearest < worst[1]:
+                worst = name, nearest, report
         if worst[0] is not None:
             name, testedwith, report = worst
             if not isinstance(testedwith, str):
                 testedwith = '.'.join([str(c) for c in testedwith])
@@ -314,9 +321,12 @@  def _runcatch(req):
     return -1
 
 def tuplever(v):
     try:
-        return tuple([int(i) for i in v.split('.')])
+        # Assertion: tuplever is only used for extension compatibility
+        # checking. Otherwise, the discarding of extra version fields is
+        # incorrect.
+        return tuple([int(i) for i in v.split('.')[0:2]])
     except ValueError:
         return tuple()
 
 def aliasargs(fn, givenargs):
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -857,9 +857,9 @@  Broken disabled extension and command:
   (try "hg help --keyword foo")
   [255]
 
   $ cat > throw.py <<EOF
-  > from mercurial import cmdutil, commands
+  > from mercurial import cmdutil, commands, util
   > cmdtable = {}
   > command = cmdutil.command(cmdtable)
   > class Bogon(Exception): pass
   > @command('throw', [], 'hg throw', norepo=True)
@@ -909,9 +909,9 @@  If the extensions declare outdated versi
   $ rm -f throw.pyc throw.pyo
   $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
   >   throw 2>&1 | egrep '^\*\*'
   ** Unknown exception encountered with possibly-broken third-party extension older
-  ** which supports versions 1.9.3 of Mercurial.
+  ** which supports versions 1.9 of Mercurial.
   ** Please disable older and try your action again.
   ** If that fixes the bug please report it to the extension author.
   ** Python * (glob)
   ** Mercurial Distributed SCM (version 2.2)
@@ -922,9 +922,9 @@  One extension only tested with older, on
   $ rm -f older.pyc older.pyo
   $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
   >   throw 2>&1 | egrep '^\*\*'
   ** Unknown exception encountered with possibly-broken third-party extension older
-  ** which supports versions 1.9.3 of Mercurial.
+  ** which supports versions 1.9 of Mercurial.
   ** Please disable older and try your action again.
   ** If that fixes the bug please report it to the extension author.
   ** Python * (glob)
   ** Mercurial Distributed SCM (version 2.1)
@@ -935,9 +935,9 @@  Older extension is tested with current v
   $ rm -f older.pyc older.pyo
   $ hg --config extensions.throw=throw.py --config extensions.older=older.py \
   >   throw 2>&1 | egrep '^\*\*'
   ** Unknown exception encountered with possibly-broken third-party extension throw
-  ** which supports versions 2.1.1 of Mercurial.
+  ** which supports versions 2.1 of Mercurial.
   ** Please disable throw and try your action again.
   ** If that fixes the bug please report it to http://example.com/bts
   ** Python * (glob)
   ** Mercurial Distributed SCM (version 1.9.3)
@@ -953,8 +953,19 @@  Declare the version as supporting this h
   ** Python * (glob)
   ** Mercurial Distributed SCM (*) (glob)
   ** Extensions loaded: throw
 
+Patch version is ignored during compatibility check
+  $ echo "testedwith = '3.2'" >> throw.py
+  $ echo "util.version = lambda:'3.2.2'" >> throw.py
+  $ rm -f throw.pyc throw.pyo
+  $ hg --config extensions.throw=throw.py throw 2>&1 | egrep '^\*\*'
+  ** unknown exception encountered, please report by visiting
+  ** http://mercurial.selenic.com/wiki/BugTracker
+  ** Python * (glob)
+  ** Mercurial Distributed SCM (*) (glob)
+  ** Extensions loaded: throw
+
 Test version number support in 'hg version':
   $ echo '__version__ = (1, 2, 3)' >> throw.py
   $ rm -f throw.pyc throw.pyo
   $ hg version -v