[PATCH 0/10 v3] improve refspec handling in push

[PATCH 0/10 v3] improve refspec handling in push

[Steffen Prohaska]

This is a replacement for sp/push-refspec
(93e296613306311ef02dabb19a6538be2f52aa1c).

Compared to the v2 series the following changed (v2 patch numbers):
    1/8 implementation should be better readable.
    2/8 adjusted to 1/8 changes.
    3/8 removed.
    4/8 removed.
    5/8 much simpler implementation, second patch "git push HEAD" added.
    6/8 chose more explicit naming
        ref_abbrev_matches_full_with_rev_parse_rules;
        unified argument order with ref_matches_abbrev,
        which was renamed to ref_abbrev_matches_full_with_fetch_rules.
    7/8 adjusted to 6/8 changes.
    8/8 report summary;
        --verbose fixed;
        added test that remote tracking branches are unchanged.

All tests pass.

Here's a summary of the series:

 Documentation/git-http-push.txt |    6 ++
 Documentation/git-push.txt      |   16 +++-
 Documentation/git-send-pack.txt |   18 +++-
 builtin-push.c                  |   23 +++++-
 cache.h                         |    1 +
 http-push.c                     |    9 ++-
 remote.c                        |   50 +++++++-----
 remote.h                        |    2 +-
 send-pack.c                     |   77 +++++++++++++----
 sha1_name.c                     |   14 +++
 t/t5516-fetch-push.sh           |  181 ++++++++++++++++++++++++++++++++++++++-
 transport.c                     |   12 ++-
 transport.h                     |    2 +
 13 files changed, 358 insertions(+), 53 deletions(-)

 [PATCH 01/10] push: change push to fail if short refname does not exist
 [PATCH 02/10] push: teach push new flag --create

 [PATCH 03/10] push: support pushing HEAD to real branch name
 [PATCH 04/10] push: add "git push HEAD" shorthand for 'push current branch to default repo'
    Junio doesn't like this patch. But I had it ready, so here it is.
    Junio described an alternative in
    http://marc.info/?l=git&m=119358745026345&w=2

 [PATCH 05/10] rename ref_matches_abbrev() to ref_abbrev_matches_full_with_fetch_rules()
 [PATCH 06/10] add ref_abbrev_matches_full_with_rev_parse_rules() comparing abbrev with full ref name
 [PATCH 07/10] push: use same rules as git-rev-parse to resolve refspecs
    Maybe the matching rules could be further unified.
    Code cleanup would be needed here.
    But this is a different story.

 [PATCH 08/10] push: teach push to accept --verbose option
 [PATCH 09/10] push: teach push to pass --verbose option to transport layer
 [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref

    Steffen

[PATCH 01/10] push: change push to fail if short refname does not exist

[Steffen Prohaska]

Pushing a short refname used to create a new ref on 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. If you specify a
branch name that does not yet exist on the remote side, git push
will print a suggestion to push the full refname instead.

The new behaviour is more defensive than the old one. You can
now explicitly distinguish between "push existing branch" and
"create new branch on the remote side". The old implementation
allowed the same command line in both cases.

A follow-up patch will add a flag '--create' that provides an
alternative to using full refnames if creation of new refs is
intended.

Another follow-up patch will support "push origin HEAD". In this
case, the existence check is 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.

The implementation in this patch is less "clever" and hopefully
better readable than an ealier version of the patch. Thanks for
the suggestions to Daniel Barkalow <barkalow@iabervon.org>.

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

diff --git a/remote.c b/remote.c
index 170015a..cf6441a 100644
--- a/remote.c
+++ b/remote.c
@@ -610,7 +610,8 @@ static int match_explicit(struct ref *src, struct ref *dst,
 {
 	struct ref *matched_src, *matched_dst;
 
-	const char *dst_value = rs->dst;
+	const char *lit_dst_value;
+	const char *search_dst_value;
 
 	if (rs->pattern)
 		return errs;
@@ -637,27 +638,33 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	if (!matched_src)
 		errs = 1;
 
-	if (!dst_value) {
+	if (rs->dst) {
+		lit_dst_value = search_dst_value = rs->dst;
+	} else {
 		if (!matched_src)
 			return errs;
-		dst_value = matched_src->name;
+		lit_dst_value = rs->src;
+		search_dst_value = matched_src->name;
 	}
 
-	switch (count_refspec_match(dst_value, dst, &matched_dst)) {
+	switch (count_refspec_match(search_dst_value, dst, &matched_dst)) {
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(dst_value, "refs/", 5))
-			matched_dst = make_linked_ref(dst_value, dst_tail);
-		else
+		if (!memcmp(lit_dst_value , "refs/", 5))
+			matched_dst = make_linked_ref(lit_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/.", lit_dst_value);
+			if (!rs->dst)
+				error("Did you mean %s?\n", search_dst_value);
+		}
 		break;
 	default:
 		matched_dst = NULL;
 		error("dst refspec %s matches more than one.",
-		      dst_value);
+		      lit_dst_value);
 		break;
 	}
 	if (errs || !matched_dst)
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.439.ge8b49

[PATCH 02/10] push: teach push new flag --create

[Steffen Prohaska]

If you want to push a branch that does not yet exist on the
remote side you can push using a full refspec. For example you
can "push origin refs/heads/master".

This commit changes push such that refs that do not start with
'refs/' will be created at the remote as the matching local ref
if --create is used. If you want to create a new ref at the
remote, you can now say "git push --create origin master".

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                        |   24 +++++++++++++++---------
 remote.h                        |    2 +-
 send-pack.c                     |    9 +++++++--
 t/t5516-fetch-push.sh           |    8 ++++++++
 transport.c                     |    8 ++++++--
 transport.h                     |    1 +
 11 files changed, 74 insertions(+), 21 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 cf6441a..687eb8e 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;
 
@@ -653,13 +653,19 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 0:
 		if (!memcmp(lit_dst_value , "refs/", 5))
 			matched_dst = make_linked_ref(lit_dst_value, dst_tail);
-		else {
+		else if (!memcmp(search_dst_value, "refs/", 5))
+			if (create)
+				matched_dst = make_linked_ref(search_dst_value, dst_tail);
+			else
+				error("dst refspec %s does not match any "
+				      "existing ref on the remote.\n"
+				      "To create it use --create "
+				      "or the full ref %s.",
+				       lit_dst_value, search_dst_value);
+		else
 			error("dst refspec %s does not match any "
 			      "existing ref on the remote and does "
 			      "not start with refs/.", lit_dst_value);
-			if (!rs->dst)
-				error("Did you mean %s?\n", search_dst_value);
-		}
 		break;
 	default:
 		matched_dst = NULL;
@@ -683,11 +689,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;
 }
 
@@ -717,12 +723,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.439.ge8b49

[PATCH 03/10] push: support pushing HEAD to real branch name

[Steffen Prohaska]

This teaches "push <remote> HEAD" to resolve HEAD on the local
side to its real branch name, e.g. master, and then act as if
the real branch name was specified. So we have a shorthand for
pushing the current branch. Besides HEAD, no other symbolic ref
is resolved.

Thanks to Daniel Barkalow <barkalow@iabervon.org> for suggesting
this implementation, which is much simpler than the
implementation proposed before.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 builtin-push.c        |    9 +++++++++
 t/t5516-fetch-push.sh |   31 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 4ab1401..2e3c8c6 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -40,6 +40,15 @@ static void set_refspecs(const char **refs, int nr)
 			strcat(tag, refs[i]);
 			ref = tag;
 		}
+		if (!strcmp("HEAD", ref)) {
+			unsigned char sha1_dummy[20];
+			ref = resolve_ref(ref, sha1_dummy, 1, NULL);
+			if (!ref)
+				die("HEAD cannot be resolved.");
+			if (strncmp(ref, "refs/heads/", 11))
+				die("HEAD cannot be resolved to branch.");
+			ref = xstrdup(ref + 11);
+		}
 		add_refspec(ref);
 	}
 }
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 42ca0ff..8becaf8 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -282,6 +282,37 @@ test_expect_success 'push with colon-less refspec (4)' '
 
 '
 
+test_expect_success 'push with HEAD' '
+
+	mk_test heads/master &&
+	git checkout master &&
+	git push testrepo HEAD &&
+	check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with HEAD (--create)' '
+
+	mk_test &&
+	git checkout master &&
+	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.439.ge8b49

[PATCH 04/10] push: add "git push HEAD" shorthand for 'push current branch to default repo'

[Steffen Prohaska]

Sometimes it is handy to push only the current branch to the
default remote repository. For example, if you created a branch
using the '--track' option git knows that the current branch
is linked to a specific remote. But up to now you needed to say
"git push <defaultremote> <thisbranch>", which was quite
annoying.  You could have said "git push" but then _all_ branches
would have been pushed to the default remote.

This commit introduces "git push HEAD", which resolves HEAD to
the current branch and pushes only the current branch to its
default remote.

Setups that have a remote named HEAD will break. But such a setup
if unlikely to exist; and is not very sensible anyway.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-push.txt |    6 +++++-
 builtin-push.c             |    2 ++
 t/t5516-fetch-push.sh      |   12 ++++++++++++
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 67b354b..236898f 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git-push' [--all] [--dry-run] [--create] [--tags] [--receive-pack=<git-receive-pack>]
-           [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]
+           [--repo=all] [-f | --force] [-v] [HEAD | <repository> <refspec>...]
 
 DESCRIPTION
 -----------
@@ -25,6 +25,10 @@ documentation for gitlink:git-receive-pack[1].
 
 OPTIONS
 -------
+HEAD::
+	Tells push to push the current branch to the default
+	remote repository.
+
 <repository>::
 	The "remote" repository that is destination of a push
 	operation.  See the section <<URLS,GIT URLS>> below.
