Git development
 help / color / mirror / Atom feed
* Re: Query on git commit amend
From: Vijay Lakshminarayanan @ 2011-12-06 15:46 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <4EDDD0E4.6040003@st.com>

Viresh Kumar <viresh.kumar@st.com> writes:

> Hello,
>
> Suppose i want to add few new changes to my last commit (HEAD).
> The way i do it is
> $ git add all_changed_files
> $ git commit --amend
>
> OR
> $ git commit --amend -a
>
> With both these ways, i get a screen to edit the message too.
>
> I want to know if there is a way to skip this screen.
>
> i.e.
> $ git commit --amend -a -some_other_option
>
> which simply adds new changes to existing commit, without asking to change
> message.
>
> If there is no such way, then can we add a patch for this, if it looks a valid
> case.

I've found 

$ GIT_EDITOR=cat git commit --amend

useful.

The benefit of this technique is that it even works for git-rebase -i.

In my typical git usage, I do a lot of git-commit --fixup's.  After
reaching a level of stability, I change the history with:

GIT_EDITOR=cat git rebase -i --autosquash

and my history is adjusted without requiring manual intervention.

-- 
Cheers
~vijay

Gnus should be more complicated.

^ permalink raw reply

* [PATCH] userdiff: allow * between cpp funcname words
From: Thomas Rast @ 2011-12-06 16:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The cpp pattern, used for C and C++, would not match the start of a
declaration such as

  static char *prepare_index(int argc,

because it did not allow for * anywhere between the various words that
constitute the modifiers, type and function name.  Fix it.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This is a really sneaky one-character bug that I cannot believe went
unnoticed for so long, seeing as there are plenty of instances within
git itself where it matters.


 userdiff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/userdiff.c b/userdiff.c
index 7c983c1..76109da 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -118,7 +118,7 @@
 	 /* Jump targets or access declarations */
 	 "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n"
 	 /* C/++ functions/methods at top level */
-	 "^([A-Za-z_][A-Za-z_0-9]*([ \t]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n"
+	 "^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[ \t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n"
 	 /* compound type at top level */
 	 "^((struct|class|enum)[^;]*)$",
 	 /* -- */
-- 
1.7.8.424.gcb840

^ permalink raw reply related

* git svn rebase “out of memory” error after merging two tracking branches
From: Onsager @ 2011-12-06 16:40 UTC (permalink / raw)
  To: git

@list

... more detailed explanation on stackoverflow:

http://stackoverflow.com/questions/8398405/git-svn-rebase-out-of-memory-error-after-merging-two-tracking-branches

Is there something, I can do about this issue as a common user?

Michael

^ permalink raw reply

* [PATCH 0/5] cache-tree revisited
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Carlos Martín Nieto

Junio C Hamano wrote:
> Ahh, I forgot all about that exchange.
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/178480/focus=178515
> 
> The cache-tree mechanism has traditionally been one of the more important
> optimizations and it would be very nice if we can resurrect the behaviour
> for "git commit" too.

Oh, I buried that.  Let's try something other than the aggressive
strategy I had there: only compute cache-tree if

* we know we're going to need it soon, and we're about to write out
  the index anyway (as in git-commit)

* we know we can rebuild it from a tree, and we're about to write out
  the index anyway (as in git-reset)

Both of these are essentially free, so I'm not sure I should even
bother with timings, but I ran the silly script at the end on my
favourite linux v2.6.37-rc2 repo, and it takes

  before-:  real 0m29.103s   user 0m17.335s   sys 0m8.415s
  before+:  real 0m30.860s   user 0m9.741s    sys 0m7.287s
  after-:   real 0m28.798s   user 0m17.367s   sys 0m8.435s
  after+:   real 0m17.563s   user 0m8.246s    sys 0m7.122s

where the + signifies starting out with fully valid cache-tree data,
and - means starting out with none at all.

So it's really free when it's starting out in bad shape, and it's
much faster when it starts out with valid data.

If you insist on writing cache-tree at *every* (except --only) commit,
then we might make it so that it writes out the index again at the
very end if it didn't already update it earlier.  It would (and does,
I've tried... ok ok, patch is at the end) of course give the +
performance in the - case, but it's not free so other operations may
be affected.



---- 8< ---- test script for the above timings ---- 8< ----
#!/bin/sh

for i in $(seq 1 100); do
    echo $i > arch/alpha/boot/foo
    git add arch/alpha/boot/foo
    git commit -m$i
done
---- >8 ----

---- 8< ---- patch to let git-commit always write the index ---- 8< ----
diff --git i/builtin/commit.c w/builtin/commit.c
index 57d028e..8e0c773 100644
--- i/builtin/commit.c
+++ w/builtin/commit.c
@@ -333,6 +333,8 @@ static void refresh_cache_or_die(int refresh_flags)
 		die_resolve_conflict("commit");
 }
 
