[PATCH 0/8 v2] improve push's refspec handling

[PATCH 0/8 v2] improve push's refspec handling

[Steffen Prohaska]

This patch series improves the refspec handling in push.

It is a replacement for the series in sp/push-refspec
(666df53d6868bf56ca6c9ed0a927d612c67fe68c).

The series addresses some issues that were recently discussed on the
mailing list.
- creating remote refs requires a more explicit command [1].
- the current branch can be pushed as "git push HEAD" [2].
- matching of refs use same rules as git rev-parse [3].
- annoying error messages when working with shared repos are supressed [4].

[1] http://marc.info/?l=git&m=119286893014690&w=2
[2] http://marc.info/?l=git&m=119089831513994&w=2
[3] http://marc.info/?l=git&m=119224567631084&w=2
[4] http://marc.info/?t=119305127000001&r=1&w=2


Note, existing setups may break. Therefore, we need to decide if this
series can be applied before git 1.6.

All tests pass.

    Steffen

 Documentation/git-http-push.txt |    6 ++
 Documentation/git-push.txt      |    8 ++-
 Documentation/git-send-pack.txt |   18 ++++-
 builtin-push.c                  |    6 ++-
 builtin-rev-parse.c             |   27 +++++---
 cache.h                         |    2 +
 http-push.c                     |    9 ++-
 remote.c                        |   41 ++++++++----
 remote.h                        |    2 +-
 send-pack.c                     |   70 ++++++++++++++++-----
 sha1_name.c                     |   52 +++++++++++++----
 t/t5516-fetch-push.sh           |  127 ++++++++++++++++++++++++++++++++++++++-
 transport.c                     |    8 ++-
 transport.h                     |    1 +
 14 files changed, 311 insertions(+), 66 deletions(-)

 [PATCH 1/8] push: change push to fail if short ref name does not exist
 [PATCH 2/8] push: teach push new flag --create
    these two should be kept together.

 [PATCH 3/8] add get_sha1_with_real_ref() returning full name of ref on demand

 [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name
    This one is a bit off-topic and could be skipped.

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

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

 [PATCH 8/8] push: teach push to be quiet if local ref is strict subset of remote ref

[PATCH 1/8] push: change push to fail if short ref name does not exist

[Steffen Prohaska]

You can use a branch's shortname to push it. Push used to create
the branch on the remote side if it did not yet exist. If you
specified the wrong branch accidentally it was created. A safety
valve that pushes only existing branches may help to avoid
errors.

This commit changes push to 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.

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 170015a..ec992c9 100644
--- a/remote.c
+++ b/remote.c
@@ -611,6 +611,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;
@@ -647,12 +648,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 4fbd5b1..5ba09e2 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.1261.g626eb

[PATCH 2/8] push: teach push new flag --create

[Steffen Prohaska]

Refs that do not start with 'refs/' will only be created
at the remote if --create is used.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-http-push.txt |    6 ++++++
 Documentation/git-push.txt      |    8 +++++++-
 Documentation/git-send-pack.txt |   14 +++++++++++---
 builtin-push.c                  |    6 +++++-
 http-push.c                     |    9 +++++++--
 remote.c                        |   18 +++++++++++++-----
 remote.h                        |    2 +-
 send-pack.c                     |    9 +++++++--
 t/t5516-fetch-push.sh           |    8 ++++++++
 transport.c                     |    8 ++++++--
 transport.h                     |    1 +
 11 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-http-push.txt b/Documentation/git-http-push.txt
index 3a69b71..8753611 100644
--- a/Documentation/git-http-push.txt
+++ b/Documentation/git-http-push.txt
@@ -30,6 +30,12 @@ OPTIONS
 	the remote repository can lose commits; use it with
 	care.
 
+\--create::
+	Usually, the command refuses to create a remote ref that is
+	not specified by its full name, i.e. starting with 'refs/'.
+	This flag tells the command to create the remote ref under
+	the full name of the local matching ref.
+
 --dry-run::
 	Do everything except actually send the updates.
 
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index e5dd4c1..67b354b 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
 SYNOPSIS
 --------
 [verse]
-'git-push' [--all] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>]
+'git-push' [--all] [--dry-run] [--create] [--tags] [--receive-pack=<git-receive-pack>]
            [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]
 
 DESCRIPTION
@@ -86,6 +86,12 @@ the remote repository.
 	This flag disables the check.  This can cause the
 	remote repository to lose commits; use it with care.
 
+\--create::
+	Usually, the command refuses to create a remote ref that is
+	not specified by its full name, i.e. starting with 'refs/'.
+	This flag tells the command to create the remote ref under
+	the full name of the local matching ref.
+
 \--repo=<repo>::
 	When no repository is specified the command defaults to
 	"origin"; this overrides it.
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 2fa01d4..01495df 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -44,6 +44,12 @@ OPTIONS
 	the remote repository can lose commits; use it with
 	care.
 
+\--create::
+	Usually, the command refuses to create a remote ref that is
+	not specified by its full name, i.e. starting with 'refs/'.
+	This flag tells the command to create the remote ref under
+	the full name of the local matching ref.
+
 \--verbose::
 	Run verbosely.
 
@@ -97,9 +103,11 @@ destination side.
    * it has to start with "refs/"; <dst> is used as the
      destination literally in this case.
 