diff --git a/builtin-push.c b/builtin-push.c
index 2e3c8c6..7c08e19 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -102,6 +102,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		const char *arg = argv[i];
 
 		if (arg[0] != '-') {
+			if (!strcmp("HEAD", arg))
+				break;
 			repo = arg;
 			i++;
 			break;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8becaf8..2650e36 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -291,6 +291,18 @@ test_expect_success 'push with HEAD' '
 
 '
 
+test_expect_success 'push HEAD' '
+
+	mk_test heads/track &&
+	git remote add test testrepo &&
+	git fetch test &&
+	git checkout -b track test/track &&
+	git reset --hard master &&
+	git push HEAD &&
+	check_push_result $the_commit heads/track
+
+'
+
 test_expect_success 'push with HEAD (--create)' '
 
 	mk_test &&
-- 
1.5.3.4.439.ge8b49

[PATCH 05/10] rename ref_matches_abbrev() to ref_abbrev_matches_full_with_fetch_rules()

[Steffen Prohaska]

The new naming makes the order of the arguments and the rules used
for matching more explicit. This will avoid confusion with
ref_abbrev_matches_full_with_rev_parse_rules(), which will be
introduced in a follow-up commit.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 remote.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index 687eb8e..59e6485 100644
--- a/remote.c
+++ b/remote.c
@@ -421,7 +421,7 @@ int remote_has_url(struct remote *remote, const char *url)
  * Returns true if, under the matching rules for fetching, name is the
  * same as the given full name.
  */
-static int ref_matches_abbrev(const char *name, const char *full)
+static int ref_abbrev_matches_full_with_fetch_rules(const char *name, const char *full)
 {
 	if (!prefixcmp(name, "refs/") || !strcmp(name, "HEAD"))
 		return !strcmp(name, full);
@@ -820,7 +820,7 @@ int branch_merge_matches(struct branch *branch,
 {
 	if (!branch || i < 0 || i >= branch->merge_nr)
 		return 0;
-	return ref_matches_abbrev(branch->merge[i]->src, refname);
+	return ref_abbrev_matches_full_with_fetch_rules(branch->merge[i]->src, refname);
 }
 
 static struct ref *get_expanded_map(struct ref *remote_refs,
@@ -859,7 +859,7 @@ static struct ref *find_ref_by_name_abbrev(struct ref *refs, const char *name)
 {
 	struct ref *ref;
 	for (ref = refs; ref; ref = ref->next) {
-		if (ref_matches_abbrev(name, ref->name))
+		if (ref_abbrev_matches_full_with_fetch_rules(name, ref->name))
 			return ref;
 	}
 	return NULL;
-- 
1.5.3.4.439.ge8b49

[PATCH 06/10] add ref_abbrev_matches_full_with_rev_parse_rules() comparing abbrev with full ref name

[Steffen Prohaska]

ref_abbrev_matches_full_with_rev_parse_rules(abbrev_name, full_name)
expands abbrev_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().

The function name is very long to make the rule set used
explicit. We have a different set of rules for matching refspecs.
It would be a good thing to unify all different rule sets. But
this commit doesn't address this challenge. It only makes the
git-rev-parse rules available for string comparison.

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

Thanks to Daniel Barkalow <barkalow@iabervon.org> for pointing
out that ref_matches_abbrev in remote.c solves a similar problem
and care should be take to avoid confusion.

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 27485d3..bb10ade 100644
--- a/cache.h
+++ b/cache.h
@@ -405,6 +405,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_abbrev_matches_full_with_rev_parse_rules(const char *abbrev_name, const char *full_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 2d727d5..944e318 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -249,6 +249,20 @@ static const char *ref_fmt[] = {
 	NULL
 };
 
+int ref_abbrev_matches_full_with_rev_parse_rules(const char *abbrev_name, const char *full_name)
+{
+	const char **p;
+	const int abbrev_name_len = strlen(abbrev_name);
+
+	for (p = ref_fmt; *p; p++) {
+		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {
+			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.439.ge8b49

[PATCH 07/10] 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 may break 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 59e6485..9c33fcf 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_abbrev_matches_full_with_rev_parse_rules(pattern, name))
 			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 2650e36..6708ec1 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.439.ge8b49

[PATCH 08/10] push: teach push to accept --verbose option

[Steffen Prohaska]

Before this commit, git push only knew '-v'.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Documentation/git-push.txt |    4 ++--
 builtin-push.c             |    4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 236898f..865f183 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git-push' [--all] [--dry-run] [--create] [--tags] [--receive-pack=<git-receive-pack>]
-           [--repo=all] [-f | --force] [-v] [HEAD | <repository> <refspec>...]
+           [--repo=all] [-f | --force] [-v | --verbose] [HEAD | <repository> <refspec>...]
 
 DESCRIPTION
 -----------
@@ -105,7 +105,7 @@ the remote repository.
 	transfer spends extra cycles to minimize the number of
 	objects to be sent and meant to be used on slower connection.
 
--v::
+-v, \--verbose::
 	Run verbosely.
 
 include::urls-remotes.txt[]
diff --git a/builtin-push.c b/builtin-push.c
index 7c08e19..9103d57 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -112,6 +112,10 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			verbose=1;
 			continue;
 		}
+		if (!strcmp(arg, "--verbose")) {
+			verbose=1;
+			continue;
+		}
 		if (!prefixcmp(arg, "--repo=")) {
 			repo = arg+7;
 			continue;
-- 
1.5.3.4.439.ge8b49

[PATCH 09/10] push: teach push to pass --verbose option to transport layer

[Steffen Prohaska]

A --verbose option to push should also be passed to the
transport layer, i.e. git-send-pack, git-http-push.

git push is modified to do so.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 builtin-push.c |    2 ++
 transport.c    |    8 ++++++--
 transport.h    |    1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 9103d57..27eaca5 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -110,10 +110,12 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		}
 		if (!strcmp(arg, "-v")) {
 			verbose=1;
+			flags |= TRANSPORT_PUSH_VERBOSE;
 			continue;
 		}
 		if (!strcmp(arg, "--verbose")) {
 			verbose=1;
+			flags |= TRANSPORT_PUSH_VERBOSE;
 			continue;
 		}
 		if (!prefixcmp(arg, "--repo=")) {
diff --git a/transport.c b/transport.c
index fbdbd0d..e6bca93 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 + 12) * sizeof(char *));
+	argv = xmalloc((refspec_nr + 13) * sizeof(char *));
 	argv[0] = "http-push";
 	argc = 1;
 	if (flags & TRANSPORT_PUSH_ALL)
@@ -396,6 +396,8 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons
 		argv[argc++] = "--dry-run";
 	if (flags & TRANSPORT_PUSH_CREATE)
 		argv[argc++] = "--create";
+	if (flags & TRANSPORT_PUSH_VERBOSE)
+		argv[argc++] = "--verbose";
 	argv[argc++] = transport->url;
 	while (refspec_nr--)
 		argv[argc++] = *refspec++;
@@ -660,7 +662,7 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
 	int argc;
 	int err;
 
-	argv = xmalloc((refspec_nr + 12) * sizeof(char *));
+	argv = xmalloc((refspec_nr + 13) * sizeof(char *));
 	argv[0] = "send-pack";
 	argc = 1;
 	if (flags & TRANSPORT_PUSH_ALL)
@@ -671,6 +673,8 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
 		argv[argc++] = "--dry-run";
 	if (flags & TRANSPORT_PUSH_CREATE)
 		argv[argc++] = "--create";
+	if (flags & TRANSPORT_PUSH_VERBOSE)
+		argv[argc++] = "--verbose";
 	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 1d6a926..a387eed 100644
--- a/transport.h
+++ b/transport.h
@@ -31,6 +31,7 @@ struct transport {
 #define TRANSPORT_PUSH_FORCE 2
 #define TRANSPORT_PUSH_DRY_RUN 4
 #define TRANSPORT_PUSH_CREATE 8
+#define TRANSPORT_PUSH_VERBOSE 16
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);
-- 
1.5.3.4.439.ge8b49

[PATCH 10/10] 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 for local refs that are
strict subsets of the matching remote refs and no refspec is
specified on the command line. If the --verbose flag is used a
"note" is printed instead of silently ignoring the refs.
If no notes have been printed the number of ignored refs will
be reported in the final summary.

If refs are ignored their matching remote tracking refs will not
be changed.

git push now allows you pushing a couple of branches that have
advanced, while ignoring all branches that have no local changes,
but are lagging behind their matching remote refs. This is done
without reporting errors.

Thanks to Junio C. Hamano <gitster@pobox.com> for suggesting to
report in the summary that refs have been ignored.

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

diff --git a/send-pack.c b/send-pack.c
index 77acae1..68a4692 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -187,6 +187,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
 	int ask_for_status_report = 0;
 	int allow_deleting_refs = 0;
 	int expect_status_report = 0;
+	int ignored_refs = 0;
 
 	/* No funny business with the matcher */
 	remote_tail = get_remote_heads(in, &remote_refs, 0, NULL, REF_NORMAL);
@@ -259,24 +260,56 @@ 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);
+				} else
+					ignored_refs++;
+				continue;
 			}
 		}
 		hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
@@ -335,6 +368,11 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha
 			ret = -4;
 	}
 
+	if (ignored_refs)
+		fprintf(stderr,
+			"Ignored %d local refs that are strict subsets of matching remote ref. "
+			"Use --verbose for more details.\n",
+			ignored_refs);
 	if (!new_refs && ret == 0)
 		fprintf(stderr, "Everything up-to-date\n");
 	return ret;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 6708ec1..1f740b2 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -56,6 +56,22 @@ check_push_result () {
 	)
 }
 
+check_local_result () {
+	(
+		it="$1" &&
+		shift
+		for ref in "$@"
+		do
+			r=$(git show-ref -s --verify refs/$ref) &&
+			test "z$r" = "z$it" || {
+				echo "Oops, refs/$ref is wrong"
+				exit 1
+			}
+		done &&
+		git fsck --full
+	)
+}
+
 test_expect_success setup '
 
 	: >path1 &&
@@ -345,4 +361,72 @@ 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
+	else
+		check_push_result $the_commit heads/foo
+	fi
+
+'
+
+test_expect_success 'push with local is strict subset (must not update remotes)' '
+
+	mk_test heads/foo &&
+	git push testrepo $the_commit:refs/heads/foo &&
+	git branch -f foo $old_commit &&
+	git fetch test &&
+	check_local_result $the_commit remotes/test/foo &&
+	if git push test 2>&1 | grep ^error
+	then
+		echo "Oops, should not report error"
+		false
+	else
+		check_push_result $the_commit heads/foo &&
+		check_local_result $the_commit remotes/test/foo
+	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
+	else
+		check_push_result $the_commit heads/foo
+	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
+	else
+		check_push_result $the_commit heads/foo
+	fi
+
+'
+
 test_done
-- 
1.5.3.4.439.ge8b49

Re: [PATCH 10/10] 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 now allows you pushing a couple of branches that have
> advanced, while ignoring all branches that have no local changes,
> but are lagging behind their matching remote refs. This is done
> without reporting errors.
>
> Thanks to Junio C. Hamano <gitster@pobox.com> for suggesting to
> report in the summary that refs have been ignored.

I do not think this is a good idea at all.  Furthermore, I never
suggested anything about summary.  You are robbing the
information from the pusher which ones are pushed and which ones
are left behind.

It simply is insane to make this strange rule 10/10 introduces
the default behaviour.  It is too specific to a particular
workflow (that is, working with a shared central repository,
having many locally tracking branches that are not often used
and become stale, and working on only things to completion
between pushes).

I think we could live with an optional behaviour, in addition to
the current "matching refs" behaviour, that is "matching refs,
ignoring strict ancestors", though, but I doubt it is worth the
addition.

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

[Steffen Prohaska]

On Oct 30, 2007, at 9:29 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> git push now allows you pushing a couple of branches that have
>> advanced, while ignoring all branches that have no local changes,
>> but are lagging behind their matching remote refs. This is done
>> without reporting errors.
>>
>> Thanks to Junio C. Hamano <gitster@pobox.com> for suggesting to
>> report in the summary that refs have been ignored.
>
> I do not think this is a good idea at all.  Furthermore, I never
> suggested anything about summary.

Yeah, sorry. You only asked if the summary does mention
something; not suggesting it should do so.


> You are robbing the
> information from the pusher which ones are pushed and which ones
> are left behind.

Absolutely; because the branches left behind are not
interesting. The remote already is ahead of the local
branches. The local branches are just left were they are. They
have no new information on them.  Forcing an push would _rewind_
the remote without adding anything to it.

If you really intended to do a rewind you should have passed
'--force' in the first place and my report would never be
printed.


> It simply is insane to make this strange rule 10/10 introduces
> the default behaviour.  It is too specific to a particular
> workflow (that is, working with a shared central repository,
> having many locally tracking branches that are not often used
> and become stale, and working on only things to completion
> between pushes).

I don't think its very strange behaviour if you see it in the
light of what the user wants to achieve. We are talking about
the case were only fast forward pushes are allowed. So, we
only talk about a push that has the goal of adding new local
changes to the remote. The user says "git push" and means
push my new local changes to the remote.

Unfortunately, the remote may have advanced differently from
the local branch, and the push must fail because someone needs
to merge first. git push recommends to do a pull and retry, which
is the right thing to do.

My strange rule 10/10 adds a check that verifies if the local
side has something interesting to push. Only in this case a
pull make sense. If you do not have something new, a pull will
be a fast-forward, and just a waste of time.

In this light I think the current behaviour is insane, because
it asks the user to spend time on things that do not add any
value. No new commits, no new information, no need to merge, no
need to push again, no need to report errors ...

> I think we could live with an optional behaviour, in addition to
> the current "matching refs" behaviour, that is "matching refs,
> ignoring strict ancestors", though, but I doubt it is worth the
> addition.

... just ignore strict ancestors by default.

	Steffen

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

[Andreas Ericsson]

Steffen Prohaska wrote:
> 
> My strange rule 10/10 adds a check that verifies if the local
> side has something interesting to push. Only in this case a
> pull make sense. If you do not have something new, a pull will
> be a fast-forward, and just a waste of time.
> 

Err... fast-forward pulls are not a waste of time. What a strange
notion. Perhaps I misunderstood, but this sentence jumped out at
me and immediately got filed under "decidedly odd".

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

[Steffen Prohaska]

On Oct 30, 2007, at 11:26 AM, Andreas Ericsson wrote:

> Steffen Prohaska wrote:
>> My strange rule 10/10 adds a check that verifies if the local
>> side has something interesting to push. Only in this case a
>> pull make sense. If you do not have something new, a pull will
>> be a fast-forward, and just a waste of time.
>
> Err... fast-forward pulls are not a waste of time. What a strange
> notion. Perhaps I misunderstood, but this sentence jumped out at
> me and immediately got filed under "decidedly odd".

If the local branch is a strict ancestor, a pull is only
interesting if you want to start to work on such a branch
locally. But pull is a waste of time if you're only goal is to
push. Push suggests to pull first. So you pull; and then you
push again; and the result on the remote is the same. Only
the error message is gone that could have been avoided in the
first place. -> waste of time.

If you _pull_ it would be interesting to learn that you probably
want to merge to more than the current local branch. At that
time you expressed the intention to integrate new changes from
the remote. And it's probably a good idea to integrate changes
on all local branches that are set up to automatically merge
from the same remote you just pulled.

But if you push you want to push. You'd probably only interested
in pulls that add immediate value to the push. That is if the
result of a subsequent push modified the remote.

	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 30, 2007, at 9:29 AM, Junio C Hamano wrote:
>
>> It simply is insane to make this strange rule 10/10 introduces
>> the default behaviour.  It is too specific to a particular
>> workflow (that is, working with a shared central repository,
>> having many locally tracking branches that are not often used
>> and become stale, and working on only things to completion
>> between pushes).
>
> I don't think its very strange behaviour if you see it in the
> light of what the user wants to achieve. We are talking about
> the case were only fast forward pushes are allowed. So, we
> only talk about a push that has the goal of adding new local
> changes to the remote. The user says "git push" and means
> push my new local changes to the remote.

If you want to push a specific subset of branches, you should
not be invoking the "matching refs" to begin with.  And breaking
the "matching refs" behaviour is not the way to fix it.

You can rewind a wrong branch by mistake locally and run push.
With your change you would not notice that mistake.

        $ git checkout bar
        $ work work work; commit commit commit
	$ git checkout test
        $ git merge bar
	... integrate, build, test
        ... notice that the tip commit of bar is not ready
        $ git checkout foo ;# oops, mistake
        $ git reset --hard HEAD^
	$ git push