+static int wrote_index_already = 0;
+
 static char *prepare_index(int argc, const char **argv, const char *prefix,
 			   const struct commit *current_head, int is_status)
 {
@@ -395,6 +397,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
 		update_main_cache_tree(1);
+		wrote_index_already = 1;
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
 			die(_("unable to write new_index file"));
@@ -415,6 +418,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
 		if (active_cache_changed) {
+			wrote_index_already = 1;
 			update_main_cache_tree(1);
 			if (write_cache(fd, active_cache, active_nr) ||
 			    commit_locked_index(&index_lock))
@@ -639,6 +643,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	const char *hook_arg2 = NULL;
 	int ident_shown = 0;
 	int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
+	int fd;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -863,11 +868,17 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	 * the editor and after we invoke run_status above.
 	 */
 	discard_cache();
+	if (!wrote_index_already)
+		fd = hold_locked_index(&index_lock, 1);
 	read_cache_from(index_file);
 	if (update_main_cache_tree(0)) {
 		error(_("Error building trees"));
 		return 0;
 	}
+	if (!wrote_index_already && commit_style != COMMIT_PARTIAL)
+		if (write_cache(fd, active_cache, active_nr) ||
+		    close_lock_file(&index_lock))
+			die(_("unable to write new_index file"));
 
 	if (run_hook(index_file, "prepare-commit-msg",
 		     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
---- >8 ----


Thomas Rast (5):
  Add test-scrap-cache-tree
  Test the current state of the cache-tree optimization
  Refactor cache_tree_update idiom from commit
  commit: write cache-tree data when writing index anyway
  reset: update cache-tree data when appropriate

 .gitignore              |    1 +
 Makefile                |    1 +
 builtin/commit.c        |    7 +--
 builtin/reset.c         |    7 +++
 cache-tree.c            |   19 +++++++--
 cache-tree.h            |    4 +-
 merge-recursive.c       |    2 +-
 t/t0090-cache-tree.sh   |   95 +++++++++++++++++++++++++++++++++++++++++++++++
 test-dump-cache-tree.c  |    2 +-
 test-scrap-cache-tree.c |   17 ++++++++
 10 files changed, 144 insertions(+), 11 deletions(-)
 create mode 100755 t/t0090-cache-tree.sh
 create mode 100644 test-scrap-cache-tree.c

-- 
1.7.8.425.ga639d3

^ permalink raw reply related

* [PATCH 1/5] Add test-scrap-cache-tree
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>

A simple utility that invalidates all existing cache-tree data.  We
need this for tests.  (We don't need a tool to rebuild the cache-tree
data; git read-tree HEAD works for that.)

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 .gitignore              |    1 +
 Makefile                |    1 +
 test-scrap-cache-tree.c |   17 +++++++++++++++++
 3 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 test-scrap-cache-tree.c

diff --git a/.gitignore b/.gitignore
index 8572c8c..122336c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -171,6 +171,7 @@
 /test-date
 /test-delta
 /test-dump-cache-tree
+/test-scrap-cache-tree
 /test-genrandom
 /test-index-version
 /test-line-buffer
diff --git a/Makefile b/Makefile
index cbb5601..6bfa10b 100644
--- a/Makefile
+++ b/Makefile
@@ -436,6 +436,7 @@ TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
 TEST_PROGRAMS_NEED_X += test-dump-cache-tree
+TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
 TEST_PROGRAMS_NEED_X += test-genrandom
 TEST_PROGRAMS_NEED_X += test-index-version
 TEST_PROGRAMS_NEED_X += test-line-buffer
diff --git a/test-scrap-cache-tree.c b/test-scrap-cache-tree.c
new file mode 100644
index 0000000..4728013
--- /dev/null
+++ b/test-scrap-cache-tree.c
@@ -0,0 +1,17 @@
+#include "cache.h"
+#include "tree.h"
+#include "cache-tree.h"
+
+static struct lock_file index_lock;
+
+int main(int ac, char **av)
+{
+	int fd = hold_locked_index(&index_lock, 1);
+	if (read_cache() < 0)
+		die("unable to read index file");
+	active_cache_tree = NULL;
+	if (write_cache(fd, active_cache, active_nr)
+	    || commit_lock_file(&index_lock))
+		die("unable to write index file");
+	return 0;
+}
-- 
1.7.8.425.ga639d3

^ permalink raw reply related

* [PATCH 3/5] Refactor cache_tree_update idiom from commit
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>

We'll need to safely create or update the cache-tree data of the_index
from other places.  While at it, give it an argument that lets us
silence the messages produced by unmerged entries (which prevent it
from working).

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 builtin/commit.c       |    5 +----
 cache-tree.c           |   19 +++++++++++++++----
 cache-tree.h           |    4 +++-
 merge-recursive.c      |    2 +-
 test-dump-cache-tree.c |    2 +-
 5 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index e36e9ad..0163086 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -862,10 +862,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	 */
 	discard_cache();
 	read_cache_from(index_file);
-	if (!active_cache_tree)
-		active_cache_tree = cache_tree();
-	if (cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0) < 0) {
+	if (update_main_cache_tree(0)) {
 		error(_("Error building trees"));
 		return 0;
 	}
diff --git a/cache-tree.c b/cache-tree.c
index f755590..8de3959 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -150,7 +150,7 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
 }
 
 static int verify_cache(struct cache_entry **cache,
-			int entries)
+			int entries, int silent)
 {
 	int i, funny;
 
@@ -159,6 +159,8 @@ static int verify_cache(struct cache_entry **cache,
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
 		if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
+			if (silent)
+				return -1;
 			if (10 < ++funny) {
 				fprintf(stderr, "...\n");
 				break;
@@ -370,10 +372,11 @@ int cache_tree_update(struct cache_tree *it,
 		      struct cache_entry **cache,
 		      int entries,
 		      int missing_ok,
-		      int dryrun)
+		      int dryrun,
+		      int silent)
 {
 	int i;
-	i = verify_cache(cache, entries);
+	i = verify_cache(cache, entries, silent);
 	if (i)
 		return i;
 	i = update_one(it, cache, entries, "", 0, missing_ok, dryrun);
@@ -573,7 +576,7 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
 
 		if (cache_tree_update(active_cache_tree,
 				      active_cache, active_nr,
-				      missing_ok, 0) < 0)
+				      missing_ok, 0, 0) < 0)
 			return WRITE_TREE_UNMERGED_INDEX;
 		if (0 <= newfd) {
 			if (!write_cache(newfd, active_cache, active_nr) &&
@@ -668,3 +671,11 @@ int cache_tree_matches_traversal(struct cache_tree *root,
 		return it->entry_count;
 	return 0;
 }
+
+int update_main_cache_tree (int silent)
+{
+	if (!the_index.cache_tree)
+		the_index.cache_tree = cache_tree();
+	return cache_tree_update(the_index.cache_tree,
+				 the_index.cache, the_index.cache_nr, 0, 0, silent);
+}
diff --git a/cache-tree.h b/cache-tree.h
index 3df641f..0ec0b2a 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -29,7 +29,9 @@ struct cache_tree {
 struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
 
 int cache_tree_fully_valid(struct cache_tree *);
-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int);
+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);
+
+int update_main_cache_tree(int);
 
 /* bitmasks to write_cache_as_tree flags */
 #define WRITE_TREE_MISSING_OK 1
diff --git a/merge-recursive.c b/merge-recursive.c
index 5a2db29..d83cd6c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
 
 	if (!cache_tree_fully_valid(active_cache_tree) &&
 	    cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0) < 0)
+			      active_cache, active_nr, 0, 0, 0) < 0)
 		die("error building trees");
 
 	result = lookup_tree(active_cache_tree->sha1);
diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c
index 1f73f1e..e6c2923 100644
--- a/test-dump-cache-tree.c
+++ b/test-dump-cache-tree.c
@@ -59,6 +59,6 @@ int main(int ac, char **av)
 	struct cache_tree *another = cache_tree();
 	if (read_cache() < 0)
 		die("unable to read index file");
-	cache_tree_update(another, active_cache, active_nr, 0, 1);
+	cache_tree_update(another, active_cache, active_nr, 0, 1, 0);
 	return dump_cache_tree(active_cache_tree, another, "");
 }