-   * <src> == <dst> and the ref that matched the <src> must not
-     exist in the set of remote refs; the ref matched <src>
-     locally is used as the name of the destination.
+   * Only <src> is specified and the ref that matched
+     <src> must not exist in the set of remote refs;
+     and the '--create' flag is used;
+     the ref matched <src> locally is used as the name of
+     the destination.
 
 Without '--force', the <src> ref is stored at the remote only if
 <dst> does not exist, or <dst> is a proper subset (i.e. an
diff --git a/builtin-push.c b/builtin-push.c
index 4b39ef3..4ab1401 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -8,7 +8,7 @@
 #include "remote.h"
 #include "transport.h"
 
-static const char push_usage[] = "git-push [--all] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
+static const char push_usage[] = "git-push [--all] [--dry-run] [--create] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]";
 
 static int thin, verbose;
 static const char *receivepack;
@@ -113,6 +113,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			flags |= TRANSPORT_PUSH_DRY_RUN;
 			continue;
 		}
+		if (!strcmp(arg, "--create")) {
+			flags |= TRANSPORT_PUSH_CREATE;
+			continue;
+		}
 		if (!strcmp(arg, "--tags")) {
 			add_refspec("refs/tags/*");
 			continue;
diff --git a/http-push.c b/http-push.c
index c02a3af..4ad9f26 100644
--- a/http-push.c
+++ b/http-push.c
@@ -13,7 +13,7 @@
 #include <expat.h>
 
 static const char http_push_usage[] =
-"git-http-push [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";
+"git-http-push [--all] [--dry-run] [--create] [--force] [--verbose] <remote> [<head>...]\n";
 
 #ifndef XML_STATUS_OK
 enum XML_Status {
@@ -81,6 +81,7 @@ static int push_verbosely;
 static int push_all;
 static int force_all;
 static int dry_run;
+static int create;
 
 static struct object_list *objects;
 
@@ -2307,6 +2308,10 @@ int main(int argc, char **argv)
 				dry_run = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--create")) {
+				create = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--verbose")) {
 				push_verbosely = 1;
 				continue;
@@ -2389,7 +2394,7 @@ int main(int argc, char **argv)
 	if (!remote_tail)
 		remote_tail = &remote_refs;
 	if (match_refs(local_refs, remote_refs, &remote_tail,
-		       nr_refspec, refspec, push_all))
+		       nr_refspec, refspec, push_all, create))
 		return -1;
 	if (!remote_refs) {
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n");
diff --git a/remote.c b/remote.c
index ec992c9..8908a19 100644
--- a/remote.c
+++ b/remote.c
@@ -606,7 +606,7 @@ static struct ref *make_linked_ref(const char *name, struct ref ***tail)
 static int match_explicit(struct ref *src, struct ref *dst,
 			  struct ref ***dst_tail,
 			  struct refspec *rs,
-			  int errs)
+			  int errs, int create)
 {
 	struct ref *matched_src, *matched_dst;
 
@@ -650,6 +650,14 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 0:
 		if (!memcmp(orig_dst_value , "refs/", 5))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
+		else if (!memcmp(dst_value, "refs/", 5))
+			if (create)
+				matched_dst = make_linked_ref(dst_value, dst_tail);
+			else
+				error("dst refspec %s does not match any "
+				      "existing ref on the remote. "
+				      "If you want to create is use --create.",
+				       orig_dst_value);
 		else
 			error("dst refspec %s does not match any "
 			      "existing ref on the remote and does "
@@ -677,11 +685,11 @@ static int match_explicit(struct ref *src, struct ref *dst,
 
 static int match_explicit_refs(struct ref *src, struct ref *dst,
 			       struct ref ***dst_tail, struct refspec *rs,
-			       int rs_nr)
+			       int rs_nr, int create)
 {
 	int i, errs;
 	for (i = errs = 0; i < rs_nr; i++)
-		errs |= match_explicit(src, dst, dst_tail, &rs[i], errs);
+		errs |= match_explicit(src, dst, dst_tail, &rs[i], errs, create);
 	return -errs;
 }
 
@@ -711,12 +719,12 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
  * without thinking.
  */
 int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
-	       int nr_refspec, char **refspec, int all)
+	       int nr_refspec, char **refspec, int all, int create)
 {
 	struct refspec *rs =
 		parse_ref_spec(nr_refspec, (const char **) refspec);
 
-	if (match_explicit_refs(src, dst, dst_tail, rs, nr_refspec))
+	if (match_explicit_refs(src, dst, dst_tail, rs, nr_refspec, create))
 		return -1;
 
 	/* pick the remainder */
diff --git a/remote.h b/remote.h
index c62636d..7d731b1 100644
--- a/remote.h
+++ b/remote.h
@@ -57,7 +57,7 @@ void ref_remove_duplicates(struct ref *ref_map);
 struct refspec *parse_ref_spec(int nr_refspec, const char **refspec);
 
 int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
-	       int nr_refspec, char **refspec, int all);
+	       int nr_refspec, char **refspec, int all, int create);
 
 /*
  * Given a list of the remote refs and the specification of things to
diff --git a/send-pack.c b/send-pack.c
index e9b9a39..77acae1 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -7,7 +7,7 @@
 #include "remote.h"
 
 static const char send_pack_usage[] =
-"git-send-pack [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
+"git-send-pack [--all] [--dry-run] [--create] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
 "  --all and explicit <ref> specification are mutually exclusive.";
 static const char *receivepack = "git-receive-pack";
 static int verbose;
@@ -15,6 +15,7 @@ static int send_all;
 static int force_update;
 static int use_thin_pack;
 static int dry_run;
+static int create;
 
 /*
  * Make a pack stream and spit it out into file descriptor fd
@@ -201,7 +202,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
 	if (!remote_tail)
 		remote_tail = &remote_refs;
 	if (match_refs(local_refs, remote_refs, &remote_tail,
-		       nr_refspec, refspec, send_all))
+		       nr_refspec, refspec, send_all, create))
 		return -1;
 
 	if (!remote_refs) {
@@ -398,6 +399,10 @@ int main(int argc, char **argv)
 				dry_run = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--create")) {
+				create = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--force")) {
 				force_update = 1;
 				continue;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 5ba09e2..42ca0ff 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -156,6 +156,14 @@ test_expect_success 'push nonexisting (3)' '
 
 '
 
+test_expect_success 'push nonexisting (4)' '
+
+	mk_test &&
+	git push testrepo --create master &&
+	check_push_result $the_commit heads/master
+
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&
diff --git a/transport.c b/transport.c
index 400af71..fbdbd0d 100644
--- a/transport.c
+++ b/transport.c
@@ -385,7 +385,7 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons
 	int argc;
 	int err;
 
-	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
+	argv = xmalloc((refspec_nr + 12) * sizeof(char *));
 	argv[0] = "http-push";
 	argc = 1;
 	if (flags & TRANSPORT_PUSH_ALL)
@@ -394,6 +394,8 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons
 		argv[argc++] = "--force";
 	if (flags & TRANSPORT_PUSH_DRY_RUN)
 		argv[argc++] = "--dry-run";
+	if (flags & TRANSPORT_PUSH_CREATE)
+		argv[argc++] = "--create";
 	argv[argc++] = transport->url;
 	while (refspec_nr--)
 		argv[argc++] = *refspec++;
@@ -658,7 +660,7 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
 	int argc;
 	int err;
 
-	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
+	argv = xmalloc((refspec_nr + 12) * sizeof(char *));
 	argv[0] = "send-pack";
 	argc = 1;
 	if (flags & TRANSPORT_PUSH_ALL)
@@ -667,6 +669,8 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
 		argv[argc++] = "--force";
 	if (flags & TRANSPORT_PUSH_DRY_RUN)
 		argv[argc++] = "--dry-run";
+	if (flags & TRANSPORT_PUSH_CREATE)
+		argv[argc++] = "--create";
 	if (data->receivepack) {
 		char *rp = xmalloc(strlen(data->receivepack) + 16);
 		sprintf(rp, "--receive-pack=%s", data->receivepack);
diff --git a/transport.h b/transport.h
index df12ea7..1d6a926 100644
--- a/transport.h
+++ b/transport.h
@@ -30,6 +30,7 @@ struct transport {
 #define TRANSPORT_PUSH_ALL 1
 #define TRANSPORT_PUSH_FORCE 2
 #define TRANSPORT_PUSH_DRY_RUN 4
+#define TRANSPORT_PUSH_CREATE 8
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
-- 
1.5.3.4.1261.g626eb

[PATCH 3/8] add get_sha1_with_real_ref() returning full name of ref on demand

[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 27485d3..726948b 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.1261.g626eb

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

[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.1261.g626eb

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

[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 |   29 +++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index 8908a19..1c96659 100644
--- a/remote.c
+++ b/remote.c
@@ -575,6 +575,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);
@@ -582,12 +584,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 42ca0ff..0ff791a 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -282,6 +282,35 @@ 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 (--create)' '
+
+	mk_test &&
+	git push --create 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 &&
+	if git push testrepo HEAD
+	then
+		echo "Oops, should have failed"
+		false
+	else
+		check_push_result $the_first_commit heads/master
+	fi
+'
+
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&
-- 
1.5.3.4.1261.g626eb

[PATCH 6/8] add ref_cmp_full_short() comparing full ref name with a short name

[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 |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 726948b..e1385ac 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..2a1e093 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -249,6 +249,20 @@ 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.1261.g626eb

[PATCH 7/8] push: use same rules as git-rev-parse to resolve refspecs

[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".

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 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 01495df..08bcc25 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -91,7 +91,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 <src> does not match exactly one of the
    local refs.
diff --git a/remote.c b/remote.c
index 1c96659..176457c 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 (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 0ff791a..9ec8216 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -183,11 +183,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.1261.g626eb

[PATCH 8/8] push: teach push to be quiet if local ref is strict subset of remote ref

[Steffen Prohaska]

git push reports errors if a remote ref is not a strict subset
of a local ref. The push wouldn't be a fast-forward and is
therefore refused. This is in general a good idea.

But these messages can be annoying if you work with a shared
remote repository. Branches at the remote may have advanced and
you haven't pulled to all of your local branches. In this
situation, local branches may be strict subsets of the remote
heads. Pushing such branches wouldn't add any information to the
remote. It would only reset the remote to an ancestor. A merge
between the remote and the local branch is not very interested
either because it would just be a fast forward of the local
branch. In these cases you're not interested in the error
message.

This commit teaches git push to be quiet if the local is a strict
subset of the remote and no refspec is explicitly specified on
the command line. If the --verbose flag is used a "note:" is
printed for each ignored branch.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 send-pack.c           |   61 +++++++++++++++++++++++++++++++++++++------------
 t/t5516-fetch-push.sh |   44 +++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 15 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index 77acae1..b95bed9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -259,24 +259,55 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
 		    !will_delete_ref &&
 		    !is_null_sha1(ref->old_sha1) &&
 		    !ref->force) {
-			if (!has_sha1_file(ref->old_sha1) ||
-			    !ref_newer(ref->peer_ref->new_sha1,
-				       ref->old_sha1)) {
-				/* We do not have the remote ref, or
-				 * we know that the remote ref is not
-				 * an ancestor of what we are trying to
-				 * push.  Either way this can be losing
-				 * commits at the remote end and likely
-				 * we were not up to date to begin with.
+			if (!has_sha1_file(ref->old_sha1)) {
+				/* We do not have the remote ref.
+				 * This can be losing commits at
+				 * the remote end.
 				 */