If you checked out foo instead of bar by mistake at the last
"git checkout" step like this, your change will make 'foo' an
ancestor of the other side of the connection, and push silently
ignores it instead of failing.

Also, the behaviour is too specific to your workflow of working
on things only to completion between pushes.  If you work a bit
on branch 'foo' (but not complete), and work much on branch
'bar', 'baz', and 'boo' making all of them ready to be
published, you cannot say "git push" anyway.  Instead you have
to say "git push $remote bar baz boo".

This discourages people from making commits that are not ready
to be published, which is a very wrong thing to do, as a major
selling point of distributed revision control is the
dissociation between committing and publishing.

You work and commit freely, and at any point some of your
branches are ready to be published while some others
aren't. Inconvenience of "matching refs" may need to be worked
around.  I liked your "current branch only", with "git push
$remote HEAD" (I presume that "remote.$remote.push = HEAD" and
"branch.$current.remote = $remote" would let you do that with
"git push"), exactly because the way it specifies which branch
is to be published is very clearly defined and easy to
understand.  This "matching but only ff" does not have that
attractive clarity.

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

[Steffen Prohaska]

On Oct 30, 2007, at 8:19 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 30, 2007, at 9:29 AM, Junio C Hamano wrote:
>>
>>> It simply is insane to make this strange rule 10/10 introduces
>>> the default behaviour.  It is too specific to a particular
>>> workflow (that is, working with a shared central repository,
>>> having many locally tracking branches that are not often used
>>> and become stale, and working on only things to completion
>>> between pushes).
>>
>> I don't think its very strange behaviour if you see it in the
>> light of what the user wants to achieve. We are talking about
>> the case were only fast forward pushes are allowed. So, we
>> only talk about a push that has the goal of adding new local
>> changes to the remote. The user says "git push" and means
>> push my new local changes to the remote.
>
> If you want to push a specific subset of branches, you should
> not be invoking the "matching refs" to begin with.  And breaking
> the "matching refs" behaviour is not the way to fix it.

ok.

So, git push shall guarantee that all matching refs point
to the _same_ commit if a push was successful. Otherwise,
git push shall report an error.

Would it be acceptable if the error was less severe in the
case of local being a strict subset of remote?
Daniel proposed
"%s: nothing to push to %s, but you are not up-to-date and
may want to pull"
It would still be an error, but a less severe one.

It could also be a good idea to teach git push transactional
behaviour. It could check in advance ('--dry-run') if the
push will succeed. If not it should report the errors without
actually pushing. Then, _nothing_ would have been changed on
the remote. Only if everything is ok "git push" would modify
the remote. Well, I think it might be hard to avoid the race
condition when someone else pushes simultaneously to a shared
repo. But this hopefully rarely happens.


> You can rewind a wrong branch by mistake locally and run push.
> With your change you would not notice that mistake.
>
>         $ git checkout bar
>         $ work work work; commit commit commit
> 	$ git checkout test
>         $ git merge bar
> 	... integrate, build, test
>         ... notice that the tip commit of bar is not ready
>         $ git checkout foo ;# oops, mistake
>         $ git reset --hard HEAD^
> 	$ git push
>
> If you checked out foo instead of bar by mistake at the last
> "git checkout" step like this, your change will make 'foo' an
> ancestor of the other side of the connection, and push silently
> ignores it instead of failing.

Yes, there are many ways you can mess up ;)


> Also, the behaviour is too specific to your workflow of working
> on things only to completion between pushes.  If you work a bit
> on branch 'foo' (but not complete), and work much on branch
> 'bar', 'baz', and 'boo' making all of them ready to be
> published, you cannot say "git push" anyway.  Instead you have
> to say "git push $remote bar baz boo".

Ok and this is the root why I work only to completion between
pushes. I tried to figure out a "safe" workflow. If you
accidentally type "git push" nothing wrong should happen. I
am sure that people will sometimes type "git push" forgetting
to mention anything. At least, I am sure that _I_ will do this.

The only comfortable way to make "git push" safe with
the current behaviour is to work on local branches only to
completion. Then, you can push to any repository at any time
and nothing bad can happen.

Alternatives with existing git are

- never use "git push", but always tell git explicitly what you
   want. This is too dangerous for me because at some point I'll
   type "git push". The problem with "git push" is that it's
   really hard to undo. It's near to impossible if you pushed
   to a public remote. Therefore, I really want to avoid this danger.

- Configure specific push rules for remotes that switch off
   the "matching branches" default. You can for example 'switch'
   off the default by configuring
   "remote.$remote.push = nonexisting". But then I started
   to get annoyed by all the configuration work. I do not want
   to explain such details to people who get started with git.
   And you do not get reasonable messages either. And btw I'd
   prefer if git push just did the right thing.


Alternatives that require changing git push are

- git push would do _nothing_ by default. git push would ask
   "what do you mean? Need at least a remote, or better remote
    and branch."
   Options could be provided to push current branch (--current)
   or all matching branches (--matching).

- git push _by default_ would only push the current branch. This
   would at least be a "safer" default.

- git push would first run --dry-run and then ask for
   confirmation. Something like:
   "Do you really want to push this to that remote? Here is
   the URL and the branches. Did you really mean this?
   WARNING: you can't undo this operation. And btw if you say
   yes, I'll report errors anyway because some remotes are not
   strict subsets. So maybe you want to fix things first."

- git push can be configuration to push only the current
   branch, as outlined below. This would certainly work. What
   I do not like is that you first need to do some configuration
   before you get a safe working environment.


> This discourages people from making commits that are not ready
> to be published, which is a very wrong thing to do, as a major
> selling point of distributed revision control is the
> dissociation between committing and publishing.

Yes, the current default behaviour of git push discourages me
to work that way.


> You work and commit freely, and at any point some of your
> branches are ready to be published while some others
> aren't. Inconvenience of "matching refs" may need to be worked
> around.  I liked your "current branch only", with "git push
> $remote HEAD" (I presume that "remote.$remote.push = HEAD" and
> "branch.$current.remote = $remote" would let you do that with
> "git push"), exactly because the way it specifies which branch
> is to be published is very clearly defined and easy to
> understand.  This "matching but only ff" does not have that
> attractive clarity.

In my view, that would be safer than what we have now.

	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> Would it be acceptable if the error was less severe in the
> case of local being a strict subset of remote?
> Daniel proposed
> "%s: nothing to push to %s, but you are not up-to-date and
> may want to pull"
> It would still be an error, but a less severe one.

I am not convinced there is one true total order of "error
severity" that applies uniformly across different workflows, so
I would not immediately agree if you are suggesting to introduce
"severity levels".  But it certainly makes a lot of sense to be
able to _differentiate_ kinds of errors, and to have the calling
scripts and the push command itself react to them.

What are the possible error conditions?

 1. Error on the sending side.  The ref parameters given to
    git-push were bogus, or they were good commits but they were
    not fully connected to the commits the other side has
    (i.e. local repository corruption).  pack-objects will abort
    and no remote (nor local tracking ref that tracks what we
    pushed to the remote) would be updated.  This should be
    "most severe" in _any_ workflow, so I do not mind calling
    this "fatal".

 2. Push to a ref does fast forward, but the update hook on the
    remote side declines.  The ref on the remote nor the
    corresponding local tracking ref would not be updated, and
    the command would fail.

For all the other classes of errors, the ref on the remote nor
the corresponding local tracking ref would not be updated, and
by default, an error on any ref causes the command to error out.
For each of these classes of errors, we _could_ have an option
to let you tell the command not to error out because of it.

 3. Push to a ref does not fast forward and --force is not
    given, but you can prove the remote is strict subset of
    local (what your 10/10 wants to do).

 4. Same as #3 but you cannot prove the remote is strict subset
    of local.

Any other classes?

It might be a good idea to generalize 3 & 4, by the way.  The
remote being a strict descendant of what is being pushed might
be something you happened to want today, but somebody else may
come up with a different rule tomorrow.  So, 

 3'. Push to a ref does not fast forward and --force is not
     given, but there is a configuration (would this be per
     remote?, per remote branch?, or per local branch?) that
     tells git-push to call a hook on the local side that takes
     <ref being pushed, ref on the remote> as its parameter.
     The result from the hook does not change the fact that this
     is still an error, but it can instruct git-push not to
     error out due to this condition.

In some other workflows, it might make sense to maybe even
making 2. not to cause the error from git-push.  I dunno.

> It could also be a good idea to teach git push transactional
> behaviour.

That is certainly true.  I am not sure about other transports,
but it should be a relatively straightforward protocol extension
for the git native transport.

> - git push can be configuration to push only the current
>   branch, as outlined below. This would certainly work. What
>   I do not like is that you first need to do some configuration
>   before you get a safe working environment.

I would not doubt it would be safer for _your_ workflow, but you
should consider the risk of making things more cumbersome for
workflows of others by enforcing that policy.

In other words, don't change anything unless you have a very
good reason to convince everybody else that it is universally
a good change to the default.

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

[Junio C Hamano]

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

>  1. Error on the sending side.  The ref parameters given to
>     git-push were bogus, or they were good commits but they were
>     not fully connected to the commits the other side has
>     (i.e. local repository corruption).  pack-objects will abort
>     and no remote (nor local tracking ref that tracks what we
>     pushed to the remote) would be updated.  This should be
>     "most severe" in _any_ workflow, so I do not mind calling
>     this "fatal".

By the way, as git-push allows an arbitrary SHA-1 on the left
hand side of a refspec, you can have the above error without a
corrupted repository.  Here is how.

 * You run git-fetch from elsewhere.  It is a small fetch and we
   decide not to keep the pack (iow, run unpack-objects instead
   of index-pack on the local side).  Or the fetch is over dumb
   transport that walks commits one-by-one.

   This git-fetch is interrupted.  We do _not_ update any refs
   in such a case, but we do not eradicate loose objects that
   were downloaded.  They stay dangling.

 * You push one of the commits downloaded above.  I.e. it is
   not connected to any of your ref.

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

[Steffen Prohaska]

On Oct 31, 2007, at 9:45 AM, Junio C Hamano wrote:

>> - git push can be configuration to push only the current
>>   branch, as outlined below. This would certainly work. What
>>   I do not like is that you first need to do some configuration
>>   before you get a safe working environment.
>
> I would not doubt it would be safer for _your_ workflow, but you
> should consider the risk of making things more cumbersome for
> workflows of others by enforcing that policy.

Together with the '--create' flag it would be safer in all
cases, because it would always do _less_ than what git push
currently does. The safest choice would be if "git push"
refused to do anything until configured appropriately.

"safer" is independent of the workflow.

But I see that it may be more cumbersome depending on the
workflow.

I'm mainly interested in using git against a shared repo,
and make it as simple and as safe as possible to use in
such a setup. I suspect that git is more optimized for the
workflow used for the Linux kernel and for developing git,
which heavily rely on sending patches to mailing lists and
pulling fro read-only repos.


> In other words, don't change anything unless you have a very
> good reason to convince everybody else that it is universally
> a good change to the default.

What I can imagine would not be universally better, but it
would be universally safer. You'd need to either explicitly
tell git push how to act (e.g. '--current' or '--matching'
flags), or you could explicitly configure git to always act in
a specific way. But it would only start to act this way _after_
being configured appropriately.

	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 31, 2007, at 9:45 AM, Junio C Hamano wrote:
>
>> I would not doubt it would be safer for _your_ workflow, but you
>> should consider the risk of making things more cumbersome for
>> workflows of others by enforcing that policy.
>
> Together with the '--create' flag it would be safer in all
> cases, because it would always do _less_ than what git push
> currently does. The safest choice would be if "git push"
> refused to do anything until configured appropriately.
>
> "safer" is independent of the workflow.

By your definition, a command that does not do anything by
default is safer regardless of the workflow.

That may be theoretically true --- it cannot do any harm by
default.  But that is not useful.

> I'm mainly interested in using git against a shared repo,
> and make it as simple and as safe as possible to use in
> such a setup. I suspect that git is more optimized for the
> workflow used for the Linux kernel and for developing git,
> which heavily rely on sending patches to mailing lists and
> pulling fro read-only repos.

You forgot a lot more important part.  Pushing into publishing
repositories.  And the discussion is about git-push command.

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

[Steffen Prohaska]

On Oct 31, 2007, at 7:51 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 31, 2007, at 9:45 AM, Junio C Hamano wrote:
>>
>>> I would not doubt it would be safer for _your_ workflow, but you
>>> should consider the risk of making things more cumbersome for
>>> workflows of others by enforcing that policy.
>>
>> Together with the '--create' flag it would be safer in all
>> cases, because it would always do _less_ than what git push
>> currently does. The safest choice would be if "git push"
>> refused to do anything until configured appropriately.
>>
>> "safer" is independent of the workflow.
>
> By your definition, a command that does not do anything by
> default is safer regardless of the workflow.
>
> That may be theoretically true --- it cannot do any harm by
> default.  But that is not useful.

If different workflows have contradicting needs, doing nothing
by default might be a good choice. Not theoretically, but in
practice.


