Git development
 help / color / mirror / Atom feed
* [PATCH 02/10] push: teach push new flag --create
From: Steffen Prohaska @ 2007-10-28 17:46 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska
In-Reply-To: <11935935812741-git-send-email-prohaska@zib.de>

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

^ permalink raw reply related

* [PATCH 05/10] rename ref_matches_abbrev() to ref_abbrev_matches_full_with_fetch_rules()
From: Steffen Prohaska @ 2007-10-28 17:46 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska
In-Reply-To: <11935935812185-git-send-email-prohaska@zib.de>

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

^ permalink raw reply related

* [PATCH 10/10] push: teach push to be quiet if local ref is strict subset of remote ref
From: Steffen Prohaska @ 2007-10-28 17:46 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska
In-Reply-To: <11935935823496-git-send-email-prohaska@zib.de>

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

^ permalink raw reply related

* [PATCH 03/10] push: support pushing HEAD to real branch name
From: Steffen Prohaska @ 2007-10-28 17:46 UTC (permalink / raw)
  To: git; +Cc: Steffen Prohaska
In-Reply-To: <1193593581114-git-send-email-prohaska@zib.de>

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

^ permalink raw reply related

* Re: [RFH] gcc constant expression warning...
From: Linus Torvalds @ 2007-10-28 17:09 UTC (permalink / raw)
  To: Antti-Juhani Kaijanaho; +Cc: git
In-Reply-To: <slrnfi8pj7.mb4.antti-juhani@kukkaseppele.kaijanaho.fi>



On Sun, 28 Oct 2007, Antti-Juhani Kaijanaho wrote:
>
> A correct fix would be to check for the size of off_t in some other (and
> defined) manner, but I don't know off_t well enough to suggest one.

In this case, it's trying to make sense that "off_t" can hold more than 32
bits. So I think that test can just be rewritten as

	if (sizeof(off_t) <= 4) {
		munmap(idx_map, idx_size);
		return error("pack too large for current definition of off_t in %s", path);
	}

instead.

			Linus

^ permalink raw reply

* Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"
From: Johannes Schindelin @ 2007-10-28 16:51 UTC (permalink / raw)
  To: Scott R Parish; +Cc: git
In-Reply-To: <1193582654-12100-1-git-send-email-srp@srparish.net>

Hi,

On Sun, 28 Oct 2007, Scott R Parish wrote:

> diff --git a/help.c b/help.c
> index 34ac5db..07cf67a 100644
> --- a/help.c
> +++ b/help.c
> @@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
>  		putchar(c);
>  }
>  
> -static struct cmdname {
> -	size_t len;
> -	char name[1];
> -} **cmdname;
> -static int cmdname_alloc, cmdname_cnt;
> -
> -static void add_cmdname(const char *name, int len)
> +static struct cmdnames {
> +	int alloc;
> +	int cnt;
> +	struct cmdname {
> +		size_t len;
> +		char name[1];
> +	} **names;
> +} main_cmds, other_cmds;
> +
> +static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
>  {
>  	struct cmdname *ent;
> -	if (cmdname_alloc <= cmdname_cnt) {
> -		cmdname_alloc = cmdname_alloc + 200;
> -		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
> +	if (cmds->alloc <= cmds->cnt) {
> +		cmds->alloc = cmds->alloc + 200;
> +		cmds->names = xrealloc(cmds->names,
> +				       cmds->alloc * sizeof(*cmds->names));

Looks like a candidate for ALLOC_GROW() ...

> @@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
>  	return strcmp(a->name, b->name);
>  }
>  
> -static void pretty_print_string_list(struct cmdname **cmdname, int longest)
> +static void uniq(struct cmdnames *cmds)
> +{
> +	int i, j;
> +
> +	if (!cmds->cnt)
> +		return;
> +
> +	for (i = j = 1; i < cmds->cnt; i++) {
> +		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
> +			cmds->names[j++] = cmds->names[i];
> +		}
> +	}

Losing the curly brackets would make this look much nicer.

> +
> +	cmds->cnt = j;
> +}
> +
> +static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {

Maybe "exclude_cmds()", and choose more suggestive names for the 
parameters?

> -	DIR *dir = opendir(exec_path);
> +	DIR *dirp = opendir(dir);

I am not sure that a rename from "dir" to "dirp" is needed here.  It 
distracts a little from the real content of your patch.

Thanks,
Dscho

^ permalink raw reply

* Re: New features in gitk
From: Linus Torvalds @ 2007-10-28 16:50 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <18212.13862.637991.30536@cargo.ozlabs.ibm.com>



On Sun, 28 Oct 2007, Paul Mackerras wrote:
>
> Yeah.  Actually what I'd like is to know how many commits git log is
> going to give me

That's not known until later.

> With --topo-order (or --date-order) it has to get to the last commit 
> before it outputs the first commit, doesn't it?

The cost is not generally in outputting the commits. The real cost is in 
traversing them in the first place. 

So yes, we could output the number of commits once we know it, but 
generally, by that time, it's not an interesting number any more! You 
might as well just read the list, because git is going to feed it to you 
as fast as it can (which is plenty fast - you'll probably get hundreds of 
megabytes of SHA1 values per second at that point).

So basically, by the time you start getting SHA1's from --topo-order, the 
best thing you can do is just lay back and think of England. The *last* 
thing you want to do is bother with any graphics and updates, because it's 
just going to slow things down.

It's before you even start getting the SHA1's, _or_ if you don't use 
"--date/topo-order" in the first place, that you want to have a "wait, I'm 
thinking" thing. And at neither time do you know how long it's going to 
be.

(And as mentioned many times earlier - if you can avoid topo-order and 
date-order entirely, you are going to perform a million times better at 
startup for the cold-cache case. Since you seem to be doing the graph 
layout lazily now, maybe you could aim for that some day? It does mean 
that you might - occasionally - end up having to add a commit to 
*before* one you already laid out).

		Linus

^ permalink raw reply

* [BUG?] git-pull and git-merge don't support option --ff as the document says
From: Yin Ping @ 2007-10-28 16:36 UTC (permalink / raw)
  To: git

Released git version 1.5.3.4

yinping@kooxoo235:~/git$ git-pull --ff
Usage: /usr/local/bin/git-fetch <fetch-options> <repository> <refspec>...
yinping@kooxoo235:~/git$ git-pull --no-ff
Usage: /usr/local/bin/git-fetch <fetch-options> <repository> <refspec>...


yinping@kooxoo235:~/git$ git-merge --ff origin/master
Usage: /usr/local/bin/git-merge [-n] [--summary] [--no-commit]
[--squash] [-s <strategy>] [-m=<merge-message>] <commit>+
yinping@kooxoo235:~/git$ git-merge --no-ff origin/master
Usage: /usr/local/bin/git-merge [-n] [--summary] [--no-commit]
[--squash] [-s <strategy>] [-m=<merge-message>] <commit>+

-- 
franky

^ permalink raw reply

* Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name
From: Steffen Prohaska @ 2007-10-28 16:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7il7usyx.fsf@gitster.siamese.dyndns.org>


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

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

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

	Steffen

^ permalink raw reply

* Re: [RFH] gcc constant expression warning...
From: Daniel Barkalow @ 2007-10-28 16:28 UTC (permalink / raw)
  To: Florian Weimer; +Cc: git
In-Reply-To: <87ir4rletv.fsf@mid.deneb.enyo.de>

On Sun, 28 Oct 2007, Florian Weimer wrote:

> * Junio C. Hamano:
> 
> > The offending lines are:
> >
> >         if (idx_size != min_size) {
> >                 /* make sure we can deal with large pack offsets */
> >                 off_t x = 0x7fffffffUL, y = 0xffffffffUL;
> >                 if (x > (x + 1) || y > (y + 1)) {
> >                         munmap(idx_map, idx_size);
> 
> x and y must be unsigned for this test to work (signed overflow is
> undefined).

I believe the test is trying to determine if signed addition on numbers of 
a certain size is safe in this environment. Doing the test with unsigned 
variables would cause the test to give a predictable but irrelevant 
result. I think gcc is being annoying in assuming that signed overflow 
doesn't occur (even when it must), rather than assuming that the result of 
signed overflow is some arbitrary and likely not useful value. If we have 
an overflow possible with off_t in the way we'd use it, then one of those 
tests should be automatically true due to the limited size of the type 
(except that I think the test should be >= instead of >). I think we 
should be able to assume that the result of a signed overflow, whatever 
undefined value it is, is a possible value of its type and therefore not 
more than the maximum value of its type, but gcc may be screwing this up.

It's probably best just to test the size of off_t.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name
From: Junio C Hamano @ 2007-10-28 16:03 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <7vd4uzuu1g.fsf@gitster.siamese.dyndns.org>

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

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>>
>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>
>>>> This teaches "push <remote> HEAD" to resolve HEAD on the local
>>>> side to its real ref name, e.g. refs/heads/master, and then
>>>> use the real ref name on the remote side to search a matching
>>>> remote ref.
>>>
>>> This probably is a good idea.
>>
>> I'll add an even shorter shorthand: "git push HEAD" will push
>> the current branch to its default remote.
>
> Ugh, that looks way too magicky.  The first parameter to push if
> one ever exists has _always_ been the remote, and the above
> breaks it.
>
> Please don't.

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

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

	$ git push
	$ git push $remote

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

Then, introduce another option

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

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

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

	[remote "origin"]
        	push_default = HEAD

and your

	$ git push

will push only the current branch.

^ permalink raw reply

* Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name
From: Steffen Prohaska @ 2007-10-28 15:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4uzuu1g.fsf@gitster.siamese.dyndns.org>


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

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>>
>>> Steffen Prohaska <prohaska@zib.de> writes:
>>>
>>>> This teaches "push <remote> HEAD" to resolve HEAD on the local
>>>> side to its real ref name, e.g. refs/heads/master, and then
>>>> use the real ref name on the remote side to search a matching
>>>> remote ref.
>>>
>>> This probably is a good idea.
>>
>> I'll add an even shorter shorthand: "git push HEAD" will push
>> the current branch to its default remote.
>
> Ugh, that looks way too magicky.  The first parameter to push if
> one ever exists has _always_ been the remote, and the above
> breaks it.

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


> Please don't.

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

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

	Steffen

^ permalink raw reply

* Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name
From: Junio C Hamano @ 2007-10-28 15:40 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <722152C5-299C-435E-B720-D2D331D16249@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

> On Oct 28, 2007, at 8:28 AM, Junio C Hamano wrote:
>
>> Steffen Prohaska <prohaska@zib.de> writes:
>>
>>> This teaches "push <remote> HEAD" to resolve HEAD on the local
>>> side to its real ref name, e.g. refs/heads/master, and then
>>> use the real ref name on the remote side to search a matching
>>> remote ref.
>>
>> This probably is a good idea.
>
> I'll add an even shorter shorthand: "git push HEAD" will push
> the current branch to its default remote.

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

Please don't.

^ permalink raw reply

* Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name
From: Brian Gernhardt @ 2007-10-28 15:10 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <30641295-495B-4E5E-9D44-5CAF7C480DF2@zib.de>


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

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

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

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

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

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

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

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

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

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

~~ Brian

^ permalink raw reply

* Re: [PATCH 5/8] push, send-pack: support pushing HEAD to real ref name
From: Steffen Prohaska @ 2007-10-28 15:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwst7wvdr.fsf@gitster.siamese.dyndns.org>


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

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> This teaches "push <remote> HEAD" to resolve HEAD on the local
>> side to its real ref name, e.g. refs/heads/master, and then
>> use the real ref name on the remote side to search a matching
>> remote ref.
>
> This probably is a good idea.

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

	Steffen

^ permalink raw reply

* [PATCH 6/7] include $PATH in generating list of commands for "help -a"
From: Scott R Parish @ 2007-10-28 14:44 UTC (permalink / raw)
  To: git; +Cc: Scott R Parish
In-Reply-To: <1193474215-6728-6-git-send-email-srp@srparish.net>

Git had previously been using the $PATH for scripts--a previous
patch moved exec'ed commands to also use the $PATH. For consistency
"help -a" should also list commands in the $PATH.

The main commands are still listed from the git_exec_path(), but
the $PATH is walked and other git commands (probably extensions) are
listed.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |  163 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 125 insertions(+), 38 deletions(-)

diff --git a/help.c b/help.c
index 34ac5db..07cf67a 100644
--- a/help.c
+++ b/help.c
@@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdname {
-	size_t len;
-	char name[1];
-} **cmdname;
-static int cmdname_alloc, cmdname_cnt;
-
-static void add_cmdname(const char *name, int len)
+static struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[1];
+	} **names;
+} main_cmds, other_cmds;
+
+static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
 	struct cmdname *ent;
-	if (cmdname_alloc <= cmdname_cnt) {
-		cmdname_alloc = cmdname_alloc + 200;
-		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
+	if (cmds->alloc <= cmds->cnt) {
+		cmds->alloc = cmds->alloc + 200;
+		cmds->names = xrealloc(cmds->names,
+				       cmds->alloc * sizeof(*cmds->names));
 	}
 	ent = xmalloc(sizeof(*ent) + len);
 	ent->len = len;
 	memcpy(ent->name, name, len);
 	ent->name[len] = 0;
-	cmdname[cmdname_cnt++] = ent;
+	cmds->names[cmds->cnt++] = ent;
 }
 
 static int cmdname_compare(const void *a_, const void *b_)
@@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
 	return strcmp(a->name, b->name);
 }
 
-static void pretty_print_string_list(struct cmdname **cmdname, int longest)
+static void uniq(struct cmdnames *cmds)
+{
+	int i, j;
+
+	if (!cmds->cnt)
+		return;
+
+	for (i = j = 1; i < cmds->cnt; i++) {
+		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
+			cmds->names[j++] = cmds->names[i];
+		}
+	}
+
+	cmds->cnt = j;
+}
+
+static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
+	int ai, aj, bi;
+	int cmp;
+
+	ai = aj = bi = 0;
+	while (ai < a->cnt && bi < b->cnt) {
+		cmp = strcmp(a->names[ai]->name, b->names[bi]->name);
+		if (cmp < 0)
+			a->names[aj++] = a->names[ai++];
+		else if (cmp == 0)
+			ai++, bi++;
+		else if (cmp > 0)
+			bi++;
+	}
+
+	while (ai < a->cnt)
+		a->names[aj++] = a->names[ai++];
+
+	a->cnt = aj;
+}
+
+static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
 	int cols = 1, rows;
 	int space = longest + 1; /* min 1 SP between words */