-				error("remote '%s' is not a strict "
-				      "subset of local ref '%s'. "
-				      "maybe you are not up-to-date and "
-				      "need to pull first?",
-				      ref->name,
-				      ref->peer_ref->name);
+				error("You don't have the commit"
+				      "for the remote ref '%s'."
+				      "This may cause losing commits"
+				      "that cannot be recovered.",
+				      ref->name);
 				ret = -2;
 				continue;
+			} else if (!ref_newer(ref->peer_ref->new_sha1,
+			                      ref->old_sha1)) {
+				/* We know that the remote ref is not
+				 * an ancestor of what we are trying to
+				 * push. This can be losing commits at
+				 * the remote end and likely we were not
+				 * up to date to begin with.
+				 *
+				 * Therefore, we don't push.
+				 *
+				 * If no explicit refspec was passed on the
+				 * commandline, then we only report an error
+				 * if the local is not a strict subset of the
+				 * remote.  If the local is a strict subset we
+				 * don't have new commits for the remote.
+				 * Pulling and pushing wouldn't add anything to
+				 * the remote.
+				 *
+				 */
+				if (nr_refspec ||
+				    !ref_newer(ref->old_sha1, ref->peer_ref->new_sha1)) {
+					error("remote '%s' is not a strict "
+					      "subset of local ref '%s'. "
+					      "maybe you are not up-to-date and "
+					      "need to pull first?",
+					      ref->name,
+					      ref->peer_ref->name);
+					ret = -2;
+				} else if (verbose) {
+					fprintf(stderr,
+					        "note: ignoring local ref '%s' "
+					        "because it is a strict "
+					        "subset of remote '%s'.\n",
+					        ref->peer_ref->name,
+					        ref->name);
+				}
+				continue;
 			}
 		}
 		hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 9ec8216..c8493f9 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -331,4 +331,48 @@ test_expect_success 'push with dry-run' '
 	check_push_result $old_commit heads/master
 '
 