>> I'm mainly interested in using git against a shared repo,
>> and make it as simple and as safe as possible to use in
>> such a setup. I suspect that git is more optimized for the
>> workflow used for the Linux kernel and for developing git,
>> which heavily rely on sending patches to mailing lists and
>> pulling from read-only repos.
>>
>
> You forgot a lot more important part.  Pushing into publishing
> repositories.  And the discussion is about git-push command.

Exactly, here are two examples:

If you push only to publishing repositories that are read
only by others, you'll never encounter the problem that
10/10 tried to solve. The publishing repository is never
changed by others. You are the only one who pushes to this
repository. Therefore the remote never advances unexpectedly.

A shared repository behaves differently. Others push to the
repository as well. Hence, branches can advance unexpectedly.


Another difference is the way changes are integrated. In
a workflow without shared repositories, only pull is used
for integration, while push in only used for publishing the
changes. After a push you always need to request someone else
to pull. For example:

- Alice publishes branch foo.
- Bob clones Alice's repository and checks out foo as his
   local branch bar.
- Bob later publishes his branch by pushing bar to his
   public repository and asks Alice to pull.
- Alice pulls bar from Bobs public repository and merges
   with foo. She then publishes the integrated changes
   by pushing foo to her public repository.

My point is: there is no need to push from branch bar to
branch foo. Alice and Bob both push to branches that are named
identical in their private and their public repositories.
Only pull is used to merge changes from the branch named bar
to the branch named foo.

This is different if you work with a shared repository. Bob
checks out the shared branch foo to his local branch bar and
later he needs to push bar back to the shared branch foo. Bob
needs to push changes from his local branch bar to the branch
foo in the remote repository, a branch with a different name.
This need does not emerge when working with two publishing
repositories, as described above.


This was the extended version of what I meant above. The
workflow used for the Linux kernel and for developing git is
focused on pull. Push is normally only used for publishing
branches under identical name. The interesting stuff happens
during the pull.

	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

>> You forgot a lot more important part.  Pushing into publishing
>> repositories.  And the discussion is about git-push command.
>
> Exactly, here are two examples:
>
> If you push only to publishing repositories that are read
> only by others, you'll never encounter the problem that
> 10/10 tried to solve. The publishing repository is never
> changed by others. You are the only one who pushes to this
> repository. Therefore the remote never advances unexpectedly.

Wrong.

People can and do work from more than one private repositories
(I do).  In a sense, that is sharing the repository with
oneself.

I may do an emergency patch to fix breakage on 'maint' (and
'maint' only) from a location that is not my primary development
box and push the fix out.  I fully expect that the push will
push out 'maint' and expect the other branches such as 'master'
on the remote side to stay the same, as I haven't touched
'master' on that box for quite a while and it is now stale.  In
that situation, I _want_ the "git push" itself to report failure
to notify me that it did not push what _I_ asked it to push out,
so that I can be reminded that I'd better do "git push $remote
maint" the next time.  In the meantime, even though it reports
a failure, 'master' on the remote side is _not_ updated, so the
behaviour is still _safe_.

> Another difference is the way changes are integrated. In
> a workflow without shared repositories, only pull is used
> for integration, while push in only used for publishing the
> changes.

Wrong.  push is a mirror of fetch and does not do _any_
integration.  It is just a safe (because it insists on
fast-forward) propagation mechanism.  Your integration still
happens with pull (actually, shared repository people seem to
prefer "fetch + rebase" over "pull" which is "fetch + merge").

> This is different if you work with a shared repository. Bob
> checks out the shared branch foo to his local branch bar and
> later he needs to push bar back to the shared branch foo. Bob
> needs to push changes from his local branch bar to the branch
> foo in the remote repository, a branch with a different name.
> This need does not emerge when working with two publishing
> repositories, as described above.

So you do "git push $remote bar:foo".  If you do that regulary,
there are configuration mechanisms to help you reduce your
keyboard wear.  What's the problem?

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

[Steffen Prohaska]

On Oct 31, 2007, at 10:31 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>>> You forgot a lot more important part.  Pushing into publishing
>>> repositories.  And the discussion is about git-push command.
>>
>> Exactly, here are two examples:
>>
>> If you push only to publishing repositories that are read
>> only by others, you'll never encounter the problem that
>> 10/10 tried to solve. The publishing repository is never
>> changed by others. You are the only one who pushes to this
>> repository. Therefore the remote never advances unexpectedly.
>
> Wrong.
>
> People can and do work from more than one private repositories
> (I do).  In a sense, that is sharing the repository with
> oneself.

I do, too. But as long as I do not forget what I've done, the
branches do not advance _unexpectedly_. I am in full control.


> I may do an emergency patch to fix breakage on 'maint' (and
> 'maint' only) from a location that is not my primary development
> box and push the fix out.  I fully expect that the push will
> push out 'maint' and expect the other branches such as 'master'
> on the remote side to stay the same, as I haven't touched
> 'master' on that box for quite a while and it is now stale.  In
> that situation, I _want_ the "git push" itself to report failure
> to notify me that it did not push what _I_ asked it to push out,
> so that I can be reminded that I'd better do "git push $remote
> maint" the next time.  In the meantime, even though it reports
> a failure, 'master' on the remote side is _not_ updated, so the
> behaviour is still _safe_.

You're right it is safe, but it may be confusing.


>> Another difference is the way changes are integrated. In
>> a workflow without shared repositories, only pull is used
>> for integration, while push in only used for publishing the
>> changes.
>
> Wrong.  push is a mirror of fetch and does not do _any_
> integration.  It is just a safe (because it insists on
> fast-forward) propagation mechanism.  Your integration still
> happens with pull (actually, shared repository people seem to
> prefer "fetch + rebase" over "pull" which is "fetch + merge").

Right; but you can't push without doing the integration. If you
have new changes on the remote side you _must_ pull before
you can push. You're forced to do the integration immediately.
Your main objective was to push, but the shared workflow forces
you to do the integration _now_ (by using pull). In a pull-only
workflow, you can just push and defere the integration for later.

Some people claim fetch + rebase is superior to fetch + merge.
The only point I can see is that fetch + rebase gives a linear
history without loops, which is nicer to visualize. I recently
asked on the list if there are any benefits of fetch + rebase
over fetch + merge, besides a nicer visualization. I didn't
receive many interesting comments. One comment explained
that rebase can shift the merge conflict resolution from
the maintainer (merge) to the original author (rebase). But
this is not very interesting in a shared workflow, because
the author must resolve conflicts in any case before he can
push. It doesn't matter much if he uses merge or rebase to
do so.

I evaluated if teaching people fetch + rebase before teaching
fetch + merge is a good idea. Therefore I tested some scenarios
with people who are new to git. The result is that there are
too many situations where fetch + rebase might be confusing.
I abandoned my idea.

I decided that fetch + merge is _easier_. It works in all
situations, it's easier to explain, it's better supported
(automerge), it can be used to work on shared topic branches.
Definitely fetch + merge is the first workflow you should
learn. At the moment I'm not anymore interested in the fetch +
rebase approach.


>> This is different if you work with a shared repository. Bob
>> checks out the shared branch foo to his local branch bar and
>> later he needs to push bar back to the shared branch foo. Bob
>> needs to push changes from his local branch bar to the branch
>> foo in the remote repository, a branch with a different name.
>> This need does not emerge when working with two publishing
>> repositories, as described above.
>
> So you do "git push $remote bar:foo".  If you do that regulary,
> there are configuration mechanisms to help you reduce your
> keyboard wear.  What's the problem?

Too complex and not flexible enough.

The configuration is in the remote section. Therefore I can
tell git what to do only on a per-branch basis. What do you
think about my recent proposal to add branch.$name.push?


And I want to avoid that people need to learn about the details
of the configuration mechanism on the first time they use git.

I am searching for a solution that just works for them. They
currently use CVS. I'll give them a detailed getting started
document for git. The workflow described should be as simple as
possible, but safe and reliable. No confusing error messages
should appear. Only a few commands should be needed to
contribute to a shared branch. The workflow described should
use git in a sane way that provides opportunities to use more
of its power later.

So here is what I'd like to have.

    git clone ssh://server/git/project.git project

    [ On Windows the hassel already starts because it actually is

	git clone -n ssh://sever/git/project.git project
	git config core.autocrlf true

      And here's the next point. git config doesn't validate the
      variable. It accepts _any_ variable. If you have a typo
      you go without autocrlf. ... but this is a different story. ]

    cd project
    git checkout -b devel origin/devel
    # work, commit, work, commit
    git push  # maybe git pull first, but git would tell you

The last command, git push, can already cause trouble. git
automatically created a local master and the remote master
may have advanced, so git push would complain with an error.
Currently the correct command would be
"git push origin devel".


An alternative scenario is that you want to start work that
will not be ready right away. So you start a topic branch

    git checkout -b topic origin/devel
    # work, commit, some time passes, work, commit
    git pull 	# integrate changes from devel
    # work, commit
    git pull
    git push 	# this one should push to origin/devel


In scenario three you planned to finish your work right away
but the problem turned out to be harder. Here, the following
would be nice

    git checkout -b devel origin/devel
    # work, commit, hmm... much harder ...
    git branch -m devel dolater

    # do something else

    git checkout dolater
    # finish work
    git pull    # integrate with other work on devel
    git push    # push back to shared branch


Another question is what to do with a local branch after
you finished work. We recently had the
"Re: best git practices, was Re: Git User's Survey 2007
unfinished summary continued" aka the 200-local-branches
discussion.

There were different suggestions what to do. A reasonable
suggestion was to delete the local branch after you're done.
This clearly distinguishes between remote branches (which are
mirrored as a remote tracking branch) and local branches. Local
branches are _your_ branches while the remote branches contain
the shared work. If you're done with your local work, delete
your local branch. So maybe you should do

    git checkout origin/devel
    git branch -d devel

Now you're on a detached branch that points to origin/work.
But how to do you get new changes from others? git pull would
not work and git fetch neither.

Independently of what the best practice is, leaving the local
work branch there shouldn't do any harm because I'm sure that
some devs will forget to clean up, independently of what I tell
them.

	Steffen

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

[Andreas Ericsson]

Steffen Prohaska wrote:
> 
> On Oct 31, 2007, at 10:31 PM, Junio C Hamano wrote:
> 
>> Steffen Prohaska <prohaska@zib.de> writes:
>>
>>>> You forgot a lot more important part.  Pushing into publishing
>>>> repositories.  And the discussion is about git-push command.
>>>
>>> Exactly, here are two examples:
>>>
>>> If you push only to publishing repositories that are read
>>> only by others, you'll never encounter the problem that
>>> 10/10 tried to solve. The publishing repository is never
>>> changed by others. You are the only one who pushes to this
>>> repository. Therefore the remote never advances unexpectedly.
>>
>> Wrong.
>>
>> People can and do work from more than one private repositories
>> (I do).  In a sense, that is sharing the repository with
>> oneself.
> 
> I do, too. But as long as I do not forget what I've done, the
> branches do not advance _unexpectedly_. I am in full control.
> 
> 
>> I may do an emergency patch to fix breakage on 'maint' (and
>> 'maint' only) from a location that is not my primary development
>> box and push the fix out.  I fully expect that the push will
>> push out 'maint' and expect the other branches such as 'master'
>> on the remote side to stay the same, as I haven't touched
>> 'master' on that box for quite a while and it is now stale.  In
>> that situation, I _want_ the "git push" itself to report failure
>> to notify me that it did not push what _I_ asked it to push out,
>> so that I can be reminded that I'd better do "git push $remote
>> maint" the next time.  In the meantime, even though it reports
>> a failure, 'master' on the remote side is _not_ updated, so the
>> behaviour is still _safe_.
> 
> You're right it is safe, but it may be confusing.
> 
> 
>>> Another difference is the way changes are integrated. In
>>> a workflow without shared repositories, only pull is used
>>> for integration, while push in only used for publishing the
>>> changes.
>>
>> Wrong.  push is a mirror of fetch and does not do _any_
>> integration.  It is just a safe (because it insists on
>> fast-forward) propagation mechanism.  Your integration still
>> happens with pull (actually, shared repository people seem to
>> prefer "fetch + rebase" over "pull" which is "fetch + merge").
> 
> Right; but you can't push without doing the integration. If you
> have new changes on the remote side you _must_ pull before
> you can push.

Yes, because otherwise you'd rewrite published history. That's not
a good thing.

> You're forced to do the integration immediately.

Yes, but you get to choose how. Perhaps git-push should list more
options than just git-pull, such as the three commands required to
rebase the currently checked out branch onto its remote counterpart.
That would support more workflows.

> Your main objective was to push, but the shared workflow forces
> you to do the integration _now_ (by using pull). In a pull-only
> workflow, you can just push and defere the integration for later.
> 

No, you can also fetch + rebase.

> Some people claim fetch + rebase is superior to fetch + merge.
> The only point I can see is that fetch + rebase gives a linear
> history without loops, which is nicer to visualize. I recently
> asked on the list if there are any benefits of fetch + rebase
> over fetch + merge, besides a nicer visualization.


It's easier to bisect. If git bisect lands you on a merge-commit,
you need to start a new bisect for each of the parents included
in the merge. Hopefully the nature of the merge gives a clue so
the user can make an educated guess as to which parent introduced
the bogus commit, but for an "evil octopus" (unusual) or if the
merge had conflicts which were resolved in a buggy way (not
exactly uncommon), it can be quite a hassle to get things right.
With a mostly linear history, this problem goes away.