@@ -73,9 +114,7 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 
 	if (space < max_cols)
 		cols = max_cols / space;
-	rows = (cmdname_cnt + cols - 1) / cols;
-
-	qsort(cmdname, cmdname_cnt, sizeof(*cmdname), cmdname_compare);
+	rows = (cmds->cnt + cols - 1) / cols;
 
 	for (i = 0; i < rows; i++) {
 		printf("  ");
@@ -83,31 +122,29 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 		for (j = 0; j < cols; j++) {
 			int n = j * rows + i;
 			int size = space;
-			if (n >= cmdname_cnt)
+			if (n >= cmds->cnt)
 				break;
-			if (j == cols-1 || n + rows >= cmdname_cnt)
+			if (j == cols-1 || n + rows >= cmds->cnt)
 				size = 1;
-			printf("%-*s", size, cmdname[n]->name);
+			printf("%-*s", size, cmds->names[n]->name);
 		}
 		putchar('\n');
 	}
 }
 
-static void list_commands(const char *exec_path)
+static unsigned int list_commands_in_dir(struct cmdnames *cmds, const char *dir)
 {
 	unsigned int longest = 0;
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	DIR *dir = opendir(exec_path);
+	DIR *dirp = opendir(dir);
 	struct dirent *de;
+	struct stat st;
 
-	if (!dir || chdir(exec_path)) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
-	}
+	if (!dirp || chdir(dir))
+		return 0;
 
