Git development
 help / color / mirror / Atom feed
* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 18:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604162006050.19560@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> There's another reason it does not show it. ATM, you have to add "-p" to 
> the command line, or "-c" will not show *any* patch, let alone a combined 
> one.

Oh, it will show the "raw" patch, which is often very useful. I've grown 
quite fond of it, because it shows what happened on a bigger level, 
without showing the details within a file ("--name-status" makes it more 
readable, but I'm too lazy to type it, so I often just look at the raw 
git patch).

> Thanks for all your help, but in this case it was not irrelevant. Because 
> I *had* the function in my working copy. And I had changed it. So I had to 
> find out where to move the change.

Right, but it was irrelevant as far as "top-of-head" was concerned (which 
is all that "git log" shows you - it doesn't care about your working 
tree).

The fact that it _had_ been relevant in the state you used to be at is not 
something "git log" and friends know or care about.

Now, I'm not disputing that we might want to make it easier to see what 
_had_ been relevant at some earlier time. But you'd have to specify that 
earlier time somehow. I assume you had tried to do a "git rebase", and the 
problem with that is that git rebase really doesn't help you at all when 
things go wrong, exactly because "rebase" - by design - screws up the 
history and loses that information for you.

If your problem state had been as a result of a "git merge", you'd 
actually have had much better tools available to you, exactly because 
merge doesn't screw up history, so you've got both sides of the merge you 
can look at (HEAD and MERGE_HEAD, and "git diff --ours" and "--theirs").

That said, even "rebase" will help somewhat. You've got "ORIG_HEAD" to 
use, and that should help at least a _bit_. In particular, you can do

	gitk ORIG_HEAD.. 

to see all the changes you rebased across. But right now you'd have to 
fall back on the "-m -p" thing if you wanted to see it all..

		Linus

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 18:09 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161052310.3701@g5.osdl.org>

Hi,

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> > 
> > I finally found the commit which removed parse_whatchanged_opt() from 
> > log-tree.c by
> > 
> > 	git-rev-list 4a617..next | while read commit; do \
> > 		echo $commit; \
> > 		git diff $commit^..$commit log-tree.c | \
> > 			grep parse_whatchanged; \
> > 	done | less
> 
> Heh. You really should learn about "-m -p", which does the above, but 
> better (it compares against _all_ parents - you would have missed the 
> thing _again_ if the "lt/logopt" branch had been the main branch ;)

Somehow I forgot. Probably because I don't like the format either.

> [...] even
> 
> 	git show -c 43f934aa90
> 
> won't show that "log-tree.c" file AT ALL, because there was no content 
> conflict: the whole file was taken from one branch, unmodified. 

There's another reason it does not show it. ATM, you have to add "-p" to 
the command line, or "-c" will not show *any* patch, let alone a combined 
one.

> So "--cc" really does ignore everything that is irrelevant for the end 
> result, and in this case you are very much trying to find somethign that 
> is totally irrelevant for the end result, since the function you look for 
> had never even _existed_ in the file as far as the end result goes..

Thanks for all your help, but in this case it was not irrelevant. Because 
I *had* the function in my working copy. And I had changed it. So I had to 
find out where to move the change.

Again, thanks a bunch,
Dscho

^ permalink raw reply

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 17:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161931530.19020@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> I finally found the commit which removed parse_whatchanged_opt() from 
> log-tree.c by
> 
> 	git-rev-list 4a617..next | while read commit; do \
> 		echo $commit; \
> 		git diff $commit^..$commit log-tree.c | \
> 			grep parse_whatchanged; \
> 	done | less

Heh. You really should learn about "-m -p", which does the above, but 
better (it compares against _all_ parents - you would have missed the 
thing _again_ if the "lt/logopt" branch had been the main branch ;)

> However, the combined diff of that commit does not show it, while the diff 
> to the first parent does:
> 
> 	git-show --cc 43f934aa90 | grep parse_whatchanged

Combined merges really only show conflicts where the different parents do 
something different from the end result. Since the whole file was taken 
from the lt/logopt branch, even

	git show -c 43f934aa90

won't show that "log-tree.c" file AT ALL, because there was no content 
conflict: the whole file was taken from one branch, unmodified. 

If you want to see all changes against all parents, you really do need 
"-m -p" (or just "-m", which will show the raw diffs, and which will 
show how the file changes from one parent, but not the other).

Note that NORMALLY, you'd really never want to use "-m -p". It's a very 
very inconvenient format, since normally you want to see only the stuff 
that changed wrt the end result.

So "--cc" really does ignore everything that is irrelevant for the end 
result, and in this case you are very much trying to find somethign that 
is totally irrelevant for the end result, since the function you look for 
had never even _existed_ in the file as far as the end result goes..

		Linus

^ permalink raw reply

* [PATCH] Make cg-commit work with accented letters.
From: Yann Dirson @ 2006-04-16 17:57 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git


git-diff-index in default mode has an annoying behaviour wrt filenames
containing non-ascii chars.  As suggested by Pasky, we can use -z
mode, which gives us a much better way of handling all other special
chars.  With associated testcases ensuring it works with simple and
double quotes, backslashes, and spaces as well.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 cg-commit               |   10 ++++----
 t/t9900-specialchars.sh |   59 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/cg-commit b/cg-commit
index 8dac57c..9ec8289 100755
--- a/cg-commit
+++ b/cg-commit
@@ -274,8 +274,8 @@ if [ "$ARGS" -o "$_git_relpath" ]; then
 		echo "${_git_relpath}$file" >>"$filter"
 	done
 
-	eval "commitfiles=($(cat "$filter" | path_xargs git-diff-index -r -m HEAD -- | \
-		sed -e 's/"\|\\/\\&/g' -e 's/^\([^	]*\)\(.\)	\(.*\)\(	.*\)*$/"\2 \3"/'))"
+	eval "commitfiles=($(cat "$filter" | path_xargs git-diff-index -z -r -m HEAD -- | \
+		perl -n0e 'if (defined $meta) { s/([\"\\])/\\\1/; print "\"$meta $_\"\n"; $meta=undef } else { $meta = (split(/\s/))[4] }'))"
 	customfiles=1
 
 	[ "$review" ] && cat "$filter" | path_xargs git-diff-index -r -m -p HEAD -- > "$PATCH"
@@ -292,8 +292,8 @@ else
 	if [ ! "$ignorecache" ]; then
 		# \t instead of the tab character itself works only with new
 		# sed versions.
-		eval "commitfiles=($(git-diff-index -r -m HEAD | \
-			sed -e 's/"\|\\/\\&/g' -e 's/^\([^	]*\)\(.\)	\(.*\)\(	.*\)*$/"\2 \3"/'))"
+		eval "commitfiles=($(git-diff-index -z -r -m HEAD | \
+			perl -n0e 'if (defined $meta) { s/([\"\\])/\\\1/; print "\"$meta $_\"\n"; $meta=undef } else { $meta = (split(/\s/))[4] }'))"
 
 		if [ -s "$_git/commit-ignore" ]; then
 			newcommitfiles=()
@@ -439,7 +439,7 @@ __END__
 		exit 1
 	fi
 	if [ ! "$ignorecache" ] && [ ! "$merging" ] && [ ! "$review" ]; then