-- 
1.7.8.425.ga639d3

^ permalink raw reply related

* [PATCH 2/5] Test the current state of the cache-tree optimization
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>

The cache-tree optimization originally helped speed up write-tree
operation.  However, many commands no longer properly maintain -- or
use an opportunity to cheaply generate -- the cache-tree data.  In
particular, this affects commit, checkout and reset.  The notable
examples that *do* write cache-tree data are read-tree and write-tree.

This sadly means most people no longer benefit from the optimization,
as they would not normally use the plumbing commands.

Document the current state of affairs in a test file, in preparation
for improvements in the area.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t0090-cache-tree.sh |   95 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 95 insertions(+), 0 deletions(-)
 create mode 100755 t/t0090-cache-tree.sh

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
new file mode 100755
index 0000000..3d0702a
--- /dev/null
+++ b/t/t0090-cache-tree.sh
@@ -0,0 +1,95 @@
+#!/bin/sh
+
+test_description="Test whether cache-tree is properly updated
+
+Tests whether various commands properly update and/or rewrite the
+cache-tree extension.
+"
+ . ./test-lib.sh
+
+cmp_cache_tree () {
+	test-dump-cache-tree >actual &&
+	sed "s/$_x40/SHA/" <actual >filtered &&
+	test_cmp "$1" filtered
+}
+
+# We don't bother with actually checking the SHA1:
+# test-dump-cache-tree already verifies that all existing data is
+# correct.
+test_shallow_cache_tree () {
+	echo "SHA " \
+	    "($(git ls-files|wc -l) entries, 0 subtrees)" >expect &&
+	cmp_cache_tree expect
+}
+
+test_invalid_cache_tree () {
+	echo "invalid                                   (0 subtrees)" >expect &&
+	echo "SHA #(ref) " \
+	    "($(git ls-files|wc -l) entries, 0 subtrees)" >>expect &&
+	cmp_cache_tree expect
+}
+
+test_no_cache_tree () {
+	: >expect &&
+	cmp_cache_tree expect
+}
+
+test_expect_failure 'initial commit has cache-tree' '
+	test_commit foo &&
+	test_shallow_cache_tree
+'
+
+test_expect_success 'read-tree HEAD establishes cache-tree' '
+	git read-tree HEAD &&
+	test_shallow_cache_tree
+'
+
+test_expect_success 'git-add invalidates cache-tree' '
+	test_when_finished "git reset --hard; git read-tree HEAD" &&
+	echo "I changed this file" > foo &&
+	git add foo &&
+	test_invalid_cache_tree
+'
+
+test_expect_success 'update-index invalidates cache-tree' '
+	test_when_finished "git reset --hard; git read-tree HEAD" &&
+	echo "I changed this file" > foo &&
+	git update-index --add foo &&
+	test_invalid_cache_tree
+'
+
+test_expect_success 'write-tree establishes cache-tree' '
+	test-scrap-cache-tree &&
+	git write-tree &&
+	test_shallow_cache_tree
+'
+
+test_expect_success 'test-scrap-cache-tree works' '
+	git read-tree HEAD &&
+	test-scrap-cache-tree &&
+	test_no_cache_tree
+'
+
+test_expect_failure 'second commit has cache-tree' '
+	test_commit bar &&
+	test_shallow_cache_tree
+'
+
+test_expect_failure 'reset --hard gives cache-tree' '
+	test-scrap-cache-tree &&
+	git reset --hard &&
+	test_shallow_cache_tree
+'
+
+test_expect_failure 'reset --hard without index gives cache-tree' '
+	rm -f .git/index &&
+	git reset --hard &&
+	test_shallow_cache_tree
+'
+
+test_expect_failure 'checkout gives cache-tree' '
+	git checkout HEAD^ &&
+	test_shallow_cache_tree
+'
+
+test_done
-- 
1.7.8.425.ga639d3

