Patchwork [STABLE] dispatch: handle empty `testedwith` value in extension

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 4, 2013, 12:43 p.m.
Message ID <b52aaf26693658506497.1357303401@crater2.logilab.fr>
Download mbox | patch
Permalink /patch/390/
State Accepted
Commit 0f9013112ebae38ae07ad58501b64a38757aeea9
Headers show

Comments

Pierre-Yves David - Jan. 4, 2013, 12:43 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
# Date 1357295150 -3600
# Node ID b52aaf2669365850649775170131b5d221e3d4c2
# Parent  c565761dde6aa1a70b688cd2eebad69e9b5a16db
dispatch: handle empty `testedwith` value in extension

When extensions had an empty `testedwith` attribute the code tried to parse it
and failed. As a result the actual error were shallowed by a This crash.

We now treat empty strip as 'unknown'
Kevin Bullock - Jan. 4, 2013, 5:20 p.m.
On Jan 4, 2013, at 6:43 AM, pierre-yves.david at logilab.fr wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1357295150 -3600
> # Node ID b52aaf2669365850649775170131b5d221e3d4c2
> # Parent  c565761dde6aa1a70b688cd2eebad69e9b5a16db
> dispatch: handle empty `testedwith` value in extension
> 
> When extensions had an empty `testedwith` attribute the code tried to parse it
> and failed. As a result the actual error were shallowed by a This crash.

Surely it would be pretty easy to add a test case for this?

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Kevin Bullock - Jan. 4, 2013, 7:07 p.m.
On Jan 4, 2013, at 11:20 AM, Kevin Bullock wrote:

> On Jan 4, 2013, at 6:43 AM, pierre-yves.david at logilab.fr wrote:
> 
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>> # Date 1357295150 -3600
>> # Node ID b52aaf2669365850649775170131b5d221e3d4c2
>> # Parent  c565761dde6aa1a70b688cd2eebad69e9b5a16db
>> dispatch: handle empty `testedwith` value in extension
>> 
>> When extensions had an empty `testedwith` attribute the code tried to parse it
>> and failed. As a result the actual error were shallowed by a This crash.
> 
> Surely it would be pretty easy to add a test case for this?

After discussion on IRC, pulled this patch with test case added and pushed to crew as 0f9013112eba.

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -215,15 +215,15 @@  def _runcatch(req):
         # version number and try updating.
         compare = myver.split('+')[0]
         ct = tuplever(compare)
         worst = None, ct, ''
         for name, mod in extensions.extensions():
-            testedwith = getattr(mod, 'testedwith', 'unknown')
+            testedwith = getattr(mod, 'testedwith', '')
             report = getattr(mod, 'buglink', _('the extension author.'))
-            if testedwith == 'unknown':
+            if not testedwith.strip():
                 # We found an untested extension. It's likely the culprit.
-                worst = name, testedwith, report
+                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)