-		eval "newcommitfiles=($(grep ^CG:F "$LOGMSG2" | sed 's/^CG:F *\(.*\)$/"\1"/'))"
+		eval "newcommitfiles=($(grep ^CG:F "$LOGMSG2" | sed -e 's/\"/\\&/g' -e 's/^CG:F *\(.*\)$/"\1"/'))"
 		if [ ! "$force" ] && [ ! "${newcommitfiles[*]}" ]; then
 			rm "$LOGMSG" "$LOGMSG2"
 			[ "$quiet" ] && exit 0 || die 'Nothing to commit'
diff --git a/t/t9900-specialchars.sh b/t/t9900-specialchars.sh
new file mode 100755
index 0000000..a705052
--- /dev/null
+++ b/t/t9900-specialchars.sh
@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2006 Yann Dirson
+#
+test_description="Tests various commands with shell-special chars.
+
+Filenames with embedded spaces, quotes, non-ascii letter, you name it."
+
+. ./test-lib.sh
+
+rm -rf .git
+cg-init -m .
+
+touch "a space"
+test_expect_success 'add file with space' 'cg-add "a space"'
+test_expect_success 'commit file with space' 'cg-commit -m . "a space"'
+
+touch "a'quote"
+test_expect_success 'add file with quote' "cg-add \"a'quote\""
+test_expect_success 'commit file with quote' "cg-commit -m . \"a'quote\""
+
+touch "d\"quote"
+test_expect_success 'add file with accent' 'cg-add "d\"quote"'
+test_expect_success 'commit file with quote' 'cg-commit -m . "d\"quote"'
+
+touch "back\\slash"
+test_expect_success 'add file with accent' 'cg-add "back\\slash"'
+test_expect_success 'commit file with quote' 'cg-commit -m . "back\\slash"'
+
+touch "accént"
+test_expect_success 'add file with accent' "cg-add accént"
+test_expect_success 'commit file with quote' "cg-commit -m . accént"
+
+## same without a file arg to cg-commit
+
+rm -rf * .git
+cg-init -m .
+
+touch "a space"
+test_expect_success 'add file with space' 'cg-add "a space"'
+test_expect_success 'commit file with space' 'cg-commit -m .'
+
+touch "a'quote"
+test_expect_success 'add file with quote' "cg-add \"a'quote\""
+test_expect_success 'commit file with quote' "cg-commit -m ."
+
+touch "d\"quote"
+test_expect_success 'add file with accent' 'cg-add "d\"quote"'
+test_expect_success 'commit file with quote' 'cg-commit -m .'
+
+touch "back\\slash"
+test_expect_success 'add file with accent' 'cg-add "back\\slash"'
+test_expect_success 'commit file with quote' 'cg-commit -m .'
+
+touch "accént"
+test_expect_success 'add file with accent' "cg-add accént"
+test_expect_success 'commit file with quote' "cg-commit -m ."
+
+test_done