-	while ((de = readdir(dir)) != NULL) {
-		struct stat st;
+	while ((de = readdir(dirp)) != NULL) {
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
@@ -125,16 +162,68 @@ static void list_commands(const char *exec_path)
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + prefix_len, entlen);
+		add_cmdname(cmds, de->d_name + prefix_len, entlen);
+	}
+	closedir(dirp);
+
+	return longest;
+}
+
+static void list_commands(void)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+	const char *exec_path = git_exec_path();
+
+	if (exec_path)
+		longest = list_commands_in_dir(&main_cmds, exec_path);
+
+	if (!env_path) {
+		fprintf(stderr, "PATH not set\n");
+		exit(1);
+	}
+
+	path = paths = xstrdup(env_path);
+	while (1) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(&other_cmds, path);
+		if (len > longest)
+			longest = len;
+
+		if (!colon)
+			break;
+		path = colon + 1;
+	}
+	free(paths);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	qsort(other_cmds.names, other_cmds.cnt,
+	      sizeof(*other_cmds.names), cmdname_compare);
+	uniq(&other_cmds);
+	subtract_cmds(&other_cmds, &main_cmds);
+
+	if (main_cmds.cnt) {
+		printf("available git commands in '%s'\n", exec_path);
+		printf("----------------------------");
+		mput_char('-', strlen(exec_path));
+		putchar('\n');
+		pretty_print_string_list(&main_cmds, longest);
+		putchar('\n');
+	}
+
+	if (other_cmds.cnt) {
+		printf("git commands available from elsewhere on your $PATH\n");
+		printf("---------------------------------------------------\n");
+		pretty_print_string_list(&other_cmds, longest);
+		putchar('\n');
 	}
-	closedir(dir);
-
-	printf("git commands available in '%s'\n", exec_path);
-	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
-	putchar('\n');
-	pretty_print_string_list(cmdname, longest);
-	putchar('\n');
 }
 
 void list_common_cmds_help(void)
@@ -188,7 +277,6 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
-	const char *exec_path = git_exec_path();
 
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -198,8 +286,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
-		if(exec_path)
-			list_commands(exec_path);
+		list_commands();
 		exit(0);
 	}
 
-- 
1.5.3.4.401.g19778-dirty

^ permalink raw reply related

* Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"
From: Scott Parish @ 2007-10-28 14:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5p7v5j7.fsf@gitster.siamese.dyndns.org>

On Sun, Oct 28, 2007 at 04:32:12AM -0700, Junio C Hamano wrote:

> Scott R Parish <srp@srparish.net> writes:
> 
> > +	while (1) {
> > +		if ((colon = strchr(path, ':')))
> > +			*colon = 0;
> > +
> > +		len = list_commands_in_dir(&other_cmds, path);
> > +		longest = MAX(longest, len);
> 
> Where do we borrow this MAX() macro?
> 
> On Linux with glibc, /usr/include/sys/param.h which is included
> by git-compat-util.h (meaning, for everybody) is where we find
> it, but that somehow does not sound portable assumption.

Awesome catch

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: [PATCH 4/8] rev-parse: teach "git rev-parse --symbolic" to print the full ref name
From: Steffen Prohaska @ 2007-10-28 13:49 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0710271748440.7345@iabervon.org>


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

> On Sat, 27 Oct 2007, Steffen Prohaska wrote:
>
>> "git rev-parse --symbolic" used to return the ref name as it was
>> specified on the command line. This is changed to returning the
>> full matched ref name, i.e. "git rev-parse --symbolic master"
>> now typically returns "refs/heads/master".
>>
>> Note, this changes output of an established command. It might
>> break existing setups. I checked that it does not break scripts
>> in git.git.
>
> I think this makes the --create option to push unnecessary, as  
> interactive
> users could use a suggested explicit value (or whatever they actually
> meant), while scripts could replace $name with $(git rev-parse -- 
> symbolic
> $name) as easily as they could add --create, and by more explicit  
> as to
> what they're doing.

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

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

	Steffen

^ permalink raw reply

* [PATCH] gitweb : disambiguate heads and tags withs the same name
From: Guillaume Seguin @ 2007-10-28 13:12 UTC (permalink / raw)
  To: pasky; +Cc: git

Avoid wrong disambiguation that would link logs/trees of tags and heads which
share the same name to the same page, leading to a disambiguation that would
prefer the tag, thus making it impossible to access the corresponding
head log and
tree without hacking the url by hand.

---
 gitweb/gitweb.perl |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 48e21da..f918c00 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3534,6 +3534,7 @@ sub git_tags_body {
 	for (my $i = $from; $i <= $to; $i++) {
 		my $entry = $taglist->[$i];
 		my %tag = %$entry;
+		my $name = "refs/tags/$tag{'name'}";
 		my $comment = $tag{'subject'};
 		my $comment_short;
 		if (defined $comment) {
@@ -3570,8 +3571,8 @@ sub git_tags_body {
 		      "<td class=\"link\">" . " | " .
 		      $cgi->a({-href => href(action=>$tag{'reftype'},
hash=>$tag{'refid'})}, $tag{'reftype'});
 		if ($tag{'reftype'} eq "commit") {
-			print " | " . $cgi->a({-href => href(action=>"shortlog",
hash=>$tag{'name'})}, "shortlog") .
-			      " | " . $cgi->a({-href => href(action=>"log",
hash=>$tag{'name'})}, "log");
+			print " | " . $cgi->a({-href => href(action=>"shortlog",
hash=>$name)}, "shortlog") .
+			      " | " . $cgi->a({-href => href(action=>"log", hash=>$name)}, "log");
 		} elsif ($tag{'reftype'} eq "blob") {
 			print " | " . $cgi->a({-href => href(action=>"blob_plain",
hash=>$tag{'refid'})}, "raw");
 		}
@@ -3597,6 +3598,7 @@ sub git_heads_body {
 	for (my $i = $from; $i <= $to; $i++) {
 		my $entry = $headlist->[$i];
 		my %ref = %$entry;
+		my $name = "refs/heads/$ref{'name'}";
 		my $curr = $ref{'id'} eq $head;
 		if ($alternate) {
 			print "<tr class=\"dark\">\n";
@@ -3606,13 +3608,13 @@ sub git_heads_body {
 		$alternate ^= 1;
 		print "<td><i>$ref{'age'}</i></td>\n" .
 		      ($curr ? "<td class=\"current_head\">" : "<td>") .
-		      $cgi->a({-href => href(action=>"shortlog", hash=>$ref{'name'}),
+		      $cgi->a({-href => href(action=>"shortlog", hash=>$name),
 		               -class => "list name"},esc_html($ref{'name'})) .
 		      "</td>\n" .
 		      "<td class=\"link\">" .
-		      $cgi->a({-href => href(action=>"shortlog",
hash=>$ref{'name'})}, "shortlog") . " | " .
-		      $cgi->a({-href => href(action=>"log", hash=>$ref{'name'})},
"log") . " | " .
-		      $cgi->a({-href => href(action=>"tree", hash=>$ref{'name'},
hash_base=>$ref{'name'})}, "tree") .
+		      $cgi->a({-href => href(action=>"shortlog", hash=>$name)},
"shortlog") . " | " .
+		      $cgi->a({-href => href(action=>"log", hash=>$name)}, "log") . " | " .
+		      $cgi->a({-href => href(action=>"tree", hash=>$name,
hash_base=>$name)}, "tree") .
 		      "</td>\n" .
 		      "</tr>";
 	}
-- 
1.5.3.4.395.g85b0

^ permalink raw reply related

* Re: How to merge git://repo.or.cz/git-gui into git.git
From: Yin Ping @ 2007-10-28 12:44 UTC (permalink / raw)
  To: Peter Baumann; +Cc: git
In-Reply-To: <20071028111443.GA29183@xp.machine.xx>

On 10/28/07, Peter Baumann <waste.manager@gmx.de> wrote:

> Have a look at the subtree merge strategy [1] and at the following
> explanations how git-gui got initally merged.
>
> -Peter
>
> [1]: http://www.gelato.unsw.edu.au/archives/git/0702/40139.html
> [2]: http://www.gelato.unsw.edu.au/archives/git/0702/39661.html
>

3x. I have seen that subtree stategy is introduced in commit 68fa.
However, I don't find any description in manual of git-merge. Should
this be added to this manual or any other document?


-- 
franky

^ permalink raw reply

* Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"
From: Junio C Hamano @ 2007-10-28 11:32 UTC (permalink / raw)
  To: Scott R Parish; +Cc: git
In-Reply-To: <1193570329-11656-1-git-send-email-srp@srparish.net>

Scott R Parish <srp@srparish.net> writes:

> +	while (1) {
> +		if ((colon = strchr(path, ':')))
> +			*colon = 0;
> +
> +		len = list_commands_in_dir(&other_cmds, path);
> +		longest = MAX(longest, len);

Where do we borrow this MAX() macro?

On Linux with glibc, /usr/include/sys/param.h which is included
by git-compat-util.h (meaning, for everybody) is where we find
it, but that somehow does not sound portable assumption.

^ permalink raw reply

* Re: [RFH] gcc constant expression warning...
From: Antti-Juhani Kaijanaho @ 2007-10-28 10:37 UTC (permalink / raw)
  To: git
In-Reply-To: <7vy7dnvd6w.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> kirjoitti 28.10.2007:
> With the recent gcc, we get:
>
> sha1_file.c: In check_packed_git_:
> sha1_file.c:527: warning: assuming signed overflow does not
> occur when assuming that (X + c) < X is always false
> sha1_file.c:527: warning: assuming signed overflow does not
> occur when assuming that (X + c) < X is always false
>
> when compiling with
>
>     -O2 -Werror -Wall -Wold-style-definition \
>     -ansi -pedantic -std=c99 -Wdeclaration-after-statement

-ansi and -std=c99 in the same command line is a bit weird, BTW :)

> The offending lines are:
>
>         if (idx_size != min_size) {
>                 /* make sure we can deal with large pack offsets */
>                 off_t x = 0x7fffffffUL, y = 0xffffffffUL;
>                 if (x > (x + 1) || y > (y + 1)) {
>                         munmap(idx_map, idx_size);

The second if line invokes undefined behavior if off_t cannot represent
0x7fffffffUL + 1.  GCC apparently takes that as a license to ignore
overflow and rewrite that if as "if (0) { ...".

A fast fix is to compile with -fwrapv or with -fno-strict-overflow:

-fstrict-overflow
    Allow the compiler to assume strict signed overflow rules, depending
    on the language being compiled. For C (and C++) this means that
    overflow when doing arithmetic with signed numbers is undefined,
    which means that the compiler may assume that it will not happen.
    This permits various optimizations. For example, the compiler will
    assume that an expression like i + 10 > i will always be true for
    signed i. This assumption is only valid if signed overflow is
    undefined, as the expression is false if i + 10 overflows when using
    twos complement arithmetic. When this option is in effect any
    attempt to determine whether an operation on signed numbers will
    overflow must be written carefully to not actually involve overflow.

    See also the -fwrapv option. Using -fwrapv means that signed
    overflow is fully defined: it wraps. When -fwrapv is used, there is
    no difference between -fstrict-overflow and -fno-strict-overflow.
    With -fwrapv certain types of overflow are permitted. For example,
    if the compiler gets an overflow when doing arithmetic on constants,
    the overflowed value can still be used with -fwrapv, but not
    otherwise.

    The -fstrict-overflow option is enabled at levels -O2, -O3, -Os. 

-fwrapv
    This option instructs the compiler to assume that signed arithmetic
    overflow of addition, subtraction and multiplication wraps around
    using twos-complement representation. This flag enables some
    optimizations and disables others. This option is enabled by default
    for the Java front-end, as required by the Java language
    specification. 

(From the GCC 4.2.2 manual.)

A correct fix would be to check for the size of off_t in some other (and
defined) manner, but I don't know off_t well enough to suggest one.
Considering that the size of off_t won't change at runtime, the test
ought to be compile (or configure) time.  Reading POSIX, there seem to
be some rather cumbersome sysconf stuff one could test for, and of
course CHAR_BIT * sizeof(off_t) also tells one something.  GNU autoconf
might also have a solution at hand.

-- 
Antti-Juhani Kaijanaho, Jyväskylä, Finland

^ permalink raw reply

* [PATCH 6/7] include $PATH in generating list of commands for "help -a"
From: Scott R Parish @ 2007-10-28 11:18 UTC (permalink / raw)
  To: git; +Cc: Scott R Parish
In-Reply-To: <1193474215-6728-6-git-send-email-srp@srparish.net>

Git had previously been using the $PATH for scripts--a previous
patch moved exec'ed commands to also use the $PATH. For consistency
"help -a" should also list commands in the $PATH.

The main commands are still listed from the git_exec_path(), but
the $PATH is walked and other git commands (probably extensions) are
listed.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |  162 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 124 insertions(+), 38 deletions(-)

diff --git a/help.c b/help.c
index 34ac5db..2534bf0 100644
--- a/help.c
+++ b/help.c
@@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdname {
-	size_t len;
-	char name[1];
-} **cmdname;
-static int cmdname_alloc, cmdname_cnt;
-
-static void add_cmdname(const char *name, int len)
+static struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[1];
+	} **names;
+} main_cmds, other_cmds;
+
+static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
 	struct cmdname *ent;
-	if (cmdname_alloc <= cmdname_cnt) {
-		cmdname_alloc = cmdname_alloc + 200;
-		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
+	if (cmds->alloc <= cmds->cnt) {
+		cmds->alloc = cmds->alloc + 200;
+		cmds->names = xrealloc(cmds->names,
+				       cmds->alloc * sizeof(*cmds->names));
 	}
 	ent = xmalloc(sizeof(*ent) + len);
 	ent->len = len;
 	memcpy(ent->name, name, len);
 	ent->name[len] = 0;
-	cmdname[cmdname_cnt++] = ent;
+	cmds->names[cmds->cnt++] = ent;
 }
 
 static int cmdname_compare(const void *a_, const void *b_)
@@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
 	return strcmp(a->name, b->name);
 }
 
-static void pretty_print_string_list(struct cmdname **cmdname, int longest)
+static void uniq(struct cmdnames *cmds)
+{
+	int i, j;
+
+	if (!cmds->cnt)
+		return;
+
+	for (i = j = 1; i < cmds->cnt; i++) {
+		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
+			cmds->names[j++] = cmds->names[i];
+		}
+	}
+
+	cmds->cnt = j;
+}
+
+static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
+	int ai, aj, bi;
+	int cmp;
+
+	ai = aj = bi = 0;
+	while (ai < a->cnt && bi < b->cnt) {
+		cmp = strcmp(a->names[ai]->name, b->names[bi]->name);
+		if (cmp < 0)
+			a->names[aj++] = a->names[ai++];
+		else if (cmp == 0)
+			ai++, bi++;
+		else if (cmp > 0)
+			bi++;
+	}
+
+	while (ai < a->cnt)
+		a->names[aj++] = a->names[ai++];
+
+	a->cnt = aj;
+}
+
+static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
 	int cols = 1, rows;
 	int space = longest + 1; /* min 1 SP between words */
@@ -73,9 +114,7 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 
 	if (space < max_cols)
 		cols = max_cols / space;
-	rows = (cmdname_cnt + cols - 1) / cols;
-
-	qsort(cmdname, cmdname_cnt, sizeof(*cmdname), cmdname_compare);
+	rows = (cmds->cnt + cols - 1) / cols;
 
 	for (i = 0; i < rows; i++) {
 		printf("  ");
@@ -83,31 +122,29 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 		for (j = 0; j < cols; j++) {
 			int n = j * rows + i;
 			int size = space;
-			if (n >= cmdname_cnt)
+			if (n >= cmds->cnt)
 				break;
-			if (j == cols-1 || n + rows >= cmdname_cnt)
+			if (j == cols-1 || n + rows >= cmds->cnt)
 				size = 1;
-			printf("%-*s", size, cmdname[n]->name);
+			printf("%-*s", size, cmds->names[n]->name);
 		}
 		putchar('\n');
 	}
 }
 
-static void list_commands(const char *exec_path)
+static unsigned int list_commands_in_dir(struct cmdnames *cmds, const char *dir)
 {
 	unsigned int longest = 0;
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	DIR *dir = opendir(exec_path);
+	DIR *dirp = opendir(dir);
 	struct dirent *de;
+	struct stat st;
 
-	if (!dir || chdir(exec_path)) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
-	}
+	if (!dirp || chdir(dir))
+		return 0;
 
-	while ((de = readdir(dir)) != NULL) {
-		struct stat st;
+	while ((de = readdir(dirp)) != NULL) {
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
@@ -125,16 +162,67 @@ static void list_commands(const char *exec_path)
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + prefix_len, entlen);
+		add_cmdname(cmds, de->d_name + prefix_len, entlen);
+	}
+	closedir(dirp);
+
+	return longest;
+}
+
+static void list_commands(void)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+	const char *exec_path = git_exec_path();
+
+	if (exec_path)
+		longest = list_commands_in_dir(&main_cmds, exec_path);
+
+	if (!env_path) {
+		fprintf(stderr, "PATH not set\n");
+		exit(1);
+	}
+
+	path = paths = xstrdup(env_path);
+	while (1) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(&other_cmds, path);
+		longest = MAX(longest, len);
+
+		if (!colon)
+			break;
+		path = colon + 1;
+	}
+	free(paths);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	qsort(other_cmds.names, other_cmds.cnt,
+	      sizeof(*other_cmds.names), cmdname_compare);
+	uniq(&other_cmds);
+	subtract_cmds(&other_cmds, &main_cmds);
+
+	if (main_cmds.cnt) {
+		printf("available git commands in '%s'\n", exec_path);
+		printf("----------------------------");
+		mput_char('-', strlen(exec_path));
+		putchar('\n');
+		pretty_print_string_list(&main_cmds, longest);
+		putchar('\n');
+	}
+
+	if (other_cmds.cnt) {
+		printf("git commands available from elsewhere on your $PATH\n");
+		printf("---------------------------------------------------\n");
+		pretty_print_string_list(&other_cmds, longest);
+		putchar('\n');
 	}
-	closedir(dir);
-
-	printf("git commands available in '%s'\n", exec_path);
-	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
-	putchar('\n');
-	pretty_print_string_list(cmdname, longest);
-	putchar('\n');
 }
 
 void list_common_cmds_help(void)
@@ -188,7 +276,6 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
-	const char *exec_path = git_exec_path();
 
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -198,8 +285,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
-		if(exec_path)
-			list_commands(exec_path);
+		list_commands();
 		exit(0);
 	}
 
-- 
1.5.3.4.401.g19778-dirty

^ permalink raw reply related

* [PATCH 5/7] use only the $PATH for exec'ing git commands
From: Scott R Parish @ 2007-10-28 11:17 UTC (permalink / raw)
  To: git; +Cc: Scott R Parish
In-Reply-To: <1193474215-6728-5-git-send-email-srp@srparish.net>

We need to correctly set up $PATH for non-c based git commands.
Since we already do this, we can just use that $PATH and execvp,
instead of looping over the paths with execve.

This patch adds a setup_path() function to exec_cmd.c, which sets
the $PATH order correctly for our search order. execv_git_cmd() is
stripped down to setting up argv and calling execvp(). git.c's
main() only only needs to call setup_path().

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |  121 ++++++++++++++++++++++++++----------------------------------
 exec_cmd.h |    1 +
 git.c      |   43 +++------------------
 3 files changed, 60 insertions(+), 105 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 8b681d0..53d0f3c 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -29,85 +29,68 @@ const char *git_exec_path(void)
 	return builtin_exec_path;
 }
 
+static void add_path(struct strbuf *out, const char *path)
+{
+	if (path && *path) {
+		if (is_absolute_path(path))
+			strbuf_addstr(out, path);
+		else
+			strbuf_addstr(out, make_absolute_path(path));
+
+		strbuf_addch(out, ':');
+	}
+}
+
+void setup_path(const char *cmd_path)
+{
+	const char *old_path = getenv("PATH");
+	struct strbuf new_path;
+
+	strbuf_init(&new_path, 0);
+
+	add_path(&new_path, argv_exec_path);
+	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
+	add_path(&new_path, builtin_exec_path);
+	add_path(&new_path, cmd_path);
+
+	if (old_path)
+		strbuf_addstr(&new_path, old_path);
+	else
+		strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin");
+
+	setenv("PATH", new_path.buf, 1);
+
+	strbuf_release(&new_path);
+}
 
 int execv_git_cmd(const char **argv)
 {
-	char git_command[PATH_MAX + 1];
-	int i;
-	const char *paths[] = { argv_exec_path,
-				getenv(EXEC_PATH_ENVIRONMENT),
-				builtin_exec_path };
-
-	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
-		size_t len;
-		int rc;
-		const char *exec_dir = paths[i];
-		const char *tmp;
-
-		if (!exec_dir || !*exec_dir) continue;
-
-		if (*exec_dir != '/') {
-			if (!getcwd(git_command, sizeof(git_command))) {
-				fprintf(stderr, "git: cannot determine "
-					"current directory: %s\n",
-					strerror(errno));
-				break;
-			}
-			len = strlen(git_command);
-
-			/* Trivial cleanup */
-			while (!prefixcmp(exec_dir, "./")) {
-				exec_dir += 2;
-				while (*exec_dir == '/')
-					exec_dir++;
-			}
-
-			rc = snprintf(git_command + len,
-				      sizeof(git_command) - len, "/%s",
-				      exec_dir);
-			if (rc < 0 || rc >= sizeof(git_command) - len) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-		} else {
-			if (strlen(exec_dir) + 1 > sizeof(git_command)) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-			strcpy(git_command, exec_dir);
-		}
-
-		len = strlen(git_command);
-		rc = snprintf(git_command + len, sizeof(git_command) - len,
-			      "/git-%s", argv[0]);
-		if (rc < 0 || rc >= sizeof(git_command) - len) {
-			fprintf(stderr,
-				"git: command name given is too long.\n");
-			break;
-		}
+	struct strbuf cmd;
+	const char *tmp;
 
-		/* argv[0] must be the git command, but the argv array
-		 * belongs to the caller, and my be reused in
-		 * subsequent loop iterations. Save argv[0] and
-		 * restore it on error.
-		 */
+	strbuf_init(&cmd, 0);
+	strbuf_addf(&cmd, "git-%s", argv[0]);
 
-		tmp = argv[0];
-		argv[0] = git_command;
+	/* argv[0] must be the git command, but the argv array
+	 * belongs to the caller, and my be reused in
+	 * subsequent loop iterations. Save argv[0] and
+	 * restore it on error.
+	 */
+	tmp = argv[0];
+	argv[0] = cmd.buf;
 
-		trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf(argv, -1, "trace: exec:");
 
-		/* execve() can only ever return if it fails */
-		execve(git_command, (char **)argv, environ);
+	/* execvp() can only ever return if it fails */
+	execvp(cmd.buf, (char **)argv);
 
-		trace_printf("trace: exec failed: %s\n", strerror(errno));
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-		argv[0] = tmp;
-	}
-	return -1;
+	argv[0] = tmp;
 
+	strbuf_release(&cmd);
+
+	return -1;
 }
 
 