> I didn't
> receive many interesting comments. One comment explained
> that rebase can shift the merge conflict resolution from
> the maintainer (merge) to the original author (rebase). But
> this is not very interesting in a shared workflow, because
> the author must resolve conflicts in any case before he can
> push. It doesn't matter much if he uses merge or rebase to
> do so.
> 

It depends. When commit ordering doesn't matter the original
author can use "git rebase --skip" and then continue with the
rebase to get as much as possible out as quickly as possible.
I'm in the unfortunate position of having a boss that likes
to fiddle with help-texts in code when it's in alpha-testing.
Sometimes that causes conflicts but it's often not important
enough to spend 30 minutes figuring out how to resolve it
properly. I tend to just skip those patches and send them as
emails to our tech-writer instead, asking him to rephrase the
text to incorporate both changes, and then manually applying
the text to the end result.

> 
> I am searching for a solution that just works for them. They
> currently use CVS. I'll give them a detailed getting started
> document for git. The workflow described should be as simple as
> possible, but safe and reliable.


If they're used to CVS and want to use more than one branch without
having to learn additional syntax, nothing can help, methinks.

> 
> Another question is what to do with a local branch after
> you finished work. We recently had the
> "Re: best git practices, was Re: Git User's Survey 2007
> unfinished summary continued" aka the 200-local-branches
> discussion.
> 

We're at 224 branches now, having added 7 new repos.

> There were different suggestions what to do. A reasonable
> suggestion was to delete the local branch after you're done.

Except that it doesn't work unless you either detach the HEAD
(which prints a big fat ugly message) or give it -D to force
it, which I really, really don't recommend. We use git because
I'm pretty confident in its capabilities of never ever losing
anything. Using the seemingly harmless -D switch to git-branch
puts us at risk of wiping history quite without noticing.

> This clearly distinguishes between remote branches (which are
> mirrored as a remote tracking branch) and local branches. Local
> branches are _your_ branches while the remote branches contain
> the shared work. If you're done with your local work, delete
> your local branch. So maybe you should do
> 
>    git checkout origin/devel

Except that this gives a warning-esque message:
Note: moving to "origin/devel" which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
  git checkout -b <new_branch_name>
HEAD is now at deadbeef... Ma! Pa butchered all the cows!

To me, this indicates I've done something git thinks I shouldn't have.

> 
> Independently of what the best practice is, leaving the local
> work branch there shouldn't do any harm because I'm sure that
> some devs will forget to clean up, independently of what I tell
> them.
> 

I wholeheartedly agree with this one.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

[Steffen Prohaska]

On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:

> Steffen Prohaska wrote:
>> On Oct 31, 2007, at 10:31 PM, Junio C Hamano wrote:
>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>
>>>> Another difference is the way changes are integrated. In
>>>> a workflow without shared repositories, only pull is used
>>>> for integration, while push in only used for publishing the
>>>> changes.
>>>
>>> Wrong.  push is a mirror of fetch and does not do _any_
>>> integration.  It is just a safe (because it insists on
>>> fast-forward) propagation mechanism.  Your integration still
>>> happens with pull (actually, shared repository people seem to
>>> prefer "fetch + rebase" over "pull" which is "fetch + merge").
>> Right; but you can't push without doing the integration. If you
>> have new changes on the remote side you _must_ pull before
>> you can push.
>
> Yes, because otherwise you'd rewrite published history. That's not
> a good thing.
>
>> You're forced to do the integration immediately.
>
> Yes, but you get to choose how. Perhaps git-push should list more
> options than just git-pull, such as the three commands required to
> rebase the currently checked out branch onto its remote counterpart.
> That would support more workflows.

I agree. Providing better hints would be good.


>> Your main objective was to push, but the shared workflow forces
>> you to do the integration _now_ (by using pull). In a pull-only
>> workflow, you can just push and defer the integration for later.
>
> No, you can also fetch + rebase.

Right. My point was than one cannot defer the integration. It
must be addressed immediately.


>> Some people claim fetch + rebase is superior to fetch + merge.
>> The only point I can see is that fetch + rebase gives a linear
>> history without loops, which is nicer to visualize. I recently
>> asked on the list if there are any benefits of fetch + rebase
>> over fetch + merge, besides a nicer visualization.
>
>
> It's easier to bisect. If git bisect lands you on a merge-commit,
> you need to start a new bisect for each of the parents included
> in the merge. Hopefully the nature of the merge gives a clue so
> the user can make an educated guess as to which parent introduced
> the bogus commit, but for an "evil octopus" (unusual) or if the
> merge had conflicts which were resolved in a buggy way (not
> exactly uncommon), it can be quite a hassle to get things right.
> With a mostly linear history, this problem goes away.

This is really an interesting point. I did not start to use
git bisect regularly. But I certainly plan to do so in the future.

Couldn't bisect learn to better cope with non-linear history?

[...]


>> I am searching for a solution that just works for them. They
>> currently use CVS. I'll give them a detailed getting started
>> document for git. The workflow described should be as simple as
>> possible, but safe and reliable.
>
>
> If they're used to CVS and want to use more than one branch without
> having to learn additional syntax, nothing can help, methinks.

They will learn. But they must not get frustrated too early.
I also don't wont to see them lining up in front of my office.


BTW, what do you thing about the proposal to add branch.$name.push [1]?

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


[...]

>> There were different suggestions what to do. A reasonable
>> suggestion was to delete the local branch after you're done.
>
> Except that it doesn't work unless you either detach the HEAD
> (which prints a big fat ugly message) or give it -D to force
> it, which I really, really don't recommend. We use git because
> I'm pretty confident in its capabilities of never ever losing
> anything. Using the seemingly harmless -D switch to git-branch
> puts us at risk of wiping history quite without noticing.

I don't like -D either. I liked the idea mentioned recently
to check -d against the remotes. If a remote tracking branch
has the history it should be considered fully merged.

Another idea may be to distinguish between detached head and
checkout of remote tracking branch. Maybe we could do some
useful things if get knew that the user is 'on a remote tracking
branch'. Committing could be forbidden. A suggestion would be
printed instead to use "git checkout -b something", which could act
as if the remote branch was mentioned on the command line.

Something like that would be needed before I'd seriously
suggest to delete local branches after you finished your work.


>> This clearly distinguishes between remote branches (which are
>> mirrored as a remote tracking branch) and local branches. Local
>> branches are _your_ branches while the remote branches contain
>> the shared work. If you're done with your local work, delete
>> your local branch. So maybe you should do
>>    git checkout origin/devel
>
> Except that this gives a warning-esque message:
> Note: moving to "origin/devel" which isn't a local branch
> If you want to create a new branch from this checkout, you may do so
> (now or later) by using -b with the checkout command again. Example:
>  git checkout -b <new_branch_name>
> HEAD is now at deadbeef... Ma! Pa butchered all the cows!
>
> To me, this indicates I've done something git thinks I shouldn't have.

I agree. This could probably be suppressed if git handled remote
tracking branches a bit differently from other detached heads.


>> Independently of what the best practice is, leaving the local
>> work branch there shouldn't do any harm because I'm sure that
>> some devs will forget to clean up, independently of what I tell
>> them.
>
> I wholeheartedly agree with this one.

So I think we need to resolve this first.

Do you already have post-checkout script that makes useful
suggestions.  I remember you mentioned something like that
during the 200-local-branches discussion.

	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:
>
>> Steffen Prohaska wrote:
>>
>>> You're forced to do the integration immediately.

The context of this "forced" is that you say (in the following
paragraph) the user's main objective was to "push", but I do not
think "to push" is ever the main objective.

 - If it is to give integrated result for others to work further
   on, then you need to resolve before being able to achieve
   that goal.  There is no escaping from it.

 - On the other hand, if it is to show what you did as early as
   possible in a working shape, and if the updated shared
   repository has changes from somebody else that conflicts you,
   in a CVS/SVN style shared workflow, there is no way for you
   to show what you did in isolation.  If you try to follow that
   model in git and insist pushing to the same branch, then you
   are forced to resolve first.

   But you do not have to.  You could push out to another new
   branch, and say "Here is how you could do it, although this
   is based on an older codebase and conflicts with what
   recently happened to the tip".  You could even ask other
   party whose changes conflict with yours to help with the
   merge by saying "I pushed it out, you are more familiar with
   that area of the code and with your changes near the tip of
   the trunk, so could you merge it and push out the result?"

>> Yes, but you get to choose how. Perhaps git-push should list more
>> options than just git-pull, such as the three commands required to
>> rebase the currently checked out branch onto its remote counterpart.
>> That would support more workflows.
>
> I agree. Providing better hints would be good.

I am not so sure about that.  If there are three different
workflows, should git-push give hints suitable for all of them?

The current hint was added in response to users' requests, and I
think it could be generalized.  What we would want the end user
to realize is:

    What I tried to push out is stale, I do not want to push out
    something that does not contain what the other side has
    done, so I need to integrate my work with what the other
    side have before pushing to that branch at the remote.

    In my workflow, that means doing rebase of the branch I
    tried to push out on top of the remote branch I was trying
    to push to.

The second paragraph depends on the workflow.  Do we want to
(can we afford the space to) give a laundry list here?  Probably
not.

>>> Your main objective was to push, but the shared workflow forces
>>> you to do the integration _now_ (by using pull). In a pull-only
>>> workflow, you can just push and defer the integration for later.
>>
>> No, you can also fetch + rebase.
>
> Right. My point was than one cannot defer the integration. It
> must be addressed immediately.

See above.

>>> Some people claim fetch + rebase is superior to fetch + merge.
>>> The only point I can see is that fetch + rebase gives a linear
>>> history without loops, which is nicer to visualize. I recently
>>> asked on the list if there are any benefits of fetch + rebase
>>> over fetch + merge, besides a nicer visualization.
>>
>>
>> It's easier to bisect...
>> With a mostly linear history, this problem goes away.
>
> This is really an interesting point. I did not start to use
> git bisect regularly. But I certainly plan to do so in the future.
>
> Couldn't bisect learn to better cope with non-linear history?

It copes with it as best as it can.

Another thing to think about is how "everybody fetches, merges
and pushes out" would interact with the concept of "mainline".
Strictly speaking, the point of distributed development is that
there is no mainline, but workflows based on "fetch + rebase"
allows --first-parent to give a reasonable approximation of what
people would naively expect how the mainline would look like.
If everybody fetches, merges and pushes out, there is no
"mainline" and --first-parent would give totally useless
history.

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

[Steffen Prohaska]

On Nov 1, 2007, at 9:18 PM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:
>>
>>> Steffen Prohaska wrote:
>>>
>>>> You're forced to do the integration immediately.
>
> The context of this "forced" is that you say (in the following
> paragraph) the user's main objective was to "push", but I do not
> think "to push" is ever the main objective.

Right. I should probably describe a bit more of the context.

We have a shared branch for a group of developer who are located
in the same building. We are allowed to commit reasonably stable
code to this branch. Changes should compile and the commiter
should be convinced that it does something useful without
breaking other code. But failing to meet these requirements
is acceptable. For example it is sufficient to compile on
only one architecture and have good reason to believe that
the other architectures will work, too.

A nightly job builds the shared branch on all of our
architectures and creates a report that is available the next
day. If problems happen you should fix them asap. If someone
spots problems causes by others that need to be addressed
right away, he can walk over to the office of the one who
caused the problem.

In this setting a user really want to push. Because only then
the code will be tested and available for all others. ...



>  - If it is to give integrated result for others to work further
>    on, then you need to resolve before being able to achieve
>    that goal.  There is no escaping from it.
>
>  - On the other hand, if it is to show what you did as early as
>    possible in a working shape, and if the updated shared
>    repository has changes from somebody else that conflicts you,
>    in a CVS/SVN style shared workflow, there is no way for you
>    to show what you did in isolation.  If you try to follow that
>    model in git and insist pushing to the same branch, then you
>    are forced to resolve first.
>
>    But you do not have to.  You could push out to another new
>    branch, and say "Here is how you could do it, although this
>    is based on an older codebase and conflicts with what
>    recently happened to the tip".  You could even ask other
>    party whose changes conflict with yours to help with the
>    merge by saying "I pushed it out, you are more familiar with
>    that area of the code and with your changes near the tip of
>    the trunk, so could you merge it and push out the result?"


... I know we could use git to establish a more complex workflow
that would give better guarantees on the published branches.

But it's a judgement how much complexity you want to
add. Pushing to a different branch instead of solving
conflicts right away may be a good model to postpone conflict
resolution. But it requires more knowledge of git and more
commands. Right now, the users are trained on a CVS workflow
and they expect that conflicts may occur and if so need to
be addressed right away. The next step is probably to learn
how git could help them to do this.  (index vs. work tree,
mergetool, ...)

Btw, I have another 'stable' branch, which I have full control
over. This branch is built and tested prior to pushing to the
public repository. So, if the shared branch completely breaks
down, we can fall-back to the stable branch.

We haven't figured out much more of our workflow. The first
milestone is to migrate from CVS to git continuing to use a
CVS-style workflow.