^ permalink raw reply related

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 17:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161905010.18184@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> just to make it clearer what I want:
> 
> 	git-whatchanged -p next | grep parse_whatchanged
> 
> as well as
> 
> 	git log -p next | grep parse_whatchanged
> 
> do not find that any line like
> 
> 	int parse_whatchanged_opt(int ac, [...]
> 
> was removed, but they find that this line was added. However, in the 
> working tree (which has a fresh checkout of next), there is no such line 
> in log-tree.c. So I really would like to know where it vanished!

It was removed by merge 43f934aa: "Merge branch 'lt/logopt' into next".

The "parse_whatchanged_opt()" function never existed in that lt/logopt 
branch, and merging it (manually, I assume) it doesn't exist in the result 
either.

General hint: if you don't find it in "--cc", then that means that it's 
almost certainly a merge. "--cc" will only find things that _clash_, ie 
the end result is different from either of the branches (and in this case, 
it wasn't different, since parse_whatchanged_opt() simply didn't exist in 
the branch that was merged).

Now, finding things in merges can be a bit painful, but the sure-fire safe 
way is the old one: use "-m -p" to show _all_ sides of a merge as a diff. 
That's a really inconvenient format for reading, but it literally shows 
all changes to all parents.

Again, "git log log-tree.c" actually does do the right thing: the current 
state of "log-tree.c" really has _all_ of its history coming from the 
lt/logopt branch, which is why when you do

	git log -m -p log-tree.c | grep int parse_whatchanged_opt

you won't get any result at all: the _current_ state of log-tree.c really 
has no history what-so-ever that involved parse_whatchanged_opt. That may 
sound strange, but it really is very true. Doing a "gitk log-tree.c" shows 
what the real history of the contents of that file is.

And this actually comes back to a very fundamental git issue: git tracks 
_contents_. It doesn't care one whit if there was another branch that had 
some other history for that file: if that other branch didn't affect the 
contents of the file, then that other branch simply doesn't exist as far 
as that particular file history is concerned. It only exists as a "bigger" 
issue.

But that "-m -p" thing can be useful when you do want to see the bigger 
issue. As might "--no-prune-merges".

		Linus

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 17:39 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161000550.3701@g5.osdl.org>

Hi,

me again.

I finally found the commit which removed parse_whatchanged_opt() from 
log-tree.c by

	git-rev-list 4a617..next | while read commit; do \
		echo $commit; \
		git diff $commit^..$commit log-tree.c | \
			grep parse_whatchanged; \
	done | less

where obviously 4a617 is the commit I last merged with. The winner is: 

	43f934aa90... Merge branch 'lt/logopt' into next

However, the combined diff of that commit does not show it, while the diff 
to the first parent does:

	git-show --cc 43f934aa90 | grep parse_whatchanged

shows nothing (-c -p shows half of the truth), while

	git-diff 43f934aa90^..43f934aa90 | grep parse_whatchanged

shows something, and 

	git-diff 43f934aa90^2..43f934aa90 | grep parse_whatchanged

again does not.

Time to understand the combined diff logic, I guess...

Ciao,
Dscho

^ permalink raw reply

* Re: path limiting broken (NOT)
From: Johannes Schindelin @ 2006-04-16 17:27 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604161000550.3701@g5.osdl.org>

Hi,

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> > 
> > But you missed my point. The file log-tree.c had some opt handling. I 
> > changed it (as can be seen in my patch for combined diffstat), so the 
> > merge did not go well with next, which removed that code.
> > 
> > So I tried to be clever and find the (non-merge) commit where this 
> > happened. With what you describe, I should have hit *exactly one* commit 
> > removing the code.
> > 
> > But I hit *exactly none* with "git log --cc master log-tree.c".
> 
> Do you have the tree somewhere?

I have no quick way to publish it, if you mean that.

> In the current git tree, there really _is_ just one commit that touches 
> log-tree.c in "master". And "git log log-tree.c" picks that out without 
> trouble.

It is my master. Not Junio's.

The problem seems to be that the current next does not have the code. But 
when I last merged, it had it. So basically, I have a problem here that I 
thought "git log --cc" can solve, but it doesn't.

"git log -cc" is good to find out when code *that is present now* has 
crept in, but it is no good to find out when code that was in commit 
such-and-such, and is no longer in commit such-and-that, has been culled.

Pity.

Thanks for your help,
Dscho

^ permalink raw reply

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 17:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161835410.17985@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> But you missed my point. The file log-tree.c had some opt handling. I 
> changed it (as can be seen in my patch for combined diffstat), so the 
> merge did not go well with next, which removed that code.
> 
> So I tried to be clever and find the (non-merge) commit where this 
> happened. With what you describe, I should have hit *exactly one* commit 
> removing the code.
> 
> But I hit *exactly none* with "git log --cc master log-tree.c".

Do you have the tree somewhere?

In the current git tree, there really _is_ just one commit that touches 
log-tree.c in "master". And "git log log-tree.c" picks that out without 
trouble.

Did you mean to do

	git log --cc next log-tree.c

instead, perhaps? (Note the "next" branch specifier) That definitely shows 
three commits, including very much the one that moves the common option 
parsing to revision.c (do "git log --cc --full-diff next log-tree.c" if 
you want to see the whole diff whenever something touches log-tree.c).

Maybe I'm still missing something, but it seems to do the right thing 
for me..

		Linus

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 17:05 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604160850230.3701@g5.osdl.org>

Hi,

just to make it clearer what I want:

	git-whatchanged -p next | grep parse_whatchanged

as well as

	git log -p next | grep parse_whatchanged

do not find that any line like

	int parse_whatchanged_opt(int ac, [...]

was removed, but they find that this line was added. However, in the 
working tree (which has a fresh checkout of next), there is no such line 
in log-tree.c. So I really would like to know where it vanished!

Ciao,
Dscho

^ permalink raw reply

* Re: path limiting broken
From: Johannes Schindelin @ 2006-04-16 16:45 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0604160850230.3701@g5.osdl.org>

Hi,

On Sun, 16 Apr 2006, Linus Torvalds wrote:

> On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> > 
> > I am not intelligent enough to find out why there are three revisions 
> > which get culled.
> 
> Path limiting by default looks at all parents of a merge, and if any of 
> them is identical to the child (within the path limiter), it will pick 
> _that_ parent, and that parent only, and drop all other parents.

That's okay.

But you missed my point. The file log-tree.c had some opt handling. I 
changed it (as can be seen in my patch for combined diffstat), so the 
merge did not go well with next, which removed that code.

So I tried to be clever and find the (non-merge) commit where this 
happened. With what you describe, I should have hit *exactly one* commit 
removing the code.

But I hit *exactly none* with "git log --cc master log-tree.c".

Ciao,
Dscho

^ permalink raw reply

* Re: path limiting broken
From: Linus Torvalds @ 2006-04-16 16:06 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0604161411120.15345@wbgn013.biozentrum.uni-wuerzburg.de>



On Sun, 16 Apr 2006, Johannes Schindelin wrote:
> 
> I am not intelligent enough to find out why there are three revisions 
> which get culled.
> 
> Ideas?

Path limiting by default looks at all parents of a merge, and if any of 
them is identical to the child (within the path limiter), it will pick 
_that_ parent, and that parent only, and drop all other parents.

(If there are multiple identical parents, it will pick the first one).

For example, if you do

		 HEAD
		   |
		   a
		  / \
		 b   c
		 |  /|
		 | / d
		 |/  |
		 e   f
		 |  /
		 g /
		 |/
		 h

and the file was modified in commit "b" and nowhere else, then think about 
what the final history should look like.. Since it was modified in "b", 
it's going to be the same in "a" and "b", so the path limiting will 
totally drop the whole "c" subtree, and will only ever look at the tree 
a->b->e->g->h.

After that, it will simplify the history by dropping commits that don't 
change the path, and the whole history will end up being just "b".

Which is exactly what you want.

Now, realize that if the exact _same_ change is done in "f", the same 
thing will happen. "f" will never be shown, because "f" simply isn't 
interesting. We picked up the same change in "b", and while the merge "a" 
had _both_ parents identical in the file, it would pick only the first 
one. So we'll never even _look_ at "f".

Now, this is _important_. Pruning merges is really fundamental to giving a 
reasonable file history, and not just a performance optimization. If the 
same change came from two branches, we really don't care who did it: we'll 
hit _one_ of the changes even after pruning. But yes, it could miss the 
other one.

Here's a patch to show what an unpruned history ends up looking like.

With this patch applied, try the following:

	gitk -- ls-tree.c &
	gitk --no-prune-merges -- ls-tree.c &

and compare the two. You'll see _immediately_ why I think merge pruning is 
not just a good idea, it's something we _have_ to do.

		Linus
----
diff --git a/revision.c b/revision.c
index 0505f3f..25338b6 100644
--- a/revision.c
+++ b/revision.c
@@ -291,7 +291,7 @@ static void try_to_simplify_commit(struc
 		parse_commit(p);
 		switch (rev_compare_tree(revs, p->tree, commit->tree)) {
 		case REV_TREE_SAME:
-			if (p->object.flags & UNINTERESTING) {
+			if (revs->no_merge_pruning || (p->object.flags & UNINTERESTING)) {
 				/* Even if a merge with an uninteresting
 				 * side branch brought the entire change
 				 * we are interested in, we do not want
@@ -640,6 +640,10 @@ int setup_revisions(int argc, const char
 				revs->unpacked = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--no-prune-merges")) {
+				revs->no_merge_pruning = 1;
+				continue;
+			}
 			*unrecognized++ = arg;
 			left++;
 			continue;
diff --git a/revision.h b/revision.h
index 8970b57..a116305 100644
--- a/revision.h
+++ b/revision.h
@@ -26,6 +26,7 @@ struct rev_info {
 	/* Traversal flags */
 	unsigned int	dense:1,
 			no_merges:1,
+			no_merge_pruning:1,
 			remove_empty_trees:1,
 			lifo:1,
 			topo_order:1,

^ permalink raw reply related

* path limiting broken
From: Johannes Schindelin @ 2006-04-16 12:26 UTC (permalink / raw)
  To: git

Hi,

I just tried to find out when certain changes percolated into log-tree.c. 
So I issued "git log --cc next log-tree.c". Wonders of wonders, the 
patches did not contain *anything* about what I tried to find, even if the 
file contains the key words. Because the path limiting is overeager (I 
finally found what I looked for in commit f4235f8b):

	git-name-rev $(git-rev-list --pretty=oneline \
	f4235f8b2ef875b85ead74ffa199d827f9ee9d8d..next log-tree.c | \
	sed "s/ .*$//")

yields

	cb8f64b4e3f263c113b7a2f156af74b810e969ff next^2
	cd2bdc5309461034e5cc58e1d3e87535ed9e093b next~10^2~2

while without path limiting,

	git-name-rev $(git-whatchanged.sh --pretty=oneline \
	f4235f8b2ef875b85ead74ffa199d827f9ee9d8d^..next log-tree.c | \
	sed -n "s/^diff-tree \([^ ]*\).*$/\1/p")

I get

	cb8f64b4e3f263c113b7a2f156af74b810e969ff next^2
	c5ccd8be43df4b916752a176512a9adaf3b94df9 next~4^2
	f4235f8b2ef875b85ead74ffa199d827f9ee9d8d next~6^2
	183df63940bf92ea626af64d0057165b8aad24f6 next~8^2
	cd2bdc5309461034e5cc58e1d3e87535ed9e093b next~10^2~2

I am not intelligent enough to find out why there are three revisions 
which get culled.

Ideas?

Ciao,
Dscho

^ permalink raw reply

* [BUG] comments in grafts file broken in current master
From: Yann Dirson @ 2006-04-16 12:35 UTC (permalink / raw)
  To: GIT list

While looking at allowing empty lines in grafts files, I discovered
that comments had already been implemented.  However, current
git-read-tree segfaults when there is a comment line in info/grafts:

dwitch@gandelf:/export/work/yann/git/git$ cat .git/info/grafts 
c118c026e44f02c3dbad00d924285eef2340f700
# foo
dwitch@gandelf:/export/work/yann/git/git$ git-read-tree master
Segmentation fault


The commit introducing the problem is
5040f17eba15504bad66b14a645bddd9b015ebb7 (blame -S <ancestry-file>),
which changes quite some things in the grafts area.
    

BTW, after segfaulting, .git/index.lock is still there, and the
"unable to create new cachefile" is not so helpful - I had to strace
to see what was happenning.

Hope this helps,
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* [BUG] gitk: breaks with both version and file limits
From: Yann Dirson @ 2006-04-16 11:54 UTC (permalink / raw)
  To: Paul Mackerras, GIT list

As an example, on a current git tree, the following command triggers
an 'Error: expected boolean value but got ""' dialog when scrolling to
the bottom of the graph, and leaves the bottom of the graph not drawn.
This happens with current master, but not with 1.2.4.

gitk --all ^v1.0.6 -- Makefile 

While checking with git-bisect while limiting to path gitk, I noticed
the amusing detail that "git-bisect visualize" is impacted by this bug.

git-bisect shows that the commit triggering the problem is
79b2c75e043ad85f9a6b1a8d890b601a2f761a0e ("gitk: replace parent and
children arrays with lists" on Apr 2)

Hope this helps,
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* [PATCH 8/9] Exercise "stg pull" on patches just appending lines.
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


It indeed reveals a problem in "push": appended lines are appended
again, as the already-applied patch is not detected.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 t/t1201-pull-trailing.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/t/t1201-pull-trailing.sh b/t/t1201-pull-trailing.sh
new file mode 100755
index 0000000..142f894
--- /dev/null
+++ b/t/t1201-pull-trailing.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='test
+
+'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Setup and clone tree, and setup changes' \
+    "(cd foo &&
+      printf 'a\nb\n' > file && git add file && git commit -m .
+     ) &&
+     stg clone foo bar &&
+     (cd bar && stg new p1 -m p1
+      printf 'c\n' >> file && stg refresh
+     )
+"
+
+test_expect_success \
+    'Port those patches to orig tree' \
+    "(cd foo &&
+      GIT_DIR=../bar/.git git-format-patch --stdout bases/master..HEAD |
+      git-am -3 -k
+     )
+"
+
+test_expect_success \
+    'Pull those patches applied upstream' \
+    "(cd bar && stg pull
+     )
+"
+
+test_expect_success \
+    'Check that all went well' \
+    "diff -u foo/file bar/file
+"
+
+test_done

^ permalink raw reply related

* [PATCH 9/9] Look for templates in ~/.stgit/templates as well
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


This can be quite useful to avoid adding one's sig again and again to
ever covermail, and to use a patchmail template that insert
Signed-off-by lines, for those of us who prefer this to adding them to
commits.

Also make sure to use os.path.join() instead of hardcoded slashes.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/export.py |    9 ++++++---
 stgit/commands/mail.py   |   18 ++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/stgit/commands/export.py b/stgit/commands/export.py
index e7de902..fbf5690 100644
--- a/stgit/commands/export.py
+++ b/stgit/commands/export.py
@@ -31,8 +31,9 @@ usage = """%prog [options] [<dir>]
 
 Export the applied patches into a given directory (defaults to
 'patches') in a standard unified GNU diff format. A template file
-(defaulting to '.git/patchexport.tmpl or
-/usr/share/stgit/templates/patchexport.tmpl') can be used for the
+(defaulting to '.git/patchexport.tmpl' or
+'~/.stgit/templates/patchexport.tmpl' or
+'/usr/share/stgit/templates/patchexport.tmpl') can be used for the
 patch format. The following variables are supported in the template
 file:
 
@@ -145,8 +146,10 @@ def func(parser, options, args):
         patch_tmpl_list = []
 
     patch_tmpl_list += [os.path.join(basedir.get(), 'patchexport.tmpl'),
+                        os.path.join(os.path.expanduser('~'), '.stgit', 'templates',
+                                     'patchexport.tmpl'),
                         os.path.join(sys.prefix,
-                                     'share/stgit/templates/patchexport.tmpl')]
+                                     'share', 'stgit', 'templates', 'patchexport.tmpl')]
     tmpl = ''
     for patch_tmpl in patch_tmpl_list:
         if os.path.isfile(patch_tmpl):
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 470cf65..5e01ea1 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -30,16 +30,18 @@ usage = """%prog [options] [<patch> [<pa
 Send a patch or a range of patches (defaulting to the applied patches)
 by e-mail using the 'smtpserver' configuration option. The From
 address and the e-mail format are generated from the template file
-passed as argument to '--template' (defaulting to .git/patchmail.tmpl
-or /usr/share/stgit/templates/patchmail.tmpl). The To/Cc/Bcc addresses
+passed as argument to '--template' (defaulting to
+'.git/patchmail.tmpl' or '~/.stgit/templates/patchmail.tmpl' or or
+'/usr/share/stgit/templates/patchmail.tmpl'). The To/Cc/Bcc addresses
 can either be added to the template file or passed via the
 corresponding command line options.
 
 A preamble e-mail can be sent using the '--cover' and/or '--edit'
 options. The first allows the user to specify a file to be used as a
 template. The latter option will invoke the editor on the specified
-file (defaulting to .git/covermail.tmpl or
-/usr/share/stgit/templates/covermail.tmpl).
+file (defaulting to '.git/covermail.tmpl' or
+'~/.stgit/templates/covermail.tmpl' or
+'/usr/share/stgit/templates/covermail.tmpl').
 
 All the subsequent e-mails appear as replies to the first e-mail sent
 (either the preamble or the first patch). E-mails can be seen as
@@ -444,8 +446,10 @@ def func(parser, options, args):
             tfile_list = [options.cover]
         else:
             tfile_list = [os.path.join(basedir.get(), 'covermail.tmpl'),
+                          os.path.join(os.path.expanduser('~'), '.stgit', 'templates',
+                                       'covermail.tmpl'),
                           os.path.join(sys.prefix,
-                                       'share/stgit/templates/covermail.tmpl')]
+                                       'share', 'stgit', 'templates', 'covermail.tmpl')]
 
         tmpl = None
         for tfile in tfile_list:
@@ -476,8 +480,10 @@ def func(parser, options, args):
         tfile_list = [options.template]
     else:
         tfile_list = [os.path.join(basedir.get(), 'patchmail.tmpl'),
+                      os.path.join(os.path.expanduser('~'), '.stgit', 'templates',
+                                   'patchmail.tmpl'),
                       os.path.join(sys.prefix,
-                                   'share/stgit/templates/patchmail.tmpl')]
+                                   'share', 'stgit', 'templates', 'patchmail.tmpl')]
     tmpl = None
     for tfile in tfile_list:
         if os.path.isfile(tfile):

^ permalink raw reply related

* [PATCH 4/9] Make branch creation atomic
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


This patch adds an optional create_at parameter to Series.init(), to
pass a git.branch_create() parameter if we want it to be called.  This
parameter can be None, so the test for False has to be explicit.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/branch.py |    6 +-----
 stgit/stack.py           |    8 +++++---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index be501a8..c7561a8 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -125,12 +125,8 @@ def func(parser, options, args):
         tree_id = None
         if len(args) == 2:
             tree_id = git_id(args[1])
-
-        if git.branch_exists(args[0]):
-            raise CmdException, 'Branch "%s" already exists' % args[0]
         
-        stack.Series(args[0]).init()
-        git.create_branch(args[0], tree_id)
+        stack.Series(args[0]).init(create_at = tree_id)
 
         print 'Branch "%s" created.' % args[0]
         return
diff --git a/stgit/stack.py b/stgit/stack.py
index c14e029..19bb62a 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -426,7 +426,7 @@ class Series:
         """
         return os.path.isdir(self.__patch_dir)
 
-    def init(self):
+    def init(self, create_at=False):
         """Initialises the stgit series
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
@@ -438,6 +438,9 @@ class Series:
         if os.path.exists(self.__base_file):
             raise StackException, self.__base_file + ' already exists'
 
+        if (create_at!=False):
+            git.create_branch(self.__name, create_at)
+
         os.makedirs(self.__patch_dir)
 
         if not os.path.isdir(bases_dir):
@@ -509,8 +512,7 @@ class Series:
         """Clones a series
         """
         base = read_string(self.get_base_file())
-        git.create_branch(target_series, tree_id = base)
-        Series(target_series).init()
+        Series(target_series).init(create_at = base)
         new_series = Series(target_series)
 
         # generate an artificial description file

^ permalink raw reply related

* [PATCH 5/9] Correctly handle refs/patches on series rename
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


When renaming a series, the refs/patches dir was not moved, and by
chance a new one was created by the repository-upgrade code, but that
left the old one behind as cruft (which the safety checks added in a
former patch now detects).

Also adds a regression test to assert that nothing by the old name
is left behind.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/stack.py           |    2 ++
 t/t1001-branch-rename.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/stgit/stack.py b/stgit/stack.py
index 19bb62a..975ac21 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -505,6 +505,8 @@ class Series:
             os.rename(self.__series_dir, to_stack.__series_dir)
         if os.path.exists(self.__base_file):
             os.rename(self.__base_file, to_stack.__base_file)
+        if os.path.exists(self.__refs_dir):
+            os.rename(self.__refs_dir, to_stack.__refs_dir)
 
         self.__init__(to_name)
 
diff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh
new file mode 100755
index 0000000..28da15c
--- /dev/null
+++ b/t/t1001-branch-rename.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch renames.
+
+Exercises branch renaming commands.
+'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'Create an stgit branch from scratch' \
+    'stg init &&
+     stg branch -c foo &&
+     stg new p1 -m "p1"
+'
+
+test_expect_failure \
+    'Rename the current stgit branch' \
+    'stg branch -r foo bar
+'
+
+test_expect_success \
+    'Rename an stgit branch' \
+    'stg branch -c buz &&
+     stg branch -r foo bar &&
+     test -z `find .git -name foo | tee /dev/stderr`
+'
+
+test_done

^ permalink raw reply related

* [PATCH 7/9] Test that pulls a patch creating a file that got modified afterwards
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


This demonstrates an issue wite has bitten me more than once: the stg
branch adds a file in one patch, and modifies it in a later patch; then all
patches get integrated in upstream tree, and at "stg pull" time, stgit
believes there is a conflict, even one the patches are exactly the same.

"stg push" apparently does not consider patches one by one, or it
would have noticed that the patches were "already applied".  It reports:

 Pushing patch "p1"...Error: File "file2" added in branches but different
 The merge failed during "push". Use "refresh" after fixing the conflicts
 stg push: GIT index merging failed (possible conflicts)

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 t/t1200-push-modified.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
new file mode 100755
index 0000000..18a4df2
--- /dev/null
+++ b/t/t1200-push-modified.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Exercise pushing patches applied upstream.
+
+Especially, consider the case of a patch that adds a file,
+while a subsequent one modifies it.
+'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Clone tree and setup changes' \
+    "stg clone foo bar &&
+     (cd bar && stg new p1 -m p1
+      printf 'a\nc\n' > file && stg add file && stg refresh &&
+      stg new p2 -m p2
+      printf 'a\nb\nc\n' > file && stg refresh
+     )
+"
+
+test_expect_success \
+    'Port those patches to orig tree' \
+    "(cd foo &&
+      GIT_DIR=../bar/.git git-format-patch --stdout bases/master..HEAD |
+      git-am -3 -k
+     )
+"
+
+test_expect_success \
+    'Pull to sync with parent, preparing for the problem' \
+    "(cd bar && stg pop --all &&
+      stg pull
+     )
+"
+
+test_expect_success \
+    'Now attempt to push those patches applied upstream' \
+    "(cd bar && stg push --all
+     )
+"
+
+test_done