+test_expect_success 'push with local is strict subset (must not report error)' '
+
+	mk_test heads/foo &&
+	git push testrepo $the_commit:refs/heads/foo &&
+	git branch -f foo $old_commit &&
+	if git push testrepo 2>&1 | grep ^error
+	then
+		echo "Oops, should not report error"
+		false
+	fi
+
+'
+
+test_expect_success 'push with explicit refname, local is strict subset (must report error)' '
+
+	mk_test heads/foo &&
+	git push testrepo $the_commit:refs/heads/foo &&
+	git branch -f foo $old_commit &&
+	if ! git push testrepo foo 2>&1 | grep ^error
+	then
+		echo "Oops, should have reported error"
+		false
+	fi
+
+'
+
+test_expect_success 'push with neither local nor remote is strict subset (must report error)' '
+
+	mk_test heads/foo &&
+	git push testrepo $the_commit:refs/heads/foo &&
+	git branch -f foo $old_commit &&
+	git checkout foo &&
+	: >path3 &&
+	git add path3 &&
+	test_tick &&
+	git commit -a -m branched &&
+	if ! git push testrepo 2>&1 | grep ^error
+	then
+		echo "Oops, should have reported error"
+		false
+	fi
+
+'
+
 test_done
-- 
1.5.3.4.1261.g626eb

Re: [PATCH 8/8] push: teach push to be quiet if local ref is strict subset of remote ref

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> git push reports errors if a remote ref is not a strict subset
> of a local ref. The push wouldn't be a fast-forward and is
> therefore refused. This is in general a good idea.

> This commit teaches git push to be quiet if the local is a strict
> subset of the remote and no refspec is explicitly specified on
> the command line. If the --verbose flag is used a "note:" is
> printed for each ignored branch.

What happens to the summary reporting after such a push?  Does
it say "branch foo was not pushed because you did not touch it
since you fetched and it is already stale with respect to the
remote side which has updates since then"?

How does this interact with the "pretend we have fetched
immediately after we pushed by updating the corresponding remote
tracking branch" logic?

Re: [PATCH 8/8] push: teach push to be quiet if local ref is strict subset of remote ref

[Steffen Prohaska]

On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> git push reports errors if a remote ref is not a strict subset
>> of a local ref. The push wouldn't be a fast-forward and is
>> therefore refused. This is in general a good idea.
>
>> This commit teaches git push to be quiet if the local is a strict
>> subset of the remote and no refspec is explicitly specified on
>> the command line. If the --verbose flag is used a "note:" is
>> printed for each ignored branch.
>
> What happens to the summary reporting after such a push?  Does
> it say "branch foo was not pushed because you did not touch it
> since you fetched and it is already stale with respect to the
> remote side which has updates since then"?

It says nothing, it's quiet, as promised in the commit message ;)
That's the point of this patch.

But I see your point. Maybe it should say something like
"ignored 2 branches, which are strict subsets of remote."
"use --verbose for details."
?

> How does this interact with the "pretend we have fetched
> immediately after we pushed by updating the corresponding remote
> tracking branch" logic?

I doesn't change a remote tracking branch. I'll add a test
confirming this.

	Steffen

Re: [PATCH 7/8] push: use same rules as git-rev-parse to resolve refspecs

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> 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".

It may "break" but I think it is a good change.

