Submitter | Siddharth Agarwal |
---|---|
Date | April 16, 2013, 9:44 p.m. |
Message ID | <58f26562aa8cc468c651.1366148658@sid0x220> |
Download | mbox | patch |
Permalink | /patch/1369/ |
State | Accepted, archived |
Headers | show |
Comments
On 04/16/2013 02:44 PM, Siddharth Agarwal wrote: > # HG changeset patch > # User Siddharth Agarwal <sid0@fb.com> > # Date 1366148377 25200 > # Tue Apr 16 14:39:37 2013 -0700 > # Node ID 58f26562aa8cc468c651d65b55c16e2c0f9946df > # Parent 1e9704ba956a08cb298be3afb9a90fcf9d54c142 > dispatch: print 'abort:' when a pre-command hook fails > > This also changes the exit code from whatever the hook returned to 255. This > brings it in line with all the other hooks that abort. RFC because I'm wondering whether this count as breaking BC.
On 04/16/2013 11:44 PM, Siddharth Agarwal wrote: > # HG changeset patch > # User Siddharth Agarwal <sid0@fb.com> > # Date 1366148377 25200 > # Tue Apr 16 14:39:37 2013 -0700 > # Node ID 58f26562aa8cc468c651d65b55c16e2c0f9946df > # Parent 1e9704ba956a08cb298be3afb9a90fcf9d54c142 > dispatch: print 'abort:' when a pre-command hook fails > > This also changes the exit code from whatever the hook returned to 255. This > brings it in line with all the other hooks that abort. For the record: The core of this change is to set throw=True when calling hook() - not completely trivial to spot when looking at the diff. /Mads > > diff -r 1e9704ba956a -r 58f26562aa8c mercurial/dispatch.py > --- a/mercurial/dispatch.py Thu Mar 28 00:30:40 2013 -0700 > +++ b/mercurial/dispatch.py Tue Apr 16 14:39:37 2013 -0700 > @@ -508,10 +508,8 @@ def _earlygetopt(aliases, args): > > def runcommand(lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions): > # run pre-hook, and abort if it fails > - ret = hook.hook(lui, repo, "pre-%s" % cmd, False, args=" ".join(fullargs), > - pats=cmdpats, opts=cmdoptions) > - if ret: > - return ret > + hook.hook(lui, repo, "pre-%s" % cmd, True, args=" ".join(fullargs), > + pats=cmdpats, opts=cmdoptions) > ret = _runcommand(ui, options, cmd, d) > # run post-hook, passing command result > hook.hook(lui, repo, "post-%s" % cmd, False, args=" ".join(fullargs), > diff -r 1e9704ba956a -r 58f26562aa8c tests/test-hook.t > --- a/tests/test-hook.t Thu Mar 28 00:30:40 2013 -0700 > +++ b/tests/test-hook.t Tue Apr 16 14:39:37 2013 -0700 > @@ -71,8 +71,8 @@ test generic hooks > > $ hg id > pre-identify hook: HG_ARGS=id HG_OPTS={'bookmarks': None, 'branch': None, 'id': None, 'insecure': None, 'num': None, 'remotecmd': '', 'rev': '', 'ssh': '', 'tags': None} HG_PATS=[] > - warning: pre-identify hook exited with status 1 > - [1] > + abort: pre-identify hook exited with status 1 > + [255] > $ hg cat b > pre-cat hook: HG_ARGS=cat b HG_OPTS={'decode': None, 'exclude': [], 'include': [], 'output': '', 'rev': ''} HG_PATS=['b'] > b > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Tue, 2013-04-16 at 14:44 -0700, Siddharth Agarwal wrote: > # HG changeset patch > # User Siddharth Agarwal <sid0@fb.com> > # Date 1366148377 25200 > # Tue Apr 16 14:39:37 2013 -0700 > # Node ID 58f26562aa8cc468c651d65b55c16e2c0f9946df > # Parent 1e9704ba956a08cb298be3afb9a90fcf9d54c142 > dispatch: print 'abort:' when a pre-command hook fails > This also changes the exit code from whatever the hook returned to 255. This > brings it in line with all the other hooks that abort. Queued for default with added (BC) tag. I think this is clearly more correct.
Patch
diff -r 1e9704ba956a -r 58f26562aa8c mercurial/dispatch.py --- a/mercurial/dispatch.py Thu Mar 28 00:30:40 2013 -0700 +++ b/mercurial/dispatch.py Tue Apr 16 14:39:37 2013 -0700 @@ -508,10 +508,8 @@ def _earlygetopt(aliases, args): def runcommand(lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions): # run pre-hook, and abort if it fails - ret = hook.hook(lui, repo, "pre-%s" % cmd, False, args=" ".join(fullargs), - pats=cmdpats, opts=cmdoptions) - if ret: - return ret + hook.hook(lui, repo, "pre-%s" % cmd, True, args=" ".join(fullargs), + pats=cmdpats, opts=cmdoptions) ret = _runcommand(ui, options, cmd, d) # run post-hook, passing command result hook.hook(lui, repo, "post-%s" % cmd, False, args=" ".join(fullargs), diff -r 1e9704ba956a -r 58f26562aa8c tests/test-hook.t --- a/tests/test-hook.t Thu Mar 28 00:30:40 2013 -0700 +++ b/tests/test-hook.t Tue Apr 16 14:39:37 2013 -0700 @@ -71,8 +71,8 @@ test generic hooks $ hg id pre-identify hook: HG_ARGS=id HG_OPTS={'bookmarks': None, 'branch': None, 'id': None, 'insecure': None, 'num': None, 'remotecmd': '', 'rev': '', 'ssh': '', 'tags': None} HG_PATS=[] - warning: pre-identify hook exited with status 1 - [1] + abort: pre-identify hook exited with status 1 + [255] $ hg cat b pre-cat hook: HG_ARGS=cat b HG_OPTS={'decode': None, 'exclude': [], 'include': [], 'output': '', 'rev': ''} HG_PATS=['b'] b