[PATCH] Prevent type exception on concatination if diffstat returns None

Sean Dague sean at dague.net
Thu Feb 1 19:42:31 CST 2007


On Tue, Jan 30, 2007 at 10:38:43AM -0400, Sean Dague wrote:
> Diffstat output:
> 
>  notify.py |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> # HG changeset patch
> # User Sean Dague <sean at dague.net>
> # Date 1170171325 18000
> # Node ID 7a16078f83386217e23405898ebad805dd6e7d70
> # Parent  cc08d2543593a0d17e5bddd7c40b1544acbabe55
> Prevent type exception on concatination if diffstat returns None.
> This will most often occur if diffstat is not installed in the
> target platform, though may also happen in other cases where
> diffstat fails to execute.
> 
> Signed-off-by: Sean Dague <sean at dague.net>
> 
> diff -r cc08d2543593 -r 7a16078f8338 hgext/notify.py
> --- a/hgext/notify.py	Tue Jan 23 17:41:53 2007 -0600
> +++ b/hgext/notify.py	Tue Jan 30 10:35:25 2007 -0500
> @@ -240,7 +240,9 @@ class notifier(object):
>          difflines = self.ui.popbuffer().splitlines(1)
>          if self.ui.configbool('notify', 'diffstat', True):
>              s = patch.diffstat(difflines)
> -            self.ui.write('\ndiffstat:\n\n' + s)
> +            # s may be nil, don't include the header if it is
> +            if s:
> +                self.ui.write('\ndiffstat:\n\n%s' % s)
>          if maxdiff > 0 and len(difflines) > maxdiff:
>              self.ui.write(_('\ndiffs (truncated from %d to %d lines):\n\n') %
>                            (len(difflines), maxdiff))

I probably should have prefaced this with a bit more information.

Right now if you do a push to an hg repo with notify enabled, and you don't
have diffstat installed on that machine, notify will die with an exception
due to the following 2 lines:

              s = patch.diffstat(difflines)
              self.ui.write('\ndiffstat:\n\n' + s)

patch.diffstat uses a popen call of the diffstat executable, and masks any
exceptions raised by the popen call itself.  This s is None.  Concattenating
None to a String raises and exception, and we die.

The patch below is the simplest fix for this, which follows the flow logic
that patchbomb uses (just check that s is defined before doing the append). 
I also changed from the straight + to sprintfing to prevent any other type
of type explosion that might creep in in the future.

There was also an opinion on IRC that the real fix should be to let the
exception from diffstat filter up, and catch it in patchbomb and notify. 
I'd be happy to redo the patch like that if it was desired.  It seems a lot
more intrusive, and makes the code a bit less readable to me, but opinions
may vary.

	-Sean

-- 
__________________________________________________________________

Sean Dague                                       Mid-Hudson Valley
sean at dague dot net                            Linux Users Group
http://dague.net                                 http://mhvlug.org

There is no silver bullet.  Plus, werewolves make better neighbors
than zombies, and they tend to keep the vampire population down.
__________________________________________________________________
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://www.selenic.com/pipermail/mercurial-devel/attachments/20070201/404ec060/attachment.pgp


More information about the Mercurial-devel mailing list