[PATCH] receive-pack: refuse all commands if one fails in atomic mode

2014-12-16 Thread Stefan Beller
This patch will be incorporated into the right places in v3 of the series.

Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
I don't want to resend the patch series today to accumulate comments,
but this makes the last test pass, which is the whole point of the series.

I'll put it into the right places in a reroll.

 builtin/receive-pack.c | 13 -
 t/t5543-atomic-push.sh |  2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0803fd2..3477f7c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1122,7 +1122,18 @@ static void execute_commands(struct command *commands,
}
 
if (use_atomic) {
-   if (ref_transaction_commit(transaction, err)) {
+   /* update(...) may abort early (i.e. because the hook refused to
+* update that ref), which then doesn't even record a 
transaction
+* regarding that ref. Make sure all commands are without error
+* and then commit atomically. */
+   for (cmd = commands; cmd; cmd = cmd-next)
+   if (cmd-error_string)
+   break;
+   if (cmd) {
+   for (cmd = commands; cmd; cmd = cmd-next)
+   if (!cmd-error_string)
+   cmd-error_string = atomic push 
failure;
+   } else if (ref_transaction_commit(transaction, err)) {
rp_error(%s, err.buf);
for (cmd = commands; cmd; cmd = cmd-next)
cmd-error_string = err.buf;
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
index 6354fc0..f0e54d9 100755
--- a/t/t5543-atomic-push.sh
+++ b/t/t5543-atomic-push.sh
@@ -142,7 +142,7 @@ test_expect_success 'atomic push fails if one tag fails 
remotely' '
test_ref_upstream second $(git -C workbench rev-parse --verify HEAD@{1})
 '
 
-test_expect_failure 'atomic push obeys update hook preventing a branch to be 
pushed' '
+test_expect_success 'atomic push obeys update hook preventing a branch to be 
pushed' '
mk_repo_pair 
(
cd workbench 
-- 
2.2.0.31.gad78000.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] receive-pack: refuse all commands if one fails in atomic mode

2014-12-16 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 This patch will be incorporated into the right places in v3 of the series.

 Signed-off-by: Stefan Beller sbel...@google.com
 ---

 Notes:
 I don't want to resend the patch series today to accumulate comments,
 but this makes the last test pass, which is the whole point of the series.
 
 I'll put it into the right places in a reroll.

  builtin/receive-pack.c | 13 -
  t/t5543-atomic-push.sh |  2 +-
  2 files changed, 13 insertions(+), 2 deletions(-)

 diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
 index 0803fd2..3477f7c 100644
 --- a/builtin/receive-pack.c
 +++ b/builtin/receive-pack.c
 @@ -1122,7 +1122,18 @@ static void execute_commands(struct command *commands,
   }
  
   if (use_atomic) {
 - if (ref_transaction_commit(transaction, err)) {
 + /* update(...) may abort early (i.e. because the hook refused to
 +  * update that ref), which then doesn't even record a 
 transaction
 +  * regarding that ref. Make sure all commands are without error
 +  * and then commit atomically. */

/*
 * The first line of our multi-line comment
 * has only opening slash-asterisk and nothing else.
 * The last line has asterisk-slash and nothing else.
 */

 + for (cmd = commands; cmd; cmd = cmd-next)
 + if (cmd-error_string)
 + break;
 + if (cmd) {
 + for (cmd = commands; cmd; cmd = cmd-next)
 + if (!cmd-error_string)
 + cmd-error_string = atomic push 
 failure;
 + } else if (ref_transaction_commit(transaction, err)) {
   rp_error(%s, err.buf);
   for (cmd = commands; cmd; cmd = cmd-next)
   cmd-error_string = err.buf;

Makes sense.

 diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
 index 6354fc0..f0e54d9 100755
 --- a/t/t5543-atomic-push.sh
 +++ b/t/t5543-atomic-push.sh
 @@ -142,7 +142,7 @@ test_expect_success 'atomic push fails if one tag fails 
 remotely' '
   test_ref_upstream second $(git -C workbench rev-parse --verify HEAD@{1})
  '
  
 -test_expect_failure 'atomic push obeys update hook preventing a branch to be 
 pushed' '
 +test_expect_success 'atomic push obeys update hook preventing a branch to be 
 pushed' '
   mk_repo_pair 
   (
   cd workbench 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html