>>> Yes, but you get to choose how. Perhaps git-push should list more
>>> options than just git-pull, such as the three commands required to
>>> rebase the currently checked out branch onto its remote counterpart.
>>> That would support more workflows.
>>
>> I agree. Providing better hints would be good.
>
> I am not so sure about that.  If there are three different
> workflows, should git-push give hints suitable for all of them?
>
> The current hint was added in response to users' requests, and I
> think it could be generalized.  What we would want the end user
> to realize is:
>
>     What I tried to push out is stale, I do not want to push out
>     something that does not contain what the other side has
>     done, so I need to integrate my work with what the other
>     side have before pushing to that branch at the remote.
>
>     In my workflow, that means doing rebase of the branch I
>     tried to push out on top of the remote branch I was trying
>     to push to.
>
> The second paragraph depends on the workflow.  Do we want to
> (can we afford the space to) give a laundry list here?  Probably
> not.

I agree.

But how many different ways of integrating do we have? I only know
of merge or rebase. So, we may just mention both.

Or we only print an extended message if '--verbose' is given. The
short message could be even shorter and refer to '--verbose':

error: remote 'refs/heads/master' is ahead of local 'refs/heads/ 
master'. Use --verbose for more details.

If the user passes --verbose he gets the full story:

- A more detailed description of 'ahead'. For example,
   local could be a strict subset of remote, or local could have
   new commits that are not already at remote.

- We could give all sorts of hints, for example how to list the
   commits that are new on the local side. Recommendations how to
   solve the issue (merge, rebase). The message shouldn't get
   too verbose, though.



>>>> Your main objective was to push, but the shared workflow forces
>>>> you to do the integration _now_ (by using pull). In a pull-only
>>>> workflow, you can just push and defer the integration for later.
>>>
>>> No, you can also fetch + rebase.
>>
>> Right. My point was than one cannot defer the integration. It
>> must be addressed immediately.
>
> See above.

See above.


>>>> Some people claim fetch + rebase is superior to fetch + merge.
>>>> The only point I can see is that fetch + rebase gives a linear
>>>> history without loops, which is nicer to visualize. I recently
>>>> asked on the list if there are any benefits of fetch + rebase
>>>> over fetch + merge, besides a nicer visualization.
>>>
>>>
>>> It's easier to bisect...
>>> With a mostly linear history, this problem goes away.
>>
>> This is really an interesting point. I did not start to use
>> git bisect regularly. But I certainly plan to do so in the future.
>>
>> Couldn't bisect learn to better cope with non-linear history?
>
> It copes with it as best as it can.

I should try out git bisect to understand the details.


> Another thing to think about is how "everybody fetches, merges
> and pushes out" would interact with the concept of "mainline".
> Strictly speaking, the point of distributed development is that
> there is no mainline, but workflows based on "fetch + rebase"
> allows --first-parent to give a reasonable approximation of what
> people would naively expect how the mainline would look like.
> If everybody fetches, merges and pushes out, there is no
> "mainline" and --first-parent would give totally useless
> history.

Building a main line needs more control and more knowledge
about git.

Here is what I think can be done. It's only a sketch so
far. It's not yet reality. Therefore it might turn out to be
infeasible. I'd adjust my plans then.

We actually have at least three groups of developers that work
at three different locations. They'll work on different shared
branches. We also will create shared topic branches if a smaller
group of developers needs to work together on a prototype.

At some point shared branches need to become stable. They
need to be tested and maybe some of the changes need to be
reverted if they turn out to be useless. Finally we'll have a
tip of a shared branch that is stable. Stable depends on the
quality criteria, which may vary depending on where we are
in the release cycle. But at least some minimal requirements,
like "compiles on all platforms" or "passes all tests" will be
verified. Such a stable tip will now be merged with '--no-ff'
to the mainline. The merge will be thoroughly tested.

The chain of commits along first parent establishes a mainline
that matches certain quality criteria. The criteria are not
necessarily met by commits on the side branches. Therefore
fast-forward must not be used for the merge.

If we feel comfortable with git, we may consider creating
better topic branches in the first place. But for now I want
to start with shared branches containing a mixed bag of
commits.

Do all this make sense?

	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Nov 1, 2007, at 9:18 PM, Junio C Hamano wrote:
>
>> The context of this "forced" is that you say (in the following
>> paragraph) the user's main objective was to "push", but I do not
>> think "to push" is ever the main objective.
>
> Right. I should probably describe a bit more of the context.

Boring ;-)

> We have a shared branch for a group of developer who are located
> ...
> In this setting a user really want to push. Because only then
> the code will be tested and available for all others. ...

Pretty much expected, sane, and unsurprising.  Then you are in
the first category I quoted, and...

>>  - If it is to give integrated result for others to work further
>>    on, then you need to resolve before being able to achieve
>>    that goal.  There is no escaping from it.

... it still holds that what the developer wants to do is not
just "to push", but "to push after making sure what he is going
to push is in a good enough shape to be pushed".  Your _workflow_
is forcing to integrate right away before pushing; don't blame
git for this.

>>  - On the other hand, if it is to show what you did as early as
>>    possible in a working shape, and if the updated shared
>>    repository has changes from somebody else that conflicts you,
>>    in a CVS/SVN style shared workflow, there is no way for you
>>    to show what you did in isolation.  If you try to follow that
>>    model in git and insist pushing to the same branch, then you
>>    are forced to resolve first.
>>
>>    But you do not have to.  You could push out to another new
>>    branch, and say "Here is how you could do it, although this
>>    is based on an older codebase and conflicts with what
>>    recently happened to the tip".  You could even ask other
>>    party whose changes conflict with yours to help with the
>>    merge by saying "I pushed it out, you are more familiar with
>>    that area of the code and with your changes near the tip of
>>    the trunk, so could you merge it and push out the result?"
>
> ... I know we could use git to establish a more complex workflow
> that would give better guarantees on the published branches.

Don't get me wrong.  You do not always have to use the "push to
a side branch and ask for help from others", but git opens the
door for you to do so more conveniently, rather than strictly
sticking to the CVS workflow.    I re-quoted the whole "On the
other hand" part because I think this is something not often
done by people with CVS background --- with CVS you can do
exactly the same thing but it is too cumbersome and people don't
do so in practice.  With git, such an interaction is not just
possible but is a very natural thing to do.

Your more advanced people can be the first ones to employ this
"new communication medium" to help work better among them.  You
do not have to force the "side communication" as an official
part of workflow to the whole group.

SCM is just a tool to help developer communication.  Use it
wisely.

> We haven't figured out much more of our workflow. The first
> milestone is to migrate from CVS to git continuing to use a
> CVS-style workflow.

I think that is an interesting admission.  As somebody else on
the thread already said, if you are sticking to CVS workflow,
there are things that can and cannot be naturally done with
git.  Don't break git when you hit the situation in the latter
category without understanding how the world works.

> error: remote 'refs/heads/master' is ahead of local 'refs/heads/
> master'. Use --verbose for more details.

I'd rather have "Read section XXX of the user's guide".

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

[Steffen Prohaska]

On Nov 2, 2007, at 8:52 AM, Junio C Hamano wrote:

> ... it still holds that what the developer wants to do is not
> just "to push", but "to push after making sure what he is going
> to push is in a good enough shape to be pushed".  Your _workflow_
> is forcing to integrate right away before pushing; don't blame
> git for this.

I don't blame git for forcing the developers to merge. I blame
git for not supporting this workflow well enough.

I still believe that

- in a pull-oriented workflow (Linux kernel, git) there's less
   need to handle unexpected changes on the remote you want to
   push to. There's maybe also less need to push to heads named
   differently on the local and the remote (though I'm not sure
   if this really true).

- in a workflow that is base on shared branches (CVS-style),
   the remote heads certainly will advance unexpectedly, and
   git push should support developers to cope with this situation.
   In addition push should push back to the remote branch a local
   topic was originally branched off. This makes the need for
   pushing to a branch named differently on the remote side more
   likely than in a pull-oriented workflow, where you would
   publish under your local branch name and ask someone else
   to pull.

[...]

>
>> We haven't figured out much more of our workflow. The first
>> milestone is to migrate from CVS to git continuing to use a
>> CVS-style workflow.
>
> I think that is an interesting admission.  As somebody else on
> the thread already said, if you are sticking to CVS workflow,
> there are things that can and cannot be naturally done with
> git.  Don't break git when you hit the situation in the latter
> category without understanding how the world works.

Fair enough. I absolutely agree that it will never be a design
goal of git to directly support a CVS workflow ;)

But I strongly believe that there is a more universal question
behind. It makes sense to have good support for a workflow
that is based on a shared repository. A shared repository
can be a way
- to make it easy for the average developer to get started.
   Only clone to a local working repository is needed, but no
   publishing repository.
- to facilitate that commits will be pushed to at a central
   place. The default is to push back to the shared repository
   (btw, it's easy to setup hooks to do some access control to
   avoid havoc). This may increase visibility of changes. It may
   help doing backups. It may be easy to encourage early integration.

For small projects with developers available for direct
communication it may even be an option to have just this single
shared branch.

For larger project a better infrastructure and more control
over the changes is certainly a good idea. And git greatly
supports more complex workflows. That's the main reason why
I decided to choose git in the first place.

But for me the question is how can git be efficiently used to
support a workflow based on a shared repository. It should be
easy and safe to use and only few commands should be needed
to get started.


>> error: remote 'refs/heads/master' is ahead of local 'refs/heads/
>> master'. Use --verbose for more details.
>
> I'd rather have "Read section XXX of the user's guide".

Ok; do I need to write the section first or is there? ;)


And maybe we could do two things (at least for msysgit):

- Rename or link user-manual.html to git-user-manual.html,
   which would allow saying "git help user-manual".

- Support HTML anchors, such that
   "git help user-manual#section5" would open the user manual
   and jump to the right section.
	
	Steffen

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

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> - in a pull-oriented workflow (Linux kernel, git) ...
>   ... There's maybe also less need to push to heads named
>   differently on the local and the remote (though I'm not sure
>   if this really true).

That's far from true but is irrelevant to the discussion of
supporting shared repositories better.

> - in a workflow that is base on shared branches (CVS-style),
>   ...
>   In addition push should push back to the remote branch a local
>   topic was originally branched off.

Why?  If it is shared, and if you are shooting for the simplest
set of commands, wouldn't you work this way?

	$ git clone $public my-work-dir
        $ cd my-work-dir
        $ git checkout -b --track foo origin/foo
        $ hack hack hack, commit, commit, commit *on* *foo*
        $ git push $public foo

I think the recent git defaults to --track anyway so the third
step do not spell out --track.

With your "remote.$public.push = HEAD", the last step would be
"git push" without any parameter.

If you do use private topics, then the story would change this
way:

        $ git checkout -b --track foo origin/foo
        $ git checkout -b topic1 foo ;# or origin/foo
        $ hack hack hack, commit, commit, commit on topic1
        $ git checkout -b topic2 foo ;# or origin/foo
        $ hack hack hack, commit, commit, commit on topic2
        $ git checkout foo
        $ git merge topic1
        $ test test test; # test _your_ changes
        $ git merge topic2
        $ test test test; # test _your_ changes
        $ git push ;# again push 'foo' out

This may fail to fast forward.  You may at this time want to
"git fetch" first, rebase topic1 or topic2 that conflict with
the other side on top of updated origin/foo, rebuild foo and
push the result out, like this:

	$ git fetch
        $ git rebase origin/foo topic1
        $ git branch -f foo origin/foo
        $ git checkout foo
        $ git merge topic1
        $ git merge topic2
        $ test test test
        $ git push

>   ... This makes the need for
>   pushing to a branch named differently on the remote side more
>   likely than in a pull-oriented workflow,

So I do not understand this remark.

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

[Steffen Prohaska]

On Nov 2, 2007, at 11:44 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> - in a workflow that is base on shared branches (CVS-style),
>>   ...
>>   In addition push should push back to the remote branch a local
>>   topic was originally branched off.
>
> Why?  If it is shared, and if you are shooting for the simplest
> set of commands, wouldn't you work this way?

Yes. I would work exactly this way with current git.


> 	$ git clone $public my-work-dir
>         $ cd my-work-dir
>         $ git checkout -b --track foo origin/foo

So the implicit rule here is
"name a branch identical in all repositories you're dealing with"
right?

That is foo is named foo at the remote, named foo as a tracking
branch (git handles this automatically) and is named foo as your
local branch.

I believe it is reasonable. Though I have two questions:

1) If this is best practice, why doesn't save git me from typos?
    Why do I need to type "foo" correctly twice?

2) What shall I do if I am dealing with more than one shared
    repository?  Andreas' group should already run into problems
    here. They have several shared repos and if they want to
    checkout several local branches from different repos they
    need to somehow encode the name of the remote in the name
    of the local branch.


>         $ hack hack hack, commit, commit, commit *on* *foo*
>         $ git push $public foo
>
> I think the recent git defaults to --track anyway so the third
> step do not spell out --track.

It does.


> With your "remote.$public.push = HEAD", the last step would be
> "git push" without any parameter.

Indeed. Or with my "branch.$name.push" it would just be "git push"
as well. And I'd be probably happy then.


> If you do use private topics, then the story would change this
> way:
>
>         $ git checkout -b --track foo origin/foo
>         $ git checkout -b topic1 foo ;# or origin/foo