^ permalink raw reply related

* [PATCH 5/5] reset: update cache-tree data when appropriate
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>

In the case of --mixed and --hard, we throw away the old index and
rebuild everything from the tree argument (or HEAD).  So we have an
opportunity here to fill in the cache-tree data, just as read-tree
did.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 builtin/reset.c       |    7 +++++++
 t/t0090-cache-tree.sh |    4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 811e8e2..8c2c1d5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -43,6 +43,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 	int nr = 1;
 	int newfd;
 	struct tree_desc desc[2];
+	struct tree *tree;
 	struct unpack_trees_options opts;
 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 
@@ -84,6 +85,12 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 		return error(_("Failed to find tree of %s."), sha1_to_hex(sha1));
 	if (unpack_trees(nr, desc, &opts))
 		return -1;
+
+	if (reset_type == MIXED || reset_type == HARD) {
+		tree = parse_tree_indirect(sha1);
+		prime_cache_tree(&active_cache_tree, tree);
+	}
+
 	if (write_cache(newfd, active_cache, active_nr) ||
 	    commit_locked_index(lock))
 		return error(_("Could not write new index file."));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index a3527a5..f972562 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -75,13 +75,13 @@ test_expect_success 'second commit has cache-tree' '
 	test_shallow_cache_tree
 '
 
-test_expect_failure 'reset --hard gives cache-tree' '
+test_expect_success 'reset --hard gives cache-tree' '
 	test-scrap-cache-tree &&
 	git reset --hard &&
 	test_shallow_cache_tree
 '
 
-test_expect_failure 'reset --hard without index gives cache-tree' '
+test_expect_success 'reset --hard without index gives cache-tree' '
 	rm -f .git/index &&
 	git reset --hard &&
 	test_shallow_cache_tree
-- 
1.7.8.425.ga639d3

^ permalink raw reply related

* [PATCH 4/5] commit: write cache-tree data when writing index anyway
From: Thomas Rast @ 2011-12-06 17:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Carlos Martín Nieto
In-Reply-To: <cover.1323191497.git.trast@student.ethz.ch>

In prepare_index(), we refresh the index, and then write it to disk if
this changed the index data.  After running hooks we re-read the index
and compute the root tree sha1 with the cache-tree machinery.

This gives us a mostly free opportunity to write up-to-date cache-tree
data: we can compute it in prepare_index() immediately before writing
the index to disk.

If we do this, we were going to write the index anyway, and the later
cache-tree update has no further work to do.  If we don't do it, we
don't do any extra work, though we still don't have have cache-tree
data after the commit.

The only case that suffers badly is when the pre-commit hook changes
many trees in the index.  I'm writing this off as highly unusual.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Don't ask me why git thinks the index has been changed only when
already building upon an earlier commit.  I don't know.  I suspect
it's some raciness issue though.

 builtin/commit.c      |    2 ++
 t/t0090-cache-tree.sh |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 0163086..57d028e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -394,6 +394,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
+		update_main_cache_tree(1);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
 			die(_("unable to write new_index file"));
