Stefan Beller <sbel...@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.

>  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;
>  }
--
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

Reply via email to