Merge branch 'ma/create-pseudoref-with-null-old-oid'
authorJunio C Hamano <gitster@pobox.com>
Wed, 30 May 2018 05:04:08 +0000 (14:04 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 30 May 2018 05:04:08 +0000 (14:04 +0900)
"git update-ref A B" is supposed to ensure that ref A does not yet
exist when B is a NULL OID, but this check was not done correctly
for pseudo-refs outside refs/ hierarchy, e.g. MERGE_HEAD.

* ma/create-pseudoref-with-null-old-oid:
refs: handle zero oid for pseudorefs
t1400: add tests around adding/deleting pseudorefs
refs.c: refer to "object ID", not "sha1", in error messages

1  2 
refs.c
diff --combined refs.c
index dabcd850a9eb8c08dca8fe5c7d636a6271cbff20,26af07fc51791e511200c5011e8655822be18337..20fb35d8952611527e7ade5c2e394c4bde79291b
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -303,7 -303,7 +303,7 @@@ enum peel_status peel_object(const stru
        struct object *o = lookup_unknown_object(name->hash);
  
        if (o->type == OBJ_NONE) {
 -              int type = oid_object_info(name, NULL);
 +              int type = oid_object_info(the_repository, name, NULL);
                if (type < 0 || !object_as_type(o, type, 0))
                        return PEEL_INVALID;
        }
@@@ -615,8 -615,7 +615,8 @@@ int dwim_log(const char *str, int len, 
  static int is_per_worktree_ref(const char *refname)
  {
        return !strcmp(refname, "HEAD") ||
 -              starts_with(refname, "refs/bisect/");
 +              starts_with(refname, "refs/bisect/") ||
 +              starts_with(refname, "refs/rewritten/");
  }
  
  static int is_pseudoref_syntax(const char *refname)
@@@ -660,7 -659,7 +660,7 @@@ static int write_pseudoref(const char *
  {
        const char *filename;
        int fd;
 -      static struct lock_file lock;
 +      struct lock_file lock = LOCK_INIT;
        struct strbuf buf = STRBUF_INIT;
        int ret = -1;
  
        strbuf_addf(&buf, "%s\n", oid_to_hex(oid));
  
        filename = git_path("%s", pseudoref);
 -      fd = hold_lock_file_for_update_timeout(&lock, filename,
 -                                             LOCK_DIE_ON_ERROR,
 +      fd = hold_lock_file_for_update_timeout(&lock, filename, 0,
                                               get_files_ref_lock_timeout_ms());
        if (fd < 0) {
                strbuf_addf(err, "could not open '%s' for writing: %s",
        if (old_oid) {
                struct object_id actual_old_oid;
  
-               if (read_ref(pseudoref, &actual_old_oid))
-                       die("could not read ref '%s'", pseudoref);
-               if (oidcmp(&actual_old_oid, old_oid)) {
-                       strbuf_addf(err, "unexpected sha1 when writing '%s'", pseudoref);
+               if (read_ref(pseudoref, &actual_old_oid)) {
+                       if (!is_null_oid(old_oid)) {
+                               strbuf_addf(err, "could not read ref '%s'",
+                                           pseudoref);
+                               rollback_lock_file(&lock);
+                               goto done;
+                       }
+               } else if (is_null_oid(old_oid)) {
+                       strbuf_addf(err, "ref '%s' already exists",
+                                   pseudoref);
+                       rollback_lock_file(&lock);
+                       goto done;
+               } else if (oidcmp(&actual_old_oid, old_oid)) {
+                       strbuf_addf(err, "unexpected object ID when writing '%s'",
+                                   pseudoref);
                        rollback_lock_file(&lock);
                        goto done;
                }
@@@ -705,27 -716,25 +716,28 @@@ done
  
  static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)
  {
 -      static struct lock_file lock;
        const char *filename;
  
        filename = git_path("%s", pseudoref);
  
        if (old_oid && !is_null_oid(old_oid)) {
 +              struct lock_file lock = LOCK_INIT;
                int fd;
                struct object_id actual_old_oid;
  
                fd = hold_lock_file_for_update_timeout(
 -                              &lock, filename, LOCK_DIE_ON_ERROR,
 +                              &lock, filename, 0,
                                get_files_ref_lock_timeout_ms());
 -              if (fd < 0)
 -                      die_errno(_("Could not open '%s' for writing"), filename);
 +              if (fd < 0) {
 +                      error_errno(_("could not open '%s' for writing"),
 +                                  filename);
 +                      return -1;
 +              }
                if (read_ref(pseudoref, &actual_old_oid))
                        die("could not read ref '%s'", pseudoref);
                if (oidcmp(&actual_old_oid, old_oid)) {
-                       warning("Unexpected sha1 when deleting %s", pseudoref);
+                       error("unexpected object ID when deleting '%s'",
+                             pseudoref);
                        rollback_lock_file(&lock);
                        return -1;
                }
@@@ -962,10 -971,10 +974,10 @@@ void ref_transaction_free(struct ref_tr
                /* OK */
                break;
        case REF_TRANSACTION_PREPARED:
 -              die("BUG: free called on a prepared reference transaction");
 +              BUG("free called on a prepared reference transaction");
                break;
        default:
 -              die("BUG: unexpected reference transaction state");
 +              BUG("unexpected reference transaction state");
                break;
        }
  
@@@ -987,7 -996,7 +999,7 @@@ struct ref_update *ref_transaction_add_
        struct ref_update *update;
  
        if (transaction->state != REF_TRANSACTION_OPEN)
 -              die("BUG: update called for transaction that is not open");
 +              BUG("update called for transaction that is not open");
  
        FLEX_ALLOC_STR(update, refname, refname);
        ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
@@@ -1037,7 -1046,7 +1049,7 @@@ int ref_transaction_create(struct ref_t
                           struct strbuf *err)
  {
        if (!new_oid || is_null_oid(new_oid))
 -              die("BUG: create called without valid new_oid");
 +              BUG("create called without valid new_oid");
        return ref_transaction_update(transaction, refname, new_oid,
                                      &null_oid, flags, msg, err);
  }
@@@ -1049,7 -1058,7 +1061,7 @@@ int ref_transaction_delete(struct ref_t
                           struct strbuf *err)
  {
        if (old_oid && is_null_oid(old_oid))
 -              die("BUG: delete called with old_oid set to zeros");
 +              BUG("delete called with old_oid set to zeros");
        return ref_transaction_update(transaction, refname,
                                      &null_oid, old_oid,
                                      flags, msg, err);
@@@ -1062,7 -1071,7 +1074,7 @@@ int ref_transaction_verify(struct ref_t
                           struct strbuf *err)
  {
        if (!old_oid)
 -              die("BUG: verify called with old_oid set to NULL");
 +              BUG("verify called with old_oid set to NULL");
        return ref_transaction_update(transaction, refname,
                                      NULL, old_oid,
                                      flags, NULL, err);
@@@ -1660,7 -1669,7 +1672,7 @@@ static struct ref_store *ref_store_init
        struct ref_store *refs;
  
        if (!be)
 -              die("BUG: reference backend %s is unknown", be_name);
 +              BUG("reference backend %s is unknown", be_name);
  
        refs = be->init(gitdir, flags);
        return refs;
@@@ -1671,9 -1680,6 +1683,9 @@@ struct ref_store *get_main_ref_store(st
        if (r->refs)
                return r->refs;
  
 +      if (!r->gitdir)
 +              BUG("attempting to get main_ref_store outside of repository");
 +
        r->refs = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
        return r->refs;
  }
@@@ -1691,7 -1697,7 +1703,7 @@@ static void register_ref_store_map(stru
                hashmap_init(map, ref_store_hash_cmp, NULL, 0);
  
        if (hashmap_put(map, alloc_ref_store_hash_entry(name, refs)))
 -              die("BUG: %s ref_store '%s' initialized twice", type, name);
 +              BUG("%s ref_store '%s' initialized twice", type, name);
  }
  
  struct ref_store *get_submodule_ref_store(const char *submodule)
@@@ -1837,7 -1843,7 +1849,7 @@@ int ref_update_reject_duplicates(struc
                                    refnames->items[i].string);
                        return 1;
                } else if (cmp > 0) {
 -                      die("BUG: ref_update_reject_duplicates() received unsorted list");
 +                      BUG("ref_update_reject_duplicates() received unsorted list");
                }
        }
        return 0;
@@@ -1853,13 -1859,13 +1865,13 @@@ int ref_transaction_prepare(struct ref_
                /* Good. */
                break;
        case REF_TRANSACTION_PREPARED:
 -              die("BUG: prepare called twice on reference transaction");
 +              BUG("prepare called twice on reference transaction");
                break;
        case REF_TRANSACTION_CLOSED:
 -              die("BUG: prepare called on a closed reference transaction");
 +              BUG("prepare called on a closed reference transaction");
                break;
        default:
 -              die("BUG: unexpected reference transaction state");
 +              BUG("unexpected reference transaction state");
                break;
        }
  
@@@ -1886,10 -1892,10 +1898,10 @@@ int ref_transaction_abort(struct ref_tr
                ret = refs->be->transaction_abort(refs, transaction, err);
                break;
        case REF_TRANSACTION_CLOSED:
 -              die("BUG: abort called on a closed reference transaction");
 +              BUG("abort called on a closed reference transaction");
                break;
        default:
 -              die("BUG: unexpected reference transaction state");
 +              BUG("unexpected reference transaction state");
                break;
        }
  
@@@ -1914,10 -1920,10 +1926,10 @@@ int ref_transaction_commit(struct ref_t
                /* Fall through to finish. */
                break;
        case REF_TRANSACTION_CLOSED:
 -              die("BUG: commit called on a closed reference transaction");
 +              BUG("commit called on a closed reference transaction");
                break;
        default:
 -              die("BUG: unexpected reference transaction state");
 +              BUG("unexpected reference transaction state");
                break;
        }
  
@@@ -1998,7 -2004,7 +2010,7 @@@ int refs_verify_refname_available(struc
        }
  
        if (ok != ITER_DONE)
 -              die("BUG: error while iterating over references");
 +              BUG("error while iterating over references");
  
        extra_refname = find_descendant_ref(dirname.buf, extras, skip);
        if (extra_refname)