@@ -414,6 +415,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 		fd = hold_locked_index(&index_lock, 1);
 		refresh_cache_or_die(refresh_flags);
 		if (active_cache_changed) {
+			update_main_cache_tree(1);
 			if (write_cache(fd, active_cache, active_nr) ||
 			    commit_locked_index(&index_lock))
 				die(_("unable to write new_index file"));
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 3d0702a..a3527a5 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -70,7 +70,7 @@ test_expect_success 'test-scrap-cache-tree works' '
 	test_no_cache_tree
 '
 
-test_expect_failure 'second commit has cache-tree' '
+test_expect_success 'second commit has cache-tree' '
 	test_commit bar &&
 	test_shallow_cache_tree
 '
-- 
1.7.8.425.ga639d3

^ permalink raw reply related

* Re: git svn rebase  “out of memory” error after merging two tracking branches
From: Thomas Rast @ 2011-12-06 17:56 UTC (permalink / raw)
  To: Onsager; +Cc: git
In-Reply-To: <4EDE4589.9030601@gmx.net>

Onsager wrote:
> @list
> 
> ... more detailed explanation on stackoverflow:
> 
> http://stackoverflow.com/questions/8398405/git-svn-rebase-out-of-memory-error-after-merging-two-tracking-branches
> 
> Is there something, I can do about this issue as a common user?

In this age of 5-second attention spans, I'm not sure you can expect
us to follow a link to an external site when a simple cut&paste job
would have sufficed, like so:

  I'm tracking two customer SVN branches with msysgit 1.7.7.1 (Win 7 64bit):

    SVN                Git

    trunk          --> master
    release_x_y_z  --> git-local-release_x_y_z 

  After a successful local merge of 'git-local-release_x_y_z' into
  'master' I'm running out of memory, when trying to synchronize the
  result with svn:

    mb@MMPEPA23 /c/git/MySoft (master)
    $ git svn rebase
    First, rewinding head to replay your work on top of it...
    Applying:
    fatal: Out of memory, realloc failed
    Repository lacks necessary blobs to fall back on 3-way merge.
    Cannot fall back to three-way merge.
    Patch failed at 0001

    When you have resolved this problem run "git rebase --continue".
    If you would prefer to skip this patch, instead run "git rebase --skip".
    To check out the original branch and stop rebasing run "git rebase --abort".

    rebase refs/remotes/git-svn: command returned error: 1 

  The .git/rebase-apply directory contains a few files ('patch', '0001')
  with more than 1GB size. What can I do in order to apply this patch?


First you should find out whether something went wrong with the patch
generation, or if that 1GB size is plausible.  Did your merge bring in
blobs that were that big?

Second, you could try with the -m option.  This will use a 3-way merge
to rebase, which avoids generating a full patch.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] Set hard limit on delta chain depth
From: Shawn Pearce @ 2011-12-06 18:12 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Michael Haggerty, kusmabite, git, Junio C Hamano
In-Reply-To: <CACsJy8Btwn0=PGS+PJV-6DqYBmzOp7LTB2=R_kCx4SJHA2YDRw@mail.gmail.com>

On Tue, Dec 6, 2011 at 07:30, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> index-pack is called at server side as part of a push. So in theory
> the sending side can generate very long delta chains and bring down
> the server side.

It is also called at client side during fetch. So in theory the server
can produce very long delta chains and take down a client.

>>  Because if not, then the "creator" had better never be configured
>> to use a chain depth that the "reader" cannot handle.
>
> Normal creators (i.e. C Git) use default depth 50 so we should be safe.

JGit is also a "normal creator", and it sometimes produces chains
deeper than 50. Junio identified a 255 deep chain a week or two ago.
Some people have repacked their repositories very aggressively with
deeper chains when they are trying to optimize for space and don't
access historical revisions very often. I doubt anyone has packed
deeper than 120ish intentionally... but we shouldn't assume that in
the code.

>>  This in turn
>> imply that there should be a common limit that is supported by all git
>> clients and is a documented part of the protocol.  (Or the code has to
>> be rewritten to use an explicit stack instead of recursion.)
>
> It's the implementation limitation, not the protocol. If the server
> propagates error messages from index-pack back to client (I'm not
> sure), then users can adjust --depth to be accepted by server. We
> could negotiate the limit over the protocol but not sure we would want
> to go that route.

What about clients fetching from a server? The client can't change the
depth the server sends it.

> The troubled code could be rewritten to avoid recursion. However long
> delta chains may degrade performance, I'd rather have support to split
> long chains (which can be done with --depth at client already) instead
> of just recursion elimination. This patch is simpler, so it would be
> easier to back port it if someone wants to do so.

JGit long ago changed its IndexPack routine to use a manually managed
heap based stack instead of recursion, making it immune from
overrunning the stack due to a long delta chain. That is probably the
cleaner route. But you also have to fix sha1_file.c and its recursion
based unpacking of a delta chain... again which JGit fixed a long time
ago.

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 18:20 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: git
In-Reply-To: <CALkWK0mpPoZJWviBesWgy2dZ4xJrNyhED2znFid8iGbSTirPhQ@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> 0. You can merge this into `master` resolving the conflicts: it's a
> fairly straightforward resolution.  As soon as you publish the new
> `master`, I can continue working on `rr/sequencer`.
> 1. I can post the `rr/revert-cherry-pick` to the list, hoping that it
> will make it to `master` without more disruptions.  I can rebase
> `rr/sequencer` on this new series and continue working.  For your
> reference, this [1] is what I'll be posting to the list if we pick
> this option.

Merging

    https://github.com/artagnon/git.git rr/revert-cherry-pick ;# 1b7c2632

and merging rr/revert-cherry-pick currently in 'pu' (6a156fd) to 'master'
results in identical trees, so I think these amount to the same thing.