Re: [PATCH 6/8] add ref_cmp_full_short() comparing full ref name with a short name

[Daniel Barkalow]

On Sat, 27 Oct 2007, Steffen Prohaska wrote:

> 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.

I think this and ref_matches_abbrev in remote.c should be both be 
named to be more explicit as to which sets of rules they implement, and 
should agree on order of arguments.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 6/8] add ref_cmp_full_short() comparing full ref name with a short name

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> diff --git a/sha1_name.c b/sha1_name.c
> index b820909..2a1e093 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -249,6 +249,20 @@ 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) {

We usually say "!strcmp()" instead for readability.

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Daniel Barkalow]

On Sat, 27 Oct 2007, Steffen Prohaska wrote:

> 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.

I'd prefer this to be in set_refspecs in builtin-push. That way, it only 
applies to command-line arguments and matches your description better. (If 
you use a symbolic name on the command line, it follows it, and acts 
exactly as if you used the name it points to).

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> 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.

This probably is a good idea.

> +	if (real_name) {
> +		free(real_name);
> +	}

Excess and unnecessary brace pair.

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Steffen Prohaska]

On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> 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.
>
> This probably is a good idea.

I'll think about Daniel's suggestion of implementing it
differently.


>> +	if (real_name) {
>> +		free(real_name);
>> +	}
>
> Excess and unnecessary brace pair.

When are excess and unnecessary braces acceptable?
Is

	if (something) {
  		printf("text %d"
   		       "more text %d"
                        "and even more %d\n"
		       , a, b, c);
	}

ok? Or should I avoid braces there, too?

	Steffen

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Steffen Prohaska]

On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> 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.
>
> This probably is a good idea.

I'll add an even shorter shorthand: "git push HEAD" will push
the current branch to its default remote.

	Steffen

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>
>> Steffen Prohaska <prohaska@zib.de> writes:
>>
>>> 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.
>>
>> This probably is a good idea.
>
> I'll add an even shorter shorthand: "git push HEAD" will push
> the current branch to its default remote.

Ugh, that looks way too magicky.  The first parameter to push if
one ever exists has _always_ been the remote, and the above
breaks it.

Please don't.

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Steffen Prohaska]

On Oct 28, 2007, at 4:40 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>>
>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>
>>>> 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.
>>>
>>> This probably is a good idea.
>>
>> I'll add an even shorter shorthand: "git push HEAD" will push
>> the current branch to its default remote.
>
> Ugh, that looks way too magicky.  The first parameter to push if
> one ever exists has _always_ been the remote, and the above
> breaks it.

Yes. It breaks setups that have a remote named HEAD.


> Please don't.

I already did it -- it was too easy. But it's a separate patch.
So it can easily be skipped.

What would you propose for pushing only the current branch to
its default remote repository? All information is there. Only
a way to tell push to restrict push to the current branch
is missing. Would you prefer something like
"git push --only-current-branch"?

	Steffen

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>>
>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>
>>>> 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.
>>>
>>> This probably is a good idea.
>>
>> I'll add an even shorter shorthand: "git push HEAD" will push
>> the current branch to its default remote.
>
> Ugh, that looks way too magicky.  The first parameter to push if
> one ever exists has _always_ been the remote, and the above
> breaks it.
>
> Please don't.

An alternative, just to let me keep my nicer public image by
pretending to be constructive ;-)

Introduce a configuration "remote.$name.push_default" whose
value can be a list of refs.  Teach the push command without
refspecs:

	$ git push
	$ git push $remote

to pretend as if the listed refspecs are given, instead of the
traditional "matching branches" behaviour.

Then, introduce another option

	$ git push --matching
	$ git push --matching $remote

to override that configuration, if set, so that the user who
usually pushes only the selected branches can use the "matching
branches" behaviour when needed.

Along with your earlier "git push $remote HEAD" patch, this will
allow you to say:

	[remote "origin"]
        	push_default = HEAD

and your

	$ git push

will push only the current branch.

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Steffen Prohaska]

On Oct 28, 2007, at 5:03 PM, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Steffen Prohaska <prohaska@zib.de> writes:
>>
>>> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>>>
>>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>>
>>>>> 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.
>>>>
>>>> This probably is a good idea.
>>>
>>> I'll add an even shorter shorthand: "git push HEAD" will push
>>> the current branch to its default remote.
>>
>> Ugh, that looks way too magicky.  The first parameter to push if
>> one ever exists has _always_ been the remote, and the above
>> breaks it.
>>
>> Please don't.
>
> An alternative, just to let me keep my nicer public image by
> pretending to be constructive ;-)
>
> Introduce a configuration "remote.$name.push_default" whose
> value can be a list of refs.  Teach the push command without
> refspecs:
>
> 	$ git push
> 	$ git push $remote
>
> to pretend as if the listed refspecs are given, instead of the
> traditional "matching branches" behaviour.
>
> Then, introduce another option
>
> 	$ git push --matching
> 	$ git push --matching $remote
>
> to override that configuration, if set, so that the user who
> usually pushes only the selected branches can use the "matching
> branches" behaviour when needed.
>
> Along with your earlier "git push $remote HEAD" patch, this will
> allow you to say:
>
> 	[remote "origin"]
>         	push_default = HEAD
>
> and your
>
> 	$ git push
>
> will push only the current branch.

