Merge branch 'jc/branch-name-sanity'
authorJunio C Hamano <gitster@pobox.com>
Tue, 28 Nov 2017 04:41:49 +0000 (13:41 +0900)
committerJunio C Hamano <gitster@pobox.com>
Tue, 28 Nov 2017 04:41:49 +0000 (13:41 +0900)
"git branch" and "git checkout -b" are now forbidden from creating
a branch whose name is "HEAD".

* jc/branch-name-sanity:
builtin/branch: remove redundant check for HEAD
branch: correctly reject refs/heads/{-dash,HEAD}
branch: split validate_new_branchname() into two
branch: streamline "attr_only" handling in validate_new_branchname()

1  2 
branch.c
builtin/branch.c
builtin/checkout.c
sha1_name.c
diff --combined branch.c
index 62f7b0d8c2d6a89fde5abffb6c836d0ce7706ac4,2c3a364a0b0fe9eeb0aaec78ea3b3ec1424246ab..fe1e1c3676b26a4f35feaf8137700b25916ad5ee
+++ b/branch.c
@@@ -178,24 -178,40 +178,40 @@@ int read_branch_desc(struct strbuf *buf
        return 0;
  }
  
- int validate_new_branchname(const char *name, struct strbuf *ref,
-                           int force, int attr_only)
+ /*
+  * Check if 'name' can be a valid name for a branch; die otherwise.
+  * Return 1 if the named branch already exists; return 0 otherwise.
+  * Fill ref with the full refname for the branch.
+  */
+ int validate_branchname(const char *name, struct strbuf *ref)
  {
        if (strbuf_check_branch_ref(ref, name))
                die(_("'%s' is not a valid branch name."), name);
  
-       if (!ref_exists(ref->buf))
+       return ref_exists(ref->buf);
+ }
+ /*
+  * Check if a branch 'name' can be created as a new branch; die otherwise.
+  * 'force' can be used when it is OK for the named branch already exists.
+  * Return 1 if the named branch already exists; return 0 otherwise.
+  * Fill ref with the full refname for the branch.
+  */
+ int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+ {
+       const char *head;
+       if (!validate_branchname(name, ref))
                return 0;
-       else if (!force && !attr_only)
-               die(_("A branch named '%s' already exists."), ref->buf + strlen("refs/heads/"));
  
-       if (!attr_only) {
-               const char *head;
+       if (!force)
+               die(_("A branch named '%s' already exists."),
+                   ref->buf + strlen("refs/heads/"));
+       head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+       if (!is_bare_repository() && head && !strcmp(head, ref->buf))
+               die(_("Cannot force update the current branch."));
  
-               head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
-               if (!is_bare_repository() && head && !strcmp(head, ref->buf))
-                       die(_("Cannot force update the current branch."));
-       }
        return 1;
  }
  
@@@ -242,9 -258,9 +258,9 @@@ void create_branch(const char *name, co
        if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
                explicit_tracking = 1;
  
-       if (validate_new_branchname(name, &ref, force,
-                                   track == BRANCH_TRACK_OVERRIDE ||
-                                   clobber_head)) {
+       if ((track == BRANCH_TRACK_OVERRIDE || clobber_head)
+           ? validate_branchname(name, &ref)
+           : validate_new_branchname(name, &ref, force)) {
                if (!force)
                        dont_change_ref = 1;
                else
                die(_("Not a valid object name: '%s'."), start_name);
        }
  
 -      switch (dwim_ref(start_name, strlen(start_name), oid.hash, &real_ref)) {
 +      switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) {
        case 0:
                /* Not branching from any existing branch */
                if (explicit_tracking)
                transaction = ref_transaction_begin(&err);
                if (!transaction ||
                    ref_transaction_update(transaction, ref.buf,
 -                                         oid.hash, forcing ? NULL : null_sha1,
 +                                         &oid, forcing ? NULL : &null_oid,
                                           0, msg, &err) ||
                    ref_transaction_commit(transaction, &err))
                        die("%s", err.buf);
diff --combined builtin/branch.c
index 33fd5fcfd10be978f4d8ccdb6869eb05e86eb956,945790b60b355b06cebebbf76d29fa428d414d0e..5fc57cdc982f86d6e449d8358004c476707d909c
@@@ -93,7 -93,7 +93,7 @@@ static int git_branch_config(const cha
                        return config_error_nonbool(var);
                return color_parse(value, branch_colors[slot]);
        }
 -      return git_default_config(var, value, cb);
 +      return git_color_default_config(var, value, cb);
  }
  
  static const char *branch_get_color(enum color_branch ix)
@@@ -125,7 -125,7 +125,7 @@@ static int branch_merged(int kind, cons
                if (upstream &&
                    (reference_name = reference_name_to_free =
                     resolve_refdup(upstream, RESOLVE_REF_READING,
 -                                  oid.hash, NULL)) != NULL)
 +                                  &oid, NULL)) != NULL)
                        reference_rev = lookup_commit_reference(&oid);
        }
        if (!reference_rev)
