Patchwork [2,of,2] bundle2: abort when a mandatory pushkey part fails

login
register
mail settings
Submitter Pierre-Yves David
Date June 6, 2015, 8:57 p.m.
Message ID <40c4968954f175698a21.1433624241@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/9547/
State Accepted
Headers show

Comments

Pierre-Yves David - June 6, 2015, 8:57 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1432729720 25200
#      Wed May 27 05:28:40 2015 -0700
# Node ID 40c4968954f175698a21ddb11aaa3db4c72c3887
# Parent  1965f29f800fc0761ae9053165d9c2c75943d21f
bundle2: abort when a mandatory pushkey part fails

So far result of a pushkey operation had no consequence on the transaction
(beside the change). We makes it respect the 'mandatory' flag of part so that
failed pushkey call abort the whole transaction. This will allow rejecting
changes (primary target: changesets) regarding phases or bookmark criteria in
the future (when we will push such data in a mandatory part).

We currently raise an abort error because all clients supports it. We'll
introduce a more precise error in the next changesets.
Augie Fackler - June 8, 2015, 2:35 p.m.
On Sat, Jun 06, 2015 at 01:57:21PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1432729720 25200
> #      Wed May 27 05:28:40 2015 -0700
> # Node ID 40c4968954f175698a21ddb11aaa3db4c72c3887
> # Parent  1965f29f800fc0761ae9053165d9c2c75943d21f
> bundle2: abort when a mandatory pushkey part fails

queued, thanks!