^ permalink raw reply related

* [PATCH 3/9] Add a couple of safety checks to series creation
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


- we must check first whether the operation can complete, instead of
  bombing out halfway.  That means checking that nothing will prevent
  the creation of stgit data (note that calling is_initialised() is
  not enough, as it does not catch a bogus file), which is tested by
  the 3 first couple of testcases, and fixed in stack.py

- creating the git branch unconditionally before knowing whether
  creation of the stgit stuff can be completed is a problem as well:
  being atomic would be much much better.  To emulate atomicity (which
  comeds in the next patch), this patch does a somewhat dirty hack to
  branch.py: we first attempt to create the stgit stuff, and if that
  succeeds, we create the git branch: it is much easier to do at first
  a quick check that the latter will succeed.  Testcase 7/8 ensure
  that such a safety check has not been forgotten.

- when git already reports a problem with that head we would like to
  create, we should catch it.  Testcase 9/10 creates such a situation,
  and the fix to git.py allows to catch the error spit out by
  git-rev-parse.  I cannot tell why the stderr lines were not included
  by the Popen3 object.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/branch.py |    5 ++-
 stgit/git.py             |    4 ++
 stgit/stack.py           |    7 +++-
 t/t1000-branch-create.sh |   82 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index c4b5945..be501a8 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -126,8 +126,11 @@ def func(parser, options, args):
         if len(args) == 2:
             tree_id = git_id(args[1])
 
