From 1bc6d022b731e4cf69de42fea75c5f6366d2dbae Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 16:55:01 -0500 Subject: [PATCH 01/13] tests: at-combinations: simplify setup The test is setting up an upstream branch, but there's a much simpler way of doing that: git branch -u. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t1508-at-combinations.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh index d5d6244178..46e3f166a6 100755 --- a/t/t1508-at-combinations.sh +++ b/t/t1508-at-combinations.sh @@ -31,10 +31,8 @@ test_expect_success 'setup' ' git checkout -b new-branch && test_commit new-one && test_commit new-two && - git config branch.old-branch.remote . && - git config branch.old-branch.merge refs/heads/master && - git config branch.new-branch.remote . && - git config branch.new-branch.merge refs/heads/upstream-branch + git branch -u master old-branch && + git branch -u upstream-branch new-branch ' check HEAD new-two From c8a81e90acda9da55bae627928e80b04e98980fc Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 16:55:02 -0500 Subject: [PATCH 02/13] tests: at-combinations: check ref names directly Some committishes might point to the same commit, but through a different ref, that's why it's better to check directly for the ref, rather than the commit message. We can do that by calling rev-parse --symbolic-full-name, and to differentiate the old from the new behavior we add an extra argument to the check() helper. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t1508-at-combinations.sh | 39 ++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh index 46e3f166a6..1126125672 100755 --- a/t/t1508-at-combinations.sh +++ b/t/t1508-at-combinations.sh @@ -4,17 +4,24 @@ test_description='test various @{X} syntax combinations together' . ./test-lib.sh check() { -test_expect_${3:-success} "$1 = $2" " - echo '$2' >expect && - git log -1 --format=%s '$1' >actual && - test_cmp expect actual -" + test_expect_${4:-success} "$1 = $3" " + echo '$3' >expect && + if test '$2' = 'commit' + then + git log -1 --format=%s '$1' >actual + else + git rev-parse --symbolic-full-name '$1' >actual + fi && + test_cmp expect actual + " } + nonsense() { -test_expect_${2:-success} "$1 is nonsensical" " - test_must_fail git log -1 '$1' -" + test_expect_${2:-success} "$1 is nonsensical" " + test_must_fail git log -1 '$1' + " } + fail() { "$@" failure } @@ -35,14 +42,14 @@ test_expect_success 'setup' ' git branch -u upstream-branch new-branch ' -check HEAD new-two -check "@{1}" new-one -check "@{-1}" old-two -check "@{-1}@{1}" old-one -check "@{u}" upstream-two -check "@{u}@{1}" upstream-one -check "@{-1}@{u}" master-two -check "@{-1}@{u}@{1}" master-one +check HEAD ref refs/heads/new-branch +check "@{1}" commit new-one +check "@{-1}" ref refs/heads/old-branch +check "@{-1}@{1}" commit old-one +check "@{u}" ref refs/heads/upstream-branch +check "@{u}@{1}" commit upstream-one +check "@{-1}@{u}" ref refs/heads/master +check "@{-1}@{u}@{1}" commit master-one nonsense "@{u}@{-1}" nonsense "@{1}@{u}" From 89d5dd4e2f47186687d70223847cec8ec3fb16dd Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 16:55:03 -0500 Subject: [PATCH 03/13] tests: at-combinations: improve nonsense() In some circumstances 'git log' might fail, but not because the @ parsing failed. For example: 'git rev-parse' might succeed and return a bad object, and then 'git log' would fail. The layer we want to test is revision parsing, so let's test that directly. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t1508-at-combinations.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh index 1126125672..d45a410362 100755 --- a/t/t1508-at-combinations.sh +++ b/t/t1508-at-combinations.sh @@ -18,7 +18,7 @@ check() { nonsense() { test_expect_${2:-success} "$1 is nonsensical" " - test_must_fail git log -1 '$1' + test_must_fail git rev-parse --verify '$1' " } From f58dc19e571bea9b7b079041198c21ec15ac3cea Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Tue, 7 May 2013 16:55:04 -0500 Subject: [PATCH 04/13] tests: at-combinations: increase coverage Add more tests exercising documented functionality. [fc: commit message and extra tests] Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t1508-at-combinations.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh index d45a410362..31e95a5466 100755 --- a/t/t1508-at-combinations.sh +++ b/t/t1508-at-combinations.sh @@ -44,13 +44,21 @@ test_expect_success 'setup' ' check HEAD ref refs/heads/new-branch check "@{1}" commit new-one +check "HEAD@{1}" commit new-one +check "@{now}" commit new-two +check "HEAD@{now}" commit new-two check "@{-1}" ref refs/heads/old-branch +check "@{-1}@{0}" commit old-two check "@{-1}@{1}" commit old-one check "@{u}" ref refs/heads/upstream-branch +check "HEAD@{u}" ref refs/heads/upstream-branch check "@{u}@{1}" commit upstream-one check "@{-1}@{u}" ref refs/heads/master check "@{-1}@{u}@{1}" commit master-one nonsense "@{u}@{-1}" +nonsense "@{0}@{0}" nonsense "@{1}@{u}" +nonsense "HEAD@{-1}" +nonsense "@{-1}@{-1}" test_done From 723b74ee3e09bfb19b43218da19c52f805bb69ae Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Tue, 7 May 2013 16:55:05 -0500 Subject: [PATCH 05/13] tests: at-combinations: @{N} versus HEAD@{N} All the tests so far check that @{N} is the same as HEAD@{N} (for positive N). However, this is not always the case; write a couple of tests for this. [fc: simplify some wording] Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- t/t1508-at-combinations.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh index 31e95a5466..e5aea3b896 100755 --- a/t/t1508-at-combinations.sh +++ b/t/t1508-at-combinations.sh @@ -61,4 +61,17 @@ nonsense "@{1}@{u}" nonsense "HEAD@{-1}" nonsense "@{-1}@{-1}" +# @{N} versus HEAD@{N} + +check "HEAD@{3}" commit old-two +nonsense "@{3}" + +test_expect_success 'switch to old-branch' ' + git checkout old-branch +' + +check HEAD ref refs/heads/old-branch +check "HEAD@{1}" commit new-two +check "@{1}" commit old-one + test_done From 1ef2d8dacc992b98444170ea9ee17de417433146 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 16:55:06 -0500 Subject: [PATCH 06/13] sha1_name: remove no-op 'at' is always 0, since we can reach this point only if !len && reflog_len, and len=at when reflog is assigned. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_name.c b/sha1_name.c index 3820f28ae7..01e49a92dd 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -464,7 +464,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) struct strbuf buf = STRBUF_INIT; int ret; /* try the @{-N} syntax for n-th checkout */ - ret = interpret_branch_name(str+at, &buf); + ret = interpret_branch_name(str, &buf); if (ret > 0) { /* substitute this branch name and restart */ return get_sha1_1(buf.buf, buf.len, sha1, 0); From b5f769a0d71bc3f13f916db849ef6b4768359c4e Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 16:55:07 -0500 Subject: [PATCH 07/13] sha1_name: remove unnecessary braces Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 01e49a92dd..6530ddd6d8 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -465,12 +465,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) int ret; /* try the @{-N} syntax for n-th checkout */ ret = interpret_branch_name(str, &buf); - if (ret > 0) { + if (ret > 0) /* substitute this branch name and restart */ return get_sha1_1(buf.buf, buf.len, sha1, 0); - } else if (ret == 0) { + else if (ret == 0) return -1; - } /* allow "@{...}" to mean the current branch reflog */ refs_found = dwim_ref("HEAD", 4, sha1, &real_ref); } else if (reflog_len) From e883a057a9da3c738dd6f6e2f1aa9498885ae1e8 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Tue, 7 May 2013 16:55:09 -0500 Subject: [PATCH 08/13] sha1_name: don't waste cycles in the @-parsing loop The @-parsing loop unnecessarily checks for the sequence "@{" from (len - 2) unnecessarily. We can safely check from (len - 4). Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_name.c b/sha1_name.c index 6530ddd6d8..7b59f6f4ec 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -445,7 +445,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) /* 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 (!upstream_mark(str + at, len - at)) { reflog_len = (len-1) - (at+2); From 128fd54daeff23fbef0d0b7ec1a31f38d0803f63 Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 16:55:10 -0500 Subject: [PATCH 09/13] sha1_name: reorganize get_sha1_basic() Through the years the functionality to handle @{-N} and @{u} has moved around the code, and as a result, code that once made sense, doesn't any more. There is no need to call this function recursively with the branch of @{-N} substituted because dwim_{ref,log} already replaces it. However, there's one corner-case where @{-N} resolves to a detached HEAD, in which case we wouldn't get any ref back. So we parse the nth-prior manually, and deal with it depending on whether it's a SHA-1, or a ref. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 7b59f6f4ec..45b89d9df0 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -431,13 +431,14 @@ static inline int upstream_mark(const char *string, int len) } 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."; char *real_ref = NULL; int refs_found = 0; - int at, reflog_len; + int at, reflog_len, nth_prior = 0; if (len == 40 && !get_sha1_hex(str, sha1)) return 0; @@ -447,6 +448,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) if (len && str[len-1] == '}') { for (at = len-4; at >= 0; at--) { if (str[at] == '@' && str[at+1] == '{') { + if (at == 0 && str[2] == '-') { + nth_prior = 1; + continue; + } if (!upstream_mark(str + at, len - at)) { reflog_len = (len-1) - (at+2); len = at; @@ -460,19 +465,22 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) 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, &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); From 83d16bc7bec92ea3f705f559c5a13589cb7c1f92 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Tue, 7 May 2013 16:55:11 -0500 Subject: [PATCH 10/13] sha1_name: check @{-N} errors sooner It's trivial to check for them in the @{N} parsing loop. Signed-off-by: Ramkumar Ramachandra Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 45b89d9df0..446cc62c4e 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -448,7 +448,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) if (len && str[len-1] == '}') { for (at = len-4; at >= 0; at--) { if (str[at] == '@' && str[at+1] == '{') { - if (at == 0 && str[2] == '-') { + if (str[at+2] == '-') { + if (at != 0) + /* @{-N} not at start */ + return -1; nth_prior = 1; continue; } @@ -497,10 +500,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) 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]; From 7a0a49a7ca787433255280b66e7652b56f7beb6e Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 17:04:30 -0500 Subject: [PATCH 11/13] sha1_name: refactor reinterpret() This code essentially replaces part of ref with another ref, for example '@{-1}@{u}' is replaced with 'master@{u}', but this can be reused for other purposes other than nth prior checkouts. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 446cc62c4e..f60c951234 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -972,6 +972,27 @@ int get_sha1_mb(const char *name, unsigned char *sha1) return st; } +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 @@ -1005,25 +1026,8 @@ int interpret_branch_name(const char *name, struct strbuf *buf) 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 (0 < len) + return reinterpret(name, namelen, len, buf); cp = strchr(name, '@'); if (!cp) From cdfd94837b27c220f70f032b596ea993d195488f Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 7 May 2013 17:04:31 -0500 Subject: [PATCH 12/13] Add new @ shortcut for HEAD Typing 'HEAD' is tedious, especially when we can use '@' instead. The reason for choosing '@' is that it follows naturally from the ref@op syntax (e.g. HEAD@{u}), except we have no ref, and no operation, and when we don't have those, it makes sens to assume 'HEAD'. So now we can use 'git show @~1', and all that goody goodness. Until now '@' was a valid name, but it conflicts with this idea, so let's make it invalid. Probably very few people, if any, used this name. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- Documentation/git-check-ref-format.txt | 2 ++ Documentation/revisions.txt | 3 +++ refs.c | 4 ++++ sha1_name.c | 17 +++++++++++++++++ t/t1508-at-combinations.sh | 2 ++ 5 files changed, 28 insertions(+) diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt index ec1739a896..e8035ece42 100644 --- a/Documentation/git-check-ref-format.txt +++ b/Documentation/git-check-ref-format.txt @@ -54,6 +54,8 @@ Git imposes the following rules on how references are named: . They cannot contain a sequence `@{`. +. They cannot be the single character `@`. + . They cannot contain a `\`. These rules make it easy for shell script based tools to parse diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index d477b3f6bc..09896a37b1 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -58,6 +58,9 @@ the '$GIT_DIR/refs' directory or from the '$GIT_DIR/packed-refs' file. While the ref name encoding is unspecified, UTF-8 is preferred as some output processing may assume ref names in UTF-8. +'@':: + '@' alone is a shortcut for 'HEAD'. + '@\{\}', e.g. 'master@\{yesterday\}', 'HEAD@\{5 minutes ago\}':: A ref followed by the suffix '@' with a date specification enclosed in a brace diff --git a/refs.c b/refs.c index de2d8eb866..4e70b3eca0 100644 --- a/refs.c +++ b/refs.c @@ -72,6 +72,10 @@ int check_refname_format(const char *refname, int flags) { int component_len, component_count = 0; + if (!strcmp(refname, "@")) + /* Refname is a single character '@'. */ + return -1; + while (1) { /* We are at the start of a path component. */ component_len = check_refname_component(refname, flags); diff --git a/sha1_name.c b/sha1_name.c index f60c951234..4428df19a7 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -972,6 +972,17 @@ int get_sha1_mb(const char *name, unsigned char *sha1) 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 */ @@ -1032,9 +1043,15 @@ int interpret_branch_name(const char *name, struct strbuf *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); diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh index e5aea3b896..4db1613f8a 100755 --- a/t/t1508-at-combinations.sh +++ b/t/t1508-at-combinations.sh @@ -55,6 +55,8 @@ check "HEAD@{u}" ref refs/heads/upstream-branch check "@{u}@{1}" commit upstream-one check "@{-1}@{u}" ref refs/heads/master check "@{-1}@{u}@{1}" commit master-one +check "@" commit new-two +check "@@{u}" ref refs/heads/upstream-branch nonsense "@{u}@{-1}" nonsense "@{0}@{0}" nonsense "@{1}@{u}" From 1f27e7d56b65fdc232a5a2a4de2ee97ff5eb176b Mon Sep 17 00:00:00 2001 From: Felipe Contreras Date: Tue, 30 Apr 2013 16:49:11 -0500 Subject: [PATCH 13/13] sha1_name: compare variable with constant, not constant with variable And restructure the if/else to factor out the common "is len positive?" test into a single conditional. Signed-off-by: Felipe Contreras Signed-off-by: Junio C Hamano --- sha1_name.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 4428df19a7..5a58720d85 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1033,12 +1033,14 @@ int interpret_branch_name(const char *name, struct strbuf *buf) 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) - return reinterpret(name, namelen, len, buf); + } else if (len > 0) { + if (len == namelen) + return len; /* consumed all */ + else + return reinterpret(name, namelen, len, buf); + } cp = strchr(name, '@'); if (!cp)