Re: [PATCH 3/3] diffstat generation in hooks--update was passing "^baserev" to git-diff-tree

Previous thread: [PATCH 1/3] Fix potential command line overflow in hooks--update by Andy Parkins on Tuesday, February 13, 2007 - 7:23 am. (2 messages)

Next thread: [PATCH 2/3] Only show log entries for new revisions in hooks--update by Andy Parkins on Tuesday, February 13, 2007 - 7:24 am. (3 messages)
From: Andy Parkins
Date: Tuesday, February 13, 2007 - 7:24 am

For generating the diffstat after a branch update, git-diff-tree is
simply comparing the new revision with the base revision.  However, the
baserev was being passed with a "^" operator proceeding it - this (I
think) made git-diff-tree think that a path was being specified for
comparison rather than a second path, so only a single revision was
being summarised.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 templates/hooks--update |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/templates/hooks--update b/templates/hooks--update
index 7e8258a..ee3859c 100644
--- a/templates/hooks--update
+++ b/templates/hooks--update
@@ -193,8 +193,8 @@ case "$refname_type" in
 			git-rev-parse --not --all | git-rev-list --stdin --pretty $newrev ^$baserev
 			echo $LOGEND
 			echo ""
-			echo "Diffstat:"
-			git-diff-tree --no-color --stat -M -C --find-copies-harder $newrev ^$baserev
+			echo "Diffstat against $baserev:"
+			git-diff-tree --no-color --stat -M -C --find-copies-harder $newrev $baserev
 		fi
 		;;
 	"annotated tag")
-- 
1.5.0.rc4.364.g85b1

-

From: Linus Torvalds
Date: Tuesday, February 13, 2007 - 10:03 am

This is wrong.

	newrev ^baserev

is right. The "not baserev" tells diff-tree that the baserev is the 
starting point, so newrev is obviously the target, and thus that will 
generate a diff from baserev to newrev.

So will either of

	baserev..newrev
	baserev newrev

which mean _exactly_ the same thing as "newrev ^basrev" to "diff-tree", 
because in all cases it's obviously "baserev" that is the old one to diff 
against.

But

	newrev baserev

means the diff from "new" to "base", which is exactly the wrong way 
around.

Of course, since we did

	baserev=$(git-merge-base $oldrev $newrev)

to generate base-rev, we could actually have done

	$oldrev...$newrev

(note the _three_ dots) which means "diff from merge-base to newrev". But 
since we use "baserev" multiple times, what the update hook does right now 
is actually better - it avoids the cost of re-computing the merge base 
that we needed for other things anyway.

		Linus
-

From: Andy Parkins
Date: Tuesday, February 13, 2007 - 11:34 am

Right.  It's all come back to me now you've explained it; I obviously 
understood it when I wrote it, but it leaked out in the interim :-).  I 
was surprised because I've been using the hook for ages and the output 
seemed right. :-)

Junio - please ignore this patch.


Andy
-- 
Dr Andrew Parkins, M Eng (Hons), AMIEE
andyparkins@gmail.com
-

Previous thread: [PATCH 1/3] Fix potential command line overflow in hooks--update by Andy Parkins on Tuesday, February 13, 2007 - 7:23 am. (2 messages)

Next thread: [PATCH 2/3] Only show log entries for new revisions in hooks--update by Andy Parkins on Tuesday, February 13, 2007 - 7:24 am. (3 messages)