>
> So far result of a pushkey operation had no consequence on the transaction
> (beside the change). We makes it respect the 'mandatory' flag of part so that
> failed pushkey call abort the whole transaction. This will allow rejecting
> changes (primary target: changesets) regarding phases or bookmark criteria in
> the future (when we will push such data in a mandatory part).
>
> We currently raise an abort error because all clients supports it. We'll
> introduce a more precise error in the next changesets.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -1327,10 +1327,13 @@ def handlepushkey(op, inpart):
>      op.records.add('pushkey', record)
>      if op.reply is not None:
>          rpart = op.reply.newpart('reply:pushkey')
>          rpart.addparam('in-reply-to', str(inpart.id), mandatory=False)
>          rpart.addparam('return', '%i' % ret, mandatory=False)
> +    if inpart.mandatory and not ret:
> +        raise util.Abort(_('failed to update value for "%s/%s"')
> +                         % (namespace, key))
>
>  @parthandler('reply:pushkey', ('return', 'in-reply-to'))
>  def handlepushkeyreply(op, inpart):
>      """retrieve the result of a pushkey request"""
>      ret = int(inpart.params['return'])
> diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
> --- a/tests/test-bundle2-exchange.t
> +++ b/tests/test-bundle2-exchange.t
> @@ -715,5 +715,147 @@ Check output capture control.
>    remote: transaction abort!
>    remote: Cleaning up the mess...
>    remote: rollback completed
>    abort: pretxnchangegroup hook exited with status 1
>    [255]
> +
> +Check abort from mandatory pushkey
> +
> +  $ cat > mandatorypart.py << EOF
> +  > from mercurial import exchange
> +  > from mercurial import pushkey
> +  > from mercurial import node
> +  > @exchange.b2partsgenerator('failingpuskey')
> +  > def addfailingpushey(pushop, bundler):
> +  >     enc = pushkey.encode
> +  >     part = bundler.newpart('pushkey')
> +  >     part.addparam('namespace', enc('phases'))
> +  >     part.addparam('key', enc(pushop.repo['cd010b8cd998'].hex()))
> +  >     part.addparam('old', enc(str(0))) # successful update
> +  >     part.addparam('new', enc(str(0)))
> +  > EOF
> +  $ cat >> $HGRCPATH << EOF
> +  > [hooks]
> +  > pretxnchangegroup=
> +  > pretxnclose.failpush=
> +  > prepushkey.failpush = sh -c "echo 'do not push the key !'; false"
> +  > [extensions]
> +  > mandatorypart=$TESTTMP/mandatorypart.py
> +  > EOF
> +  $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS # reload http config
> +  $ hg -R other serve -p $HGPORT2 -d --pid-file=other.pid -E other-error.log
> +  $ cat other.pid >> $DAEMON_PIDS
> +
> +(Failure from a hook)
> +
> +  $ hg -R main push other -r e7ec4e813ba6
> +  pushing to other
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +  do not push the key !
> +  pushkey-abort: prepushkey.failpush hook exited with status 1
> +  transaction abort!
> +  Cleaning up the mess...
> +  rollback completed
> +  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
> +  [255]
> +  $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
> +  pushing to ssh://user@dummy/other
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files
> +  remote: do not push the key !
> +  remote: pushkey-abort: prepushkey.failpush hook exited with status 1
> +  remote: transaction abort!
> +  remote: Cleaning up the mess...
> +  remote: rollback completed
> +  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
> +  [255]
> +  $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
> +  pushing to http://localhost:$HGPORT2/
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files
> +  remote: do not push the key !
> +  remote: pushkey-abort: prepushkey.failpush hook exited with status 1
> +  remote: transaction abort!
> +  remote: Cleaning up the mess...
> +  remote: rollback completed
> +  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
> +  [255]
> +
> +(Failure from a the pushkey)
> +
> +  $ cat > mandatorypart.py << EOF
> +  > from mercurial import exchange
> +  > from mercurial import pushkey
> +  > from mercurial import node
> +  > @exchange.b2partsgenerator('failingpuskey')
> +  > def addfailingpushey(pushop, bundler):
> +  >     enc = pushkey.encode
> +  >     part = bundler.newpart('pushkey')
> +  >     part.addparam('namespace', enc('phases'))
> +  >     part.addparam('key', enc(pushop.repo['cd010b8cd998'].hex()))
> +  >     part.addparam('old', enc(str(4))) # will fail
> +  >     part.addparam('new', enc(str(3)))
> +  > EOF
> +  $ cat >> $HGRCPATH << EOF
> +  > [hooks]
> +  > prepushkey.failpush =
> +  > EOF
> +  $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS # reload http config
> +  $ hg -R other serve -p $HGPORT2 -d --pid-file=other.pid -E other-error.log
> +  $ cat other.pid >> $DAEMON_PIDS
> +
> +  $ hg -R main push other -r e7ec4e813ba6
> +  pushing to other
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +  transaction abort!
> +  Cleaning up the mess...
> +  rollback completed
> +  pushkey: lock state after "phases"
> +  lock:  free
> +  wlock: free
> +  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
> +  [255]
> +  $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
> +  pushing to ssh://user@dummy/other
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files
> +  remote: transaction abort!
> +  remote: Cleaning up the mess...
> +  remote: rollback completed
> +  remote: pushkey: lock state after "phases"
> +  remote: lock:  free
> +  remote: wlock: free
> +  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
> +  [255]
> +  $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
> +  pushing to http://localhost:$HGPORT2/
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files
> +  remote: transaction abort!
> +  remote: Cleaning up the mess...
> +  remote: rollback completed
> +  remote: pushkey: lock state after "phases"
> +  remote: lock:  free
> +  remote: wlock: free
> +  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
> +  [255]
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1327,10 +1327,13 @@  def handlepushkey(op, inpart):
     op.records.add('pushkey', record)
     if op.reply is not None:
         rpart = op.reply.newpart('reply:pushkey')
         rpart.addparam('in-reply-to', str(inpart.id), mandatory=False)
         rpart.addparam('return', '%i' % ret, mandatory=False)
+    if inpart.mandatory and not ret:
+        raise util.Abort(_('failed to update value for "%s/%s"')
+                         % (namespace, key))
 
 @parthandler('reply:pushkey', ('return', 'in-reply-to'))
 def handlepushkeyreply(op, inpart):
     """retrieve the result of a pushkey request"""
     ret = int(inpart.params['return'])
diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
--- a/tests/test-bundle2-exchange.t
+++ b/tests/test-bundle2-exchange.t
@@ -715,5 +715,147 @@  Check output capture control.
   remote: transaction abort!
   remote: Cleaning up the mess...
   remote: rollback completed
   abort: pretxnchangegroup hook exited with status 1
   [255]