Sounds reasonable; but it is more work. I'm not starting to
implement this today.

	Steffen

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 28, 2007, at 5:03 PM, Junio C Hamano wrote:
> ...
>> An alternative, just to let me keep my nicer public image by
>> pretending to be constructive ;-)
>>
>> Introduce a configuration "remote.$name.push_default" whose
>> value can be a list of refs.  Teach the push command without
>> refspecs:
>>
>> 	$ git push
>> 	$ git push $remote
>>
>> to pretend as if the listed refspecs are given, instead of the
>> traditional "matching branches" behaviour.
>>
>> Then, introduce another option
>>
>> 	$ git push --matching
>> 	$ git push --matching $remote
>>
>> to override that configuration, if set, so that the user who
>> usually pushes only the selected branches can use the "matching
>> branches" behaviour when needed.
>>
>> Along with your earlier "git push $remote HEAD" patch, this will
>> allow you to say:
>>
>> 	[remote "origin"]
>>         	push_default = HEAD
>>
>> and your
>>
>> 	$ git push
>>
>> will push only the current branch.
>
> Sounds reasonable; but it is more work. I'm not starting to
> implement this today.

Take your time; nobody is in a hurry.

If somebody usually uses "matching" behaviour, i.e. without
remote.$name.push_default configuration, but wants to push only
the current branch as a one-shot operation, we can obviously use
"git push $remote HEAD".  But to be complete, it may make sense
to have another option

	$ git push --current

that lets you omit $remote (and default to the value configured
with branch.$name.remote).

Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name

[Steffen Prohaska]

On Oct 28, 2007, at 9:58 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 28, 2007, at 5:03 PM, Junio C Hamano wrote:
>> ...
>>> An alternative, just to let me keep my nicer public image by
>>> pretending to be constructive ;-)
>>>
>>> Introduce a configuration "remote.$name.push_default" whose
>>> value can be a list of refs.  Teach the push command without
>>> refspecs:
>>>
>>> 	$ git push
>>> 	$ git push $remote
>>>
>>> to pretend as if the listed refspecs are given, instead of the
>>> traditional "matching branches" behaviour.
>>>
>>> Then, introduce another option
>>>
>>> 	$ git push --matching
>>> 	$ git push --matching $remote
>>>
>>> to override that configuration, if set, so that the user who
>>> usually pushes only the selected branches can use the "matching
>>> branches" behaviour when needed.
>>>
>>> Along with your earlier "git push $remote HEAD" patch, this will
>>> allow you to say:
>>>
>>> 	[remote "origin"]
>>>         	push_default = HEAD
>>>
>>> and your
>>>
>>> 	$ git push
>>>
>>> will push only the current branch.
>>
>> Sounds reasonable; but it is more work. I'm not starting to
>> implement this today.
>
> Take your time; nobody is in a hurry.
>
> If somebody usually uses "matching" behaviour, i.e. without
> remote.$name.push_default configuration, but wants to push only
> the current branch as a one-shot operation, we can obviously use
> "git push $remote HEAD".  But to be complete, it may make sense
> to have another option
>
> 	$ git push --current
>
> that lets you omit $remote (and default to the value configured
> with branch.$name.remote).

Here is an alternative proposal.

The idea is that in a workflow based on a shared repository
git pull and git push should be 'more' symmetric than they are
in a pull-only based workflow. The integration of changes is
'more' direct. Working against a shared repository may require
to integrate new changes before pushing. Changes are also
pushed directly to the remote branch you originally branched
off. Both is different from a pull-only workflow, where I first
push my changes to a privately owned but publicly readable repo
and someone else does the integration by pulling from there.

The branch in the shared repository serves as the single
'integration' branch. One can use 'git branch --track' to set
up local branches that automatically merge changes from the
shared 'integration' branch. That is git pull without further
arguments is the right command to integrate changes from the
shared branch to the local branch. (git provides more advanced
ways, but git pull is simple and in principle does the right
thing.)

What is missing is a simple way to 'push' local changes back
to shared integration branch in the remote repository. This
can be seen as a 'symmetric' operation to pulling. So, git push
should do the right thing. And the right thing is pushing the
current branch to the shared 'integration' branch.

The automerge behaviour stores information in branch.$name.remote
and branch.$name.merge that provide sufficient information to
make "git pull" the equivalent of

    git pull <remoteURL> <remoteref>

where <remoteURL> is the full URL of the remote stored in
branch.$name.remote, and <remoteref> is the value of
branch.$name.merge.

A 'symmetric' push command would push the current branch to the
remote head it originally was branched off, that is

    git push <remoteURL> <currentbranch>:<remoteref>

Now, the proposal is

- add a configuration variable branch.$name.push

- change git push to check if the push configuration variable
   is set for the current branch $name, and if so run the
   equivalent of

     git push branch.$name.remote $name:branch.$name.push


- teach git branch a flag --push/--no-push that sets up
   branch.$name.push. Add branch.autosetuppush configuration
   flag to configure if --push or --no-push is the default.
   (maybe we need better names here).


This breaks the symmetry between git fetch/git push and
replaces it with a symmetry between git pull/git push for some
branches. I believe this might be the right thing to do for
a workflow based on shared repos.

	Steffen

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Daniel Barkalow]

On Sat, 27 Oct 2007, Steffen Prohaska wrote:

> "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.

I think this makes the --create option to push unnecessary, as interactive 
users could use a suggested explicit value (or whatever they actually 
meant), while scripts could replace $name with $(git rev-parse --symbolic 
$name) as easily as they could add --create, and by more explicit as to 
what they're doing.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Steffen Prohaska]

On Oct 27, 2007, at 11:53 PM, Daniel Barkalow wrote:

> On Sat, 27 Oct 2007, Steffen Prohaska wrote:
>
>> "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.
>
> I think this makes the --create option to push unnecessary, as  
> interactive
> users could use a suggested explicit value (or whatever they actually
> meant), while scripts could replace $name with $(git rev-parse -- 
> symbolic
> $name) as easily as they could add --create, and by more explicit  
> as to
> what they're doing.

I'll remove 4/8 (git rev-parse --symbolic) from the patch
series. It is not directly related to the push behaviour
and Junio pointed out that the old behavior of git rev-parse
must be maintained as is.  I'm not particularly interested in
modifying git rev-parse. If someone else is, feel free to take
over my patch.

I'll keep the '--create' flag. Its intention is obvious and easy
to explain to users. Much easier than
"git rev-parse --dwim_ref ..." or something similar.

	Steffen

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> "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".

This is to expose "dwim_ref" logic, so it might be a good idea
to introduce a new option --dwim-ref for this purpose.

	git rev-parse --symbolic master^2

is designed to give "master^2" or fail if "master" is not a
merge.  Similarly, you would diagnose a failure if somebody asks
to show

	git rev-parse --dwim-ref master~4

instead of giving "refs/heads/master~4".

> +static void show_rev(int type, const unsigned char *sha1, const char *name, const char* real_name)
> @@ -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);

A null pointer constant in git sources is spelled "NULL" not "0".

> @@ -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;

Pointer sign '*' in git sources go next to the name not the
type, as:

	char *real_name = NULL;

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Steffen Prohaska]

On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> "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".
>
> This is to expose "dwim_ref" logic, so it might be a good idea
> to introduce a new option --dwim-ref for this purpose.
>
> 	git rev-parse --symbolic master^2
>
> is designed to give "master^2" or fail if "master" is not a
> merge.

So the idea of --symbolic is really to return the argv as is?
No change whatsoever allowed. Why would someone need this?
Is it only for convenience when writing shell code?


> Similarly, you would diagnose a failure if somebody asks
> to show
>
> 	git rev-parse --dwim-ref master~4
>
> instead of giving "refs/heads/master~4".

What I proposed is to teach git rev-parse to return a full
symbolic ref name. Maybe

	git rev-parse --full-symbolic
	git rev-parse --full-ref

But honestly, I don't care that much about this patch. Maybe
we just put it aside?


>> +static void show_rev(int type, const unsigned char *sha1, const  
>> char *name, const char* real_name)
>> @@ -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);
>
> A null pointer constant in git sources is spelled "NULL" not "0".

Ok. I'll fix this in all patches.


>> @@ -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;
>
> Pointer sign '*' in git sources go next to the name not the
> type, as:
>
> 	char *real_name = NULL;

I know and I tried hard to follow this convention, although
I think its the wrong choice ;)

	Steffen

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Shawn O. Pearce]

Steffen Prohaska <prohaska@zib.de> wrote:
> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
> >Steffen Prohaska <prohaska@zib.de> writes:
> >>@@ -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;
> >
> >Pointer sign '*' in git sources go next to the name not the
> >type, as:
> >
> >	char *real_name = NULL;
> 
> I know and I tried hard to follow this convention, although
> I think its the wrong choice ;)

Oh, hmm...

  char* a, b;

What's the type of b?  If you said "char*" you're wrong.

Git's style of putting the * next to the name makes it far easier to
spot these sort of typing problems.  At least that's my take on it.

-- 
Shawn.

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Steffen Prohaska]

On Oct 28, 2007, at 9:06 AM, Shawn O. Pearce wrote:

> Steffen Prohaska <prohaska@zib.de> wrote:
>> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>> @@ -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;
>>>
>>> Pointer sign '*' in git sources go next to the name not the
>>> type, as:
>>>
>>> 	char *real_name = NULL;
>>
>> I know and I tried hard to follow this convention, although
>> I think its the wrong choice ;)
>
> Oh, hmm...
>
>   char* a, b;
>
> What's the type of b?  If you said "char*" you're wrong.

I know. Obviously, you need a combination of conventions.

- '*' is part of the type; put it there.
- Only define a single variable per statement.

My combined rule avoids your question. I typically use C++,
which make the second rule easier to follow, because you
defer defining variables until you really need them. There's
no need to save space at the start of the function block by
defining several variables in a single line.


> Git's style of putting the * next to the name makes it far easier to
> spot these sort of typing problems.  At least that's my take on it.

Let's stop this discussion here. I'm not proposing to change the
rules. I'll follow the git coding conventions when submitting
patches to git, even if it's sometimes hard for me. I already packed
as much as possible in my editor configuration. Unfortunately,
I can't teach vim to always place the '*' correctly. At least I
don't know how to do this.

	Steffen

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Brian Gernhardt]

On Oct 28, 2007, at 4:56 AM, Steffen Prohaska wrote:

> I can't teach vim to always place the '*' correctly. At least I
> don't know how to do this.

I did some research (I was curious), and the following in your .vimrc  
will do it:  (Mildly tested)

-----8<-----

" Needed to avoid 'char * '
func s:Eatchar(pat)
let c = nr2char(getchar(0))
return (c =~ a:pat) ? '' : c
endfunc

" Make creating the abbreviation for multiple types easier
func s:FixPointer(type)
exec "iabbr " . a:type . "* " . a:type . " *<C-R>=<SID>Eatchar('\ 
\s')<CR>"
endfunc
command -nargs=1 FixPointer call <SID>FixPointer(<args>)

" Change the following pointer types (char* var -> char *var)
FixPointer 'char'
FixPointer 'int'
FixPointer 'void'

-----8<-----

Repeating the last line for each type you want to fix the pointers  
for.  There may be a simpler way to do some of this, but I don't know  
it.  But once it's set up each time you type in "char* var", Vim will  
auto-correct to "char *var".

