Git development
 help / color / mirror / Atom feed
* [PATCH 2/8] t1300: remove redundant test
From: Jeff King @ 2012-10-23 22:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

This test checks that git-config fails for an ambiguous
"get", but we check the exact same thing 3 tests beforehand.

Signed-off-by: Jeff King <peff@peff.net>
---
I update the matching test later in the series, and I didn't want to
have to do it twice.

 t/t1300-repo-config.sh | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e12dd4a..c6489dc 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -275,10 +275,6 @@ test_expect_success 'multivar replace' '
 	test_cmp expect .git/config
 '
 
-test_expect_success 'ambiguous value' '
-	test_must_fail git config nextsection.nonewline
-'
-
 test_expect_success 'ambiguous unset' '
 	test_must_fail git config --unset nextsection.nonewline
 '
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 3/8] t1300: test "git config --get-all" more thoroughly
From: Jeff King @ 2012-10-23 22:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

We check that we can "--get-all" a multi-valued variable,
but we do not actually confirm that the output is sensible.
Doing so reveals that it works fine, but this will help us
ensure we do not have regressions in the next few patches,
which will touch this area.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1300-repo-config.sh | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index c6489dc..74a297e 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -256,8 +256,13 @@ test_expect_success 'ambiguous get' '
 	test_must_fail git config --get nextsection.nonewline
 '
 
-test_expect_success 'get multivar' '
-	git config --get-all nextsection.nonewline
+test_expect_success 'multi-valued get-all returns all' '
+	cat >expect <<-\EOF &&
+	wow
+	wow2 for me
+	EOF
+	git config --get-all nextsection.nonewline >actual &&
+	test_cmp expect actual
 '
 
 cat > expect << EOF
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 4/8] git-config: remove memory leak of key regexp
From: Jeff King @ 2012-10-23 22:36 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

This is only called once per invocation, so it's not a major
leak, but it's easy to fix.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/builtin/config.c b/builtin/config.c
index e1c33e0..e660d48 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -248,6 +248,10 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, system_wide, data);
 
 	free(key);
+	if (key_regexp) {
+		regfree(key_regexp);
+		free(key_regexp);
+	}
 	if (regexp) {
 		regfree(regexp);
 		free(regexp);
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 5/8] git-config: fix regexp memory leaks on error conditions
From: Jeff King @ 2012-10-23 22:38 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

The get_value function has a goto label for cleaning up on
errors, but it only cleans up half of what the function
might allocate. Let's also clean up the key and regexp
variables there.

Note that we need to take special care when compiling the
regex fails to clean it up ourselves, since it is in a
half-constructed state (we would want to free it, but not
regfree it).

Similarly, we fix git_config_parse_key to return NULL when it
fails, not a pointer to some already-freed memory.

Signed-off-by: Jeff King <peff@peff.net>
---
The diff is annoying in an interesting way: what I actually did was move
the regex cleanup code down, but it shows it as moving the bottom bits
up. I think it is just one of those ambiguous cases where either way is
equally valid and minimal.

 builtin/config.c | 23 +++++++++++++----------
 config.c         |  1 +
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index e660d48..60d36e7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -195,7 +195,8 @@ static int get_value(const char *key_, const char *regex_)
 		key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(key_regexp, key, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid key pattern: %s\n", key_);
-			free(key);
+			free(key_regexp);
+			key_regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -215,6 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(regexp, regex_, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid pattern: %s\n", regex_);
+			free(regexp);
+			regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -247,6 +250,15 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
+	if (do_all)
+		ret = !seen;
+	else
+		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+
+free_strings:
+	free(repo_config);
+	free(global);
+	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
@@ -257,15 +269,6 @@ static int get_value(const char *key_, const char *regex_)
 		free(regexp);
 	}
 
-	if (do_all)
-		ret = !seen;
-	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
-
-free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	return ret;
 }
 
diff --git a/config.c b/config.c
index 08e47e2..2fbe634 100644
--- a/config.c
+++ b/config.c
@@ -1280,6 +1280,7 @@ out_free_ret_1:
 
 out_free_ret_1:
 	free(*store_key);
+	*store_key = NULL;
 	return -CONFIG_INVALID_KEY;
 }
 
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 6/8] git-config: collect values instead of immediately printing
From: Jeff King @ 2012-10-23 22:40 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

This is a refactor that will allow us to more easily tweak
the behavior for multi-valued variables, and it will
ultimately allow us to remove a lot git-config's custom code
in favor of the regular git_config code.

It does mean we're no longer streaming, and we're storing
more in memory for the --get-all case, but in practice it is
a tiny amount of data, and the results are instantaneous.

Signed-off-by: Jeff King <peff@peff.net>
---
The increase in line count is nicely offset by the next two patches.

 builtin/config.c | 50 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 60d36e7..08e83fc 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -15,7 +15,6 @@ static int do_not_match;
 static int use_key_regexp;
 static int do_all;
 static int do_not_match;
-static int seen;
 static char delim = '=';
 static char key_delim = ' ';
 static char term = '\n';
@@ -95,8 +94,16 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	return 0;
 }
 