-        git.create_branch(args[0], tree_id)
+        if git.branch_exists(args[0]):
+            raise CmdException, 'Branch "%s" already exists' % args[0]
+        
         stack.Series(args[0]).init()
+        git.create_branch(args[0], tree_id)
 
         print 'Branch "%s" created.' % args[0]
         return
diff --git a/stgit/git.py b/stgit/git.py
index d75b54e..8523455 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -272,9 +272,11 @@ def rev_parse(git_id):
 def branch_exists(branch):
     """Existence check for the named branch
     """
-    for line in _output_lines(['git-rev-parse', '--symbolic', '--all']):
+    for line in _output_lines('git-rev-parse --symbolic --all 2>&1'):
         if line.strip() == branch:
             return True
+        if re.compile('[ |/]'+branch+' ').search(line):
+            raise GitException, 'Bogus branch: %s' % line
     return False
 
 def create_branch(new_branch, tree_id = None):
diff --git a/stgit/stack.py b/stgit/stack.py
index f4d7490..c14e029 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -431,8 +431,13 @@ class Series:
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
 
-        if self.is_initialised():
+        if os.path.exists(self.__patch_dir):
             raise StackException, self.__patch_dir + ' already exists'
+        if os.path.exists(self.__refs_dir):
+            raise StackException, self.__refs_dir + ' already exists'
+        if os.path.exists(self.__base_file):
+            raise StackException, self.__base_file + ' already exists'
+
         os.makedirs(self.__patch_dir)
 
         if not os.path.isdir(bases_dir):
diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
new file mode 100755
index 0000000..f0c2367
--- /dev/null
+++ b/t/t1000-branch-create.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch operations.
+
+Exercises the "stg branch" commands.
+'
+
+. ./test-lib.sh
+
+stg init
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious refs/patches/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/patches/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/patches/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious patches/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/patches/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/patches/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_expect_failure \
+    'Try to create an stgit branch with a spurious refs/bases/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/bases/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/bases/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+
+test_expect_failure \
+    'Try to create an stgit branch with an existing git branch by that name' \
+    'find .git -name foo | xargs rm -rf &&
+     cp .git/refs/heads/master .git/refs/heads/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+
+test_expect_failure \
+    'Try to create an stgit branch with an invalid refs/heads/ entry' \
+    'find .git -name foo | xargs rm -rf &&
+     touch .git/refs/heads/foo &&
+     stg branch -c foo
+'
+
+test_expect_success \
+    'Check no part of the branch was created' \
+    'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo" &&
+     ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_done

^ permalink raw reply related

* [PATCH 6/9] Fix a seriously bad interaction between .git caching and repo cloning
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


Testcase 2 exhibits a problem with caching the location of .git while
cloning a repository.  Since basedir.get() is called before the clone is
built, a value may get stored in the cache if we are within a
git-controlled tree already.  Then when constructing the object for the
clone, a bogus .git is used, which can lead, when running tests in t/trash,
to corruption of the stgit .git repository.

Testcase 1 does not show any problem by chance, because since we have a
./.git prepared for use by the testsuite, value ".git" get cached, and it
happens that this value will be still valid after chdir'ing into the
newborn clone.

Clearing the cache at the appropriate place fixes the problem.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/basedir.py        |    6 ++++++
 stgit/commands/clone.py |    3 +++
 t/t1100-clone-under.sh  |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/stgit/basedir.py b/stgit/basedir.py
index c394572..81f2b40 100644
--- a/stgit/basedir.py
+++ b/stgit/basedir.py
@@ -42,3 +42,9 @@ def get():
             __base_dir = __output('git-rev-parse --git-dir 2> /dev/null')
 
     return __base_dir
+
+def clear_cache():
+    """Clear the cached location of .git
+    """
+    global __base_dir
+    __base_dir = None
diff --git a/stgit/commands/clone.py b/stgit/commands/clone.py
index 9ad76a6..455dd6e 100644
--- a/stgit/commands/clone.py
+++ b/stgit/commands/clone.py
@@ -51,6 +51,9 @@ def func(parser, options, args):
     os.chdir(local_dir)
     git.checkout(tree_id = 'HEAD')
 
+    # be sure to forget any cached value for .git, since we're going
+    # to work on a brand new repository
+    basedir.clear_cache()
     stack.Series().init()
 
     print 'done'
diff --git a/t/t1100-clone-under.sh b/t/t1100-clone-under.sh
new file mode 100755
index 0000000..c86ef61
--- /dev/null
+++ b/t/t1100-clone-under.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Check cloning in a repo subdir
+
+Check that "stg clone" works in a subdir of a git tree.
+This ensures (to some point) that a clone within a tree does
+not corrupt the enclosing repo.
+
+This test must be run before any tests making use of clone.
+'
+
+. ./test-lib.sh
+
+# Here we are in a repo, we have a ./.git
+# Do not get rid of it, or a bug may bite out stgit repo hard
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'stg clone right inside a git tree' \
+    "stg clone foo bar"
+
+# now work in a subdir
+mkdir sub
+mv foo sub
+cd sub
+
+test_expect_success \
+    'stg clone deeper under a git tree' \
+    "stg clone foo bar"
+
+test_done

^ permalink raw reply related

* [PATCH 2/9] Add list of bugs to TODO
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


Since there is no formal place to register bugs, other than the git
ml, I have added a couple of them, some of them already mentionned on
the ml but still around.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/TODO b/TODO
index d97ffd1..a13e511 100644
--- a/TODO
+++ b/TODO
@@ -7,6 +7,7 @@ The TODO list until 1.0:
 - man page
 - code execution allowed from templates
 - more regression tests
+- stg help should probably pipe through the $PAGER
 - release 1.0
 
 
@@ -17,3 +18,17 @@ The future, when time allows or if someo
   synchronising with other patches (diff format or in other
   repositories)
 - write bash-completion script for the StGIT commands
+- support for branches with / in names
+  (ml: "Handle branch names with slashes")
+- "pull" argument should default to a sane value, "origin" is wrong in
+  many cases
+
+Bugs:
+
+- the following commands break in subdirs:
+  - refresh (ml: "Running StGIT in subdirectories")
+- "stg show" on empty patch shows previous patch
+- "stg add" is accepted when no patch is applied, then any push says
+  one must refresh first, which is obviously wrong
+- "stg add" on files already added should print a notice, so that the
+  user can catch a possible typo

^ permalink raw reply related

* [PATCH 1/9] Add a testsuite framework copied from git-core
From: Yann Dirson @ 2006-04-16 10:52 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <20060416104144.9884.28167.stgit@gandelf.nowhere.earth>


See git's t/README for details on how to use this framework.

There is no integration yet in the toplevel Makefile, I'll let
python masters take care of this.  Use "make -C t" to run the
tests for now.

A patch-naming policy should be defined for stgit, since the
git one does not apply.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO             |    2 -
 t/Makefile       |   25 ++++++
 t/README         |  210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0000-dummy.sh |   17 ++++
 t/test-lib.sh    |  203 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 456 insertions(+), 1 deletions(-)

diff --git a/TODO b/TODO
index e5affe0..d97ffd1 100644
--- a/TODO
+++ b/TODO
@@ -6,7 +6,7 @@ The TODO list until 1.0:
 - debian package support
 - man page
 - code execution allowed from templates
-- regression tests
+- more regression tests
 - release 1.0
 
 