A few comments and thoughts, all minor.

 * On "revert: simplify getting commit subject in format_todo()"

   The old code used to use get_message() on cur->item, checked its return
   value (if cur->item is not parsed, or has already been used and its buffer
   cleared, cur->item->buffer would be NULL and get_message() returns -1) and
   issued an error. The new code uses find_commit_subject without first
   checking if cur->item->buffer is NULL.

   Does this mean the old code was overly defensive, or is the new code too
   lax?

   I understand that parse_insn_line() uses lookup_commit_reference() which
   calls parse_object() on the object name (and if it is a tag, deref_tag()
   will parse the tagged object until we see something that is not a tag), so
   we know cur->item is parsed before we see it, so I suspect you merely were
   being overly defensive, but I may be missing something.

 * On "revert: make commit subjects in insn sheet optional"

   After finding the verb and advancing the pointer "bol" at the beginning of
   the object name, end_of_object_name variable points at the first SP or LF
   using strcspn(bol, " \n"), but I wonder why we are not grabbing a run of
   hexdigits instead, i.e. strspn(bol, "0123456789abcdef"). Is this so that
   we can allow something like "pick rr/revert-cherry-pick~3"?

   I also wonder if this should be (sorry for pcre) "(pick|revert)\s+(\S+)\s"
   instead, i.e. allow users with fat fingers to use one or more SP or even HT
   to separate the verb and the operand.

   The last test you added to 3510 in this patch runs test_line_count
   unconditionally, by the way.

 * On "revert: allow mixed pick and revert instructions"

   Reporting what we did not understand from parse_insn_line() is a good
   idea, but I think the line number should be reported at the beginning
   of the same line.

I'd say that I'd prefer queuing re-rolled patches posted on the list; I'll
discard the rr/revert-cherry-pick (6a156fd) from my tree.

Thanks.

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 18:35 UTC (permalink / raw)
  To: Jeff King; +Cc: Erik Faye-Lund, git
In-Reply-To: <20111206055239.GA20671@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Also, let's drop the top git_getpass bits from the topic for now (they
> will not be part of my rebase). They are a separate topic that can go on
> top, but I think there was some question from Erik of whether we should
> simply roll our own getpass().

Sounds sensible.

I suspect that there may be a codepath where we could ask both username
and password; instead of making two consecutive calls to getpass() or
git_prompt(), the series may want to give a higher level abstraction, so
that GUI can show a dialog with two input fields (single-line input and
password input) and interact only once with the user. Such an input widget
could _show_ the username, and optionally even let it edited (there may be
ramifications depending on how the codepath uses the username), while
asking for the corresponding password.

>> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit
>>  - archive: don't let remote clients get unreachable commits
>>  (this branch is used by jk/maint-upload-archive.)
>> 
>> * jk/maint-upload-archive (2011-11-21) 1 commit
>>  - Merge branch 'jk/maint-1.6.2-upload-archive' into jk/maint-upload-archive
>>  (this branch uses jk/maint-1.6.2-upload-archive.)
>> 
>> Will merge to 'next' after taking another look.
>
> Thanks. I also have some followup patches to re-loosen to at least
> trees reachable from refs. Do you want to leave the tightening to the
> maint track, and then consider the re-loosening for master?

I was planning to first have the really tight version graduate to 'master'
and ship it in 1.7.9, while possibly merging that to 1.7.8.X series. If we
hear complaints from real users in the meantime before or after such
releases, we could apply loosening patch on top of these topics and call
them "regression fix", but I have been assuming that nobody would have
been using this backdoor for anything that really matters.

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Jeff King @ 2011-12-06 18:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Erik Faye-Lund, git
In-Reply-To: <7vhb1dh7ki.fsf@alter.siamese.dyndns.org>

On Tue, Dec 06, 2011 at 10:35:25AM -0800, Junio C Hamano wrote:

> > Also, let's drop the top git_getpass bits from the topic for now (they
> > will not be part of my rebase). They are a separate topic that can go on
> > top, but I think there was some question from Erik of whether we should
> > simply roll our own getpass().
> 
> Sounds sensible.
> 
> I suspect that there may be a codepath where we could ask both username
> and password; instead of making two consecutive calls to getpass() or
> git_prompt(), the series may want to give a higher level abstraction, so
> that GUI can show a dialog with two input fields (single-line input and
> password input) and interact only once with the user. Such an input widget
> could _show_ the username, and optionally even let it edited (there may be
> ramifications depending on how the codepath uses the username), while
> asking for the corresponding password.

Yes, I've considered that, too. But I think the idea of a combined
username/password is part of the credential code, and the right
call chain is something like:

  credential_fill
    -> call helpers with "get"; return if it works
    -> credential_getpass
       -> call helpers with "ask" for combined GUI prompt
       -> otherwise, use git_prompt
          -> git_prompt("username")
          -> git_prompt("password")

So the "switch getpass to a generic prompt" idea is separate from
providing that higher-level abstraction.

> >> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit
> >>  - archive: don't let remote clients get unreachable commits
> >>  (this branch is used by jk/maint-upload-archive.)
> [...]
> I was planning to first have the really tight version graduate to 'master'
> and ship it in 1.7.9, while possibly merging that to 1.7.8.X series. If we
> hear complaints from real users in the meantime before or after such
> releases, we could apply loosening patch on top of these topics and call
> them "regression fix", but I have been assuming that nobody would have
> been using this backdoor for anything that really matters.

OK. I'll hold back on the loosening then.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Jeff King @ 2011-12-06 18:52 UTC (permalink / raw)
  To: Erik Faye-Lund
  Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, Stephan Beyer
In-Reply-To: <CABPQNSbOReM71HaPmce3v_98NDu17fT3YnySR4pWzJEDa-RKnA@mail.gmail.com>

On Tue, Dec 06, 2011 at 12:22:06PM +0100, Erik Faye-Lund wrote:

> >> * jk/upload-archive-use-start-command (2011-11-21) 1 commit
> >>  - upload-archive: use start_command instead of fork
> >>
> >> What's the status of this one?
> >
> > I think what you have in pu is good, but of course I didn't actually
> > test it on Windows. Erik?
> >
> 
> I tried to check out ee27ca4 and build, and got hit by a wall of warnings:

I think you are on the wrong topic. ee27ca4 is the maint topic for
"don't let remote clients get unreachable commits", and is based on the
ancient 1.6.2. Which is why you are getting all of those warnings.

The topic in question is jk/upload-archive-use-start-command, which is
at 1bc01ef, and should be based on recent-ish master.

-Peff

^ permalink raw reply

* Re: [PATCH] Set hard limit on delta chain depth
From: Jeff King @ 2011-12-06 18:56 UTC (permalink / raw)
  To: Shawn Pearce
  Cc: Nguyen Thai Ngoc Duy, Michael Haggerty, kusmabite, git,
	Junio C Hamano
In-Reply-To: <CAJo=hJuy27Uagmubbv=RqAOMx03e6JBgZxObkjFLg9oG2x_UqA@mail.gmail.com>

On Tue, Dec 06, 2011 at 10:12:54AM -0800, Shawn O. Pearce wrote:

> > Normal creators (i.e. C Git) use default depth 50 so we should be safe.
> 
> JGit is also a "normal creator", and it sometimes produces chains
> deeper than 50. Junio identified a 255 deep chain a week or two ago.
> Some people have repacked their repositories very aggressively with
> deeper chains when they are trying to optimize for space and don't
> access historical revisions very often. I doubt anyone has packed
> deeper than 120ish intentionally... but we shouldn't assume that in
> the code.

"git gc --aggressive" will set the depth to 250.

-Peff

^ permalink raw reply

* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Jeff King @ 2011-12-06 19:02 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, git
In-Reply-To: <a639d328e15bce3057de9238ee31097d15850de1.1323189110.git.trast@student.ethz.ch>

On Tue, Dec 06, 2011 at 05:35:08PM +0100, Thomas Rast wrote:

