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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2014, 20:32 UTC
Message-ID: <xmqqfvcf9vzn.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1418757280-32412-1-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 28 quoted lines

> This patch will be incorporated into the right places in v3 of the series.
>
> Signed-off-by: Stefan Beller <sbeller@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.
         */

Show 11 quoted lines

> +		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.

Show 13 quoted lines

> 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 &&