diff --git a/t/Makefile b/t/Makefile
new file mode 100644
index 0000000..d5d7b6f
--- /dev/null
+++ b/t/Makefile
@@ -0,0 +1,25 @@
+# Run tests
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+#GIT_TEST_OPTS=--verbose --debug
+SHELL_PATH ?= $(SHELL)
+TAR ?= $(TAR)
+
+# Shell quote;
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+
+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+
+all: $(T) clean
+
+$(T):
+	@echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+
+clean:
+	rm -fr trash
+
+.PHONY: $(T) clean
+.NOPARALLEL:
+
diff --git a/t/README b/t/README
new file mode 100644
index 0000000..d88bad2
--- /dev/null
+++ b/t/README
@@ -0,0 +1,210 @@
+Core GIT Tests
+==============
+
+This directory holds many test scripts for core GIT tools.  The
+first part of this short document describes how to run the tests
+and read their output.
+
+When fixing the tools or adding enhancements, you are strongly
+encouraged to add tests in this directory to cover what you are
+trying to fix or enhance.  The later part of this short document
+describes how your test scripts should be organized.
+
+The mechanism that powers this testsuite is directly imported from the
+Core GIT Tests, in directory t/ of the git repository.  Files are base
+on Core GIT version 1.3.0.rc4.g5069.
+
+
+Running Tests
+-------------
+
+The easiest way to run tests is to say "make -C t".  This runs all
+the tests.
+
+    *** t0000-basic.sh ***
+    *   ok 1: .git/objects should be empty after git-init-db in an empty repo.
+    *   ok 2: .git/objects should have 256 subdirectories.
+    *   ok 3: git-update-index without --add should fail adding.
+    ...
+    *   ok 23: no diff after checkout and git-update-index --refresh.
+    * passed all 23 test(s)
+    *** t0100-environment-names.sh ***
+    *   ok 1: using old names should issue warnings.
+    *   ok 2: using old names but having new names should not issue warnings.
+    ...
+
+Or you can run each test individually from command line, like
+this:
+
+    $ sh ./t3001-ls-files-killed.sh
+    *   ok 1: git-update-index --add to add various paths.
+    *   ok 2: git-ls-files -k to show killed files.
+    *   ok 3: validate git-ls-files -k output.
+    * passed all 3 test(s)
+
+You can pass --verbose (or -v), --debug (or -d), and --immediate
+(or -i) command line argument to the test.
+
+--verbose::
+	This makes the test more verbose.  Specifically, the
+	command being run and their output if any are also
+	output.
+
+--debug::
+	This may help the person who is developing a new test.
+	It causes the command defined with test_debug to run.
+
+--immediate::
+	This causes the test to immediately exit upon the first
+	failed test.
+
+
+Naming Tests
+------------
+
+The test files are named as:
+
+	tNNNN-commandname-details.sh
+
+where N is a decimal digit.
+
+Here is a proposal for numbering, loosely based on the Core GIT
+numbering conventions.
+
+First two digit tells the particular command we are testing:
+
+	00 - stgit itself
+	10 - branch
+	11 - clone
+	12 - push
+
+Third and fourth digit (optionally) tells the particular switch or
+group of switches we are testing.
+
+If you create files under t/ directory (i.e. here) that is not
+the top-level test script, never name the file to match the above
+pattern.  The Makefile here considers all such files as the
+top-level test script and tries to run all of them.  A care is
+especially needed if you are creating a common test library
+file, similar to test-lib.sh, because such a library file may
+not be suitable for standalone execution.
+
+
+Writing Tests
+-------------
+
+The test script is written as a shell script.  It should start
+with the standard "#!/bin/sh" with copyright notices, and an
+assignment to variable 'test_description', like this:
+
+	#!/bin/sh
+	#
+	# Copyright (c) 2005 Junio C Hamano
+	#
+
+	test_description='xxx test (option --frotz)
+
+	This test registers the following structure in the cache
+	and tries to run git-ls-files with option --frotz.'
+
+
+Source 'test-lib.sh'
+--------------------
+
+After assigning test_description, the test script should source
+test-lib.sh like this:
+
+	. ./test-lib.sh
+
+This test harness library does the following things:
+
+ - If the script is invoked with command line argument --help
+   (or -h), it shows the test_description and exits.
+
+ - Creates an empty test directory with an empty .git/objects
+   database and chdir(2) into it.  This directory is 't/trash'
+   if you must know, but I do not think you care.
+
+ - Defines standard test helper functions for your scripts to
+   use.  These functions are designed to make all scripts behave
+   consistently when command line arguments --verbose (or -v),
+   --debug (or -d), and --immediate (or -i) is given.
+
+
+End with test_done
+------------------
+
+Your script will be a sequence of tests, using helper functions
+from the test harness library.  At the end of the script, call
+'test_done'.
+
+
+Test harness library
+--------------------
+
+There are a handful helper functions defined in the test harness
+library for your script to use.
+
+ - test_expect_success <message> <script>
+
+   This takes two strings as parameter, and evaluates the
+   <script>.  If it yields success, test is considered
+   successful.  <message> should state what it is testing.
+
+   Example:
+
+	test_expect_success \
+	    'git-write-tree should be able to write an empty tree.' \
+	    'tree=$(git-write-tree)'
+
+ - test_expect_failure <message> <script>
+
+   This is the opposite of test_expect_success.  If <script>
+   yields success, test is considered a failure.
+
+   Example:
+
+	test_expect_failure \
+	    'git-update-index without --add should fail adding.' \
+	    'git-update-index should-be-empty'
+
+ - test_debug <script>
+
+   This takes a single argument, <script>, and evaluates it only
+   when the test script is started with --debug command line
+   argument.  This is primarily meant for use during the
+   development of a new test script.
+
+ - test_done
+
+   Your test script must have test_done at the end.  Its purpose
+   is to summarize successes and failures in the test script and
+   exit with an appropriate error code.
+
+
+Tips for Writing Tests
+----------------------
+
+As with any programming projects, existing programs are the best
+source of the information.  However, do _not_ emulate
+t0000-basic.sh when writing your tests.  The test is special in
+that it tries to validate the very core of GIT.  For example, it
+knows that there will be 256 subdirectories under .git/objects/,
+and it knows that the object ID of an empty tree is a certain
+40-byte string.  This is deliberately done so in t0000-basic.sh
+because the things the very basic core test tries to achieve is
+to serve as a basis for people who are changing the GIT internal
+drastically.  For these people, after making certain changes,
+not seeing failures from the basic test _is_ a failure.  And
+such drastic changes to the core GIT that even changes these
+otherwise supposedly stable object IDs should be accompanied by
+an update to t0000-basic.sh.
+
+However, other tests that simply rely on basic parts of the core
+GIT working properly should not have that level of intimate
+knowledge of the core GIT internals.  If all the test scripts
+hardcoded the object IDs like t0000-basic.sh does, that defeats
+the purpose of t0000-basic.sh, which is to isolate that level of
+validation in one place.  Your test also ends up needing
+updating when such a change to the internal happens, so do _not_
+do it and leave the low level of validation to t0000-basic.sh.
diff --git a/t/t0000-dummy.sh b/t/t0000-dummy.sh
new file mode 100755
index 0000000..1a9bd87
--- /dev/null
+++ b/t/t0000-dummy.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Dummy test.
+
+Only to test the testing environment.
+'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'check stgit can be run' \
+    'stg version'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
new file mode 100755
index 0000000..6339c54
--- /dev/null
+++ b/t/test-lib.sh
@@ -0,0 +1,203 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+# Copyright (c) 2006 Yann Dirson - tuning for stgit
+#
+
+# For repeatability, reset the environment to known value.
+LANG=C
+LC_ALL=C
+PAGER=cat
+TZ=UTC
+export LANG LC_ALL PAGER TZ
+unset AUTHOR_DATE
+unset AUTHOR_EMAIL
+unset AUTHOR_NAME
+unset COMMIT_AUTHOR_EMAIL
+unset COMMIT_AUTHOR_NAME
+unset GIT_ALTERNATE_OBJECT_DIRECTORIES
+unset GIT_AUTHOR_DATE
+GIT_AUTHOR_EMAIL=author@example.com
+GIT_AUTHOR_NAME='A U Thor'
+unset GIT_COMMITTER_DATE
+GIT_COMMITTER_EMAIL=committer@example.com
+GIT_COMMITTER_NAME='C O Mitter'
+unset GIT_DIFF_OPTS
+unset GIT_DIR
+unset GIT_EXTERNAL_DIFF
+unset GIT_INDEX_FILE
+unset GIT_OBJECT_DIRECTORY
+unset SHA1_FILE_DIRECTORIES
+unset SHA1_FILE_DIRECTORY
+export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
+export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
+
+# Each test should start with something like this, after copyright notices:
+#
+# test_description='Description of this test...
+# This test checks if command xyzzy does the right thing...
+# '
+# . ./test-lib.sh
+
+error () {
+	echo "* error: $*"
+	trap - exit
+	exit 1
+}
+
+say () {
+	echo "* $*"
+}
+
+test "${test_description}" != "" ||
+error "Test script did not set test_description."
+
+while test "$#" -ne 0
+do
+	case "$1" in
+	-d|--d|--de|--deb|--debu|--debug)
+		debug=t; shift ;;
+	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
+		immediate=t; shift ;;
+	-h|--h|--he|--hel|--help)
+		echo "$test_description"
+		exit 0 ;;
+	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+		verbose=t; shift ;;
+	*)
+		break ;;
+	esac
+done
+
+exec 5>&1
+if test "$verbose" = "t"
+then
+	exec 4>&2 3>&1
+else
+	exec 4>/dev/null 3>/dev/null
+fi
+
+test_failure=0
+test_count=0
+
+trap 'echo >&5 "FATAL: Unexpected exit with code $?"; exit 1' exit
+
+
+# You are not expected to call test_ok_ and test_failure_ directly, use
+# the text_expect_* functions instead.
+
+test_ok_ () {
+	test_count=$(expr "$test_count" + 1)
+	say "  ok $test_count: $@"
+}
+
+test_failure_ () {
+	test_count=$(expr "$test_count" + 1)
+	test_failure=$(expr "$test_failure" + 1);
+	say "FAIL $test_count: $1"
+	shift
+	echo "$@" | sed -e 's/^/	/'
+	test "$immediate" = "" || { trap - exit; exit 1; }
+}
+
+
+test_debug () {
+	test "$debug" = "" || eval "$1"
+}
+
+test_run_ () {
+	eval >&3 2>&4 "$1"
+	eval_ret="$?"
+	return 0
+}
+
+test_expect_failure () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-failure"
+	say >&3 "expecting failure: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" != 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_success () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-success"
+	say >&3 "expecting success: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" = 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_code () {
+	test "$#" = 3 ||
+	error "bug in the test script: not 3 parameters to test-expect-code"
+	say >&3 "expecting exit code $1: $3"
+	test_run_ "$3"
+	if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+	then
+		test_ok_ "$2"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+# Most tests can use the created repository, but some amy need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	owd=`pwd`
+	repo="$1"
+	mkdir "$repo"
+	cd "$repo" || error "Cannot setup test environment"
+	git-init-db 2>/dev/null ||
+	error "cannot run git-init-db -- have you installed git-core?"
+	mv .git/hooks .git/hooks-disabled
+	echo "empty start" |
+	git-commit-tree `git-write-tree` >.git/refs/heads/master 2>/dev/null ||
+	error "cannot run git-commit -- is your git-core funtionning?"
+	cd "$owd"
+}
+
+test_done () {
+	trap - exit
+	case "$test_failure" in
+	0)
+		# We could:
+		# cd .. && rm -fr trash
+		# but that means we forbid any tests that use their own
+		# subdirectory from calling test_done without coming back
+		# to where they started from.
+		# The Makefile provided will clean this test area so
+		# we will leave things as they are.
+
+		say "passed all $test_count test(s)"
+		exit 0 ;;
+
+	*)
+		say "failed $test_failure among $test_count test(s)"
+		exit 1 ;;
+
+	esac
+}
+
+# Test the binaries we have just built.  The tests are kept in
+# t/ subdirectory and are run in trash subdirectory.
+PATH=$(pwd)/..:$PATH
+export PATH
+
+
+# Test repository
+test=trash
+rm -fr "$test"
+test_create_repo $test
+cd "$test"