> The cpp pattern, used for C and C++, would not match the start of a
> declaration such as
> 
>   static char *prepare_index(int argc,
> 
> because it did not allow for * anywhere between the various words that
> constitute the modifiers, type and function name.  Fix it.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> 
> This is a really sneaky one-character bug that I cannot believe went
> unnoticed for so long, seeing as there are plenty of instances within
> git itself where it matters.

Looks reasonable to me. You can see the difference, for instance, with:

  git show -U1 3c73a1d

(The -U1 is because of the annoying "we will start looking for the
header at the top of context, not the top of changes" behavior I
mentioned last week).

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 19:10 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CZjrKpv53Ep8ietAHAeVW4bnYeXTa6x5FGncT1HXGWQg@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> What can I do to get "build in repack" [1] series in or moved forward?

Resubmit to re-ignite interest in reviews, perhaps?

As long as it is done right at the implementation level, I do not think
there is anything controversial in the desired end result, iow it is not
like we need three people who want to have repack rewritten in C for it to
happen.

It may be tricky to get the flushing of old and new packfiles right,
though.

Use of reprepare_packed_git() is prerequisite if you want to do it in a
single process, but once you start using that API, it may not gain much
performance benefit to link the whole pack-objects logic in to the process
over a much simpler and less error prone approach of just rewriting the
shell script straight to a small C program that spawns pack-objects
binary.

^ permalink raw reply

* Re: Query on git commit amend
From: Jeff King @ 2011-12-06 19:11 UTC (permalink / raw)
  To: Vijay Lakshminarayanan; +Cc: Viresh Kumar, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <87fwgxwvn9.fsf@gmail.com>

On Tue, Dec 06, 2011 at 09:16:18PM +0530, Vijay Lakshminarayanan wrote:

> I've found 
> 
> $ GIT_EDITOR=cat git commit --amend
> 
> useful.
> 
> The benefit of this technique is that it even works for git-rebase -i.

I sometimes do a similar thing, but I don't use "cat". That will dump
all of the log message (including the generated template) to stdout
(i.e., the terminal), which is quite noisy. Instead, I use:

  GIT_EDITOR=true git commit --amend

which silently leaves the file untouched.

-Peff

^ permalink raw reply

* Re: Query on git commit amend
From: Dirk Süsserott @ 2011-12-06 19:09 UTC (permalink / raw)
  To: Konstantin Khomoutov; +Cc: Viresh Kumar, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <20111206130138.119db519.kostix@domain007.com>

Am 06.12.2011 10:01 schrieb Konstantin Khomoutov:
> On Tue, 6 Dec 2011 13:53:00 +0530
> Viresh Kumar <viresh.kumar@st.com> wrote:
> 
>> Suppose i want to add few new changes to my last commit (HEAD).
>> The way i do it is
>> $ git add all_changed_files
>> $ git commit --amend
>>
>> OR
>> $ git commit --amend -a
>>
>> With both these ways, i get a screen to edit the message too.
>>
>> I want to know if there is a way to skip this screen.
>>
>> i.e.
>> $ git commit --amend -a -some_other_option
>>
>> which simply adds new changes to existing commit, without asking to
>> change message.
>>
>> If there is no such way, then can we add a patch for this, if it
>> looks a valid case.
> git commit --amend -C HEAD

$ git commit --amend -C HEAD

works fine but will keep the authorship (name _and_ date). To change the
date to the current timestamp, use

$ git commit --amend -C HEAD --reset-author

Note that this will also change the author's name to yours, so it
depends on your case. The commiter's name and timestamp are always
updated to "you/now", independently of that option. To change only the
author's date, use --date=<date>.

Cheers,
    Dirk

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-06 19:12 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8AmwQxcKz9vhtvFJPPKpXeipufD_0OqoMv41SHQFZgqeQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> I thought this would be replaced by a new version [1] I posted a while
> ago (and forgot to address comments). Do you want me to rebase that on
> top of this or replace it?

I think what was merged to 'master' is already sane. If you have updates,
treat them just like other new topics.

Thanks.

^ permalink raw reply

* BUG: Confusing submodule error message
From: Seth Robertson @ 2011-12-06 19:30 UTC (permalink / raw)
  To: git


Someone on #git just encountered a problem where `git init && git add . &&
git status` was failing with a message about a corrupted index.

    error: bad index file sha1 signature
    fatal: index file corrupt
    fatal: git status --porcelain failed

This confused everyone for a while, until he provided access to the
directory to play with.  I eventually tracked it down to a directory
in the tree which already had a .git directory in it.  Unfortunately,
that .git repo was corrupted and was the one returning the message
about a corrupted index.  The problem is that the error message we
were seeing did not provide any direct hints that submodules were
involved or that the problem was not at the top level (`git status
--porcelain` is admittedly an indirect hint to both).  Here is a
recipe to reproduce a similar problem:

(mkdir -p z/foo; cd z/foo; git init; echo A>A; git add A; git commit -m A; cd ..; echo B>B; rm -f foo/.git/objects/*/*; git init; git add .; git status)

Providing an expanded error message which clarifies that this is
failing in a submodule directory makes everything clear.

----------------------------------------------------------------------
--- submodule.c~	2011-12-02 14:25:08.000000000 -0500
+++ submodule.c	2011-12-06 14:13:00.554413432 -0500
@@ -714,7 +714,7 @@
 	close(cp.out);
 
 	if (finish_command(&cp))
-		die("git status --porcelain failed");
+		die("git status --porcelain failed in submodule directory %s", path);
 
 	strbuf_release(&buf);
 	return dirty_submodule;
----------------------------------------------------------------------

Do more error messages in submodule.c need adjusting?  It seems likely.

					-Seth Robertson

^ permalink raw reply

* Re: Query on git commit amend
From: Junio C Hamano @ 2011-12-06 20:03 UTC (permalink / raw)
  To: Vijay Lakshminarayanan; +Cc: Viresh Kumar, git@vger.kernel.org, Shiraz HASHIM
In-Reply-To: <87fwgxwvn9.fsf@gmail.com>

Vijay Lakshminarayanan <laksvij@gmail.com> writes:

> I've found 
>
> $ GIT_EDITOR=cat git commit --amend
>
> useful.

Are you sure it is a cat?

I almost always use

    $ EDITOR=: git commit --amend

when rewriting the contents without updating the message, but I think
we should allow people to say:

    $ git commit --amend --no-edit

which is accepted from the command line but is not honoured.

^ permalink raw reply

* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Thomas Rast @ 2011-12-06 20:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111206190217.GD9492@sigill.intra.peff.net>

Jeff King wrote:
> On Tue, Dec 06, 2011 at 05:35:08PM +0100, Thomas Rast wrote:
> 
> > The cpp pattern, used for C and C++, would not match the start of a
> > declaration such as
> > 
> >   static char *prepare_index(int argc,
> > 
> > because it did not allow for * anywhere between the various words that
> > constitute the modifiers, type and function name.  Fix it.
> > 
> > Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> > ---
> > 
> > This is a really sneaky one-character bug that I cannot believe went
> > unnoticed for so long, seeing as there are plenty of instances within
> > git itself where it matters.
> 
> Looks reasonable to me. You can see the difference, for instance, with:
> 
>   git show -U1 3c73a1d
> 
> (The -U1 is because of the annoying "we will start looking for the
> header at the top of context, not the top of changes" behavior I
> mentioned last week).

Actually (sadly) I'll have to revise it.  It doesn't match much of C++
either, and I haven't yet come up with a reasonable regex that
matches, say,

  foo::Bar<int>::t& Baz::operator<<(

which I would call ludicrous, but it's valid C++.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] userdiff: allow * between cpp funcname words
From: Jeff King @ 2011-12-06 20:19 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, git
In-Reply-To: <201112062117.57690.trast@student.ethz.ch>

On Tue, Dec 06, 2011 at 09:17:56PM +0100, Thomas Rast wrote:

> > Looks reasonable to me. You can see the difference, for instance, with:
> > 
> >   git show -U1 3c73a1d
> > 
> > (The -U1 is because of the annoying "we will start looking for the
> > header at the top of context, not the top of changes" behavior I
> > mentioned last week).
> 
> Actually (sadly) I'll have to revise it.  It doesn't match much of C++
> either, and I haven't yet come up with a reasonable regex that
> matches, say,
> 
>   foo::Bar<int>::t& Baz::operator<<(
> 
> which I would call ludicrous, but it's valid C++.

Ick, yeah. Maybe it is worth doing the "*" thing for now, and then
worrying about advanced C++ stuff on top as another patch. AFAICT, your
original patch is a strict improvement.

-Peff

^ 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