All of lore.kernel.org
 help / color / mirror / Atom feed
* change push's refspec behavior to match rev-parse
@ 2007-10-14  8:54 Steffen Prohaska
  2007-10-14  8:54 ` [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
  2007-10-14  9:04 ` change push's refspec behavior to match rev-parse Steffen Prohaska
  0 siblings, 2 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git

This patch series addresses recent complaints about the behavior of push/send-pack
when expanding short refspecs. The overall idea is to change push's handling of
refspecs to match the behavior of rev-parse.


The old way of matching short refspecs in push is often unexpected as
discussed in [1]. Now "git push <ref>" resolves ref the same way as rev-parse.

[1] http://marc.info/?l=git&m=119224567631084&w=2


A related question is how to push only the current branch [2]. Now
"git push HEAD" is supported to push the current head if a matching remote
ref exists.

[2] http://marc.info/?l=git&m=119089831513994&w=2

A summary of the patch series follows below.

    Steffen

 builtin-rev-parse.c   |   27 ++++++++++++-------
 cache.h               |    2 +
 remote.c              |   23 ++++++++++------
 sha1_name.c           |   51 ++++++++++++++++++++++++++++--------
 t/t5516-fetch-push.sh |   68 ++++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 138 insertions(+), 33 deletions(-)

 [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec

	This is a bug fix that should go to maint. All following patches modifying
        the push test script require this.

 [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand

	Is required by 3/6 and 4/6

 [PATCH 3/6] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

	A bit off-topic. It demonstrates the use of get_sha1_with_real_ref.

 [PATCH 4/6] push, send-pack: support pushing HEAD to real ref name

	Requires 1/6.

 [PATCH 5/6] add ref_cmp_full_short() comparing full ref name with a short name
 [PATCH 6/6] push, send-pack: use same rules as git-rev-parse to resolve refspecs

	Requires 1/6.

	Note, an updated documentation is not yet included. I like to first wait for
	comments.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec
  2007-10-14  8:54 change push's refspec behavior to match rev-parse Steffen Prohaska
@ 2007-10-14  8:54 ` Steffen Prohaska
  2007-10-14  8:54   ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Steffen Prohaska
  2007-10-14  9:04   ` [PATCH 1/6 v2] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
  2007-10-14  9:04 ` change push's refspec behavior to match rev-parse Steffen Prohaska
  1 sibling, 2 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

A push must fail if the remote ref does not yet exist and the refspec
does not start with refs/. Remote refs must explicitly be created with
their full name.

This commit adds some tests and fixes the existence check in send-pack.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 remote.c              |    4 ++--
 t/t5516-fetch-push.sh |   34 ++++++++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/remote.c b/remote.c
index bb774d0..36071b2 100644
--- a/remote.c
+++ b/remote.c
@@ -511,12 +511,12 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(dst_value, "refs/", 5))
+		if (!memcmp(rs->dst ? rs->dst : rs->src , "refs/", 5))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
 		else
 			error("dst refspec %s does not match any "
 			      "existing ref on the remote and does "
-			      "not start with refs/.", dst_value);
+			      "not start with refs/.", rs->dst ? rs->dst : rs->src);
 		break;
 	default:
 		matched_dst = NULL;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ca46aaf..8629cf2 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -126,6 +126,36 @@ test_expect_success 'push with wildcard' '
 	)
 '
 
+test_expect_success 'push nonexisting (1)' '
+
+	mk_test &&
+	if git push testrepo master
+	then
+		echo "Oops, should have failed"
+		false
+	fi
+
+'
+
+test_expect_success 'push nonexisting (2)' '
+
+	mk_test &&
+	if git push testrepo heads/master
+	then
+		echo "Oops, should have failed"
+		false
+	fi
+
+'
+
+test_expect_success 'push nonexisting (3)' '
+
+	mk_test &&
+	git push testrepo refs/heads/master &&
+	check_push_result $the_commit heads/master
+
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&
@@ -225,7 +255,7 @@ test_expect_success 'push with colon-less refspec (3)' '
 		git tag -d frotz
 	fi &&
 	git branch -f frotz master &&
-	git push testrepo frotz &&
+	git push testrepo refs/heads/frotz &&
 	check_push_result $the_commit heads/frotz &&
 	test 1 = $( cd testrepo && git show-ref | wc -l )
 '
@@ -238,7 +268,7 @@ test_expect_success 'push with colon-less refspec (4)' '
 		git branch -D frotz
 	fi &&
 	git tag -f frotz &&
-	git push testrepo frotz &&
+	git push testrepo refs/tags/frotz &&
 	check_push_result $the_commit tags/frotz &&
 	test 1 = $( cd testrepo && git show-ref | wc -l )
 
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand
  2007-10-14  8:54 ` [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
@ 2007-10-14  8:54   ` Steffen Prohaska
  2007-10-14  8:54     ` [PATCH 3/6] rev-parse: teach "git rev-parse --symbolic" to print the full ref name Steffen Prohaska
  2007-10-14 17:21     ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Johannes Schindelin
  2007-10-14  9:04   ` [PATCH 1/6 v2] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
  1 sibling, 2 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

Deep inside get_sha1() the name of the requested ref is matched
according to the rules documented in git-rev-parse. This patch
introduces a function that returns the full name of the matched
ref to the outside.

For example 'master' is typically returned as 'refs/heads/master'.

The new function can be used by "git rev-parse" to print the full
name of the matched ref and can be used by "git send-pack" to expand
a local ref to its full name.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 cache.h     |    1 +
 sha1_name.c |   38 +++++++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index e0abcd6..f98d57a 100644
--- a/cache.h
+++ b/cache.h
@@ -401,6 +401,7 @@ static inline unsigned int hexval(unsigned char c)
 
 extern int get_sha1(const char *str, unsigned char *sha1);
 extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
+extern int get_sha1_with_real_ref(const char *str, unsigned char *sha1, char **real_ref);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
diff --git a/sha1_name.c b/sha1_name.c
index 2d727d5..b820909 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -306,7 +306,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	return logs_found;
 }
 
-static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
+static int get_sha1_basic(const char *str, int len, unsigned char *sha1, char **real_ref_out)
 {
 	static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
 	char *real_ref = NULL;
@@ -378,17 +378,21 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		}
 	}
 
-	free(real_ref);
+	if (real_ref_out) {
+		*real_ref_out = real_ref;
+	} else {
+		free(real_ref);
+	}
 	return 0;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1);
+static int get_sha1_1(const char *name, int len, unsigned char *sha1, char **real_ref);
 
 static int get_parent(const char *name, int len,
 		      unsigned char *result, int idx)
 {
 	unsigned char sha1[20];
-	int ret = get_sha1_1(name, len, sha1);
+	int ret = get_sha1_1(name, len, sha1, /*real_ref=*/ 0);
 	struct commit *commit;
 	struct commit_list *p;
 
@@ -418,7 +422,7 @@ static int get_nth_ancestor(const char *name, int len,
 			    unsigned char *result, int generation)
 {
 	unsigned char sha1[20];
-	int ret = get_sha1_1(name, len, sha1);
+	int ret = get_sha1_1(name, len, sha1, /*real_ref=*/ 0);
 	if (ret)
 		return ret;
 
@@ -471,7 +475,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
 	else
 		return -1;
 
-	if (get_sha1_1(name, sp - name - 2, outer))
+	if (get_sha1_1(name, sp - name - 2, outer, /*real_ref=*/ 0))
 		return -1;
 
 	o = parse_object(outer);
@@ -531,7 +535,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)
 	return -1;
 }
 
-static int get_sha1_1(const char *name, int len, unsigned char *sha1)
+static int get_sha1_1(const char *name, int len, unsigned char *sha1, char **real_ref)
 {
 	int ret, has_suffix;
 	const char *cp;
@@ -569,7 +573,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1)
 	if (!ret)
 		return 0;
 
-	ret = get_sha1_basic(name, len, sha1);
+	ret = get_sha1_basic(name, len, sha1, real_ref);
 	if (!ret)
 		return 0;
 
@@ -651,14 +655,14 @@ int get_sha1(const char *name, unsigned char *sha1)
 	return get_sha1_with_mode(name, sha1, &unused);
 }
 
-int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
+int get_sha1_with_mode_real_ref(const char *name, unsigned char *sha1, unsigned *mode, char** real_ref)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
 	const char *cp;
 
 	*mode = S_IFINVALID;
-	ret = get_sha1_1(name, namelen, sha1);
+	ret = get_sha1_1(name, namelen, sha1, real_ref);
 	if (!ret)
 		return ret;
 	/* sha1:path --> object name of path in ent sha1
@@ -709,9 +713,21 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
+		if (!get_sha1_1(name, cp-name, tree_sha1, real_ref))
 			return get_tree_entry(tree_sha1, cp+1, sha1,
 					      mode);
 	}
 	return ret;
 }
+
+int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
+{
+	return get_sha1_with_mode_real_ref(name, sha1, mode, 0);
+}
+
+int get_sha1_with_real_ref(const char *name, unsigned char *sha1, char **real_ref)
+{
+	unsigned unused;
+	return get_sha1_with_mode_real_ref(name, sha1, &unused, real_ref);
+}
+
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/6] rev-parse: teach "git rev-parse --symbolic" to print the full ref name
  2007-10-14  8:54   ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Steffen Prohaska
@ 2007-10-14  8:54     ` Steffen Prohaska
  2007-10-14  8:54       ` [PATCH 4/6] push, send-pack: support pushing HEAD to real " Steffen Prohaska
  2007-10-14 17:21     ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Johannes Schindelin
  1 sibling, 1 reply; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

"git rev-parse --symbolic" used to return the ref name as it was
specified on the command line. This is changed to returning the
full matched ref name, i.e. "git rev-parse --symbolic master"
now typically returns "refs/heads/master".

Note, this changes output of an established command. It might
break existing setups. I checked that it does not break scripts
in git.git.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 builtin-rev-parse.c |   27 +++++++++++++++++----------
 1 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 8d78b69..e64abeb 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -93,7 +93,7 @@ static void show(const char *arg)
 }
 
 /* Output a revision, only if filter allows it */
-static void show_rev(int type, const unsigned char *sha1, const char *name)
+static void show_rev(int type, const unsigned char *sha1, const char *name, const char* real_name)
 {
 	if (!(filter & DO_REVS))
 		return;
@@ -102,7 +102,9 @@ static void show_rev(int type, const unsigned char *sha1, const char *name)
 
 	if (type != show_type)
 		putchar('^');
-	if (symbolic && name)
+	if (symbolic && real_name)
+		show(real_name);
+	else if (symbolic && name)
 		show(name);
 	else if (abbrev)
 		show(find_unique_abbrev(sha1, abbrev));
@@ -131,7 +133,7 @@ static void show_default(void)
 
 		def = NULL;
 		if (!get_sha1(s, sha1)) {
-			show_rev(NORMAL, sha1, s);
+			show_rev(NORMAL, sha1, s, 0);
 			return;
 		}
 	}
@@ -139,7 +141,7 @@ static void show_default(void)
 
 static int show_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
-	show_rev(NORMAL, sha1, refname);
+	show_rev(NORMAL, sha1, refname, 0);
 	return 0;
 }
 
@@ -187,8 +189,8 @@ static int try_difference(const char *arg)
 	if (dotdot == arg)
 		this = "HEAD";
 	if (!get_sha1(this, sha1) && !get_sha1(next, end)) {
-		show_rev(NORMAL, end, next);
-		show_rev(symmetric ? NORMAL : REVERSED, sha1, this);
+		show_rev(NORMAL, end, next, 0);
+		show_rev(symmetric ? NORMAL : REVERSED, sha1, this, 0);
 		if (symmetric) {
 			struct commit_list *exclude;
 			struct commit *a, *b;
@@ -198,7 +200,7 @@ static int try_difference(const char *arg)
 			while (exclude) {
 				struct commit_list *n = exclude->next;
 				show_rev(REVERSED,
-					 exclude->item->object.sha1,NULL);
+					 exclude->item->object.sha1, NULL, 0);
 				free(exclude);
 				exclude = n;
 			}
@@ -213,6 +215,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 {
 	int i, as_is = 0, verify = 0;
 	unsigned char sha1[20];
+	char* real_name = 0;
 
 	git_config(git_default_config);
 
@@ -393,12 +396,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 		/* Not a flag argument */
 		if (try_difference(arg))
 			continue;
-		if (!get_sha1(arg, sha1)) {
-			show_rev(NORMAL, sha1, arg);
+		if (!get_sha1_with_real_ref(arg, sha1, &real_name)) {
+			show_rev(NORMAL, sha1, arg, real_name);
+			if(real_name) {
+				free(real_name);
+				real_name = 0;
+			}
 			continue;
 		}
 		if (*arg == '^' && !get_sha1(arg+1, sha1)) {
-			show_rev(REVERSED, sha1, arg+1);
+			show_rev(REVERSED, sha1, arg+1, 0);
 			continue;
 		}
 		as_is = 1;
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/6] push, send-pack: support pushing HEAD to real ref name
  2007-10-14  8:54     ` [PATCH 3/6] rev-parse: teach "git rev-parse --symbolic" to print the full ref name Steffen Prohaska
@ 2007-10-14  8:54       ` Steffen Prohaska
  2007-10-14  8:54         ` [PATCH 5/6] add ref_cmp_full_short() comparing full ref name with a short name Steffen Prohaska
  2007-10-14  9:05         ` [PATCH 4/6 v2] push, send-pack: support pushing HEAD to real ref name Steffen Prohaska
  0 siblings, 2 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

This teaches "push <remote> HEAD" to resolve HEAD on the local
side to its real ref name, e.g. refs/heads/master, and then
use the real ref name on the remote side to search a matching
remote ref.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 remote.c              |   18 +++++++++++++-----
 t/t5516-fetch-push.sh |   22 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index 36071b2..58bc019 100644
--- a/remote.c
+++ b/remote.c
@@ -439,6 +439,8 @@ static struct ref *try_explicit_object_name(const char *name)
 	unsigned char sha1[20];
 	struct ref *ref;
 	int len;
+	char *real_name = 0;
+	const char *best_name;
 
 	if (!*name) {
 		ref = alloc_ref(20);
@@ -446,12 +448,17 @@ static struct ref *try_explicit_object_name(const char *name)
 		hashclr(ref->new_sha1);
 		return ref;
 	}
-	if (get_sha1(name, sha1))
+	if (get_sha1_with_real_ref(name, sha1, &real_name))
 		return NULL;
-	len = strlen(name) + 1;
+	best_name = real_name ? real_name : name;
+	len = strlen(best_name) + 1;
 	ref = alloc_ref(len);
-	memcpy(ref->name, name, len);
+	memcpy(ref->name, best_name, len);
 	hashcpy(ref->new_sha1, sha1);
+
+	if (real_name) {
+		free(real_name);
+	}
 	return ref;
 }
 
@@ -475,6 +482,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	struct ref *matched_src, *matched_dst;
 
 	const char *dst_value = rs->dst;
+	const char * const orig_dst_value = rs->dst ? rs->dst : rs->src;
 
 	if (rs->pattern)
 		return errs;
@@ -511,12 +519,12 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(rs->dst ? rs->dst : rs->src , "refs/", 5))
+		if (!memcmp(orig_dst_value , "refs/", 5))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
 		else
 			error("dst refspec %s does not match any "
 			      "existing ref on the remote and does "
-			      "not start with refs/.", rs->dst ? rs->dst : rs->src);
+			      "not start with refs/.", orig_dst_value);
 		break;
 	default:
 		matched_dst = NULL;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8629cf2..97a032e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -274,4 +274,26 @@ test_expect_success 'push with colon-less refspec (4)' '
 
 '
 
+test_expect_success 'push with HEAD' '
+
+	mk_test heads/master &&
+	git push testrepo HEAD &&
+	check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with HEAD not existing at remote' '
+
+	mk_test heads/master &&
+	git checkout -b local master &&
+	if git push testrepo HEAD
+	then
+		echo "Oops, should have failed"
+		false
+	else
+		check_push_result $the_first_commit heads/master
+	fi
+
+'
+
 test_done
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/6] add ref_cmp_full_short() comparing full ref name with a short name
  2007-10-14  8:54       ` [PATCH 4/6] push, send-pack: support pushing HEAD to real " Steffen Prohaska
@ 2007-10-14  8:54         ` Steffen Prohaska
  2007-10-14  8:54           ` [PATCH 6/6] push, send-pack: use same rules as git-rev-parse to resolve refspecs Steffen Prohaska
  2007-10-14  9:05         ` [PATCH 4/6 v2] push, send-pack: support pushing HEAD to real ref name Steffen Prohaska
  1 sibling, 1 reply; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

ref_cmp_full_short(full_name, short_name) expands short_name according
to the rules documented in git-rev-parse and compares the expanded
name with full_name. It reports a match by returning 0.

This function makes the rules for resolving refs to sha1s available
for string comparison. Before this change, the rules were buried in
get_sha1*() and dwim_ref().

ref_cmp_full_short() will be used for matching refspecs in git-send-pack.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 cache.h     |    1 +
 sha1_name.c |   13 +++++++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index f98d57a..59345b5 100644
--- a/cache.h
+++ b/cache.h
@@ -406,6 +406,7 @@ extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
+extern int ref_cmp_full_short(const char *full_name, const char* short_name);
 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);
 
diff --git a/sha1_name.c b/sha1_name.c
index b820909..ae235be 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -249,6 +249,19 @@ static const char *ref_fmt[] = {
 	NULL
 };
 
+int ref_cmp_full_short(const char* full_name, const char* short_name)
+{
+	const char **p;
+    const int short_name_len = strlen(short_name);
+
+	for (p = ref_fmt; *p; p++) {
+		if (strcmp(full_name, mkpath(*p, short_name_len, short_name)) == 0) {
+            return 0;
+        }
+    }
+    return -1;
+}
+
 int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 {
 	const char **p, *r;
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/6] push, send-pack: use same rules as git-rev-parse to resolve refspecs
  2007-10-14  8:54         ` [PATCH 5/6] add ref_cmp_full_short() comparing full ref name with a short name Steffen Prohaska
@ 2007-10-14  8:54           ` Steffen Prohaska
  0 siblings, 0 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  8:54 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

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 breaks existing setups. For example "master" will no longer
resolve to "origin/master".

TODO: this patch does not yet include a modified documentation of
git-send-pack. I prefer to wait for some comments first.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 remote.c              |    5 +----
 t/t5516-fetch-push.sh |   12 +++++++++++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index 58bc019..09cb611 100644
--- a/remote.c
+++ b/remote.c
@@ -383,10 +383,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 (ref_cmp_full_short(name, pattern))
 			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 97a032e..2664060 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -175,11 +175,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 &&
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/6 v2] push, send-pack: fix test if remote branch exists for colon-less refspec
  2007-10-14  8:54 ` [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
  2007-10-14  8:54   ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Steffen Prohaska
@ 2007-10-14  9:04   ` Steffen Prohaska
  1 sibling, 0 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  9:04 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

A push must fail if the remote ref does not yet exist and the refspec
does not start with refs/. Remote refs must explicitly be created with
their full name.

This commit adds some tests and fixes the existence check in send-pack.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 remote.c              |    5 +++--
 t/t5516-fetch-push.sh |   34 ++++++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/remote.c b/remote.c
index bb774d0..f26f0e0 100644
--- a/remote.c
+++ b/remote.c
@@ -475,6 +475,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	struct ref *matched_src, *matched_dst;
 
 	const char *dst_value = rs->dst;
+	const char * const orig_dst_value = rs->dst ? rs->dst : rs->src;
 
 	if (rs->pattern)
 		return errs;
@@ -511,12 +512,12 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(dst_value, "refs/", 5))
+		if (!memcmp(orig_dst_value , "refs/", 5))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
 		else
 			error("dst refspec %s does not match any "
 			      "existing ref on the remote and does "
-			      "not start with refs/.", dst_value);
+			      "not start with refs/.", orig_dst_value);
 		break;
 	default:
 		matched_dst = NULL;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ca46aaf..8629cf2 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -126,6 +126,36 @@ test_expect_success 'push with wildcard' '
 	)
 '
 
+test_expect_success 'push nonexisting (1)' '
+
+	mk_test &&
+	if git push testrepo master
+	then
+		echo "Oops, should have failed"
+		false
+	fi
+
+'
+
+test_expect_success 'push nonexisting (2)' '
+
+	mk_test &&
+	if git push testrepo heads/master
+	then
+		echo "Oops, should have failed"
+		false
+	fi
+
+'
+
+test_expect_success 'push nonexisting (3)' '
+
+	mk_test &&
+	git push testrepo refs/heads/master &&
+	check_push_result $the_commit heads/master
+
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&
@@ -225,7 +255,7 @@ test_expect_success 'push with colon-less refspec (3)' '
 		git tag -d frotz
 	fi &&
 	git branch -f frotz master &&
-	git push testrepo frotz &&
+	git push testrepo refs/heads/frotz &&
 	check_push_result $the_commit heads/frotz &&
 	test 1 = $( cd testrepo && git show-ref | wc -l )
 '
@@ -238,7 +268,7 @@ test_expect_success 'push with colon-less refspec (4)' '
 		git branch -D frotz
 	fi &&
 	git tag -f frotz &&
-	git push testrepo frotz &&
+	git push testrepo refs/tags/frotz &&
 	check_push_result $the_commit tags/frotz &&
 	test 1 = $( cd testrepo && git show-ref | wc -l )
 
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: change push's refspec behavior to match rev-parse
  2007-10-14  8:54 change push's refspec behavior to match rev-parse Steffen Prohaska
  2007-10-14  8:54 ` [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
@ 2007-10-14  9:04 ` Steffen Prohaska
  1 sibling, 0 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  9:04 UTC (permalink / raw)
  To: Git Mailing List


On Oct 14, 2007, at 10:54 AM, Steffen Prohaska wrote:

>
>
>  [PATCH 1/6] push, send-pack: fix test if remote branch exists for  
> colon-less refspec
>
> 	This is a bug fix that should go to maint. All following patches  
> modifying
>         the push test script require this.
>
>  [PATCH 4/6] push, send-pack: support pushing HEAD to real ref name
>
> 	Requires 1/6.

These two patches got mixed. I'll send updated versions in a minute.

Sorry for the noise,
	Steffen

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/6 v2] push, send-pack: support pushing HEAD to real ref name
  2007-10-14  8:54       ` [PATCH 4/6] push, send-pack: support pushing HEAD to real " Steffen Prohaska
  2007-10-14  8:54         ` [PATCH 5/6] add ref_cmp_full_short() comparing full ref name with a short name Steffen Prohaska
@ 2007-10-14  9:05         ` Steffen Prohaska
  1 sibling, 0 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14  9:05 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska

This teaches "push <remote> HEAD" to resolve HEAD on the local
side to its real ref name, e.g. refs/heads/master, and then
use the real ref name on the remote side to search a matching
remote ref.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 remote.c              |   13 ++++++++++---
 t/t5516-fetch-push.sh |   22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index f26f0e0..58bc019 100644
--- a/remote.c
+++ b/remote.c
@@ -439,6 +439,8 @@ static struct ref *try_explicit_object_name(const char *name)
 	unsigned char sha1[20];
 	struct ref *ref;
 	int len;
+	char *real_name = 0;
+	const char *best_name;
 
 	if (!*name) {
 		ref = alloc_ref(20);
@@ -446,12 +448,17 @@ static struct ref *try_explicit_object_name(const char *name)
 		hashclr(ref->new_sha1);
 		return ref;
 	}
-	if (get_sha1(name, sha1))
+	if (get_sha1_with_real_ref(name, sha1, &real_name))
 		return NULL;
-	len = strlen(name) + 1;
+	best_name = real_name ? real_name : name;
+	len = strlen(best_name) + 1;
 	ref = alloc_ref(len);
-	memcpy(ref->name, name, len);
+	memcpy(ref->name, best_name, len);
 	hashcpy(ref->new_sha1, sha1);
+
+	if (real_name) {
+		free(real_name);
+	}
 	return ref;
 }
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8629cf2..97a032e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -274,4 +274,26 @@ test_expect_success 'push with colon-less refspec (4)' '
 
 '
 
+test_expect_success 'push with HEAD' '
+
+	mk_test heads/master &&
+	git push testrepo HEAD &&
+	check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with HEAD not existing at remote' '
+
+	mk_test heads/master &&
+	git checkout -b local master &&
+	if git push testrepo HEAD
+	then
+		echo "Oops, should have failed"
+		false
+	else
+		check_push_result $the_first_commit heads/master
+	fi
+
+'
+
 test_done
-- 
1.5.3.4.224.gc6b84

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand
  2007-10-14  8:54   ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Steffen Prohaska
  2007-10-14  8:54     ` [PATCH 3/6] rev-parse: teach "git rev-parse --symbolic" to print the full ref name Steffen Prohaska
@ 2007-10-14 17:21     ` Johannes Schindelin
  2007-10-14 21:37       ` Steffen Prohaska
  1 sibling, 1 reply; 12+ messages in thread
From: Johannes Schindelin @ 2007-10-14 17:21 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git

Hi,

On Sun, 14 Oct 2007, Steffen Prohaska wrote:

> Deep inside get_sha1() the name of the requested ref is matched
> according to the rules documented in git-rev-parse. This patch
> introduces a function that returns the full name of the matched
> ref to the outside.
> 
> For example 'master' is typically returned as 'refs/heads/master'.
> 
> The new function can be used by "git rev-parse" to print the full
> name of the matched ref and can be used by "git send-pack" to expand
> a local ref to its full name.

I have not really studies your patch, but from your description it sounds 
as if dwim_ref() does what you want.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand
  2007-10-14 17:21     ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Johannes Schindelin
@ 2007-10-14 21:37       ` Steffen Prohaska
  0 siblings, 0 replies; 12+ messages in thread
From: Steffen Prohaska @ 2007-10-14 21:37 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git


On Oct 14, 2007, at 7:21 PM, Johannes Schindelin wrote:

> Hi,
>
> On Sun, 14 Oct 2007, Steffen Prohaska wrote:
>
>> Deep inside get_sha1() the name of the requested ref is matched
>> according to the rules documented in git-rev-parse. This patch
>> introduces a function that returns the full name of the matched
>> ref to the outside.
>>
>> For example 'master' is typically returned as 'refs/heads/master'.
>>
>> The new function can be used by "git rev-parse" to print the full
>> name of the matched ref and can be used by "git send-pack" to expand
>> a local ref to its full name.
>
> I have not really studies your patch, but from your description it  
> sounds
> as if dwim_ref() does what you want.

Without my patch get_sha1_with_mode() calls get_sha1_1() calls  
get_sha1_basic()
calls dwim_ref(). I didn't analyze in detail what the *sha1*  
functions add
to what dwim_ref() provides.

The patch passes the information from dwim_ref() up the callstack.  
Maybe this
is not needed and I could directly call dwim_ref(). But I don't know.

	Steffen

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2007-10-14 21:36 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-14  8:54 change push's refspec behavior to match rev-parse Steffen Prohaska
2007-10-14  8:54 ` [PATCH 1/6] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
2007-10-14  8:54   ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Steffen Prohaska
2007-10-14  8:54     ` [PATCH 3/6] rev-parse: teach "git rev-parse --symbolic" to print the full ref name Steffen Prohaska
2007-10-14  8:54       ` [PATCH 4/6] push, send-pack: support pushing HEAD to real " Steffen Prohaska
2007-10-14  8:54         ` [PATCH 5/6] add ref_cmp_full_short() comparing full ref name with a short name Steffen Prohaska
2007-10-14  8:54           ` [PATCH 6/6] push, send-pack: use same rules as git-rev-parse to resolve refspecs Steffen Prohaska
2007-10-14  9:05         ` [PATCH 4/6 v2] push, send-pack: support pushing HEAD to real ref name Steffen Prohaska
2007-10-14 17:21     ` [PATCH 2/6] add get_sha1_with_real_ref() returning full name of ref on demand Johannes Schindelin
2007-10-14 21:37       ` Steffen Prohaska
2007-10-14  9:04   ` [PATCH 1/6 v2] push, send-pack: fix test if remote branch exists for colon-less refspec Steffen Prohaska
2007-10-14  9:04 ` change push's refspec behavior to match rev-parse Steffen Prohaska

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.