-static int show_config(const char *key_, const char *value_, void *cb)
+struct strbuf_list {
+	struct strbuf *items;
+	int nr;
+	int alloc;
+};
+
+static int collect_config(const char *key_, const char *value_, void *cb)
 {
+	struct strbuf_list *values = cb;
+	struct strbuf *buf;
 	char value[256];
 	const char *vptr = value;
 	int must_free_vptr = 0;
@@ -111,11 +118,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;
 
+	ALLOC_GROW(values->items, values->nr + 1, values->alloc);
+	buf = &values->items[values->nr++];
+	strbuf_init(buf, 0);
+
 	if (show_keys) {
-		printf("%s", key_);
+		strbuf_addstr(buf, key_);
 		must_print_delim = 1;
 	}
-	if (seen && !do_all)
+	if (values->nr > 1 && !do_all)
 		dup_error = 1;
 	if (types == TYPE_INT)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
@@ -138,15 +149,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
 		vptr = "";
 		must_print_delim = 0;
 	}
-	seen++;
 	if (dup_error) {
 		error("More than one value for the key %s: %s",
 				key_, vptr);
 	}
 	else {
 		if (must_print_delim)
-			printf("%c", key_delim);
-		printf("%s%c", vptr, term);
+			strbuf_addch(buf, key_delim);
+		strbuf_addstr(buf, vptr);
+		strbuf_addch(buf, term);
 	}
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
@@ -166,6 +177,8 @@ static int get_value(const char *key_, const char *regex_)
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
 	void *data;
+	struct strbuf_list values = {0};
+	int i;
 
 	local = given_config_file;
 	if (!local) {
@@ -223,8 +236,8 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	fn = show_config;
-	data = NULL;
+	fn = collect_config;
+	data = &values;
 	if (respect_includes) {
 		inc.fn = fn;
 		inc.data = data;
@@ -241,19 +254,26 @@ static int get_value(const char *key_, const char *regex_)
 	if (do_all)
 		git_config_from_file(fn, local, data);
 	git_config_from_parameters(fn, data);
-	if (!do_all && !seen)
+	if (!do_all && !values.nr)
 		git_config_from_file(fn, local, data);
-	if (!do_all && !seen && global)
+	if (!do_all && !values.nr && global)
 		git_config_from_file(fn, global, data);
-	if (!do_all && !seen && xdg)
+	if (!do_all && !values.nr && xdg)
 		git_config_from_file(fn, xdg, data);
-	if (!do_all && !seen && system_wide)
+	if (!do_all && !values.nr && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
 	if (do_all)
-		ret = !seen;
+		ret = !values.nr;
 	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+		ret = (values.nr == 1) ? 0 : values.nr > 1 ? 2 : 1;
+
+	for (i = 0; i < values.nr; i++) {
+		struct strbuf *buf = values.items + i;
+		fwrite(buf->buf, 1, buf->len, stdout);
+		strbuf_release(buf);
+	}
+	free(values.items);
 
 free_strings:
 	free(repo_config);
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 7/8] git-config: do not complain about duplicate entries
From: Jeff King @ 2012-10-23 22:41 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

If git-config is asked for a single value, it will complain
and exit with an error if it finds multiple instances of
that value. This is unlike the usual internal config
parsing, however, which will generally overwrite previous
values, leaving only the final one. For example:

  [set a multivar]
  $ git config user.email one@example.com
  $ git config --add user.email two@example.com

  [use the internal parser to fetch it]
  $ git var GIT_AUTHOR_IDENT
  Your Name <two@example.com> ...

  [use git-config to fetch it]
  $ git config user.email
  one@example.com
  error: More than one value for the key user.email: two@example.com

This overwriting behavior is critical for the regular
parser, which starts with the lowest-priority file (e.g.,
/etc/gitconfig) and proceeds to the highest-priority file
($GIT_DIR/config). Overwriting yields the highest priority
value at the end.

Git-config solves this problem by implementing its own
parsing. It goes from highest to lowest priorty, but does
not proceed to the next file if it has seen a value.

So in practice, this distinction never mattered much,
because it only triggered for values in the same file. And
there was not much point in doing that; the real value is in
overwriting values from lower-priority files.

However, this changed with the implementation of config
include files. Now we might see an include overriding a
value from the parent file, which is a sensible thing to do,
but git-config will flag as a duplication.

This patch drops the duplicate detection for git-config and
switches to a pure-overwrite model (for the single case;
--get-all can still be used if callers want to do something
more fancy).

As is shown by the modifications to the test suite, this is
a user-visible change in behavior. An alternative would be
to just change the include case, but this is much cleaner
for a few reasons:

  1. If you change the include case, then to what? If you
     just stop parsing includes after getting a value, then
     you will get a _different_ answer than the regular
     config parser (you'll get the first value instead of
     the last value). So you'd want to implement overwrite
     semantics anyway.

  2. Even though it is a change in behavior for git-config,
     it is bringing us in line with what the internal
     parsers already do.

  3. The file-order reimplementation is the only thing
     keeping us from sharing more code with the internal
     config parser, which will help keep differences to a
     minimum.

Going under the assumption that the primary purpose of
git-config is to behave identically to how git's internal
parsing works, this change can be seen as a bug-fix.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c       | 27 +++++++++------------------
 t/t1300-repo-config.sh |  6 ++++--
 t/t9700/test.pl        |  3 +--
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 08e83fc..77efa69 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -107,7 +107,6 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	char value[256];
 	const char *vptr = value;
 	int must_free_vptr = 0;
-	int dup_error = 0;
 	int must_print_delim = 0;
 
 	if (!use_key_regexp && strcmp(key_, key))
@@ -126,8 +125,6 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		strbuf_addstr(buf, key_);
 		must_print_delim = 1;
 	}
-	if (values->nr > 1 && !do_all)
-		dup_error = 1;
 	if (types == TYPE_INT)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
 	else if (types == TYPE_BOOL)
@@ -149,16 +146,12 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		vptr = "";
 		must_print_delim = 0;
 	}
