From 54a8ad925cfac90bb4141c9904b1f97f0c5b83d4 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Jun 2007 23:22:58 -0700 Subject: [PATCH 1/6] remote.c: refactor match_explicit_refs() This does not change functionality; just splits one block that is deeply nested and indented out of a huge loop into a separate function. Signed-off-by: Junio C Hamano --- remote.c | 171 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 92 insertions(+), 79 deletions(-) diff --git a/remote.c b/remote.c index 33c8e5055b..b53130fb56 100644 --- a/remote.c +++ b/remote.c @@ -406,90 +406,98 @@ static struct ref *try_explicit_object_name(const char *name) return ref; } +static int match_explicit(struct ref *src, struct ref *dst, + struct ref ***dst_tail, + struct refspec *rs, + int errs) +{ + struct ref *matched_src, *matched_dst; + + const char *dst_value = rs->dst; + + if (rs->pattern) + return errs; + + if (dst_value == NULL) + dst_value = rs->src; + + matched_src = matched_dst = NULL; + switch (count_refspec_match(rs->src, src, &matched_src)) { + case 1: + break; + case 0: + /* The source could be in the get_sha1() format + * not a reference name. :refs/other is a + * way to delete 'other' ref at the remote end. + */ + matched_src = try_explicit_object_name(rs->src); + if (matched_src) + break; + errs = 1; + error("src refspec %s does not match any.", + rs->src); + break; + default: + errs = 1; + error("src refspec %s matches more than one.", + rs->src); + break; + } + switch (count_refspec_match(dst_value, dst, &matched_dst)) { + case 1: + break; + case 0: + if (!memcmp(dst_value, "refs/", 5)) { + int len = strlen(dst_value) + 1; + matched_dst = xcalloc(1, sizeof(*dst) + len); + memcpy(matched_dst->name, dst_value, len); + link_dst_tail(matched_dst, dst_tail); + } + else if (!strcmp(rs->src, dst_value) && + matched_src) { + /* pushing "master:master" when + * remote does not have master yet. + */ + int len = strlen(matched_src->name) + 1; + matched_dst = xcalloc(1, sizeof(*dst) + len); + memcpy(matched_dst->name, matched_src->name, + len); + link_dst_tail(matched_dst, dst_tail); + } + else { + errs = 1; + error("dst refspec %s does not match any " + "existing ref on the remote and does " + "not start with refs/.", dst_value); + } + break; + default: + errs = 1; + error("dst refspec %s matches more than one.", + dst_value); + break; + } + if (errs) + return errs; + if (matched_dst->peer_ref) { + errs = 1; + error("dst ref %s receives from more than one src.", + matched_dst->name); + } + else { + matched_dst->peer_ref = matched_src; + matched_dst->force = rs->force; + } + return errs; +} + static int match_explicit_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, struct refspec *rs, int rs_nr) { int i, errs; - for (i = errs = 0; i < rs_nr; i++) { - struct ref *matched_src, *matched_dst; - - const char *dst_value = rs[i].dst; - - if (rs[i].pattern) - continue; - - if (dst_value == NULL) - dst_value = rs[i].src; - - matched_src = matched_dst = NULL; - switch (count_refspec_match(rs[i].src, src, &matched_src)) { - case 1: - break; - case 0: - /* The source could be in the get_sha1() format - * not a reference name. :refs/other is a - * way to delete 'other' ref at the remote end. - */ - matched_src = try_explicit_object_name(rs[i].src); - if (matched_src) - break; - errs = 1; - error("src refspec %s does not match any.", - rs[i].src); - break; - default: - errs = 1; - error("src refspec %s matches more than one.", - rs[i].src); - break; - } - switch (count_refspec_match(dst_value, dst, &matched_dst)) { - case 1: - break; - case 0: - if (!memcmp(dst_value, "refs/", 5)) { - int len = strlen(dst_value) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, dst_value, len); - link_dst_tail(matched_dst, dst_tail); - } - else if (!strcmp(rs[i].src, dst_value) && - matched_src) { - /* pushing "master:master" when - * remote does not have master yet. - */ - int len = strlen(matched_src->name) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, matched_src->name, - len); - link_dst_tail(matched_dst, dst_tail); - } - else { - errs = 1; - error("dst refspec %s does not match any " - "existing ref on the remote and does " - "not start with refs/.", dst_value); - } - break; - default: - errs = 1; - error("dst refspec %s matches more than one.", - dst_value); - break; - } - if (errs) - continue; - if (matched_dst->peer_ref) { - errs = 1; - error("dst ref %s receives from more than one src.", - matched_dst->name); - } - else { - matched_dst->peer_ref = matched_src; - matched_dst->force = rs[i].force; - } - } + for (i = errs = 0; i < rs_nr; i++) + errs |= match_explicit(src, dst, dst_tail, &rs[i], errs); return -errs; } @@ -513,6 +521,11 @@ static const struct refspec *check_pattern_match(const struct refspec *rs, return NULL; } +/* + * Note. This is used only by "push"; refspec matching rules for + * push and fetch are subtly different, so do not try to reuse it + * without thinking. + */ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, int nr_refspec, char **refspec, int all) { From 163f0ee5ad0a5cb7d862431479c270ae3fef79cf Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 00:07:34 -0700 Subject: [PATCH 2/6] remote.c: refactor creation of new dst ref This refactors open-coded sequence to create a new "struct ref" and link it to the tail of dst list into a new function. Signed-off-by: Junio C Hamano --- remote.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/remote.c b/remote.c index b53130fb56..f469fb34e4 100644 --- a/remote.c +++ b/remote.c @@ -406,6 +406,18 @@ static struct ref *try_explicit_object_name(const char *name) return ref; } +static struct ref *make_dst(const char *name, struct ref ***dst_tail) +{ + struct ref *dst; + size_t len; + + len = strlen(name) + 1; + dst = xcalloc(1, sizeof(*dst) + len); + memcpy(dst->name, name, len); + link_dst_tail(dst, dst_tail); + return dst; +} + static int match_explicit(struct ref *src, struct ref *dst, struct ref ***dst_tail, struct refspec *rs, @@ -447,23 +459,13 @@ static int match_explicit(struct ref *src, struct ref *dst, case 1: break; case 0: - if (!memcmp(dst_value, "refs/", 5)) { - int len = strlen(dst_value) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, dst_value, len); - link_dst_tail(matched_dst, dst_tail); - } - else if (!strcmp(rs->src, dst_value) && - matched_src) { + if (!memcmp(dst_value, "refs/", 5)) + matched_dst = make_dst(dst_value, dst_tail); + else if (!strcmp(rs->src, dst_value) && matched_src) /* pushing "master:master" when * remote does not have master yet. */ - int len = strlen(matched_src->name) + 1; - matched_dst = xcalloc(1, sizeof(*dst) + len); - memcpy(matched_dst->name, matched_src->name, - len); - link_dst_tail(matched_dst, dst_tail); - } + matched_dst = make_dst(matched_src->name, dst_tail); else { errs = 1; error("dst refspec %s does not match any " @@ -567,11 +569,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail, goto free_name; if (!dst_peer) { /* Create a new one and link it */ - int len = strlen(dst_name) + 1; - dst_peer = xcalloc(1, sizeof(*dst_peer) + len); - memcpy(dst_peer->name, dst_name, len); + dst_peer = make_dst(dst_name, dst_tail); hashcpy(dst_peer->new_sha1, src->new_sha1); - link_dst_tail(dst_peer, dst_tail); } dst_peer->peer_ref = src; free_name: From 3c8b7df1ba6dd2093672afc460fd5de0e755f162 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 00:14:04 -0700 Subject: [PATCH 3/6] remote.c: minor clean-up of match_explicit() When checking what ref the source refspec matches, we have no business setting the default for the destination, so move that code lower. Also simplify the result from the code block that matches the source side by making it set matched_src only upon unambiguous match. Signed-off-by: Junio C Hamano --- remote.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/remote.c b/remote.c index f469fb34e4..30abdbb4d9 100644 --- a/remote.c +++ b/remote.c @@ -430,9 +430,6 @@ static int match_explicit(struct ref *src, struct ref *dst, if (rs->pattern) return errs; - if (dst_value == NULL) - dst_value = rs->src; - matched_src = matched_dst = NULL; switch (count_refspec_match(rs->src, src, &matched_src)) { case 1: @@ -445,16 +442,22 @@ static int match_explicit(struct ref *src, struct ref *dst, matched_src = try_explicit_object_name(rs->src); if (matched_src) break; - errs = 1; error("src refspec %s does not match any.", rs->src); break; default: - errs = 1; + matched_src = NULL; error("src refspec %s matches more than one.", rs->src); break; } + + if (!matched_src) + errs = 1; + + if (dst_value == NULL) + dst_value = rs->src; + switch (count_refspec_match(dst_value, dst, &matched_dst)) { case 1: break; @@ -466,21 +469,19 @@ static int match_explicit(struct ref *src, struct ref *dst, * remote does not have master yet. */ matched_dst = make_dst(matched_src->name, dst_tail); - else { - errs = 1; + else error("dst refspec %s does not match any " "existing ref on the remote and does " "not start with refs/.", dst_value); - } break; default: - errs = 1; + matched_dst = NULL; error("dst refspec %s matches more than one.", dst_value); break; } - if (errs) - return errs; + if (errs || matched_dst == NULL) + return 1; if (matched_dst->peer_ref) { errs = 1; error("dst ref %s receives from more than one src.", From 6125796f7d6e8b84431f92c13d7e79bd30f94f53 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 01:23:53 -0700 Subject: [PATCH 4/6] remote.c: fix "git push" weak match disambiguation When "git push A:B" is given, and A (or B) is not a full refname that begins with refs/, we require an unambiguous match with an existing ref. For this purpose, a match with a local branch or a tag (i.e. refs/heads/A and refs/tags/A) is called a "strong match", and any other match is called a "weak match". A partial refname is unambiguous when there is only one strong match with any number of weak matches, or when there is only one weak match and no other match. However, as reported by Sparse with Ramsay Jones recently, count_refspec_match() function had a bug where a variable in an inner block masked a different variable of the same name, which caused the weak matches to be ignored. This fixes it, and adds tests for the fix. Signed-off-by: Junio C Hamano --- remote.c | 1 - t/t5516-fetch-push.sh | 112 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 30abdbb4d9..120df36be0 100644 --- a/remote.c +++ b/remote.c @@ -333,7 +333,6 @@ static int count_refspec_match(const char *pattern, for (weak_match = match = 0; refs; refs = refs->next) { char *name = refs->name; int namelen = strlen(name); - int weak_match; if (namelen < patlen || memcmp(name + namelen - patlen, pattern, patlen)) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index dba018f667..b3b57faf9c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -15,12 +15,58 @@ mk_empty () { ) } +mk_test () { + mk_empty && + ( + for ref in "$@" + do + git push testrepo $the_first_commit:refs/$ref || { + echo "Oops, push refs/$ref failure" + exit 1 + } + done && + cd testrepo && + for ref in "$@" + do + r=$(git show-ref -s --verify refs/$ref) && + test "z$r" = "z$the_first_commit" || { + echo "Oops, refs/$ref is wrong" + exit 1 + } + done && + git fsck --full + ) +} + +check_push_result () { + ( + cd testrepo && + it="$1" && + shift + for ref in "$@" + do + r=$(git show-ref -s --verify refs/$ref) && + test "z$r" = "z$it" || { + echo "Oops, refs/$ref is wrong" + exit 1 + } + done && + git fsck --full + ) +} + test_expect_success setup ' : >path1 && git add path1 && test_tick && git commit -a -m repo && + the_first_commit=$(git show-ref -s --verify refs/heads/master) && + + : >path2 && + git add path2 && + test_tick && + git commit -a -m second && the_commit=$(git show-ref -s --verify refs/heads/master) ' @@ -79,4 +125,70 @@ test_expect_success 'push with wildcard' ' ) ' +test_expect_success 'push with matching heads' ' + + mk_test heads/master && + git push testrepo && + check_push_result $the_commit heads/master + +' + +test_expect_success 'push with no ambiguity (1)' ' + + mk_test heads/master && + git push testrepo master:master && + check_push_result $the_commit heads/master + +' + +test_expect_success 'push with no ambiguity (2)' ' + + mk_test remotes/origin/master && + git push testrepo master:master && + check_push_result $the_commit remotes/origin/master + +' + +test_expect_success 'push with weak ambiguity (1)' ' + + mk_test heads/master remotes/origin/master && + git push testrepo master:master && + check_push_result $the_commit heads/master && + check_push_result $the_first_commit remotes/origin/master + +' + +test_expect_success 'push with weak ambiguity (2)' ' + + mk_test heads/master remotes/origin/master remotes/another/master && + git push testrepo master:master && + check_push_result $the_commit heads/master && + check_push_result $the_first_commit remotes/origin/master remotes/another/master + +' + +test_expect_success 'push with ambiguity (1)' ' + + mk_test remotes/origin/master remotes/frotz/master && + if git push testrepo master:master + then + echo "Oops, should have failed" + false + else + check_push_result $the_first_commit remotes/origin/master remotes/frotz/master + fi +' + +test_expect_success 'push with ambiguity (2)' ' + + mk_test heads/frotz tags/frotz && + if git push testrepo master:frotz + then + echo "Oops, should have failed" + false + else + check_push_result $the_first_commit heads/frotz tags/frotz + fi +' + test_done From 1ed10b886bc69c129c06772ee4310c00e001657f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 01:37:14 -0700 Subject: [PATCH 5/6] remote.c: "git-push frotz" should update what matches at the source. Earlier, when the local repository has a branch "frotz" and the remote repository has a tag "frotz" (but not branch "frotz"), "git-push frotz" mistakenly updated the tag at the remote side. This was because the partial refname matching code was applied independently on both source and destination side. With this fix, when a colon-less refspec is given to git-push, we first match it with the refs in the source repository, and update the matching ref in the destination repository. Signed-off-by: Junio C Hamano --- remote.c | 7 +----- t/t5516-fetch-push.sh | 52 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/remote.c b/remote.c index 120df36be0..754d5130e3 100644 --- a/remote.c +++ b/remote.c @@ -455,7 +455,7 @@ static int match_explicit(struct ref *src, struct ref *dst, errs = 1; if (dst_value == NULL) - dst_value = rs->src; + dst_value = matched_src->name; switch (count_refspec_match(dst_value, dst, &matched_dst)) { case 1: @@ -463,11 +463,6 @@ static int match_explicit(struct ref *src, struct ref *dst, case 0: if (!memcmp(dst_value, "refs/", 5)) matched_dst = make_dst(dst_value, dst_tail); - else if (!strcmp(rs->src, dst_value) && matched_src) - /* pushing "master:master" when - * remote does not have master yet. - */ - matched_dst = make_dst(matched_src->name, dst_tail); else error("dst refspec %s does not match any " "existing ref on the remote and does " diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index b3b57faf9c..08d58e1c8c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -189,6 +189,58 @@ test_expect_success 'push with ambiguity (2)' ' else check_push_result $the_first_commit heads/frotz tags/frotz fi + +' + +test_expect_success 'push with colon-less refspec (1)' ' + + mk_test heads/frotz tags/frotz && + git branch -f frotz master && + git push testrepo frotz && + check_push_result $the_commit heads/frotz && + check_push_result $the_first_commit tags/frotz + +' + +test_expect_success 'push with colon-less refspec (2)' ' + + mk_test heads/frotz tags/frotz && + if git show-ref --verify -q refs/heads/frotz + then + git branch -D frotz + fi && + git tag -f frotz && + git push testrepo frotz && + check_push_result $the_commit tags/frotz && + check_push_result $the_first_commit heads/frotz + +' + +test_expect_success 'push with colon-less refspec (3)' ' + + mk_test && + if git show-ref --verify -q refs/tags/frotz + then + git tag -d frotz + fi && + git branch -f frotz master && + git push testrepo frotz && + check_push_result $the_commit heads/frotz && + test "$( cd testrepo && git show-ref | wc -l )" = 1 +' + +test_expect_success 'push with colon-less refspec (4)' ' + + mk_test && + if git show-ref --verify -q refs/heads/frotz + then + git branch -D frotz + fi && + git tag -f frotz && + git push testrepo frotz && + check_push_result $the_commit tags/frotz && + test "$( cd testrepo && git show-ref | wc -l )" = 1 + ' test_done From bb9fca80ce27eeb5a29a9ef1d2b4447b28882e54 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 9 Jun 2007 11:01:23 -0700 Subject: [PATCH 6/6] git-push: Update description of refspecs and add examples Signed-off-by: Junio C Hamano --- Documentation/git-push.txt | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 366c5dbdce..665f6dc709 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -53,9 +53,8 @@ side are updated. + `tag ` means the same as `refs/tags/:refs/tags/`. + -A parameter without a colon is equivalent to -`:`, hence updates in the destination from -in the source. +A parameter without a colon pushes the from the source +repository to the destination repository under the same name. + Pushing an empty allows you to delete the ref from the remote repository. @@ -98,6 +97,26 @@ the remote repository. include::urls.txt[] + +Examples +-------- + +git push origin master:: + Find a ref that matches `master` in the source repository + (most likely, it would find `refs/heads/master`), and update + the same ref (e.g. `refs/heads/master`) in `origin` repository + with it. + +git push origin :experimental:: + Find a ref that matches `experimental` in the `origin` repository + (e.g. `refs/heads/experimental`), and delete it. + +git push origin master:satellite/master:: + Find a ref that matches `master` in the source repository + (most likely, it would find `refs/heads/master`), and update + the ref that matches `satellite/master` (most likely, it would + be `refs/remotes/satellite/master`) in `origin` repository with it. + Author ------ Written by Junio C Hamano , later rewritten in C