From 47d996a20c3347bb9efbb44e8ed2d615cfdffba3 Mon Sep 17 00:00:00 2001 From: Steffen Prohaska Date: Sun, 11 Nov 2007 15:35:07 +0100 Subject: [PATCH 1/4] push: support pushing HEAD to real branch name This teaches "push HEAD" to resolve HEAD on the local side to its real branch name, e.g. master, and then act as if the real branch name was specified. So we have a shorthand for pushing the current branch. Besides HEAD, no other symbolic ref is resolved. Thanks to Daniel Barkalow for suggesting this implementation, which is much simpler than the implementation proposed before. Signed-off-by: Steffen Prohaska Signed-off-by: Junio C Hamano --- builtin-push.c | 9 +++++++++ t/t5516-fetch-push.sh | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/builtin-push.c b/builtin-push.c index 6d1da07c4..54fba0e83 100644 --- a/builtin-push.c +++ b/builtin-push.c @@ -44,6 +44,15 @@ static void set_refspecs(const char **refs, int nr) strcat(tag, refs[i]); ref = tag; } + if (!strcmp("HEAD", ref)) { + unsigned char sha1_dummy[20]; + ref = resolve_ref(ref, sha1_dummy, 1, NULL); + if (!ref) + die("HEAD cannot be resolved."); + if (prefixcmp(ref, "refs/heads/")) + die("HEAD cannot be resolved to branch."); + ref = xstrdup(ref + 11); + } add_refspec(ref); } } diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 86f9b5346..b0ff48816 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -244,6 +244,23 @@ test_expect_success 'push with colon-less refspec (4)' ' ' +test_expect_success 'push with HEAD' ' + + mk_test heads/master && + git checkout master && + git push testrepo HEAD && + check_push_result $the_commit heads/master + +' + +test_expect_success 'push with HEAD nonexisting at remote' ' + + mk_test heads/master && + git checkout -b local master && + git push testrepo HEAD && + check_push_result $the_commit heads/local +' + test_expect_success 'push with dry-run' ' mk_test heads/master && From 79803322c1d8d2f74e1a53d44f363d878180e0f5 Mon Sep 17 00:00:00 2001 From: Steffen Prohaska Date: Sun, 11 Nov 2007 15:01:46 +0100 Subject: [PATCH 2/4] add refname_match() We use at least two rulesets for matching abbreviated refnames with full refnames (starting with 'refs/'). git-rev-parse and git-fetch use slightly different rules. This commit introduces a new function refname_match (const char *abbrev_name, const char *full_name, const char **rules). abbrev_name is expanded using the rules and matched against full_name. If a match is found the function returns true. rules is a NULL-terminate list of format patterns with "%.*s", for example: const char *ref_rev_parse_rules[] = { "%.*s", "refs/%.*s", "refs/tags/%.*s", "refs/heads/%.*s", "refs/remotes/%.*s", "refs/remotes/%.*s/HEAD", NULL }; Asterisks are included in the format strings because this is the form required in sha1_name.c. Sharing the list with the functions there is a good idea to avoid duplicating the rules. Hopefully this facilitates unified matching rules in the future. This commit makes the rules used by rev-parse for resolving refs to sha1s available for string comparison. Before this change, the rules were buried in get_sha1*() and dwim_ref(). A follow-up commit will refactor the rules used by fetch. refname_match() will be used for matching refspecs in git-send-pack. Thanks to Daniel Barkalow for pointing out that ref_matches_abbrev in remote.c solves a similar problem and care should be taken to avoid confusion. Signed-off-by: Steffen Prohaska Signed-off-by: Junio C Hamano --- cache.h | 3 +++ refs.c | 24 ++++++++++++++++++++++++ sha1_name.c | 14 ++------------ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/cache.h b/cache.h index 33ebccf48..38d9a285e 100644 --- a/cache.h +++ b/cache.h @@ -415,6 +415,9 @@ extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int * extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref); extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref); +extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules); +extern const char *ref_rev_parse_rules[]; + extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg); extern int validate_headref(const char *ref); diff --git a/refs.c b/refs.c index ae532540c..fc26a93cb 100644 --- a/refs.c +++ b/refs.c @@ -643,6 +643,30 @@ int check_ref_format(const char *ref) } } +const char *ref_rev_parse_rules[] = { + "%.*s", + "refs/%.*s", + "refs/tags/%.*s", + "refs/heads/%.*s", + "refs/remotes/%.*s", + "refs/remotes/%.*s/HEAD", + NULL +}; + +int refname_match(const char *abbrev_name, const char *full_name, const char **rules) +{ + const char **p; + const int abbrev_name_len = strlen(abbrev_name); + + for (p = rules; *p; p++) { + if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) { + return 1; + } + } + + return 0; +} + static struct ref_lock *verify_lock(struct ref_lock *lock, const unsigned char *old_sha1, int mustexist) { diff --git a/sha1_name.c b/sha1_name.c index 2d727d54d..d364244dc 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -239,23 +239,13 @@ static int ambiguous_path(const char *path, int len) return slash; } -static const char *ref_fmt[] = { - "%.*s", - "refs/%.*s", - "refs/tags/%.*s", - "refs/heads/%.*s", - "refs/remotes/%.*s", - "refs/remotes/%.*s/HEAD", - NULL -}; - int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref) { const char **p, *r; int refs_found = 0; *ref = NULL; - for (p = ref_fmt; *p; p++) { + for (p = ref_rev_parse_rules; *p; p++) { unsigned char sha1_from_ref[20]; unsigned char *this_result; @@ -277,7 +267,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log) int logs_found = 0; *log = NULL; - for (p = ref_fmt; *p; p++) { + for (p = ref_rev_parse_rules; *p; p++) { struct stat st; unsigned char hash[20]; char path[PATH_MAX]; From ae36bdcf5147b1b54de852eda111ad76a3040726 Mon Sep 17 00:00:00 2001 From: Steffen Prohaska Date: Sun, 11 Nov 2007 15:01:47 +0100 Subject: [PATCH 3/4] push: use same rules as git-rev-parse to resolve refspecs This commit changes the rules for resolving refspecs to match the rules for resolving refs in rev-parse. git-rev-parse uses clear rules to resolve a short ref to its full name, which are well documented. The rules for resolving refspecs documented in git-send-pack were less strict and harder to understand. This commit replaces them by the rules of git-rev-parse. The unified rules are easier to understand and better resolve ambiguous cases. You can now push from a repository containing several branches ending on the same short name. Note, this may break existing setups. For example, "master" will no longer resolve to "origin/master" even when there is no other "master" elsewhere. Signed-off-by: Steffen Prohaska Signed-off-by: Junio C Hamano --- Documentation/git-send-pack.txt | 4 +++- remote.c | 5 +---- t/t5516-fetch-push.sh | 12 +++++++++++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt index 2fa01d4a3..a2d9cb61b 100644 --- a/Documentation/git-send-pack.txt +++ b/Documentation/git-send-pack.txt @@ -85,7 +85,9 @@ Each pattern pair consists of the source side (before the colon) and the destination side (after the colon). The ref to be pushed is determined by finding a match that matches the source side, and where it is pushed is determined by using the -destination side. +destination side. The rules used to match a ref are the same +rules used by gitlink:git-rev-parse[1] to resolve a symbolic ref +name. - It is an error if does not match exactly one of the local refs. diff --git a/remote.c b/remote.c index bec2ba1ad..4085c517e 100644 --- a/remote.c +++ b/remote.c @@ -519,10 +519,7 @@ static int count_refspec_match(const char *pattern, char *name = refs->name; int namelen = strlen(name); - if (namelen < patlen || - memcmp(name + namelen - patlen, pattern, patlen)) - continue; - if (namelen != patlen && name[namelen - patlen - 1] != '/') + if (!refname_match(pattern, name, ref_rev_parse_rules)) continue; /* A match is "weak" if it is with refs outside diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index b0ff48816..fd5f284e9 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -145,11 +145,21 @@ test_expect_success 'push with no ambiguity (1)' ' test_expect_success 'push with no ambiguity (2)' ' mk_test remotes/origin/master && - git push testrepo master:master && + git push testrepo master:origin/master && check_push_result $the_commit remotes/origin/master ' +test_expect_success 'push with colon-less refspec, no ambiguity' ' + + mk_test heads/master heads/t/master && + git branch -f t/master master && + git push testrepo master && + check_push_result $the_commit heads/master && + check_push_result $the_first_commit heads/t/master + +' + test_expect_success 'push with weak ambiguity (1)' ' mk_test heads/master remotes/origin/master && From 605b4978a105e2f40a353513f616be7d20f91c15 Mon Sep 17 00:00:00 2001 From: Steffen Prohaska Date: Sun, 11 Nov 2007 15:01:48 +0100 Subject: [PATCH 4/4] refactor fetch's ref matching to use refname_match() The old rules used by fetch were coded as a series of ifs. The old rules are: 1) match full refname if it starts with "refs/" or matches "HEAD" 2) verify that full refname starts with "refs/" 3) match abbreviated name in "refs/" if it starts with "heads/", "tags/", or "remotes/". 4) match abbreviated name in "refs/heads/" This is replaced by the new rules a) match full refname b) match abbreviated name prefixed with "refs/" c) match abbreviated name prefixed with "refs/heads/" The details of the new rules are different from the old rules. We no longer verify that the full refname starts with "refs/". The new rule (a) matches any full string. The old rules (1) and (2) were stricter. Now, the caller is responsible for using sensible full refnames. This should be the case for the current code. The new rule (b) is less strict than old rule (3). The new rule accepts abbreviated names that start with a non-standard prefix below "refs/". Despite this modifications the new rules should handle all cases as expected. Two tests are added to verify that fetch does not resolve short tags or HEAD in remotes. We may even think about loosening the rules a bit more and unify them with the rev-parse rules. This would be done by replacing ref_ref_fetch_rules with ref_ref_parse_rules. Note, the two new test would break. Signed-off-by: Steffen Prohaska Signed-off-by: Junio C Hamano --- cache.h | 1 + refs.c | 7 +++++++ remote.c | 23 ++--------------------- t/t5510-fetch.sh | 25 +++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/cache.h b/cache.h index 38d9a285e..cb8f3cabb 100644 --- a/cache.h +++ b/cache.h @@ -417,6 +417,7 @@ extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref); extern int refname_match(const char *abbrev_name, const char *full_name, const char **rules); extern const char *ref_rev_parse_rules[]; +extern const char *ref_fetch_rules[]; extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg); extern int validate_headref(const char *ref); diff --git a/refs.c b/refs.c index fc26a93cb..6a04a667b 100644 --- a/refs.c +++ b/refs.c @@ -653,6 +653,13 @@ const char *ref_rev_parse_rules[] = { NULL }; +const char *ref_fetch_rules[] = { + "%.*s", + "refs/%.*s", + "refs/heads/%.*s", + NULL +}; + int refname_match(const char *abbrev_name, const char *full_name, const char **rules) { const char **p; diff --git a/remote.c b/remote.c index 4085c517e..48812a713 100644 --- a/remote.c +++ b/remote.c @@ -417,25 +417,6 @@ int remote_has_url(struct remote *remote, const char *url) return 0; } -/* - * Returns true if, under the matching rules for fetching, name is the - * same as the given full name. - */ -static int ref_matches_abbrev(const char *name, const char *full) -{ - if (!prefixcmp(name, "refs/") || !strcmp(name, "HEAD")) - return !strcmp(name, full); - if (prefixcmp(full, "refs/")) - return 0; - if (!prefixcmp(name, "heads/") || - !prefixcmp(name, "tags/") || - !prefixcmp(name, "remotes/")) - return !strcmp(name, full + 5); - if (prefixcmp(full + 5, "heads/")) - return 0; - return !strcmp(full + 11, name); -} - int remote_find_tracking(struct remote *remote, struct refspec *refspec) { int find_src = refspec->src == NULL; @@ -804,7 +785,7 @@ int branch_merge_matches(struct branch *branch, { if (!branch || i < 0 || i >= branch->merge_nr) return 0; - return ref_matches_abbrev(branch->merge[i]->src, refname); + return refname_match(branch->merge[i]->src, refname, ref_fetch_rules); } static struct ref *get_expanded_map(struct ref *remote_refs, @@ -843,7 +824,7 @@ static struct ref *find_ref_by_name_abbrev(struct ref *refs, const char *name) { struct ref *ref; for (ref = refs; ref; ref = ref->next) { - if (ref_matches_abbrev(name, ref->name)) + if (refname_match(name, ref->name, ref_fetch_rules)) return ref; } return NULL; diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index aad863db7..20257428e 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -95,6 +95,31 @@ test_expect_success 'fetch following tags' ' ' +test_expect_failure 'fetch must not resolve short tag name' ' + + cd "$D" && + + mkdir five && + cd five && + git init && + + git fetch .. anno:five + +' + +test_expect_failure 'fetch must not resolve short remote name' ' + + cd "$D" && + git-update-ref refs/remotes/six/HEAD HEAD + + mkdir six && + cd six && + git init && + + git fetch .. six:six + +' + test_expect_success 'create bundle 1' ' cd "$D" && echo >file updated again by origin &&