I'd be more happy without 'or'. I really want to give a single
recommendation.

So the question here is: Should I branch off the local branch or
should I branch off the remote branch? When should I do what?
What is best practice and what is used for 'exceptional'
situations?


>         $ hack hack hack, commit, commit, commit on topic1
>         $ git checkout -b topic2 foo ;# or origin/foo
>         $ hack hack hack, commit, commit, commit on topic2
>         $ git checkout foo
>         $ git merge topic1
>         $ test test test; # test _your_ changes
>         $ git merge topic2
>         $ test test test; # test _your_ changes
>         $ git push ;# again push 'foo' out

This focuses testing on the integration of topic1 with topic2.

You could as well do the following

	$ git checkout -b topic1 origin/foo
	$ hack ...
	$ git checkout -b topic2 origin/foo
	$ hack ..

	[ later ]
	$ git checkout topic1
	$ git pull # or git fetch; git rebase origin/foo
	$ test test test
	$ git push origin topic1:origin/foo

	[ later ]
	$ git checkout topic2
	$ git pull # or git fetch; git rebase origin/foo
	$ test test test
	$ git push origin topic2:origin/foo

With my "branch.$name.push" it would just be "git push" here.

This workflow focuses testing on the integration of each of your
topics with the new changes on the shared branch independently
of your other topic.

You're done at this point. No need to merge a second time,
no need to reset branches.

It's probably a good idea to delete your local branches
now. And there is one minor question related to that: Where
to park your HEAD if you want to clean up _all_ of your local
branches because you have nothing left to do? Everything is
on the shared remote branch. The only thing you're interested
now is to checkout new changes from the shared branch if
interesting work was done by others.


> This may fail to fast forward.  You may at this time want to
> "git fetch" first, rebase topic1 or topic2 that conflict with
> the other side on top of updated origin/foo, rebuild foo and
> push the result out, like this:

Or you could just pull

[ this continues Junio's example from above, you are on branch foo. ]

	$ git pull
	$ test test; # test of your integration of topic1, topic2
	             # with the new changes on the shared branch
	$ git push



> 	$ git fetch
>         $ git rebase origin/foo topic1
>         $ git branch -f foo origin/foo

Here is another interesting point.

Would you recommend "git branch -f foo origin/foo" over
"git checkout foo; git reset --hard origin/foo"? I think the
first command is safer because it doesn't throw away uncommitted
changes. However it fails if you are already on branch foo. Then it
says "fatal: Cannot force update the current branch.". It is not
very intuitive if I'd ask users to first leave the branch they
want to modify, only to be able to use "git branch". "git reset"
always lets you achieve your goal. (BTW, I don't recommend having
local changes while doing integration testing ... but who knows
maybe someone feels comfortable with it.)


>         $ git checkout foo
>         $ git merge topic1
>         $ git merge topic2
>         $ test test test
>         $ git push

Using rebase requires more commands than using pull, and more
intrusive  commands like "branch -f" or "reset --hard" are involved.

That doesn't mean that you should not use rebase. But it certainly
needs more explanation.

Another related question is the following: After some time the
user decides that some help on topic1 would be appreciated and
another developer promises to help. So they agree to work on
a shared branch name topic1. The first developer starts with

	$ git push origin topic1

 From now on he _MUST NOT_ use rebase any longer! So starting
to work on the topic with a second developer completely changed
the best practice. From now no rebase is forbidden, which was
best practice before.

So the question for me is: do I want to teach developer a pull
or a rebase workflow first? Currently I believe pull will be
safer for them, better supported by git, and there will be
situations they must use pull. If the only nuisance are loops
in the history when viewing them in gitk, I'm happy to accept
this.


>>   ... This makes the need for
>>   pushing to a branch named differently on the remote side more
>>   likely than in a pull-oriented workflow,
>
> So I do not understand this remark.

Yeah, I should have added some explanation here. I had Andreas'
200-local-branches and the topic1/topic2 example in mind that
does the integration against the shared branch.

	Steffen

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

[Andreas Ericsson]

Steffen Prohaska wrote:
> 
> On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:
> 
>>
>> It's easier to bisect. If git bisect lands you on a merge-commit,
>> you need to start a new bisect for each of the parents included
>> in the merge. Hopefully the nature of the merge gives a clue so
>> the user can make an educated guess as to which parent introduced
>> the bogus commit, but for an "evil octopus" (unusual) or if the
>> merge had conflicts which were resolved in a buggy way (not
>> exactly uncommon), it can be quite a hassle to get things right.
>> With a mostly linear history, this problem goes away.
> 
> This is really an interesting point. I did not start to use
> git bisect regularly. But I certainly plan to do so in the future.
> 
> Couldn't bisect learn to better cope with non-linear history?
> 

Perhaps it could, but it's far from trivial. I started hacking on
a wrapper for git-bisect which would do just that, but gave up
rather quickly as the book-keeping required to remember each and
every parent-point tried just got out of hand, and it *still*
wouldn't run in full automatic. It broke down because I also
wanted merges on non-first-line parents to be delved into. If
that didn't happen, I wouldn't *know* the bisect would run fine
without me watching it, so then it was as useless as if I'd have
had to sit there the entire time anyway.


> 
> BTW, what do you thing about the proposal to add branch.$name.push [1]?
> 
> [1] http://marc.info/?l=git&m=119384331712996&w=2
> 

I'm not so sure about it. I rather liked the "don't warn if local is
strict subset of remote" thing though. I teach our devs to just
ignore that warning, but with the same leaden feeling in my stomach
that someone, sometime, is going to get bit by it. It's worked so
far though, perhaps because our update-hook contains a check meaning
I'm the only one allowed to do "git-push --force".

>>
>> Except that it doesn't work unless you either detach the HEAD
>> (which prints a big fat ugly message) or give it -D to force
>> it, which I really, really don't recommend. We use git because
>> I'm pretty confident in its capabilities of never ever losing
>> anything. Using the seemingly harmless -D switch to git-branch
>> puts us at risk of wiping history quite without noticing.
> 
> I don't like -D either. I liked the idea mentioned recently
> to check -d against the remotes. If a remote tracking branch
> has the history it should be considered fully merged.
> 

Yes. Since remote branches are considered when prune'ing anyway,
and the git-branch -d warning is there to make sure we don't
accidentally lose any tip pointers, it should be safe to use
*all* "named" refs when checking for git-branch -d's sake (that
is, everything under refs/{heads,remotes,tags}/**/*).

> Another idea may be to distinguish between detached head and
> checkout of remote tracking branch. Maybe we could do some
> useful things if get knew that the user is 'on a remote tracking
> branch'. Committing could be forbidden.

Committing nearly *has* to be forbidden.

> A suggestion would be
> printed instead to use "git checkout -b something", which could act
> as if the remote branch was mentioned on the command line.
> 
> Something like that would be needed before I'd seriously
> suggest to delete local branches after you finished your work.
> 

Yup. I'll never suggest using "git branch -D" to my co-workers. Sooner
or later there'll be cries of anguish echoing throughout the office
when that happens ;-)

> 
> 
>>> Independently of what the best practice is, leaving the local
>>> work branch there shouldn't do any harm because I'm sure that
>>> some devs will forget to clean up, independently of what I tell
>>> them.
>>
>> I wholeheartedly agree with this one.
> 
> So I think we need to resolve this first.
> 
> Do you already have post-checkout script that makes useful
> suggestions.  I remember you mentioned something like that
> during the 200-local-branches discussion.
> 

No. Junio suggested I'd implement it as a post-checkout hook, but it
would only save me one command and could cause confusion as diff
output would change depending on whether one has checked out the
one branch or another prior to running git diff, so I decided against
it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

[Tom Prince]

On Fri, Nov 02, 2007 at 11:03:36AM +0100, Andreas Ericsson wrote:
> Steffen Prohaska wrote:
>> On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:
>>>
>>> It's easier to bisect. If git bisect lands you on a merge-commit,
>>> you need to start a new bisect for each of the parents included
>>> in the merge. Hopefully the nature of the merge gives a clue so
>>> the user can make an educated guess as to which parent introduced
>>> the bogus commit, but for an "evil octopus" (unusual) or if the
>>> merge had conflicts which were resolved in a buggy way (not
>>> exactly uncommon), it can be quite a hassle to get things right.
>>> With a mostly linear history, this problem goes away.
>> This is really an interesting point. I did not start to use
>> git bisect regularly. But I certainly plan to do so in the future.
>> Couldn't bisect learn to better cope with non-linear history?
>
> Perhaps it could, but it's far from trivial. I started hacking on
> a wrapper for git-bisect which would do just that, but gave up
> rather quickly as the book-keeping required to remember each and
> every parent-point tried just got out of hand, and it *still*
> wouldn't run in full automatic. It broke down because I also
> wanted merges on non-first-line parents to be delved into. If
> that didn't happen, I wouldn't *know* the bisect would run fine
> without me watching it, so then it was as useless as if I'd have
> had to sit there the entire time anyway.

I haven't had occasion to use git-bisect much, but I was under the
impression that bisect could already handle merges, or any other shaped
history just fine.

If you test a merge and it is bad, git (eventually) picks a commit on one of
the branches. If that commit is good, then the merge-base is good, so that the
bug lies on some other branch. If that commit is bad, then the bug is on some
ancestor of the branch. Thus, no need for special book keeping.

  Tom

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

[Andreas Ericsson]

Tom Prince wrote:
> On Fri, Nov 02, 2007 at 11:03:36AM +0100, Andreas Ericsson wrote:
>> Steffen Prohaska wrote:
>>> On Nov 1, 2007, at 10:11 AM, Andreas Ericsson wrote:
>>>> It's easier to bisect. If git bisect lands you on a merge-commit,
>>>> you need to start a new bisect for each of the parents included
>>>> in the merge. Hopefully the nature of the merge gives a clue so
>>>> the user can make an educated guess as to which parent introduced
>>>> the bogus commit, but for an "evil octopus" (unusual) or if the
>>>> merge had conflicts which were resolved in a buggy way (not
>>>> exactly uncommon), it can be quite a hassle to get things right.
>>>> With a mostly linear history, this problem goes away.
>>> This is really an interesting point. I did not start to use
>>> git bisect regularly. But I certainly plan to do so in the future.
>>> Couldn't bisect learn to better cope with non-linear history?
>> Perhaps it could, but it's far from trivial. I started hacking on
>> a wrapper for git-bisect which would do just that, but gave up
>> rather quickly as the book-keeping required to remember each and
>> every parent-point tried just got out of hand, and it *still*
>> wouldn't run in full automatic. It broke down because I also
>> wanted merges on non-first-line parents to be delved into. If
>> that didn't happen, I wouldn't *know* the bisect would run fine
>> without me watching it, so then it was as useless as if I'd have
>> had to sit there the entire time anyway.
> 
> I haven't had occasion to use git-bisect much, but I was under the
> impression that bisect could already handle merges, or any other shaped
> history just fine.
> 

It appears the code supports your statement. I started writing on my
hack-around about a year ago, and the merge-handling code got in with
1c4fea3a40e836dcee2f16091bf7bfba96c924d0 at Wed Mar 21 22:16:24 2007.
Perhaps I shouldn't be so paranoid about useless merges anymore then.
Hmm. I shall have to look into it. Perhaps Junio can clarify how it
works? The man-page was terribly silent about how git-bisect handles
merges.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

[Steffen Prohaska]

On Nov 2, 2007, at 2:52 PM, Andreas Ericsson wrote:

>> I haven't had occasion to use git-bisect much, but I was under the
>> impression that bisect could already handle merges, or any other  
>> shaped
>> history just fine.
>
> It appears the code supports your statement. I started writing on my
> hack-around about a year ago, and the merge-handling code got in with
> 1c4fea3a40e836dcee2f16091bf7bfba96c924d0 at Wed Mar 21 22:16:24 2007.
> Perhaps I shouldn't be so paranoid about useless merges anymore then.
> Hmm. I shall have to look into it. Perhaps Junio can clarify how it
> works? The man-page was terribly silent about how git-bisect handles
> merges.

So eventually there's coming something good out of this thread,
without actually writing any code ;)

	Steffen

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

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Tom Prince wrote:
>>
>> I haven't had occasion to use git-bisect much, but I was under the
>> impression that bisect could already handle merges, or any other shaped
>> history just fine.
>
> It appears the code supports your statement. I started writing on my
> hack-around about a year ago, and the merge-handling code got in with
> 1c4fea3a40e836dcee2f16091bf7bfba96c924d0 at Wed Mar 21 22:16:24 2007.
> Perhaps I shouldn't be so paranoid about useless merges anymore then.
> Hmm. I shall have to look into it. Perhaps Junio can clarify how it
> works? The man-page was terribly silent about how git-bisect handles
> merges.

Bisecting through merge is not a problem.  Not at all, from the
very beginning of the bisect command.

	Side note.  The commit you quote does not change (let
	alone fix) the semantics at all.  It is a pure
	optimization.  The theory behind how bisect works, see
	my OLS presentation (reachable from the gitwiki).

The real problem is what to do when the culprit turns out to be
a merge commit.  How to spot what really is wrong, and figure
out how to fix.  The problem is not for the tool but for the
human, and it is real.