^ permalink raw reply related

* [PATCH 0/9] Add a testsuite to stgit (take 3), and more
From: Yann Dirson @ 2006-04-16 10:41 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

This new revision of my testsuite patches include:

- cleanup of the repo-initialization stuff in test-lib.sh: now
test_create_repo creates an empty commit to start with, so that HEAD
points to an existing master branch.  This makes test_stg_init
useless, rather let tests can "stg init" directly.

- steal the git t/README as well, and adapt it.  Proposes a numbering
convention for stgit testcases.

- activation and refining of the testcase which I had left commented
out in t1000, and a fix for the bug it shows

- fix for a nasty caching issue in stg clone, and associated test

- 2 testcases showing problems with detection of applied patches in
"push" after a pull when our patches were integrated upstream.  No fix
here, though, so you'll see t1200 and t1201 failing.


And not directly related to the testsuite:
- more bugs listed in TODO
- support for per-user templates for mail and export, in ~/.stgit/templates

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* Re: Help please :-)
From: Paolo Ciarrocchi @ 2006-04-16  9:30 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Git Mailing List
In-Reply-To: <20060415191039.GC27689@pasky.or.cz>

On 4/15/06, Petr Baudis <pasky@suse.cz> wrote:
>   Hello,
>
> Dear diary, on Sat, Apr 15, 2006 at 06:08:01PM CEST, I got a letter
> where Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> said that...
> > I'm used to keep updated my linux tree with cg-status,
> > I did that this morning but now I see the following:
> > paolo@Italia:~/linux-2.6$ cg-status
> > Heads:
> >    >master      2c5362007bc0a46461a9d94958cdd53bb027004c
> >   R origin      2c5362007bc0a46461a9d94958cdd53bb027004c
> >
> > ? arch/i386/kernel/smpboot.c.rej
> > ? drivers/md/dm-stripe.c.rej
> > ? drivers/net/chelsio/sge.c.rej
> > ? drivers/net/e100.c.rej
> > ? drivers/net/e1000/e1000_main.c.rej
> > ? fs/9p/vfs_dir.c.rej
> > ? fs/nfsctl.c.rej
> > ? kernel/fork.c.rej
> > ? kernel/posix-timers.c.rej
> > ? kernel/timer.c.rej
> > ? mm/memory.c.rej
> > ? mm/mempolicy.c.rej
> > ? mm/swap.c.rej
> > ? net/ieee80211/ieee80211_crypt_ccmp.c.rej
> > ? net/ieee80211/ieee80211_rx.c.rej
> > ? scripts/kconfig/lkc_defs.h
> > ? scripts/mod/modpost.c.rej
> > paolo@Italia:~/linux-2.6$ cg-diff
> >
> > I'm a bit lost, the tree is correctly updated, no error message but
> > why I see all these .rej?
>
>   you apparently had local changes in your working tree, did cg-update
> and then the local changes conflicted with the new changes in Linus'
> tree. cg-update should have told you further details.

Can this be due an interrupted cg-update?
However, cg-update did not complain.

> > And how can I fix this problem?
> > git reset and cg-reset don't help...
>
>   cg-clean can remove files not recognized by git.

Thanks!

Ciao,
--
Paolo
http://paolociarrocchi.googlepages.com

^ 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