reverse sense of return value from python hooks.

old scheme (False/None/0/'' == fail) made coding style
unnatural, did not allow use of mercurial commands as hooks.

new scheme (False/None/0 == pass) is pythonic, does not require peculiar
"return True" at ends of hooks, allows hooks like this:

[hooks]
# update working dir after push into this repo
changegroup.update = python:mercurial.commands.update

hgext/bugzilla.py

@@ -278,7 +278,6 def hook(ui, repo, hooktype, node=None,
             for id in ids:
                 bz.update(id, bin_node, changes)
             bz.notify(ids)
-        return True
     except MySQLdb.MySQLError, err:
         raise util.Abort(_('database error: %s') % err[1])
 

hgext/notify.py

@@ -255,4 +255,3 def hook(ui, repo, hooktype, node=None,
         n.node(node)
     n.diff(node)
     n.send(node, count)
-    return True

mercurial/localrepo.py

@@ -78,8 +78,12 class localrepository(object):
         def callhook(hname, funcname):
             '''call python hook. hook is callable object, looked up as
             name in python module. if callable returns "true", hook
-            passes, else fails. if hook raises exception, treated as
-            hook failure. exception propagates if throw is "true".'''
+            fails, else passes. if hook raises exception, treated as
+            hook failure. exception propagates if throw is "true".
+
+            reason for "true" meaning "hook failed" is so that
+            unmodified commands (e.g. mercurial.commands.update) can
+            be run as hooks without wrappers to convert return values.'''
 
             self.ui.note(_("calling hook %s: %s\n") % (hname, funcname))
             d = funcname.rfind('.')
@@ -119,11 +123,11 class localrepository(object):
                     raise
                 if self.ui.traceback:
                     traceback.print_exc()
-                return False
-            if not r:
+                return True
+            if r:
                 if throw:
                     raise util.Abort(_('%s hook failed') % hname)
-                self.ui.warn(_('error: %s hook failed\n') % hname)
+                self.ui.warn(_('warning: %s hook failed\n') % hname)
             return r
 
         def runhook(name, cmd):
@@ -135,19 +139,18 class localrepository(object):
                 desc, r = util.explain_exit(r)
                 if throw:
                     raise util.Abort(_('%s hook %s') % (name, desc))
-                self.ui.warn(_('error: %s hook %s\n') % (name, desc))
-                return False
-            return True
+                self.ui.warn(_('warning: %s hook %s\n') % (name, desc))
+            return r
 
-        r = True
+        r = False
         hooks = [(hname, cmd) for hname, cmd in self.ui.configitems("hooks")
                  if hname.split(".", 1)[0] == name and cmd]
         hooks.sort()
         for hname, cmd in hooks:
             if cmd.startswith('python:'):
-                r = callhook(hname, cmd[7:].strip()) and r
+                r = callhook(hname, cmd[7:].strip()) or r
             else:
-                r = runhook(hname, cmd) and r
+                r = runhook(hname, cmd) or r
         return r
 
     def tags(self):

tests/test-hook

@@ -100,14 +100,13 def printargs(args):
     print 'hook args:'
     for k, v in a:
        print ' ', k, v
-    return True
 
 def passhook(**args):
     printargs(args)
-    return True
 
 def failhook(**args):
     printargs(args)
+    return True
 
 class LocalException(Exception):
     pass