@@@ -241,7 -241,7 +241,7 @@@ static int delete_branches(int argc, co
                                        RESOLVE_REF_READING
                                        | RESOLVE_REF_NO_RECURSE
                                        | RESOLVE_REF_ALLOW_BAD_NAME,
 -                                      oid.hash, &flags);
 +                                      &oid, &flags);
                if (!target) {
                        error(remote_branch
                              ? _("remote-tracking branch '%s' not found.")
                        goto next;
                }
  
 -              if (delete_ref(NULL, name, is_null_oid(&oid) ? NULL : oid.hash,
 -                             REF_NODEREF)) {
 +              if (delete_ref(NULL, name, is_null_oid(&oid) ? NULL : &oid,
 +                             REF_NO_DEREF)) {
                        error(remote_branch
                              ? _("Error deleting remote-tracking branch '%s'")
                              : _("Error deleting branch '%s'"),
@@@ -463,7 -463,6 +463,6 @@@ static void copy_or_rename_branch(cons
        struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
        struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
        int recovery = 0;
-       int clobber_head_ok;
  
        if (!oldname) {
                if (copy)
         * A command like "git branch -M currentbranch currentbranch" cannot
         * cause the worktree to become inconsistent with HEAD, so allow it.
         */
-       clobber_head_ok = !strcmp(oldname, newname);
-       validate_new_branchname(newname, &newref, force, clobber_head_ok);
+       if (!strcmp(oldname, newname))
+               validate_branchname(newname, &newref);
+       else
+               validate_new_branchname(newname, &newref, force);
  
        reject_rebase_or_bisect_branch(oldref.buf);
  
@@@ -636,7 -636,7 +636,7 @@@ int cmd_branch(int argc, const char **a
  
        track = git_branch_track;
  
 -      head = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
 +      head = resolve_refdup("HEAD", 0, &head_oid, NULL);
        if (!head)
                die(_("Failed to resolve HEAD as a valid ref."));
        if (!strcmp(head, "HEAD"))
        } else if (argc > 0 && argc <= 2) {
                struct branch *branch = branch_get(argv[0]);
  
-               if (!strcmp(argv[0], "HEAD"))
-                       die(_("it does not make sense to create 'HEAD' manually"));
                if (!branch)
                        die(_("no such branch '%s'"), argv[0]);
  
diff --combined builtin/checkout.c
index 7d8bcc3833512bc262bd0a8d86db9169d991b320,697ac7dcaf475d5215747743d4bfbc12d7dc518a..3faae382de4fa345f6d54c9120d33ed86c02e2cc
@@@ -247,7 -247,7 +247,7 @@@ static int checkout_paths(const struct 
        struct object_id rev;
        struct commit *head;
        int errs = 0;
 -      struct lock_file *lock_file;
 +      struct lock_file lock_file = LOCK_INIT;
  
        if (opts->track != BRANCH_TRACK_UNSPECIFIED)
                die(_("'%s' cannot be used with updating paths"), "--track");
                return run_add_interactive(revision, "--patch=checkout",
                                           &opts->pathspec);
  
 -      lock_file = xcalloc(1, sizeof(struct lock_file));
 -
 -      hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
 +      hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
        if (read_cache_preload(&opts->pathspec) < 0)
                return error(_("index file corrupt"));
  
        }
        errs |= finish_delayed_checkout(&state);
  
 -      if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
 +      if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
                die(_("unable to write new index file"));
  
 -      read_ref_full("HEAD", 0, rev.hash, NULL);
 +      read_ref_full("HEAD", 0, &rev, NULL);
        head = lookup_commit_reference_gently(&rev, 1);
  
        errs |= post_checkout_hook(head, head, 0);
@@@ -470,9 -472,9 +470,9 @@@ static int merge_working_tree(const str
                              int *writeout_error)
  {
        int ret;
 -      struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 +      struct lock_file lock_file = LOCK_INIT;
  
 -      hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
 +      hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
        if (read_cache_preload(NULL) < 0)
                return error(_("index file corrupt"));
  
        if (!cache_tree_fully_valid(active_cache_tree))
                cache_tree_update(&the_index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
  
 -      if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
 +      if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
                die(_("unable to write new index file"));
  
        if (!opts->force && !opts->quiet)
@@@ -662,8 -664,8 +662,8 @@@ static void update_refs_for_switch(cons
        if (!strcmp(new->name, "HEAD") && !new->path && !opts->force_detach) {
                /* Nothing to do. */
        } else if (opts->force_detach || !new->path) {  /* No longer on any branch. */
 -              update_ref(msg.buf, "HEAD", new->commit->object.oid.hash, NULL,
 -                         REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
 +              update_ref(msg.buf, "HEAD", &new->commit->object.oid, NULL,
 +                         REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
                if (!opts->quiet) {
                        if (old->path &&
                            advice_detached_head && !opts->force_detach)
@@@ -825,7 -827,7 +825,7 @@@ static int switch_branches(const struc
        struct object_id rev;
        int flag, writeout_error = 0;
        memset(&old, 0, sizeof(old));
 -      old.path = path_to_free = resolve_refdup("HEAD", 0, rev.hash, &flag);
 +      old.path = path_to_free = resolve_refdup("HEAD", 0, &rev, &flag);
        if (old.path)
                old.commit = lookup_commit_reference_gently(&rev, 1);
        if (!(flag & REF_ISSYMREF))
@@@ -1036,7 -1038,7 +1036,7 @@@ static int parse_branchname_arg(int arg
        setup_branch_path(new);
  
        if (!check_refname_format(new->path, 0) &&
 -          !read_ref(new->path, branch_rev.hash))
 +          !read_ref(new->path, &branch_rev))
                oidcpy(rev, &branch_rev);
        else
                new->path = NULL; /* not an existing branch */
@@@ -1134,7 -1136,7 +1134,7 @@@ static int checkout_branch(struct check
                struct object_id rev;
                int flag;
  
 -              if (!read_ref_full("HEAD", 0, rev.hash, &flag) &&
 +              if (!read_ref_full("HEAD", 0, &rev, &flag) &&
                    (flag & REF_ISSYMREF) && is_null_oid(&rev))
                        return switch_unborn_to_new_branch(opts);
        }
@@@ -1287,11 -1289,11 +1287,11 @@@ int cmd_checkout(int argc, const char *
        if (opts.new_branch) {
                struct strbuf buf = STRBUF_INIT;
  
-               opts.branch_exists =
-                       validate_new_branchname(opts.new_branch, &buf,
-                                               !!opts.new_branch_force,
-                                               !!opts.new_branch_force);
+               if (opts.new_branch_force)
+                       opts.branch_exists = validate_branchname(opts.new_branch, &buf);
+               else
+                       opts.branch_exists =
+                               validate_new_branchname(opts.new_branch, &buf, 0);
                strbuf_release(&buf);
        }
  
diff --combined sha1_name.c
index 9a2d5caf3b785ec629fcbc2325cd63e0ef72cf57,67961d6e47b67eac4372cbe5ffb37fa3d97c5861..611c7d24ddee678470ba74cb3c2ca669a778b44a
@@@ -153,9 -153,7 +153,9 @@@ static void unique_in_pack(struct packe
        uint32_t num, last, i, first = 0;
        const struct object_id *current = NULL;
  
 -      open_pack_index(p);
 +      if (open_pack_index(p) || !p->num_objects)
 +              return;
 +
        num = p->num_objects;
        last = num;
        while (first < last) {
@@@ -476,104 -474,10 +476,104 @@@ static unsigned msb(unsigned long val
        return r;
  }
  
 -int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
 +struct min_abbrev_data {
 +      unsigned int init_len;
 +      unsigned int cur_len;
 +      char *hex;
 +      const unsigned char *hash;
 +};
 +
 +static inline char get_hex_char_from_oid(const struct object_id *oid,
 +                                       unsigned int pos)
 +{
 +      static const char hex[] = "0123456789abcdef";
 +
 +      if ((pos & 1) == 0)
 +              return hex[oid->hash[pos >> 1] >> 4];
 +      else
 +              return hex[oid->hash[pos >> 1] & 0xf];
 +}
 +
 +static int extend_abbrev_len(const struct object_id *oid, void *cb_data)
 +{
 +      struct min_abbrev_data *mad = cb_data;
 +
 +      unsigned int i = mad->init_len;
 +      while (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i))
 +              i++;
 +
 +      if (i < GIT_MAX_RAWSZ && i >= mad->cur_len)
 +              mad->cur_len = i + 1;
 +
 +      return 0;
 +}
 +
 +static void find_abbrev_len_for_pack(struct packed_git *p,
 +                                   struct min_abbrev_data *mad)
 +{
 +      int match = 0;
 +      uint32_t num, last, first = 0;
 +      struct object_id oid;
 +
 +      if (open_pack_index(p) || !p->num_objects)
 +              return;
 +
 +      num = p->num_objects;
 +      last = num;
 +      while (first < last) {
 +              uint32_t mid = first + (last - first) / 2;
 +              const unsigned char *current;
 +              int cmp;
 +
 +              current = nth_packed_object_sha1(p, mid);
 +              cmp = hashcmp(mad->hash, current);
 +              if (!cmp) {
 +                      match = 1;
 +                      first = mid;
 +                      break;
 +              }
 +              if (cmp > 0) {
 +                      first = mid + 1;
 +                      continue;
 +              }
 +              last = mid;
 +      }
 +
 +      /*
 +       * first is now the position in the packfile where we would insert
 +       * mad->hash if it does not exist (or the position of mad->hash if
 +       * it does exist). Hence, we consider a maximum of three objects
 +       * nearby for the abbreviation length.
 +       */
 +      mad->init_len = 0;
 +      if (!match) {
 +              nth_packed_object_oid(&oid, p, first);
 +              extend_abbrev_len(&oid, mad);
 +      } else if (first < num - 1) {
 +              nth_packed_object_oid(&oid, p, first + 1);
 +              extend_abbrev_len(&oid, mad);
 +      }
 +      if (first > 0) {
 +              nth_packed_object_oid(&oid, p, first - 1);
 +              extend_abbrev_len(&oid, mad);
 +      }
 +      mad->init_len = mad->cur_len;
 +}
 +
 +static void find_abbrev_len_packed(struct min_abbrev_data *mad)
  {
 -      int status, exists;
 +      struct packed_git *p;
 +
 +      prepare_packed_git();
 +      for (p = packed_git; p; p = p->next)
 +              find_abbrev_len_for_pack(p, mad);
 +}
  
 +int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
 +{
 +      struct disambiguate_state ds;
 +      struct min_abbrev_data mad;
 +      struct object_id oid_ret;
        if (len < 0) {
                unsigned long count = approximate_object_count();
                /*
        sha1_to_hex_r(hex, sha1);
        if (len == GIT_SHA1_HEXSZ || !len)
                return GIT_SHA1_HEXSZ;
 -      exists = has_sha1_file(sha1);
 -      while (len < GIT_SHA1_HEXSZ) {
 -              struct object_id oid_ret;
 -              status = get_short_oid(hex, len, &oid_ret, GET_OID_QUIETLY);
 -              if (exists
 -                  ? !status
 -                  : status == SHORT_NAME_NOT_FOUND) {
 -                      hex[len] = 0;
 -                      return len;
 -              }
 -              len++;
 -      }
 -      return len;
 +
 +      mad.init_len = len;
 +      mad.cur_len = len;
 +      mad.hex = hex;
 +      mad.hash = sha1;
 +
 +      find_abbrev_len_packed(&mad);
 +
 +      if (init_object_disambiguation(hex, mad.cur_len, &ds) < 0)
 +              return -1;
 +
 +      ds.fn = extend_abbrev_len;
 +      ds.always_call_fn = 1;
 +      ds.cb_data = (void *)&mad;
 +
 +      find_short_object_filename(&ds);
 +      (void)finish_object_disambiguation(&ds, &oid_ret);
 +
 +      hex[mad.cur_len] = 0;
 +      return mad.cur_len;
  }
  
  const char *find_unique_abbrev(const unsigned char *sha1, int len)
@@@ -706,7 -603,7 +706,7 @@@ static int get_oid_basic(const char *st
  
        if (len == GIT_SHA1_HEXSZ && !get_oid_hex(str, oid)) {
                if (warn_ambiguous_refs && warn_on_object_refname_ambiguity) {
 -                      refs_found = dwim_ref(str, len, tmp_oid.hash, &real_ref);
 +                      refs_found = dwim_ref(str, len, &tmp_oid, &real_ref);
                        if (refs_found > 0) {
                                warning(warn_msg, len, str);
                                if (advice_object_name_warning)
  
        if (!len && reflog_len)
                /* allow "@{...}" to mean the current branch reflog */
 -              refs_found = dwim_ref("HEAD", 4, oid->hash, &real_ref);
 +              refs_found = dwim_ref("HEAD", 4, oid, &real_ref);
        else if (reflog_len)
 -              refs_found = dwim_log(str, len, oid->hash, &real_ref);
 +              refs_found = dwim_log(str, len, oid, &real_ref);
        else
 -              refs_found = dwim_ref(str, len, oid->hash, &real_ref);
 +              refs_found = dwim_ref(str, len, oid, &real_ref);
  
        if (!refs_found)
                return -1;
                                return -1;
                        }
                }
 -              if (read_ref_at(real_ref, flags, at_time, nth, oid->hash, NULL,
 +              if (read_ref_at(real_ref, flags, at_time, nth, oid, NULL,
                                &co_time, &co_tz, &co_cnt)) {
                        if (!len) {
                                if (starts_with(real_ref, "refs/heads/")) {
@@@ -1434,13 -1331,20 +1434,23 @@@ void strbuf_branchname(struct strbuf *s
  
  int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
  {
 -      strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
 +      if (startup_info->have_repository)
 +              strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
 +      else
 +              strbuf_addstr(sb, name);
-       if (name[0] == '-')
-               return -1;
+       /*
+        * This splice must be done even if we end up rejecting the
+        * name; builtin/branch.c::copy_or_rename_branch() still wants
+        * to see what the name expanded to so that "branch -m" can be
+        * used as a tool to correct earlier mistakes.
+        */
        strbuf_splice(sb, 0, 0, "refs/heads/", 11);
+       if (*name == '-' ||
+           !strcmp(sb->buf, "refs/heads/HEAD"))
+               return -1;
        return check_refname_format(sb->buf, 0);
  }