Changing this to operate only in C, or only if you're starting in your  
git repository is left as an exercise for the reader.  ;-)

~~ Brian

Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> So the idea of --symbolic is really to return the argv as is?
> No change whatsoever allowed. Why would someone need this?
> Is it only for convenience when writing shell code?

rev-parse _is_ for convenience for writing shell scripts, to
sift various parameters into "rev-list arguments" and "non
rev-list arguments".

Re: [PATCH 3/8] add get_sha1_with_real_ref() returning full name of ref on demand

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> 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.

"can be"?  "will be used to implement git rev-parse --some-option"?

> +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);

A null pointer constant in git sources is spelled as "NULL", not "0".

Re: [PATCH 1/8] push: change push to fail if short ref name does not exist

[Daniel Barkalow]

On Sat, 27 Oct 2007, Steffen Prohaska wrote:

> You can use a branch's shortname to push it. Push used to create
> the branch on the remote side if it did not yet exist. If you
> specified the wrong branch accidentally it was created. A safety
> valve that pushes only existing branches may help to avoid
> errors.
> 
> This commit changes push to 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.

I agree with the change (and I think it's appropriate for master or next), 
but your implementation is a bit too clever for my tastes.

> 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 170015a..ec992c9 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -611,6 +611,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;

"lit_dst_value" would probably be a better description, and it might be 
worth handling the rs->dst == NULL case where it's handled for dst_value. 
Technically, this variable, when it doesn't match the final value of 
dst_value, has a value that dst_value never had.

>  
>  	if (rs->pattern)
>  		return errs;
> @@ -647,12 +648,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);

This should also be orig_dst_value, too. I know that dst_value and 
orig_dst_value must be the same in this case, but it takes a bunch of 
analysis to demonstrate that, and it's more intuitive to use the value 
you've just checked anyway, and also to have all of case 0 use the 
differently-computed destination.

>  		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);

Maybe the error should provide a hint if dst_value is not the same as 
orig_dst_value? (if (!rs->dst) error("Did you mean %s?\n", dst_value);)

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 1/8] push: change push to fail if short ref name does not exist

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> You can use a branch's shortname to push it. Push used to create
> the branch on the remote side if it did not yet exist. If you
> specified the wrong branch accidentally it was created. A safety
> valve that pushes only existing branches may help to avoid
> errors.

I do not agree with this change.

If you misspelled the branch name (by the way, it is not a
"shortname", but what follows refs/heads/ is _the_ name of the
branch) "frotz" as "frtoz", and if a branch with the misspelled
name did _not_ exist locally, it would fail, with or without
this change, which is a good thing.

But if you named "nitfol" that exists locally when you meant to
push "frotz" out, if "nitfol" remotely existed, we will push
that anyway by mistake, even with this change.  It will prevent
the push only when "nitfol" did not happen to exist at the
remote side.

Earlier there was a discussion to introduce an optional
configuration that makes "git push" without any parameter to
push only the current branch out, in order to help people who
work with shared remote central repository.  That might be a
better alternative to avoid pushing out wrong branch by
mistake.  That approach would also make your 8/8 unnecessary.

Re: [PATCH 1/8] push: change push to fail if short ref name does not exist

[Steffen Prohaska]

On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> You can use a branch's shortname to push it. Push used to create
>> the branch on the remote side if it did not yet exist. If you
>> specified the wrong branch accidentally it was created. A safety
>> valve that pushes only existing branches may help to avoid
>> errors.
>
> I do not agree with this change.
>
> If you misspelled the branch name (by the way, it is not a
> "shortname", but what follows refs/heads/ is _the_ name of the
> branch)

True. Is "short refname" the correct wording?


> "frotz" as "frtoz", and if a branch with the misspelled
> name did _not_ exist locally, it would fail, with or without
> this change, which is a good thing.
>
> But if you named "nitfol" that exists locally when you meant to
> push "frotz" out, if "nitfol" remotely existed, we will push
> that anyway by mistake, even with this change.  It will prevent
> the push only when "nitfol" did not happen to exist at the
> remote side.

My proposed change is more defensive than the current
implementation. It allows distinguishing between "push existing
branch" and "create new branch on the remote side", which I
believe is a good thing. The current implementation uses
the same command line in both cases.

Daniel suggested that git push should print a recommendation
what full ref to use. This would make it easy to correct
the command. Ot you could use the '--create' flag to make
your intention explicit.

If we take the "push origin HEAD" patch, the existence check is
even more important. If you're on the wrong branch and push HEAD
you may be surprised if a new branch is created. This can be
avoided by requiring either a full ref or the '--create' flag.


> Earlier there was a discussion to introduce an optional
> configuration that makes "git push" without any parameter to
> push only the current branch out, in order to help people who
> work with shared remote central repository.  That might be a
> better alternative to avoid pushing out wrong branch by
> mistake.  That approach would also make your 8/8 unnecessary.

I didn't have the impression that the discussion went in
this direction. There were quite a few people who just said
"git push" is the counter-part of "git fetch". Therefore
"git push" pushes _all_ branches. Period.

With "git push HEAD" 5/8 you could now push only the current branch
(its existence would be verified if 1/8 is also accepted).

8/8 solves a different issue, too. I never advance local branches if
I do not intend to push them. Therefore I can always say "git push"
without any argument. It does always the right thing for me; except
for the annoying error messages that are avoided by 8/8. With 8/8
I can push several local branches while ignoring the ones that I'm
not interested in.

	Steffen