diff --git a/exec_cmd.h b/exec_cmd.h
index da99287..a892355 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,6 +3,7 @@
 
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
+extern void setup_path(const char *);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index c7cabf5..4e10581 100644
--- a/git.c
+++ b/git.c
@@ -6,28 +6,6 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
-static void prepend_to_path(const char *dir, int len)
-{
-	const char *old_path = getenv("PATH");
-	char *path;
-	int path_len = len;
-
-	if (!old_path)
-		old_path = "/usr/local/bin:/usr/bin:/bin";
-
-	path_len = len + strlen(old_path) + 1;
-
-	path = xmalloc(path_len + 1);
-
-	memcpy(path, dir, len);
-	path[len] = ':';
-	memcpy(path + len + 1, old_path, path_len - len);
-
-	setenv("PATH", path, 1);
-
-	free(path);
-}
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -408,7 +386,7 @@ int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
 	char *slash = strrchr(cmd, '/');
-	const char *exec_path = NULL;
+	const char *cmd_path = NULL;
 	int done_alias = 0;
 
 	/*
@@ -418,10 +396,7 @@ int main(int argc, const char **argv)
 	 */
 	if (slash) {
 		*slash++ = 0;
-		if (*cmd == '/')
-			exec_path = cmd;
-		else
-			exec_path = xstrdup(make_absolute_path(cmd));
+		cmd_path = cmd;
 		cmd = slash;
 	}
 
