Re: [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 16, 2014, 19:35 UTC
- Message-ID
- <xmqqwq5r9yli.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1418755747-22506-4-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> * update(...) assumes to be always in a transaction > * Caring about when to begin/commit transactions is put > into execute_commands
I am obviously biased, but I find that the new code structure and separation of responsibility between update() and execute() functions a lot clearer than the previous one.
Thanks.
Show 127 quoted lines
> builtin/receive-pack.c | 64 ++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index e76e5d5..0803fd2 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -67,6 +67,8 @@ static const char *NONCE_SLOP = "SLOP";
> static const char *nonce_status;
> static long nonce_stamp_slop;
> static unsigned long nonce_stamp_slop_limit;
> +struct strbuf err = STRBUF_INIT;
> +struct ref_transaction *transaction;
>
> static enum deny_action parse_deny_action(const char *var, const char *value)
> {
> @@ -832,34 +834,32 @@ static const char *update(struct command *cmd, struct shallow_info *si)
> cmd->did_not_exist = 1;
> }
> }
> - if (delete_ref(namespaced_name, old_sha1, 0)) {
> - rp_error("failed to delete %s", name);
> + if (ref_transaction_delete(transaction,
> + namespaced_name,
> + old_sha1,
> + 0, old_sha1 != NULL,
> + "push", &err)) {
> + rp_error("%s", err.buf);
> + strbuf_release(&err);
> return "failed to delete";
> }
> return NULL; /* good */
> }
> else {
> - struct strbuf err = STRBUF_INIT;
> - struct ref_transaction *transaction;
> -
> if (shallow_update && si->shallow_ref[cmd->index] &&
> update_shallow_ref(cmd, si))
> return "shallow error";
>
> - transaction = ref_transaction_begin(&err);
> - if (!transaction ||
> - ref_transaction_update(transaction, namespaced_name,
> - new_sha1, old_sha1, 0, 1, "push",
> - &err) ||
> - ref_transaction_commit(transaction, &err)) {
> - ref_transaction_free(transaction);
> -
> + if (ref_transaction_update(transaction,
> + namespaced_name,
> + new_sha1, old_sha1,
> + 0, 1, "push",
> + &err)) {
> rp_error("%s", err.buf);
> strbuf_release(&err);
> return "failed to update ref";
> }
>
> - ref_transaction_free(transaction);
> strbuf_release(&err);
> return NULL; /* good */
> }
> @@ -1059,6 +1059,16 @@ static void execute_commands(struct command *commands,
> return;
> }
>
> + if (use_atomic) {
> + transaction = ref_transaction_begin(&err);
> + if (!transaction) {
> + error("%s", err.buf);
> + strbuf_release(&err);
> + for (cmd = commands; cmd; cmd = cmd->next)
> + cmd->error_string = "transaction error";
> + return;
> + }
> + }
> data.cmds = commands;
> data.si = si;
> if (check_everything_connected(iterate_receive_command_list, 0, &data))
> @@ -1086,8 +1096,23 @@ static void execute_commands(struct command *commands,
>
> if (cmd->skip_update)
> continue;
> -
> + if (!use_atomic) {
> + transaction = ref_transaction_begin(&err);
> + if (!transaction) {
> + rp_error("%s", err.buf);
> + strbuf_release(&err);
> + cmd->error_string = "failed to start transaction";
> + }
> + }
> cmd->error_string = update(cmd, si);
> + if (!use_atomic)
> + if (ref_transaction_commit(transaction, &err)) {
> + ref_transaction_free(transaction);
> + rp_error("%s", err.buf);
> + strbuf_release(&err);
> + cmd->error_string = "failed to update ref";
> + }
> +
> if (shallow_update && !cmd->error_string &&
> si->shallow_ref[cmd->index]) {
> error("BUG: connectivity check has not been run on ref %s",
> @@ -1096,6 +1121,14 @@ static void execute_commands(struct command *commands,
> }
> }
>
> + if (use_atomic) {
> + if (ref_transaction_commit(transaction, &err)) {
> + rp_error("%s", err.buf);
> + for (cmd = commands; cmd; cmd = cmd->next)
> + cmd->error_string = err.buf;
> + }
> + ref_transaction_free(transaction);
> + }
> if (shallow_update && !checked_connectivity)
> error("BUG: run 'git fsck' for safety.\n"
> "If there are errors, try to remove "
> @@ -1543,5 +1576,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
> sha1_array_clear(&shallow);
> sha1_array_clear(&ref);
> free((void *)push_cert_nonce);
> + strbuf_release(&err);
> return 0;
> }