Merge branch 'jc/t1512-fix'
authorJunio C Hamano <gitster@pobox.com>
Thu, 11 Jul 2013 20:06:11 +0000 (13:06 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 11 Jul 2013 20:06:11 +0000 (13:06 -0700)
A test that should have failed but didn't revealed a bug that needs
to be corrected.

* jc/t1512-fix:
  get_short_sha1(): correctly disambiguate type-limited abbreviation
  t1512: correct leftover constants from earlier edition

1  2 
sha1_name.c
t/t1512-rev-parse-disambiguation.sh

diff --combined sha1_name.c
@@@ -241,7 -241,7 +241,7 @@@ static int disambiguate_committish_only
                return 0;
  
        /* We need to do this the hard way... */
-       obj = deref_tag(lookup_object(sha1), NULL, 0);
+       obj = deref_tag(parse_object(sha1), NULL, 0);
        if (obj && obj->type == OBJ_COMMIT)
                return 1;
        return 0;
@@@ -431,49 -431,22 +431,49 @@@ static inline int upstream_mark(const c
  }
  
  static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);
 +static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);
  
  static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
  {
        static const char *warn_msg = "refname '%.*s' is ambiguous.";
 +      static const char *object_name_msg = N_(
 +      "Git normally never creates a ref that ends with 40 hex characters\n"
 +      "because it will be ignored when you just specify 40-hex. These refs\n"
 +      "may be created by mistake. For example,\n"
 +      "\n"
 +      "  git checkout -b $br $(git rev-parse ...)\n"
 +      "\n"
 +      "where \"$br\" is somehow empty and a 40-hex ref is created. Please\n"
 +      "examine these refs and maybe delete them. Turn this message off by\n"
 +      "running \"git config advice.object_name_warning false\"");
 +      unsigned char tmp_sha1[20];
        char *real_ref = NULL;
        int refs_found = 0;
 -      int at, reflog_len;
 -
 -      if (len == 40 && !get_sha1_hex(str, sha1))
 +      int at, reflog_len, nth_prior = 0;
 +
 +      if (len == 40 && !get_sha1_hex(str, sha1)) {
 +              refs_found = dwim_ref(str, len, tmp_sha1, &real_ref);
 +              if (refs_found > 0 && warn_ambiguous_refs) {
 +                      warning(warn_msg, len, str);
 +                      if (advice_object_name_warning)
 +                              fprintf(stderr, "%s\n", _(object_name_msg));
 +              }
 +              free(real_ref);
                return 0;
 +      }
  
        /* basic@{time or number or -number} format to query ref-log */
        reflog_len = at = 0;
        if (len && str[len-1] == '}') {
 -              for (at = len-2; at >= 0; at--) {
 +              for (at = len-4; at >= 0; at--) {
                        if (str[at] == '@' && str[at+1] == '{') {
 +                              if (str[at+2] == '-') {
 +                                      if (at != 0)
 +                                              /* @{-N} not at start */
 +                                              return -1;
 +                                      nth_prior = 1;
 +                                      continue;
 +                              }
                                if (!upstream_mark(str + at, len - at)) {
                                        reflog_len = (len-1) - (at+2);
                                        len = at;
        if (len && ambiguous_path(str, len))
                return -1;
  
 -      if (!len && reflog_len) {
 +      if (nth_prior) {
                struct strbuf buf = STRBUF_INIT;
 -              int ret;
 -              /* try the @{-N} syntax for n-th checkout */
 -              ret = interpret_branch_name(str+at, &buf);
 -              if (ret > 0) {
 -                      /* substitute this branch name and restart */
 -                      return get_sha1_1(buf.buf, buf.len, sha1, 0);
 -              } else if (ret == 0) {
 -                      return -1;
 +              int detached;
 +
 +              if (interpret_nth_prior_checkout(str, &buf) > 0) {
 +                      detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));
 +                      strbuf_release(&buf);
 +                      if (detached)
 +                              return 0;
                }
 +      }
 +
 +      if (!len && reflog_len)
                /* allow "@{...}" to mean the current branch reflog */
                refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 -      else if (reflog_len)
 +      else if (reflog_len)
                refs_found = dwim_log(str, len, sha1, &real_ref);
        else
                refs_found = dwim_ref(str, len, sha1, &real_ref);
        if (!refs_found)
                return -1;
  
 -      if (warn_ambiguous_refs && refs_found > 1)
 +      if (warn_ambiguous_refs &&
 +          (refs_found > 1 ||
 +           !get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
                warning(warn_msg, len, str);
  
        if (reflog_len) {
                unsigned long co_time;
                int co_tz, co_cnt;
  
 -              /* a @{-N} placed anywhere except the start is an error */
 -              if (str[at+2] == '-')
 -                      return -1;
 -
                /* Is it asking for N-th entry, or approxidate? */
                for (i = nth = 0; 0 <= nth && i < reflog_len; i++) {
                        char ch = str[at+2+i];
                }
                if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
                                &co_time, &co_tz, &co_cnt)) {
 +                      if (!len) {
 +                              if (!prefixcmp(real_ref, "refs/heads/")) {
 +                                      str = real_ref + 11;
 +                                      len = strlen(real_ref + 11);
 +                              } else {
 +                                      /* detached HEAD */
 +                                      str = "HEAD";
 +                                      len = 4;
 +                              }
 +                      }
                        if (at_time)
                                warning("Log for '%.*s' only goes "
                                        "back to %s.", len, str,
                                        show_date(co_time, co_tz, DATE_RFC2822));
                        else {
 -                              free(real_ref);
                                die("Log for '%.*s' only has %d entries.",
                                    len, str, co_cnt);
                        }
@@@ -630,7 -594,7 +630,7 @@@ struct object *peel_to_type(const char 
        while (1) {
                if (!o || (!o->parsed && !parse_object(o->sha1)))
                        return NULL;
 -              if (o->type == expected_type)
 +              if (expected_type == OBJ_ANY || o->type == expected_type)
                        return o;
                if (o->type == OBJ_TAG)
                        o = ((struct tag*) o)->tagged;
@@@ -681,8 -645,6 +681,8 @@@ static int peel_onion(const char *name
                expected_type = OBJ_TREE;
        else if (!strncmp(blob_type, sp, 4) && sp[4] == '}')
                expected_type = OBJ_BLOB;
 +      else if (!prefixcmp(sp, "object}"))
 +              expected_type = OBJ_ANY;
        else if (sp[0] == '}')
                expected_type = OBJ_NONE;
        else if (sp[0] == '/')
  
        if (expected_type == OBJ_COMMIT)
                lookup_flags = GET_SHA1_COMMITTISH;
 +      else if (expected_type == OBJ_TREE)
 +              lookup_flags = GET_SHA1_TREEISH;
  
        if (get_sha1_1(name, sp - name - 2, outer, lookup_flags))
                return -1;
@@@ -896,8 -856,8 +896,8 @@@ static int get_sha1_oneline(const char 
  }
  
  struct grab_nth_branch_switch_cbdata {
 -      long cnt, alloc;
 -      struct strbuf *buf;
 +      int remaining;
 +      struct strbuf buf;
  };
  
  static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
        struct grab_nth_branch_switch_cbdata *cb = cb_data;
        const char *match = NULL, *target = NULL;
        size_t len;
 -      int nth;
  
        if (!prefixcmp(message, "checkout: moving from ")) {
                match = message + strlen("checkout: moving from ");
  
        if (!match || !target)
                return 0;
 -
 -      len = target - match;
 -      nth = cb->cnt++ % cb->alloc;
 -      strbuf_reset(&cb->buf[nth]);
 -      strbuf_add(&cb->buf[nth], match, len);
 +      if (--(cb->remaining) == 0) {
 +              len = target - match;
 +              strbuf_reset(&cb->buf);
 +              strbuf_add(&cb->buf, match, len);
 +              return 1; /* we are done */
 +      }
        return 0;
  }
  
  static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf)
  {
        long nth;
 -      int i, retval;
 +      int retval;
        struct grab_nth_branch_switch_cbdata cb;
        const char *brace;
        char *num_end;
        brace = strchr(name, '}');
        if (!brace)
                return -1;
 -      nth = strtol(name+3, &num_end, 10);
 +      nth = strtol(name + 3, &num_end, 10);
        if (num_end != brace)
                return -1;
        if (nth <= 0)
                return -1;
 -      cb.alloc = nth;
 -      cb.buf = xmalloc(nth * sizeof(struct strbuf));
 -      for (i = 0; i < nth; i++)
 -              strbuf_init(&cb.buf[i], 20);
 -      cb.cnt = 0;
 +      cb.remaining = nth;
 +      strbuf_init(&cb.buf, 20);
 +
        retval = 0;
 -      for_each_recent_reflog_ent("HEAD", grab_nth_branch_switch, 40960, &cb);
 -      if (cb.cnt < nth) {
 -              cb.cnt = 0;
 -              for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
 +      if (0 < for_each_reflog_ent_reverse("HEAD", grab_nth_branch_switch, &cb)) {
 +              strbuf_reset(buf);
 +              strbuf_add(buf, cb.buf.buf, cb.buf.len);
 +              retval = brace - name + 1;
        }
 -      if (cb.cnt < nth)
 -              goto release_return;
 -      i = cb.cnt % nth;
 -      strbuf_reset(buf);
 -      strbuf_add(buf, cb.buf[i].buf, cb.buf[i].len);
 -      retval = brace-name+1;
 -
 -release_return:
 -      for (i = 0; i < nth; i++)
 -              strbuf_release(&cb.buf[i]);
 -      free(cb.buf);
  
 +      strbuf_release(&cb.buf);
        return retval;
  }
  
@@@ -1002,38 -974,6 +1002,38 @@@ int get_sha1_mb(const char *name, unsig
        return st;
  }
  
 +/* parse @something syntax, when 'something' is not {.*} */
 +static int interpret_empty_at(const char *name, int namelen, int len, struct strbuf *buf)
 +{
 +      if (len || name[1] == '{')
 +              return -1;
 +
 +      strbuf_reset(buf);
 +      strbuf_add(buf, "HEAD", 4);
 +      return 1;
 +}
 +
 +static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
 +{
 +      /* we have extra data, which might need further processing */
 +      struct strbuf tmp = STRBUF_INIT;
 +      int used = buf->len;
 +      int ret;
 +
 +      strbuf_add(buf, name + len, namelen - len);
 +      ret = interpret_branch_name(buf->buf, &tmp);
 +      /* that data was not interpreted, remove our cruft */
 +      if (ret < 0) {
 +              strbuf_setlen(buf, used);
 +              return len;
 +      }
 +      strbuf_reset(buf);
 +      strbuf_addbuf(buf, &tmp);
 +      strbuf_release(&tmp);
 +      /* tweak for size of {-N} versus expanded ref name */
 +      return ret - used + len;
 +}
 +
  /*
   * This reads short-hand syntax that not only evaluates to a commit
   * object name, but also can act as if the end user spelled the name
@@@ -1063,47 -1003,43 +1063,47 @@@ int interpret_branch_name(const char *n
        int len = interpret_nth_prior_checkout(name, buf);
        int tmp_len;
  
 -      if (!len)
 +      if (!len) {
                return len; /* syntax Ok, not enough switches */
 -      if (0 < len && len == namelen)
 -              return len; /* consumed all */
 -      else if (0 < len) {
 -              /* we have extra data, which might need further processing */
 -              struct strbuf tmp = STRBUF_INIT;
 -              int used = buf->len;
 -              int ret;
 -
 -              strbuf_add(buf, name + len, namelen - len);
 -              ret = interpret_branch_name(buf->buf, &tmp);
 -              /* that data was not interpreted, remove our cruft */
 -              if (ret < 0) {
 -                      strbuf_setlen(buf, used);
 -                      return len;
 -              }
 -              strbuf_reset(buf);
 -              strbuf_addbuf(buf, &tmp);
 -              strbuf_release(&tmp);
 -              /* tweak for size of {-N} versus expanded ref name */
 -              return ret - used + len;
 +      } else if (len > 0) {
 +              if (len == namelen)
 +                      return len; /* consumed all */
 +              else
 +                      return reinterpret(name, namelen, len, buf);
        }
  
        cp = strchr(name, '@');
        if (!cp)
                return -1;
 +
 +      len = interpret_empty_at(name, namelen, cp - name, buf);
 +      if (len > 0)
 +              return reinterpret(name, namelen, len, buf);
 +
        tmp_len = upstream_mark(cp, namelen - (cp - name));
        if (!tmp_len)
                return -1;
 +
        len = cp + tmp_len - name;
        cp = xstrndup(name, cp - name);
        upstream = branch_get(*cp ? cp : NULL);
 -      if (!upstream
 -          || !upstream->merge
 -          || !upstream->merge[0]->dst)
 -              return error("No upstream branch found for '%s'", cp);
 +      /*
 +       * Upstream can be NULL only if cp refers to HEAD and HEAD
 +       * points to something different than a branch.
 +       */
 +      if (!upstream)
 +              die(_("HEAD does not point to a branch"));
 +      if (!upstream->merge || !upstream->merge[0]->dst) {
 +              if (!ref_exists(upstream->refname))
 +                      die(_("No such branch: '%s'"), cp);
 +              if (!upstream->merge) {
 +                      die(_("No upstream configured for branch '%s'"),
 +                              upstream->name);
 +              }
 +              die(
 +                      _("Upstream branch '%s' not stored as a remote-tracking branch"),
 +                      upstream->merge[0]->src);
 +      }
        free(cp);
        cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0);
        strbuf_reset(buf);
  int strbuf_branchname(struct strbuf *sb, const char *name)
  {
        int len = strlen(name);
 -      if (interpret_branch_name(name, sb) == len)
 +      int used = interpret_branch_name(name, sb);
 +
 +      if (used == len)
                return 0;
 -      strbuf_add(sb, name, len);
 +      if (used < 0)
 +              used = 0;
 +      strbuf_add(sb, name + used, len - used);
        return len;
  }
  
@@@ -1193,8 -1125,7 +1193,8 @@@ int get_sha1_blob(const char *name, uns
  static void diagnose_invalid_sha1_path(const char *prefix,
                                       const char *filename,
                                       const unsigned char *tree_sha1,
 -                                     const char *object_name)
 +                                     const char *object_name,
 +                                     int object_name_len)
  {
        struct stat st;
        unsigned char sha1[20];
                prefix = "";
  
        if (!lstat(filename, &st))
 -              die("Path '%s' exists on disk, but not in '%s'.",
 -                  filename, object_name);
 +              die("Path '%s' exists on disk, but not in '%.*s'.",
 +                  filename, object_name_len, object_name);
        if (errno == ENOENT || errno == ENOTDIR) {
                char *fullname = xmalloc(strlen(filename)
                                             + strlen(prefix) + 1);
                if (!get_tree_entry(tree_sha1, fullname,
                                    sha1, &mode)) {
                        die("Path '%s' exists, but not '%s'.\n"
 -                          "Did you mean '%s:%s' aka '%s:./%s'?",
 +                          "Did you mean '%.*s:%s' aka '%.*s:./%s'?",
                            fullname,
                            filename,
 -                          object_name,
 +                          object_name_len, object_name,
                            fullname,
 -                          object_name,
 +                          object_name_len, object_name,
                            filename);
                }
 -              die("Path '%s' does not exist in '%s'",
 -                  filename, object_name);
 +              die("Path '%s' does not exist in '%.*s'",
 +                  filename, object_name_len, object_name);
        }
  }
  
@@@ -1389,8 -1320,13 +1389,8 @@@ static int get_sha1_with_context_1(cons
        }
        if (*cp == ':') {
                unsigned char tree_sha1[20];
 -              char *object_name = NULL;
 -              if (only_to_die) {
 -                      object_name = xmalloc(cp-name+1);
 -                      strncpy(object_name, name, cp-name);
 -                      object_name[cp-name] = '\0';
 -              }
 -              if (!get_sha1_1(name, cp-name, tree_sha1, GET_SHA1_TREEISH)) {
 +              int len = cp - name;
 +              if (!get_sha1_1(name, len, tree_sha1, GET_SHA1_TREEISH)) {
                        const char *filename = cp+1;
                        char *new_filename = NULL;
  
                        if (new_filename)
                                filename = new_filename;
                        ret = get_tree_entry(tree_sha1, filename, sha1, &oc->mode);
 -                      if (only_to_die) {
 +                      if (ret && only_to_die) {
                                diagnose_invalid_sha1_path(prefix, filename,
 -                                                         tree_sha1, object_name);
 -                              free(object_name);
 +                                                         tree_sha1,
 +                                                         name, len);
                        }
                        hashcpy(oc->tree, tree_sha1);
                        strncpy(oc->path, filename,
                        return ret;
                } else {
                        if (only_to_die)
 -                              die("Invalid object name '%s'.", object_name);
 +                              die("Invalid object name '%.*s'.", len, name);
                }
        }
        return ret;
@@@ -77,6 -77,7 +77,7 @@@ test_expect_success 'disambiguate blob
  
  test_expect_success 'disambiguate tree' '
        commit=$(echo "d7xm" | git commit-tree 000000000) &&
+       # this commit is fffff2e and not ambiguous with the 00000* objects
        test $(git rev-parse $commit^{tree}) = $(git rev-parse 0000000000cdc)
  '
  
@@@ -99,10 -100,14 +100,14 @@@ test_expect_success 'disambiguate commi
  
  test_expect_success 'disambiguate commit' '
        commit=$(echo "hoaxj" | git commit-tree 0000000000cdc -p 000000000) &&
+       # this commit is ffffffd8 and not ambiguous with the 00000* objects
        test $(git rev-parse $commit^) = $(git rev-parse 0000000000e4f)
  '
  
  test_expect_success 'log name1..name2 takes only commit-ishes on both ends' '
+       # These are underspecified from the prefix-length point of view
+       # to disambiguate the commit with other objects, but there is only
+       # one commit that has 00000* prefix at this point.
        git log 000000000..000000000 &&
        git log ..000000000 &&
        git log 000000000.. &&
  '
  
  test_expect_success 'rev-parse name1..name2 takes only commit-ishes on both ends' '
+       # Likewise.
        git rev-parse 000000000..000000000 &&
        git rev-parse ..000000000 &&
        git rev-parse 000000000..
  '
  
  test_expect_success 'git log takes only commit-ish' '
+       # Likewise.
        git log 000000000
  '
  
  test_expect_success 'git reset takes only commit-ish' '
+       # Likewise.
        git reset 000000000
  '
  
@@@ -131,26 -139,30 +139,30 @@@ test_expect_success 'first tag' 
  '
  
  test_expect_failure 'two semi-ambiguous commit-ish' '
+       # At this point, we have a tag 0000000000f8f that points
+       # at a commit 0000000000e4f, and a tree and a blob that
+       # share 0000000000 prefix with these tag and commit.
+       #
        # Once the parser becomes ultra-smart, it could notice that
-       # 110282 before ^{commit} name many different objects, but
+       # 0000000000 before ^{commit} name many different objects, but
        # that only two (HEAD and v1.0.0 tag) can be peeled to commit,
        # and that peeling them down to commit yield the same commit
        # without ambiguity.
-       git rev-parse --verify 110282^{commit} &&
+       git rev-parse --verify 0000000000^{commit} &&
  
        # likewise
-       git log 000000000..000000000 &&
-       git log ..000000000 &&
-       git log 000000000.. &&
-       git log 000000000...000000000 &&
-       git log ...000000000 &&
-       git log 000000000...
+       git log 0000000000..0000000000 &&
+       git log ..0000000000 &&
+       git log 0000000000.. &&
+       git log 0000000000...0000000000 &&
+       git log ...0000000000 &&
+       git log 0000000000...
  '
  
  test_expect_failure 'three semi-ambiguous tree-ish' '
        # Likewise for tree-ish.  HEAD, v1.0.0 and HEAD^{tree} share
        # the prefix but peeling them to tree yields the same thing
-       git rev-parse --verify 000000000^{tree}
+       git rev-parse --verify 0000000000^{tree}
  '
  
  test_expect_success 'parse describe name' '
@@@ -241,7 -253,7 +253,7 @@@ test_expect_success 'ambiguous commit-i
        # Now there are many commits that begin with the
        # common prefix, none of these should pick one at
        # random.  They all should result in ambiguity errors.
-       test_must_fail git rev-parse --verify 110282^{commit} &&
+       test_must_fail git rev-parse --verify 00000000^{commit} &&
  
        # likewise
        test_must_fail git log 000000000..000000000 &&
@@@ -261,22 -273,4 +273,22 @@@ test_expect_success 'rev-parse --disamb
        test "$(sed -e "s/^\(.........\).*/\1/" actual | sort -u)" = 000000000
  '
  
 +test_expect_success 'ambiguous 40-hex ref' '
 +      TREE=$(git mktree </dev/null) &&
 +      REF=`git rev-parse HEAD` &&
 +      VAL=$(git commit-tree $TREE </dev/null) &&
 +      git update-ref refs/heads/$REF $VAL &&
 +      test `git rev-parse $REF 2>err` = $REF &&
 +      grep "refname.*${REF}.*ambiguous" err
 +'
 +
 +test_expect_success 'ambiguous short sha1 ref' '
 +      TREE=$(git mktree </dev/null) &&
 +      REF=`git rev-parse --short HEAD` &&
 +      VAL=$(git commit-tree $TREE </dev/null) &&
 +      git update-ref refs/heads/$REF $VAL &&
 +      test `git rev-parse $REF 2>err` = $VAL &&
 +      grep "refname.*${REF}.*ambiguous" err
 +'
 +
  test_done