Imagine this history.

      ---Z---o---X---...---o---A---C---D
          \                       /
           o---o---Y---...---o---B

Suppose that on the upper development line, the meaning of one
of the functions existed at Z was changed at commit X.  The
commits from Z leading to A change both the function's
implementation and all calling sites that existed at Z, as well
as new calling sites they add, to be consistent.  There is no
bug at A.

Suppose in the meantime the lower development line somebody
added a new calling site for that function at commit Y.  The
commits from Z leading to B all assume the old semantics of that
function and the callers and the callee are consistent with each
other.  There is no bug at B, either.

You merge to create C.  There is no textual conflict with this
three way merge, and the result merges cleanly.  You bisect
this, because you found D is bad and you know Z was good.  Your
bisect will find that C (merge) is broken.  Understandably so,
as at C, the new calling site of the function added by the lower
branch is not converted to the new semantics, while all the
other calling sites that already existed at Z would have been
converted by the merge.  The new calling site has semantic
adjustment needed, but you do not know that yet.  You need to
find out that is the cause of the breakage by looking at the
merge commit C and the history leading to it.

How would you do that?

Both "git diff A C" and "git diff B C" would be an enormous patch.
Each of them essentially shows the whole change on each branch
since they diverged.  The developers may have well behaved to
create good commits that follow the "commit small, commit often,
commit well contained units" mantra, and each individual commit
leading from Z to A and from Z to B may be easy to review and
understand, but looking at these small and easily reviewable
steps alone would not let you spot the breakage.  You need to
have a global picture of what the upper branch did (and
among many, one of them is to change the semantics of that
particular function) and look first at the huge "diff A C"
(which shows the change the lower branch introduces), and see if
that huge change is consistent with what have been done between
Z and A.

If you linearlize the history by rebasing the lower branch on
top of upper, instead of merging, the bug becomes much easier to
find and understand.  Your history would instead be:

    ---Z---o---X'--...---o---A---o---o---Y'--...---o---B'--D'

and there is a single commit Y' between A and B' that introduced
the new calling site that still uses the new semantics of the
function that was already in A.  "git show Y'" will be a much
smaller patch than "git diff A C" and it is much easier to deal
with.

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

[Junio C Hamano]

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

> If you linearlize the history by rebasing the lower branch on
> top of upper, instead of merging, the bug becomes much easier to
> find and understand.  Your history would instead be:
>
>     ---Z---o---X'--...---o---A---o---o---Y'--...---o---B'--D'
>
> and there is a single commit Y' between A and B' that introduced
> the new calling site that still uses the new semantics of the
> function that was already in A.  "git show Y'" will be a much
> smaller patch than "git diff A C" and it is much easier to deal
> with.

Typo.  Y' uses "the old semantics of the function, even though
that was already modified at X'".

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

[Andreas Ericsson]

Junio C Hamano wrote:
> Steffen Prohaska <prohaska@zib.de> writes:
> 
>>> You forgot a lot more important part.  Pushing into publishing
>>> repositories.  And the discussion is about git-push command.
>> Exactly, here are two examples:
>>
>> If you push only to publishing repositories that are read
>> only by others, you'll never encounter the problem that
>> 10/10 tried to solve. The publishing repository is never
>> changed by others. You are the only one who pushes to this
>> repository. Therefore the remote never advances unexpectedly.
> 
> Wrong.
> 
> People can and do work from more than one private repositories
> (I do).  In a sense, that is sharing the repository with
> oneself.
> 

I believe your troubles are alleviated a great deal by the fact
that you actually know when upstream has changes, and what those
changes are supposed to be. A communications breakdown with only
one person involved is sort of hard to imagine.

> (actually, shared repository people seem to
> prefer "fetch + rebase" over "pull" which is "fetch + merge").
> 

That's definitely true. The number of useless merge-commits we
have in our repos is annoying, and has twice made bisect a bit
troublesome for no good reason.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

[Steffen Prohaska]

On Nov 1, 2007, at 9:18 AM, Andreas Ericsson wrote:

> Junio C Hamano wrote:
>
>> (actually, shared repository people seem to
>> prefer "fetch + rebase" over "pull" which is "fetch + merge").
>
> That's definitely true. The number of useless merge-commits we
> have in our repos is annoying, and has twice made bisect a bit
> troublesome for no good reason.

Can you describe a bit more what's "annoying" about them?
Is it the visualization? Or are there more problems; like
the trouble with bisect?

I'm trying to estimate if it's worth teaching _all_
developers rebase or if we should just live with the "useless"
merge-commits.

	Steffen

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

[Andreas Ericsson]

Steffen Prohaska wrote:
> 
> On Nov 1, 2007, at 9:18 AM, Andreas Ericsson wrote:
> 
>> Junio C Hamano wrote:
>>
>>> (actually, shared repository people seem to
>>> prefer "fetch + rebase" over "pull" which is "fetch + merge").
>>
>> That's definitely true. The number of useless merge-commits we
>> have in our repos is annoying, and has twice made bisect a bit
>> troublesome for no good reason.
> 
> Can you describe a bit more what's "annoying" about them?
> Is it the visualization? Or are there more problems; like
> the trouble with bisect?
> 

Visualization is a small nuissance. git-bisect troubles are more
worrisome. I've been in the seat where useless merges means git
bisect needs constant babysitting and constant manual handling.
It's no fun at all, so we're sticking with the fetch+rebase flow.

> I'm trying to estimate if it's worth teaching _all_
> developers rebase or if we should just live with the "useless"
> merge-commits.
> 

I'd say that depends on how valuable you find gitk, qgit and
git-bisect are. To me, I'd happily use any scm in the world,
so long as it has git-bisect. Otoh, I'm a lazy bastard and
love bisect so much that all our automated tests are focused
around "git bisect run". This means bugs in software released
to customers are few and far apart. When we get one reported,
we just create a new test that exposes it, fire up git-bisect
and then go to lunch. Quality costs, however. We pay that bill
by using a workflow that's perhaps more convoluted than
necessary.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

[Wincent Colaiuta]

El 31/10/2007, a las 22:31, Junio C Hamano escribió:

> Wrong.  push is a mirror of fetch and does not do _any_
> integration.  It is just a safe (because it insists on
> fast-forward) propagation mechanism.  Your integration still
> happens with pull (actually, shared repository people seem to
> prefer "fetch + rebase" over "pull" which is "fetch + merge").


Of course, it's too late too change now, but it would be nice if the  
mirror of "fetch" were "send". (I know it's been commented in the past  
that the fact that "push" and "pull" aren't mirror operations has  
surprised quite a few people.)

Cheers,
Wincent

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

[Johannes Schindelin]

Hi,

On Fri, 2 Nov 2007, Wincent Colaiuta wrote:

> Of course, it's too late too change now, but it would be nice if the 
> mirror of "fetch" were "send". (I know it's been commented in the past 
> that the fact that "push" and "pull" aren't mirror operations has 
> surprised quite a few people.)

Could you please just do

	git config --global alias.send push

and be done with it?

Hth,
Dscho

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

[Steffen Prohaska]

On Nov 2, 2007, at 1:14 PM, Johannes Schindelin wrote:

> Hi,
>
> On Fri, 2 Nov 2007, Wincent Colaiuta wrote:
>
>> Of course, it's too late too change now, but it would be nice if the
>> mirror of "fetch" were "send". (I know it's been commented in the  
>> past
>> that the fact that "push" and "pull" aren't mirror operations has
>> surprised quite a few people.)
>
> Could you please just do
>
> 	git config --global alias.send push
>
> and be done with it?

This would certainly be the easiest. But I think the following
is probably more in line with Wincent's comment:

	Makefile builds git-send instead of git-push
	git config --global alias.push send
	[ wait some time ]
	git config --unset alias.push

The comment was about how to avoid surprises for people that
are new to git, not how to let long-time users have an alias
for push.

The _only_ real solution I see right now, is to stop the
discussion and leave "git push" as is. I strongly believe that
the git community in its majority will refuse to rename push;
though I have no evidence for this.

	Steffen

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

[Wincent Colaiuta]

El 2/11/2007, a las 13:48, Steffen Prohaska escribió:

> On Nov 2, 2007, at 1:14 PM, Johannes Schindelin wrote:
>
>> On Fri, 2 Nov 2007, Wincent Colaiuta wrote:
>>
>>> Of course, it's too late too change now, but it would be nice if the
>>> mirror of "fetch" were "send". (I know it's been commented in the  
>>> past
>>> that the fact that "push" and "pull" aren't mirror operations has
>>> surprised quite a few people.)
>>
>> Could you please just do
>>
>> 	git config --global alias.send push
>>
>> and be done with it?

(snip)

> The comment was about how to avoid surprises for people that
> are new to git, not how to let long-time users have an alias
> for push.

Exactly. I was talking about the *initial* surprise for new users, not  
for people who already know the difference between push, pull and  
fetch (99% of people reading this list already, myself included).

> The _only_ real solution I see right now, is to stop the
> discussion and leave "git push" as is. I strongly believe that
> the git community in its majority will refuse to rename push;
> though I have no evidence for this.

As I said above, "Of course, it's too late to change now"... I don't  
think it will be renamed either.

Cheers,
Wincent

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

[Daniel Barkalow]

On Tue, 30 Oct 2007, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
> 
> > git push now allows you pushing a couple of branches that have
> > advanced, while ignoring all branches that have no local changes,
> > but are lagging behind their matching remote refs. This is done
> > without reporting errors.
> >
> > Thanks to Junio C. Hamano <gitster@pobox.com> for suggesting to
> > report in the summary that refs have been ignored.
> 
> I do not think this is a good idea at all.  Furthermore, I never
> suggested anything about summary.  You are robbing the
> information from the pusher which ones are pushed and which ones
> are left behind.

I think this case should be a warning rather than an error, though. It is 
certainly true that the user isn't intending to update those remote refs, 
because there is no local change to update them with. And it is also true 
that those local refs being stale is no impediment to updating the refs 
which are not stale, which is what the user does intend to do. I can't see 
a workflow which would be hurt by this change, because we know that, if 
the user follows the instructions and then tries the push again, it will 
have no effect.

If the concern is robbing the user of information, we should simply 
provide the information, rather than interrupting the user's work to make 
them act on the information before completing the essentially independant 
operation they're attempting.

In any case, it's misleading to suggest that the user "pull first", 
because we know that there would be no effect to pushing again after 
merging. In this case, it would be more accurate to suggest that the user 
"pull instead". Perhaps the message should be
"%s: nothing to push to %s, but you are not up-to-date and may want to 
pull"

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 07/10] 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.

As you introduced long names around 5/10 to have two different
ones for clarity with the goal of unifying them, so once you
unified the rules, it probably is a good idea to rename the long
"do_this_with_X_rule()" and "do_this_with_Y_rule()" functions
back to "do_this()", isn't it?

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

[Steffen Prohaska]

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

> 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.
>
> As you introduced long names around 5/10 to have two different
> ones for clarity with the goal of unifying them, so once you
> unified the rules, it probably is a good idea to rename the long
> "do_this_with_X_rule()" and "do_this_with_Y_rule()" functions
> back to "do_this()", isn't it?

Absolutely.

But I'm not sure if I'm the one who unifies them.

	Steffen

Re: [PATCH 04/10] push: add "git push HEAD" shorthand for 'push current branch to default repo'

[Junio C Hamano]

Will drop this as you already know why.

Re: [PATCH 03/10] push: support pushing HEAD to real branch name

[Junio C Hamano]

Nice.

Re: [PATCH 01/10] push: change push to fail if short refname does not exist

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> Pushing a short refname used to create a new ref on 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.

On the other hand, if you specified a wrong branch that exists
on the remote end accidentally, it still was pushed.  Do we want
to have a new "--i-really-want-to-push" option to make it safer?

I do not think so.  Why should a new branch be treated any
differently?

Will drop 1/10 and 2/10 for now.

Re: [PATCH 01/10] push: change push to fail if short refname does not exist

[Steffen Prohaska]

On Oct 30, 2007, at 9:29 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> Pushing a short refname used to create a new ref on 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.
>
> On the other hand, if you specified a wrong branch that exists
> on the remote end accidentally, it still was pushed.  Do we want
> to have a new "--i-really-want-to-push" option to make it safer?

Maybe not a bad idea ;)

But not as a command line flag but after printing the results
of a '--dry-run' and than asking the user for confirmation:
"do you want to push this?".


> I do not think so.  Why should a new branch be treated any
> differently?

Because "updating an existing branch" and "creating a new branch"
are two slightly different tasks.

If git provides a way to make this difference explicit, it
would be safer to use.


> Will drop 1/10 and 2/10 for now.

Then they'll be dropped and I'll rely on the the --dry-run flag.

Or someone else needs to step in and support my point.

	Steffen

Re: [PATCH 01/10] push: change push to fail if short refname does not exist

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 30, 2007, at 9:29 AM, Junio C Hamano wrote:
> ...
>> Will drop 1/10 and 2/10 for now.
>
> Then they'll be dropped and I'll rely on the the --dry-run flag.
>
> Or someone else needs to step in and support my point.

Yup, you exactly got what I meant by "for now".  I reserve the
right to be convinced and converted later ;-).