+
+Check abort from mandatory pushkey
+
+  $ cat > mandatorypart.py << EOF
+  > from mercurial import exchange
+  > from mercurial import pushkey
+  > from mercurial import node
+  > @exchange.b2partsgenerator('failingpuskey')
+  > def addfailingpushey(pushop, bundler):
+  >     enc = pushkey.encode
+  >     part = bundler.newpart('pushkey')
+  >     part.addparam('namespace', enc('phases'))
+  >     part.addparam('key', enc(pushop.repo['cd010b8cd998'].hex()))
+  >     part.addparam('old', enc(str(0))) # successful update
+  >     part.addparam('new', enc(str(0)))
+  > EOF
+  $ cat >> $HGRCPATH << EOF
+  > [hooks]
+  > pretxnchangegroup=
+  > pretxnclose.failpush=
+  > prepushkey.failpush = sh -c "echo 'do not push the key !'; false"
+  > [extensions]
+  > mandatorypart=$TESTTMP/mandatorypart.py
+  > EOF
+  $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS # reload http config
+  $ hg -R other serve -p $HGPORT2 -d --pid-file=other.pid -E other-error.log
+  $ cat other.pid >> $DAEMON_PIDS
+
+(Failure from a hook)
+
+  $ hg -R main push other -r e7ec4e813ba6
+  pushing to other
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  do not push the key !
+  pushkey-abort: prepushkey.failpush hook exited with status 1
+  transaction abort!
+  Cleaning up the mess...
+  rollback completed
+  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
+  [255]
+  $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
+  pushing to ssh://user@dummy/other
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files
+  remote: do not push the key !
+  remote: pushkey-abort: prepushkey.failpush hook exited with status 1
+  remote: transaction abort!
+  remote: Cleaning up the mess...
+  remote: rollback completed
+  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
+  [255]
+  $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
+  pushing to http://localhost:$HGPORT2/
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files
+  remote: do not push the key !
+  remote: pushkey-abort: prepushkey.failpush hook exited with status 1
+  remote: transaction abort!
+  remote: Cleaning up the mess...
+  remote: rollback completed
+  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
+  [255]
+
+(Failure from a the pushkey)
+
+  $ cat > mandatorypart.py << EOF
+  > from mercurial import exchange
+  > from mercurial import pushkey
+  > from mercurial import node
+  > @exchange.b2partsgenerator('failingpuskey')
+  > def addfailingpushey(pushop, bundler):
+  >     enc = pushkey.encode
+  >     part = bundler.newpart('pushkey')
+  >     part.addparam('namespace', enc('phases'))
+  >     part.addparam('key', enc(pushop.repo['cd010b8cd998'].hex()))
+  >     part.addparam('old', enc(str(4))) # will fail
+  >     part.addparam('new', enc(str(3)))
+  > EOF
+  $ cat >> $HGRCPATH << EOF
+  > [hooks]
+  > prepushkey.failpush =
+  > EOF
+  $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS # reload http config
+  $ hg -R other serve -p $HGPORT2 -d --pid-file=other.pid -E other-error.log
+  $ cat other.pid >> $DAEMON_PIDS
+
+  $ hg -R main push other -r e7ec4e813ba6
+  pushing to other
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  transaction abort!
+  Cleaning up the mess...
+  rollback completed
+  pushkey: lock state after "phases"
+  lock:  free
+  wlock: free
+  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
+  [255]
+  $ hg -R main push ssh://user@dummy/other -r e7ec4e813ba6
+  pushing to ssh://user@dummy/other
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files
+  remote: transaction abort!
+  remote: Cleaning up the mess...
+  remote: rollback completed
+  remote: pushkey: lock state after "phases"
+  remote: lock:  free
+  remote: wlock: free
+  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
+  [255]
+  $ hg -R main push http://localhost:$HGPORT2/ -r e7ec4e813ba6
+  pushing to http://localhost:$HGPORT2/
+  searching for changes
+  remote: adding changesets
+  remote: adding manifests
+  remote: adding file changes
+  remote: added 1 changesets with 1 changes to 1 files
+  remote: transaction abort!
+  remote: Cleaning up the mess...
+  remote: rollback completed
+  remote: pushkey: lock state after "phases"
+  remote: lock:  free
+  remote: wlock: free
+  abort: failed to update value for "phases/cd010b8cd998f3981a5a8115f94f8da4ab506089"
+  [255]
+