-	if (dup_error) {
-		error("More than one value for the key %s: %s",
-				key_, vptr);
-	}
-	else {
-		if (must_print_delim)
-			strbuf_addch(buf, key_delim);
-		strbuf_addstr(buf, vptr);
-		strbuf_addch(buf, term);
-	}
+
+	if (must_print_delim)
+		strbuf_addch(buf, key_delim);
+	strbuf_addstr(buf, vptr);
+	strbuf_addch(buf, term);
+
 	if (must_free_vptr)
 		/* If vptr must be freed, it's a pointer to a
 		 * dynamically allocated buffer, it's safe to cast to
@@ -263,14 +256,12 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !values.nr && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
-	if (do_all)
-		ret = !values.nr;
-	else
-		ret = (values.nr == 1) ? 0 : values.nr > 1 ? 2 : 1;
+	ret = !values.nr;
 
 	for (i = 0; i < values.nr; i++) {
 		struct strbuf *buf = values.items + i;
-		fwrite(buf->buf, 1, buf->len, stdout);
+		if (do_all || i == values.nr - 1)
+			fwrite(buf->buf, 1, buf->len, stdout);
 		strbuf_release(buf);
 	}
 	free(values.items);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 74a297e..8cb45f1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -252,8 +252,10 @@ test_expect_success 'non-match value' '
 	test wow = $(git config --get nextsection.nonewline !for)
 '
 
-test_expect_success 'ambiguous get' '
-	test_must_fail git config --get nextsection.nonewline
+test_expect_success 'multi-valued get returns final one' '
+	echo "wow2 for me" >expect &&
+	git config --get nextsection.nonewline >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'multi-valued get-all returns all' '
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 3b9b484..0d4e366 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -46,8 +46,7 @@ BEGIN
 # Save and restore STDERR; we will probably extract this into a
 # "dies_ok" method and possibly move the STDERR handling to Git.pm.
 open our $tmpstderr, ">&STDERR" or die "cannot save STDERR"; close STDERR;
-eval { $r->config("test.dupstring") };
-ok($@, "config: duplicate entry in scalar context fails");
+is($r->config("test.dupstring"), "value2", "config: multivar");
 eval { $r->config_bool("test.boolother") };
 ok($@, "config_bool: non-boolean values fail");
 open STDERR, ">&", $tmpstderr or die "cannot restore STDERR";
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH 8/8] git-config: use git_config_with_options
From: Jeff King @ 2012-10-23 22:41 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20121023223502.GA23194@sigill.intra.peff.net>

The git-config command has always implemented its own file
lookup and parsing order. This was necessary because its
duplicate-entry handling did not match the way git's
internal callbacks worked. Now that this is no longer the
case, we are free to reuse the existing parsing code.

This saves us a few lines of code, but most importantly, it
means that the logic for which files are examined is
contained only in one place and cannot diverge.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c | 44 ++------------------------------------------
 1 file changed, 2 insertions(+), 42 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 77efa69..f881053 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -165,22 +165,9 @@ static int get_value(const char *key_, const char *regex_)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = CONFIG_GENERIC_ERROR;
-	char *global = NULL, *xdg = NULL, *repo_config = NULL;
-	const char *system_wide = NULL, *local;
-	struct config_include_data inc = CONFIG_INCLUDE_INIT;
-	config_fn_t fn;
-	void *data;
 	struct strbuf_list values = {0};
 	int i;
 
-	local = given_config_file;
-	if (!local) {
-		local = repo_config = git_pathdup("config");
-		if (git_config_system())
-			system_wide = git_etc_gitconfig();
-		home_config_paths(&global, &xdg, "config");
-	}
-
 	if (use_key_regexp) {
 		char *tl;
 
@@ -229,32 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	fn = collect_config;
-	data = &values;
-	if (respect_includes) {
-		inc.fn = fn;
-		inc.data = data;
-		fn = git_config_include;
-		data = &inc;
-	}
-
-	if (do_all && system_wide)
-		git_config_from_file(fn, system_wide, data);
-	if (do_all && xdg)
-		git_config_from_file(fn, xdg, data);
-	if (do_all && global)
-		git_config_from_file(fn, global, data);
-	if (do_all)
-		git_config_from_file(fn, local, data);
-	git_config_from_parameters(fn, data);
-	if (!do_all && !values.nr)
-		git_config_from_file(fn, local, data);
-	if (!do_all && !values.nr && global)
-		git_config_from_file(fn, global, data);
-	if (!do_all && !values.nr && xdg)
-		git_config_from_file(fn, xdg, data);
-	if (!do_all && !values.nr && system_wide)
-		git_config_from_file(fn, system_wide, data);
+	git_config_with_options(collect_config, &values,
+				given_config_file, respect_includes);
 
 	ret = !values.nr;
 
@@ -267,9 +230,6 @@ free_strings:
 	free(values.items);
 
 free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
-- 
1.8.0.3.g3456896

^ permalink raw reply related

* [PATCH] Add -S, --gpg-sign option to manpage of "git commit"
From: Tom Jones @ 2012-10-21 19:46 UTC (permalink / raw)
  To: git; +Cc: tom
In-Reply-To: <7vbofvfup7.fsf@alter.siamese.dyndns.org>

git commit -S, --gpg-sign was mentioned in the program's help message,
but not in the manpage.

This adds an equivalent entry for the option in the manpage.

Signed-off-by: Tom Jones <tom@oxix.org>
---
On Sun, Oct 21, 2012 at 01:15:16PM -0700, Junio C Hamano wrote:
> Are you sure about this?  The order [...]

Good point.  Please find a revised patch, with the newly documented
option before the optional double dashes, below.

> Sign off?

Now added, too.

 Documentation/git-commit.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9594ac8..4b78bd0 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--status | --no-status]
-	   [-i | -o] [--] [<file>...]
+	   [-i | -o] [-S[keyid]] [--] [<file>...]
 
 DESCRIPTION
 -----------
@@ -276,6 +276,10 @@ configuration variable documented in linkgit:git-config[1].
 	commit message template when using an editor to prepare the
 	default commit message.
 
+-S[<keyid>]::
+--gpg-sign[=<keyid>]::
+	GPG-sign commit.
+
 \--::
 	Do not interpret any more arguments as options.
 
-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH 2/2] git svn: canonicalize_url(): use svn_path_canonicalize when available
From: Eric Wong @ 2012-10-23 22:58 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Michael G Schwern, git, gitster, robbat2, bwalton
In-Reply-To: <20121014114857.GB21106@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> wrote:
> Until Subversion 1.7 (more precisely r873487), the standard way to
> canonicalize a URI was to call svn_path_canonicalize().  Use it.
> 
> This saves "git svn" from having to rely on our imperfect
> reimplementation of the same.  If the function doesn't exist or
> returns undef, though, it can use the fallback code, which we keep to
> be conservative.  Since svn_path_canonicalize() was added before
> Subversion 1.1, hopefully that doesn't happen often.

Hi Jonathan, this fails for me using http (but not file:// or svn://).
1/2 of this RFC looks fine, though.

subversion 1.6.12dfsg-6, apache2-mpm-prefork 2.2.16-6+squeeze8
(Debian squeeze)

$ SVN_HTTPD_PORT=12345 sh t9118-git-svn-funky-branch-names.sh -v
Initialized empty Git repository in /home/ew/git-core/t/trash directory.t9118-git-svn-funky-branch-names/.git/
expecting success: 
	mkdir project project/trunk project/branches project/tags &&
	echo foo > project/trunk/foo &&
	svn_cmd import -m "$test_description" project "$svnrepo/pr ject" &&
	rm -rf project &&
	svn_cmd cp -m "fun" "$svnrepo/pr ject/trunk" \
	                "$svnrepo/pr ject/branches/fun plugin" &&
	svn_cmd cp -m "more fun!" "$svnrepo/pr ject/branches/fun plugin" \
	                      "$svnrepo/pr ject/branches/more fun plugin!" &&
	svn_cmd cp -m "scary" "$svnrepo/pr ject/branches/fun plugin" \
	              "$svnrepo/pr ject/branches/$scary_uri" &&
	svn_cmd cp -m "leading dot" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/.leading_dot" &&
	svn_cmd cp -m "trailing dot" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/trailing_dot." &&
	svn_cmd cp -m "trailing .lock" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/trailing_dotlock.lock" &&
	svn_cmd cp -m "reflog" "$svnrepo/pr ject/trunk" \
			"$svnrepo/pr ject/branches/not-a@{0}reflog@" &&
	start_httpd
	
Adding         project/trunk
Adding         project/trunk/foo
Adding         project/branches
Adding         project/tags

Committed revision 1.

Committed revision 2.

Committed revision 3.

Committed revision 4.

Committed revision 5.

Committed revision 6.

Committed revision 7.

Committed revision 8.
ok 1 - setup svnrepo

expecting success: 
	git svn clone -s "$svnrepo/pr ject" project &&
	(
		cd project &&
		git rev-parse "refs/remotes/fun%20plugin" &&
		git rev-parse "refs/remotes/more%20fun%20plugin!" &&
		git rev-parse "refs/remotes/$scary_ref" &&
		git rev-parse "refs/remotes/%2Eleading_dot" &&
		git rev-parse "refs/remotes/trailing_dot%2E" &&
		git rev-parse "refs/remotes/trailing_dotlock%2Elock" &&
		git rev-parse "refs/remotes/$non_reflog"
	)
	
Initialized empty Git repository in /home/ew/git-core/t/trash directory.t9118-git-svn-funky-branch-names/project/.git/
Bad URL passed to RA layer: URL 'http://127.0.0.1:12345/pr ject' is malformed or the scheme or host or path is missing at /home/ew/git-core/perl/blib/lib/Git/SVN.pm line 310

not ok - 2 test clone with funky branch names
#	
#		git svn clone -s "$svnrepo/pr ject" project &&
#		(
#			cd project &&
#			git rev-parse "refs/remotes/fun%20plugin" &&
#			git rev-parse "refs/remotes/more%20fun%20plugin!" &&
#			git rev-parse "refs/remotes/$scary_ref" &&
#			git rev-parse "refs/remotes/%2Eleading_dot" &&
#			git rev-parse "refs/remotes/trailing_dot%2E" &&
#			git rev-parse "refs/remotes/trailing_dotlock%2Elock" &&
#			git rev-parse "refs/remotes/$non_reflog"
#		)
#		

expecting success: 
	(
		cd project &&
		git reset --hard 'refs/remotes/more%20fun%20plugin!' &&
		echo hello >> foo &&
		git commit -m 'hello' -- foo &&
		git svn dcommit
	)
	
fatal: ambiguous argument 'refs/remotes/more%20fun%20plugin!': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
not ok - 3 test dcommit to funky branch
#	
#		(
#			cd project &&
#			git reset --hard 'refs/remotes/more%20fun%20plugin!' &&
#			echo hello >> foo &&
#			git commit -m 'hello' -- foo &&
#			git svn dcommit
#		)
#		

expecting success: 
	(
		cd project &&
		git reset --hard "refs/remotes/$scary_ref" &&
		echo urls are scary >> foo &&
		git commit -m "eep" -- foo &&
		git svn dcommit
	)
	
fatal: ambiguous argument 'refs/remotes/Abo-Uebernahme%20(Bug%20#994)': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
not ok - 4 test dcommit to scary branch
#	
#		(
#			cd project &&
#			git reset --hard "refs/remotes/$scary_ref" &&
#			echo urls are scary >> foo &&
#			git commit -m "eep" -- foo &&
#			git svn dcommit
#		)
#		

expecting success: 
	(
		cd project &&
		git reset --hard "refs/remotes/trailing_dotlock%2Elock" &&
		echo who names branches like this anyway? >> foo &&
		git commit -m "bar" -- foo &&
		git svn dcommit
	)
	
fatal: ambiguous argument 'refs/remotes/trailing_dotlock%2Elock': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
not ok - 5 test dcommit to trailing_dotlock branch
#	
#		(
#			cd project &&
#			git reset --hard "refs/remotes/trailing_dotlock%2Elock" &&
#			echo who names branches like this anyway? >> foo &&
#			git commit -m "bar" -- foo &&
#			git svn dcommit
#		)
#		

# failed 4 among 5 test(s)
1..5

^ permalink raw reply

* Re: Git submodule for a local branch?
From: W. Trevor King @ 2012-10-23 22:09 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Git
In-Reply-To: <508704D5.9020902@web.de>

[-- Attachment #1: Type: text/plain, Size: 2266 bytes --]

On Tue, Oct 23, 2012 at 10:57:57PM +0200, Jens Lehmann wrote:
> Am 22.10.2012 14:37, schrieb W. Trevor King:
> > but cloning a remote repository (vs. checking out a local branch)
> > seems to be baked into the submodule implementation.  Should I be
> > thinking about generalizing git-submodule.sh, or am I looking under
> > the wrong rock?  My ideal syntax would be something like
> > 
> >   $ git submodule add -b c --local dir-for-c/
> 
> But then we'd have to be able to have two (or more) work trees using
> the same git directory, which current submodule code can't.

And that's the problem I'm trying to solve ;).

> > The motivation is that I have website that contains a bunch of
> > sub-sites, and the sub-sites share content.  I have per-sub-site
> > branches (a, b, c) and want a master branch (index) that aggregates
> > them.  Perhaps this is too much to wedge into a single repository?
> 
> To me this sounds upside-down. I'd put the three sub-sites into
> different repositories and the shared content into a submodule that
> all three sub-sites use. At least that is how I do all my content
> sharing on the websites I have done ... does that make sense?

That makes sense, however the problem is not in the common content, it
is in the final index:

  index
  |-- sub-site a (branch of sub-site-x)
  |-- sub-site b (branch of sub-site-x)
  `-- sub-site c (branch of sub-site-x)

All of the sub-sites are branches of a single sub-site-x master:

  o--o--o--o   sub-site-x
   \--o--o--o  sub-site-1
       \--o    sub-site-2
        \--o   sub-site-3

So they all live in the same repository.  My index repository will
have submodules for each of the sub-sites, and I'd like the index
branch to *also* live in same repository as the subsites.  This last
bit is the sticky part.

For a proof-of-concept example (where I currently use public
repositories for the sub-site submodules), see

  http://wking.github.com/swc-workshop/

which uses gh-pages as the index branch, and master, 2012-10-caltech,
and 2012-10-lbl for the sub-site branches.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH 1/2] Teach --recursive to submodule sync
From: Phil Hord @ 2012-10-23 23:15 UTC (permalink / raw)
  To: git; +Cc: phil.hord, Jens Lehmann, Phil Hord
In-Reply-To: <1351034141-2641-1-git-send-email-hordp@cisco.com>

The submodule sync command was somehow left out when
--recursive was added to the other submodule commands.

Teach sync to handle the --recursive switch by recursing
when we're in a submodule we are sync'ing.

Change the report during sync to show submodule-path
instead of submodule-name to be consistent with the other
submodule commands and to help recursed paths make sense.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 git-submodule.sh | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..6dd2338 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
-   or: $dashless [--quiet] sync [--] [<path>...]"
+   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
 OPTIONS_SPEC=
 . git-sh-setup
 . git-sh-i18n
@@ -1008,7 +1008,9 @@ cmd_sync()
 		case "$1" in
 		-q|--quiet)
 			GIT_QUIET=1
-			shift
+			;;
+		--recursive)
+			recursive=1
 			;;
 		--)
 			shift
@@ -1021,6 +1023,8 @@ cmd_sync()
 			break
 			;;
 		esac
+		orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
+		shift
 	done
 	cd_to_toplevel
 	module_list "$@" |
@@ -1051,7 +1055,7 @@ cmd_sync()
 
 		if git config "submodule.$name.url" >/dev/null 2>/dev/null
 		then
-			say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
+			say "$(eval_gettext "Synchronizing submodule url for '\$prefix\$sm_path'")"
 			git config submodule."$name".url "$super_config_url"
 
 			if test -e "$sm_path"/.git
@@ -1061,6 +1065,14 @@ cmd_sync()
 				cd "$sm_path"
 				remote=$(get_default_remote)
 				git config remote."$remote".url "$sub_origin_url"
+
+				if test -n "$recursive"
+				then
+				(
+					prefix="$prefix$sm_path/"
+					eval cmd_sync "$orig_args"
+				)
+			fi
 			)
 			fi
 		fi
-- 
1.8.0.2.gcde19fc.dirty

^ permalink raw reply related

* [PATCH 2/2] Add tests for submodule sync --recursive
From: Phil Hord @ 2012-10-23 23:15 UTC (permalink / raw)
  To: git; +Cc: phil.hord, Jens Lehmann, Phil Hord
In-Reply-To: <1351034141-2641-1-git-send-email-hordp@cisco.com>

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 t/t7403-submodule-sync.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 524d5c1..94e26c4 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -17,18 +17,25 @@ test_expect_success setup '
 	git commit -m upstream &&
 	git clone . super &&
 	git clone super submodule &&
+	(cd submodule &&
+	 git submodule add ../submodule sub-submodule &&
+	 test_tick &&
+	 git commit -m "sub-submodule"
+	) &&
 	(cd super &&
 	 git submodule add ../submodule submodule &&
 	 test_tick &&
 	 git commit -m "submodule"
 	) &&
 	git clone super super-clone &&
-	(cd super-clone && git submodule update --init) &&
+	(cd super-clone && git submodule update --init --recursive) &&
 	git clone super empty-clone &&
 	(cd empty-clone && git submodule init) &&
 	git clone super top-only-clone &&
 	git clone super relative-clone &&
-	(cd relative-clone && git submodule update --init)
+	(cd relative-clone && git submodule update --init --recursive) &&
+	git clone super recursive-clone &&
+	(cd recursive-clone && git submodule update --init --recursive)
 '
 
 test_expect_success 'change submodule' '
@@ -46,6 +53,11 @@ test_expect_success 'change submodule url' '
 	 git pull
 	) &&
 	mv submodule moved-submodule &&
+	(cd moved-submodule &&
+	 git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule &&
+	 test_tick &&
+	 git commit -a -m moved-sub-submodule
+	) &&
 	(cd super &&
 	 git config -f .gitmodules submodule.submodule.url ../moved-submodule &&
 	 test_tick &&
@@ -61,6 +73,9 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
 	test -d "$(cd super-clone/submodule &&
 	 git config remote.origin.url
 	)" &&
+	test ! -d "$(cd super-clone/submodule/sub-submodule &&
+	 git config remote.origin.url
+	)" &&
 	(cd super-clone/submodule &&
 	 git checkout master &&
 	 git pull
@@ -70,6 +85,25 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
 	)
 '
 
+test_expect_success '"git submodule sync --recursive" should update all submodule URLs' '
+	(cd super-clone &&
+	 (cd submodule &&
+	  git pull --no-recurse-submodules
+	 ) &&
+	 git submodule sync --recursive
+	) &&
+	test -d "$(cd super-clone/submodule &&
+	 git config remote.origin.url
+	)" &&
+	test -d "$(cd super-clone/submodule/sub-submodule &&
+	 git config remote.origin.url
+	)" &&
+	(cd super-clone/submodule/sub-submodule &&
+	 git checkout master &&
+	 git pull
+	)
+'
+
 test_expect_success '"git submodule sync" should update known submodule URLs' '
 	(cd empty-clone &&
 	 git pull &&
@@ -107,6 +141,23 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar
 	 #actual foo/submodule
 	 test "$(git config remote.origin.url)" = "../foo/submodule"
 	)
+	(cd submodule/sub-submodule &&
+	 test "$(git config remote.origin.url)" != "../../foo/submodule"
+	)
+	)
+'
+
+test_expect_success '"git submodule sync --recursive" propagates changes in origin' '
+	(cd recursive-clone &&
+	 git remote set-url origin foo/bar &&
+	 git submodule sync --recursive &&
+	(cd submodule &&
+	 #actual foo/submodule
+	 test "$(git config remote.origin.url)" = "../foo/submodule"
+	)
+	(cd submodule/sub-submodule &&
+	 test "$(git config remote.origin.url)" = "../../foo/submodule"
+	)
 	)
 '
 
-- 
1.8.0.2.gcde19fc.dirty

^ permalink raw reply related

* git submodule sync --recursive
From: Phil Hord @ 2012-10-23 23:15 UTC (permalink / raw)
  To: git; +Cc: phil.hord, Jens Lehmann
In-Reply-To: <507EF86C.4050807@web.de>

[PATCH 1/2] Teach --recursive to submodule sync
[PATCH 2/2] Add tests for submodule sync --recursive

This series implements and tests git submodule sync --recursive

^ permalink raw reply

* Re: The config include mechanism doesn't allow for overwriting
From: John Szakmeister @ 2012-10-24  0:46 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Jeff King, Git Mailing List, Junio C Hamano
In-Reply-To: <CACBZZX5mOb7_i9r8AqNK5V3r-gVnzN+rkeY9xrhecGv1rS-anA@mail.gmail.com>

On Tue, Oct 23, 2012 at 10:13 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
[snip]
> And git config --get foo.bar will give you:
>
>     $ git config -f /tmp/test --get foo.bar
>     one
>     error: More than one value for the key foo.bar: two
>     error: More than one value for the key foo.bar: three
>
> I think that it would be better if the config mechanism just silently
> overwrote keys that clobbered earlier keys like your patch does.
>
> But in addition can we simplify things for the consumers of
> "git-{config,var} -l" by only printing:
>
>     foo.bar=three
>
> Or are there too many variables like "include.path" that can
> legitimately appear more than once.

I frequently use pushurl in my remotes to push my master branch both
to the original repo and my forked version.  I find it very helpful in
my workflow, and would hate to lose that.  That said, I do like the
idea of having a config file and the ability to override some of the
variables.

-John

^ permalink raw reply

* Re: The config include mechanism doesn't allow for overwriting
From: Jeff King @ 2012-10-24  0:51 UTC (permalink / raw)
  To: John Szakmeister
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Junio C Hamano
In-Reply-To: <CAEBDL5V0Ffyp306rVo-USCBy_AXXMHXN1yrWmkF1BhzFaq60nA@mail.gmail.com>

On Tue, Oct 23, 2012 at 08:46:47PM -0400, John Szakmeister wrote:

> On Tue, Oct 23, 2012 at 10:13 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> [snip]
> > And git config --get foo.bar will give you:
> >
> >     $ git config -f /tmp/test --get foo.bar
> >     one
> >     error: More than one value for the key foo.bar: two
> >     error: More than one value for the key foo.bar: three
> >
> > I think that it would be better if the config mechanism just silently
> > overwrote keys that clobbered earlier keys like your patch does.
> >
> > But in addition can we simplify things for the consumers of
> > "git-{config,var} -l" by only printing:
> >
> >     foo.bar=three
> >
> > Or are there too many variables like "include.path" that can
> > legitimately appear more than once.
> 
> I frequently use pushurl in my remotes to push my master branch both
> to the original repo and my forked version.  I find it very helpful in
> my workflow, and would hate to lose that.  That said, I do like the
> idea of having a config file and the ability to override some of the
> variables.

No, that won't go anywhere. We really do have two classes of variables:
things that are expected to be single values, and things that are
expected to be lists.

From the perspective of the config code, we don't know or care which is
which, and just feed all entries sequentially to a C callback. In
practice, the callbacks do one of two things:

  1. Append the values into a list.

  2. Overwrite, and end up with the final value seen.

The trouble is that git-config has to print the values in a reasonable
way, so it asks the caller to give a hint about which it wants (--get
versus --get-all). But in the single-value case did not behave like the
C callbacks, which is what my series fixes.

Using "git config -l" is more like asking the config machinery to just
feed us everything, which is what the C callbacks see. Which is more
flexible, but way less convenient for the caller. But it doesn't need to
be fixed, since the caller has all the information to implement whatever
semantics they like.

-Peff

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Linus Torvalds @ 2012-10-24  1:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Jeff King, Al Viro, Chris Metcalf, LKML, linux-arch,
	Catalin Marinas, git, Junio C Hamano
In-Reply-To: <alpine.LFD.2.02.1210232307480.2756@ionos>

On Wed, Oct 24, 2012 at 12:25 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> It is spelled:
>>
>>   git notes add -m <comment> SHA1
>
> Cool!

Don't use them for anything global.

Use them for local codeflow, but don't expect them to be distributed.
It's a separate "flow", and while it *can* be distributed, it's not
going to be for the kernel, for example. So no, don't start using this
to ack things, because the acks *will* get lost.

             Linus

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Al Viro @ 2012-10-24  1:56 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Thomas Gleixner, Jeff King, Chris Metcalf, LKML, linux-arch,
	Catalin Marinas, git, Junio C Hamano
In-Reply-To: <CA+55aFyYD2jvD3+TSe=GhBgg5UQt2RNFdYf6HGiKRX-xWzFmdw@mail.gmail.com>

On Wed, Oct 24, 2012 at 04:02:49AM +0300, Linus Torvalds wrote:
> On Wed, Oct 24, 2012 at 12:25 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> It is spelled:
> >>
> >>   git notes add -m <comment> SHA1
> >
> > Cool!
> 
> Don't use them for anything global.
> 
> Use them for local codeflow, but don't expect them to be distributed.
> It's a separate "flow", and while it *can* be distributed, it's not
> going to be for the kernel, for example. So no, don't start using this
> to ack things, because the acks *will* get lost.

How about git commit --allow-empty, with
"belated ACK for <commit>

Acked-by: <...>
" as commit message?  I mean, that ought to work and propagate sanely,
but I'm really not sure if that's something in a good taste and should
be allowed as a common practice...

^ permalink raw reply

* RE: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Marc Gauthier @ 2012-10-23 22:06 UTC (permalink / raw)
  To: Jeff King, Thomas Gleixner
  Cc: Al Viro, Chris Metcalf, LKML, linux-arch@vger.kernel.org,
	Linus Torvalds, Catalin Marinas, git@vger.kernel.org,
	Junio C Hamano
In-Reply-To: <20121023214717.GA29306@sigill.intra.peff.net>

Jeff King wrote:
> On Tue, Oct 23, 2012 at 11:25:06PM +0200, Thomas Gleixner wrote:
> > > The resulting notes are stored in a separate
> > > revision-controlled branch
> >
> > Which branch(es) is/are that ? What are the semantics of that?
[...]


Nice feature.

Can a later commit be eventually be made to reference some set
of notes added so far, so they become part of the whole history
signed by the HEAD SHA1?  hence pulled/pushed automatically as
well.  Otherwise do you not end up with a forever growing separate
tree of notes that loses some of the properties of being behind
the head SHA1 (and perhaps less scalable in manageability)?
Also that way notes are separate only temporarily.

As for automating the inclusion of notes in the flow, can that
be conditional on some pattern in the note, so that e.g. the
Acked-by's get included and folded in automatically, whereas
others do not, according to settings?

-Marc

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Linus Torvalds @ 2012-10-24  2:14 UTC (permalink / raw)
  To: Al Viro
  Cc: Thomas Gleixner, Jeff King, Chris Metcalf, LKML, linux-arch,
	Catalin Marinas, git, Junio C Hamano
In-Reply-To: <20121024015613.GB2616@ZenIV.linux.org.uk>

On Wed, Oct 24, 2012 at 4:56 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> How about git commit --allow-empty, with
> "belated ACK for <commit>

Don't bother. It's not that important, and it's just distracting.

It's not like this is vital information. If you pushed it out without
the ack, it's out without the ack. Big deal.

          Linus

^ permalink raw reply

* Re: Long clone time after "done."
From: Nguyen Thai Ngoc Duy @ 2012-10-24  4:29 UTC (permalink / raw)
  To: Uri Moszkowicz; +Cc: git
In-Reply-To: <CAMJd5AQBbnFqT5xrFuPOEsJevwDE=jUgBVFZ5KqTZk5zv5+NOw@mail.gmail.com>

On Wed, Oct 24, 2012 at 1:30 AM, Uri Moszkowicz <uri@4refs.com> wrote:
> I have a large repository which I ran "git gc --aggressive" on that
> I'm trying to clone on a local file system. I would expect it to
> complete very quickly with hard links but it's taking about 6min to
> complete with no checkout (git clone -n). I see the message "Clining
> into 'repos'... done." appear after a few seconds but then Git just
> hangs there for another 6min. Any idea what it's doing at this point
> and how I can speed it up?

"done." is printed by clone_local(), which is called in cmd_clone().
After that there are just a few more calls. Maybe you could add a few
printf in between these calls, see which one takes most time?
-- 
Duy

^ permalink raw reply

* Large number of object files
From: Uri Moszkowicz @ 2012-10-24  5:21 UTC (permalink / raw)
  To: git

Continuing to work on improving clone times, using "git gc
--aggressive" has resulted in a large number of tags combining into a
single file but now I have a large number of files in the objects
directory - 131k for a ~2.7GB repository. Any way to reduce the number
of these files to speed up clones?

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Ingo Molnar @ 2012-10-24  6:02 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Thomas Gleixner, Jeff King, Al Viro, Chris Metcalf, LKML,
	linux-arch, Catalin Marinas, git, Junio C Hamano
In-Reply-To: <CA+55aFyYD2jvD3+TSe=GhBgg5UQt2RNFdYf6HGiKRX-xWzFmdw@mail.gmail.com>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Oct 24, 2012 at 12:25 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> It is spelled:
> >>
> >>   git notes add -m <comment> SHA1
> >
> > Cool!
> 
> Don't use them for anything global.
> 
> Use them for local codeflow, but don't expect them to be 
> distributed. It's a separate "flow", and while it *can* be 
> distributed, it's not going to be for the kernel, for example. 
> So no, don't start using this to ack things, because the acks 
> *will* get lost.

I'd also add a small meta argument: that it would be actively 
wrong to *allow* 'belated' acks to be added. In practice acks 
are most useful *before* a commit gets created and they often 
have a mostly buerocratic role afterwards.

So we should encourage timely acks (which actually help 
development), and accept ack-less patches as long as they are 
correct and create no problems. More utility, less buerocracy. 
Incorrect, ack-less patches causing problems will get all the 
flames they deserve.

Thanks,

	Ingo

^ permalink raw reply

* Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE
From: Johannes Sixt @ 2012-10-24  6:02 UTC (permalink / raw)
  To: Jeff King
  Cc: Marc Gauthier, Thomas Gleixner, Al Viro, Chris Metcalf, LKML,
	linux-arch@vger.kernel.org, Linus Torvalds, Catalin Marinas,
	git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20121023222318.GA3055@sigill.intra.peff.net>

Am 10/24/2012 0:23, schrieb Jeff King:
> For the fold-on-rebase idea, I'd think you would want something similar,
> like setting rebase.foldNotes to "foo" to say "refs/notes/foo contains
> pseudo-headers that should be folded in like a signed-off-by".

If you are rebasing anyway, you can already use interactive rebase's
--autosquash option:

# a late ACK came in:
git commit --allow-empty -m'squash! tile: support GENERIC_

Acked-by: A U Thor <author@example.com>'

git rebase -i --keep-empty --autosquash $forkpoint

Requires git 1.7.11 for --keep-empty and requires to edit out the
'squash!...' headers when the editor appears during the rebase.

-- Hannes

^ permalink raw reply

* Re: [PATCH 1/8] t1300: style updates
From: Johannes Sixt @ 2012-10-24  6:33 UTC (permalink / raw)
  To: Jeff King
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Junio C Hamano
In-Reply-To: <20121023223554.GA17392@sigill.intra.peff.net>

Am 10/24/2012 0:35, schrieb Jeff King:
> -test_expect_success 'non-match value' \
> -	'test wow = $(git config --get nextsection.nonewline !for)'
> +test_expect_success 'non-match value' '
> +	test wow = $(git config --get nextsection.nonewline !for)
> +'

Here's a case you forgot to update to test_cmp.

> +test_expect_success 'get-regexp variable with no value' '
> +	git config --get-regexp novalue > output &&
> +	 test_cmp expect output'

And while you are here, you might want to remove this extra space. ;)

Otherwise, looks fine.

-- Hannes

^ permalink raw reply

* Re: [PATCH 8/8] git-config: use git_config_with_options
From: Johannes Sixt @ 2012-10-24  6:33 UTC (permalink / raw)
  To: Jeff King
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Junio C Hamano
In-Reply-To: <20121023224119.GH17392@sigill.intra.peff.net>

All looked sane. Thanks for a pleasant read!

-- Hannes

^ 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