branch.c: use ref transaction for all ref updates

Change create_branch to use a ref transaction when creating the new branch.

This also fixes a race condition in the old code where two concurrent
create_branch could race since the lock_any_ref_for_update/write_ref_sha1
did not protect against the ref already existing. I.e. one thread could end up
overwriting a branch even if the forcing flag is false.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/branch.c b/branch.c
index 660097bc29a682c4481308ba245e5e02f0661681..37ac555324e4358c300e57f8a398b12250bd5835 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -226,7 +226,6 @@ void create_branch(const char *head,
                   int force, int reflog, int clobber_head,
                   int quiet, enum branch_track track)
 {
                   int force, int reflog, int clobber_head,
                   int quiet, enum branch_track track)
 {

-       struct ref_lock *lock = NULL;

        struct commit *commit;
        unsigned char sha1[20];
        char *real_ref, msg[PATH_MAX + 20];
        struct commit *commit;
        unsigned char sha1[20];
        char *real_ref, msg[PATH_MAX + 20];


@@ -285,15 +284,6 @@ void create_branch(const char *head,
                die(_("Not a valid branch point: '%s'."), start_name);
        hashcpy(sha1, commit->object.sha1);
 
                die(_("Not a valid branch point: '%s'."), start_name);
        hashcpy(sha1, commit->object.sha1);
 

-       if (!dont_change_ref) {
-               lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
-               if (!lock)
-                       die_errno(_("Failed to lock ref for update"));
-       }
-
-       if (reflog)
-               log_all_ref_updates = 1;
-

        if (forcing)
                snprintf(msg, sizeof msg, "branch: Reset to %s",
                         start_name);
        if (forcing)
                snprintf(msg, sizeof msg, "branch: Reset to %s",
                         start_name);


@@ -301,13 +291,26 @@ void create_branch(const char *head,
                snprintf(msg, sizeof msg, "branch: Created from %s",
                         start_name);
 
                snprintf(msg, sizeof msg, "branch: Created from %s",
                         start_name);
 

+       if (reflog)
+               log_all_ref_updates = 1;
+
+       if (!dont_change_ref) {
+               struct ref_transaction *transaction;
+               struct strbuf err = STRBUF_INIT;
+
+               transaction = ref_transaction_begin(&err);
+               if (!transaction ||
+                   ref_transaction_update(transaction, ref.buf, sha1,
+                                          null_sha1, 0, !forcing, &err) ||
+                   ref_transaction_commit(transaction, msg, &err))
+                       die("%s", err.buf);
+               ref_transaction_free(transaction);
+               strbuf_release(&err);
+       }
+

        if (real_ref && track)
                setup_tracking(ref.buf + 11, real_ref, track, quiet);
 
        if (real_ref && track)
                setup_tracking(ref.buf + 11, real_ref, track, quiet);
 

-       if (!dont_change_ref)
-               if (write_ref_sha1(lock, sha1, msg) < 0)
-                       die_errno(_("Failed to write ref"));
-

        strbuf_release(&ref);
        free(real_ref);
 }
        strbuf_release(&ref);
        free(real_ref);
 }