@@ -458,16 +433,12 @@ int main(int argc, const char **argv)
 	cmd = argv[0];
 
 	/*
-	 * We execute external git command via execv_git_cmd(),
-	 * which looks at "--exec-path" option, GIT_EXEC_PATH
-	 * environment, and $(gitexecdir) in Makefile while built,
-	 * in this order.  For scripted commands, we prepend
-	 * the value of the exec_path variable to the PATH.
+	 * We use PATH to find git commands, but we prepend some higher
+	 * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH
+	 * environment, and the $(gitexecdir) from the Makefile at build
+	 * time.
 	 */
-	if (exec_path)
-		prepend_to_path(exec_path, strlen(exec_path));
-	exec_path = git_exec_path();
-	prepend_to_path(exec_path, strlen(exec_path));
+	setup_path(cmd_path);
 
 	while (1) {
 		/* See if it's an internal command */
-- 
1.5.3.4.401.g19778-dirty

^ permalink raw reply related

* Re: [PATCH 6/7] walk $PATH to generate list of commands for "help -a"
From: Scott Parish @ 2007-10-28 11:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodejv9gt.fsf@gitster.siamese.dyndns.org>

On Sun, Oct 28, 2007 at 03:07:14AM -0700, Junio C Hamano wrote:

> Scott Parish <sRp@srparish.net> writes:
> 
> > On Sat, Oct 27, 2007 at 11:18:02PM -0700, Junio C Hamano wrote:
> >
> >> > We walk all the paths in $PATH collecting the names of "git-*"
> >> > commands. To help distinguish between the main git commands
> >> > and commands picked up elsewhere (probably extensions) we
> >> > print them seperately. The main commands are the ones that
> >> > are found in the first directory in $PATH that contains the
> >> > "git" binary.
> >> ...
> > Its not clear to me what exactly you're looking for me to change,
> > just the wording i'm using in my comment? Or are you refering to
> > the approach?
> 
> "git" binary will be found as /usr/bin/git while git-foo will be
> found as /usr/libexec/git/git-foo in such an installation that
> takes advantage of $(gitexecdir).  And /usr/libexec/git/git will
> not exist.  Using existence of /usr/bin/git (I am referring to
> your 'first directory on $PATH that contains the "git" binary'
> above) as the cue for the location of "main commands" is wrong.

Thanks for the clarification, that would be a problem. I've modified
the patch to list the main commands from git_exec_path(); i have
mixed feelings, but curious what you think.

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox