change of git-diff-tree and symlinks

change of git-diff-tree and symlinks

[Kay Sievers]

Hi,
I'm catching up with gitweb.cgi to parse the changed output. Works fine
so far and is really much easier to parse. Here is something that does
not work anymore. See the difference between:

   http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6
   http://ehlo.org/~kay/gitweb.cgi?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6

On my box is the lates git with the adapted gitweb.cgi. Here is the raw
output of the old git-diff-tree:
  kay@dhcp-188:~/src/udev> /home/kay/src/cogito/git-diff-tree -r 49cedafaf893bfe348eb7598227f1a11ae24bfd6 17f2b1a7e0d10334af7f9622848788add125dea8
  *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/cciss!c0d0/device
  *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/rd!c0d0/device
  *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/sda/device
  *120000->100644 blob 1c776568bdc9dc750addd0885dded6b008a44460->1c776568bdc9dc750addd0885dded6b008a44460 test/sys/bus/pci/devices/0000:00:09.0
  *120000->100644 blob e000c77614a23ad57fed284bd007ed7c1cb7872e->e000c77614a23ad57fed284bd007ed7c1cb7872e test/sys/bus/pci/devices/0000:00:1e.0
  ...

The new one shows simply nothing.
Shouldn't it print the mode changes like the old one?

Kay

Re: change of git-diff-tree and symlinks

[Linus Torvalds]

On Wed, 25 May 2005, Kay Sievers wrote:
>
> I'm catching up with gitweb.cgi to parse the changed output. Works fine
> so far and is really much easier to parse. Here is something that does
> not work anymore. See the difference between:
> 
>    http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6
>    http://ehlo.org/~kay/gitweb.cgi?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6

Yes, the new diff-tree thing doesn't show symlinks.

The problem seems to be that we just don't have a "status" flag for it. I 
think we should call it "T" for "Type change" or something, but in the 
meantime let's just have the rule that instead of ignoring unknown state 
changes, we always print them out as "?" instead.

Ie something like this..

(And I'd suggest you make gitweb flexible enough that it does something 
sane if it sees an unknown reason code - let's see what Junio thinks about 
what status code we should use for this).

		Linus

----
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -768,7 +768,7 @@ void diff_flush(int diff_output_style, i
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		if (p->status == 0)
-			continue;
+			p->status = '?';
 		switch (diff_output_style) {
 		case DIFF_FORMAT_PATCH:
 			diff_flush_patch(p);

Re: change of git-diff-tree and symlinks

[Kay Sievers]

On Wed, May 25, 2005 at 11:08:54AM -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 25 May 2005, Kay Sievers wrote:
> >
> > I'm catching up with gitweb.cgi to parse the changed output. Works fine
> > so far and is really much easier to parse. Here is something that does
> > not work anymore. See the difference between:
> > 
> >    http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6
> >    http://ehlo.org/~kay/gitweb.cgi?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6
> 
> Yes, the new diff-tree thing doesn't show symlinks.
> 
> The problem seems to be that we just don't have a "status" flag for it. I 
> think we should call it "T" for "Type change" or something, but in the 
> meantime let's just have the rule that instead of ignoring unknown state 
> changes, we always print them out as "?" instead.
> 
> Ie something like this..
> 
> (And I'd suggest you make gitweb flexible enough that it does something 
> sane if it sees an unknown reason code - let's see what Junio thinks about 
> what status code we should use for this).

Ok, works again. Any reason not to mark it as 'M'? It's easyly to
distinguish between a content change and a mode change by looking if
the sha has changed.

Kay

Re: change of git-diff-tree and symlinks

[Junio C Hamano]

>>>>> "KS" == Kay Sievers <kay.sievers@vrfy.org> writes:

KS> Ok, works again. Any reason not to mark it as 'M'? It's easyly to
KS> distinguish between a content change and a mode change by looking if
KS> the sha has changed.

I'd need some time to think about this in the evening (I'm at
work now) but I think Linus did '?' as an interim measure.  I
suspect 'M' would work just as well without introducing a new
class 'T', but on the other hand having them distinct would be
easier for people who read diff-raw format, so my knee-jerk
reaction is that we should use 'T'.  It's the same reasoning to
have N, D, and U --- they can be gleaned by looking at mode and
sha fields and there is no need to mark them any differently but
surely marking them the way we do now is much nicer.

More importantly I need to double check that diffcore stuff
including rename/copy are doing the right thing about type
changes.  If some of them are throwing type-change pair away
under certain obscure conditions, then tweaking the part Linus
added the '?' is too late to remedy the situation.

[PATCH] Adjust show-files test for dotfiles.

[Junio C Hamano]

The earlier test was relying on the fact that dotfiles do not
appear in the output to prepare expected test results, which
inevitably got broken when we started handling dotfiles.  Change
the test to be honest about what "--other" file it creates.

The problem was originally pointed out by Mark Allen.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

t/t3000-ls-files-others.sh |    7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -20,8 +20,9 @@ mkdir path2
 date >path2/file2
 test_expect_success \
     'git-ls-files --others to show output.' \
-    'git-ls-files --others >.output'
-cat >.expected <<EOF
+    'git-ls-files --others >output'
+cat >expected <<EOF
+output
 path0
 path1
 path2/file2
@@ -29,5 +30,5 @@ EOF
 
 test_expect_success \
     'git-ls-files --others should pick up symlinks.' \
-    'diff .output .expected'
+    'diff output expected'
 test_done
------------------------------------------------

[PATCH] Fix type-change handling when assigning the status code to filepairs.

[Junio C Hamano]

The interim single-liner '?' fix resulted delete entries that
should not have emitted coming out in the output as an
unintended side effect; I caught this with the "rename" test in
the test suite.  This patch instead fixes the code that assigns
the status code to each filepair.

I verified this does not break the testcase in udev.git tree Kay
Sievers gave us, by running git-diff-tree on that tree which
showed 21 file to symlink changes.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

*** Linus, it just struck me that there is nothing inherently
*** wrong to detect whole subtree renames (diffcore-rename
*** currently does not attempt to match tree creation against
*** tree delete to produce tree rename/copy).  This is something
*** I am _not_ going to do, but it still would be interesting to
*** keep in mind...

diff.c     |   36 ++++++++++++++++++++++--------------
diffcore.h |    3 +++
2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -691,27 +691,34 @@ static void diff_resolve_rename_copy(voi
 
 	for (i = 0; i < q->nr; i++) {
 		p = q->queue[i];
-		p->status = 0;
+		p->status = 0; /* undecided */
 		if (DIFF_PAIR_UNMERGED(p))
 			p->status = 'U';
 		else if (!DIFF_FILE_VALID((p)->one))
 			p->status = 'N';
 		else if (!DIFF_FILE_VALID((p)->two)) {
 			/* Deletion record should be omitted if there
-			 * is another entry that is a rename or a copy
-			 * and it uses this one as the source.  Then we
-			 * can say the other one is a rename.
+			 * are rename/copy entries using this one as
+			 * the source.  Then we can say one of them
+			 * is a rename and the rest are copies.
 			 */
+			p->status = 'D';
 			for (j = 0; j < q->nr; j++) {
 				pp = q->queue[j];
 				if (!strcmp(pp->one->path, p->one->path) &&
-				    strcmp(pp->one->path, pp->two->path))
+				    strcmp(pp->one->path, pp->two->path)) {
+					p->status = 'X';
 					break;
+				}
 			}
-			if (j < q->nr)
-				continue; /* has rename/copy */
-			p->status = 'D';
 		}
+		else if (DIFF_PAIR_TYPE_CHANGED(p))
+			p->status = 'T';
+
+		/* from this point on, we are dealing with a pair
+		 * whose both sides are valid and of the same type, i.e.
+		 * either in-place edit or rename/copy edit.
+		 */
 		else if (strcmp(p->one->path, p->two->path)) {
 			/* See if there is somebody else anywhere that
 			 * will keep the path (either modified or
@@ -719,7 +726,7 @@ static void diff_resolve_rename_copy(voi
 			 * not a rename.  In addition, if there is
 			 * some other rename or copy that comes later
 			 * than us that uses the same source, we
-			 * cannot be a rename either.
+			 * have to be a copy, not a rename.
 			 */
 			for (j = 0; j < q->nr; j++) {
 				pp = q->queue[j];
@@ -745,10 +752,9 @@ static void diff_resolve_rename_copy(voi
 		}
 		else if (memcmp(p->one->sha1, p->two->sha1, 20))
 			p->status = 'M';
-		else {
-			/* we do not need this one */
-			p->status = 0;
-		}
+		else
+			/* this is a "no-change" entry */
+			p->status = 'X';
 	}
 	diff_debug_queue("resolve-rename-copy done", q);
 }
@@ -767,8 +773,10 @@ void diff_flush(int diff_output_style, i
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
+		if (p->status == 'X')
+			continue;
 		if (p->status == 0)
-			p->status = '?';
+			die("internal error in diff-resolve-rename-copy");
 		switch (diff_output_style) {
 		case DIFF_FORMAT_PATCH:
 			diff_flush_patch(p);
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -45,6 +45,9 @@ struct diff_filepair {
 #define DIFF_PAIR_UNMERGED(p) \
 	(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
 
+#define DIFF_PAIR_TYPE_CHANGED(p) \
+	((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode))
+
 extern int diff_unmodified_pair(struct diff_filepair *);
 
 struct diff_queue_struct {
------------------------------------------------

Re: change of git-diff-tree and symlinks

[Kay Sievers]

On Wed, May 25, 2005 at 12:40:51PM -0700, Junio C Hamano wrote:
> >>>>> "KS" == Kay Sievers <kay.sievers@vrfy.org> writes:
> 
> KS> Ok, works again. Any reason not to mark it as 'M'? It's easyly to
> KS> distinguish between a content change and a mode change by looking if
> KS> the sha has changed.
> 
> I'd need some time to think about this in the evening (I'm at
> work now) but I think Linus did '?' as an interim measure.  I
> suspect 'M' would work just as well without introducing a new
> class 'T', but on the other hand having them distinct would be
> easier for people who read diff-raw format, so my knee-jerk
> reaction is that we should use 'T'.  It's the same reasoning to
> have N, D, and U --- they can be gleaned by looking at mode and
> sha fields and there is no need to mark them any differently but
> surely marking them the way we do now is much nicer.

If we introduce 'T', how is a content _and_ a type change represented
if they happen at the same time?

Kay

Re: change of git-diff-tree and symlinks

[Junio C Hamano]

>>>>> "KS" == Kay Sievers <kay.sievers@vrfy.org> writes:

KS> If we introduce 'T', how is a content _and_ a type change represented
KS> if they happen at the same time?

If you have this pair in two trees:

    ln -s frotz xyzzy
    echo -n frotz >xyzzy

it is a 'T'.  If you instead have these in two trees:

    ln -s rezrov xyzzy
    echo -n frotz >xyzzy

it is also a 'T'.

I do not think we would want patch format to give us a diff
showing that string rezrov changing into frotz in the latter
example anyway.  When we have a type change, content change is
irrelevant.

Re: change of git-diff-tree and symlinks

[Linus Torvalds]

On Thu, 26 May 2005, Kay Sievers wrote:
> 
> If we introduce 'T', how is a content _and_ a type change represented
> if they happen at the same time?

A 'T' _always_ implies a content change, imho. 

Yes, that strange udev changeset actually had files that had the same
content as the symlinks, but from a patch perspective, that should
probably really still be a "file got entirely deleted" + "we created a
symlink with new content". Anything else just doesn't make any sense.

So in that way, 'T' really is different from 'M'. 'M' implies a patch
(which might be empty, of course), while 'T' implies that the old thing 
was deleted and entirely replaced with something totally different.

		Linus

[PATCH] Mode only changes from diff.

[Junio C Hamano]

This fixes another bug.

 - Mode-only changes were pruned incorrectly from the output.
 - Added test to catch the above problem.
 - Normalize rename/copy similarity score in the diff-raw output
   to per-cent, no matter what scale we internally use.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-helper.c        |    2 ++
diff.c               |    6 ++++--
t/t4006-diff-mode.sh |   34 ++++++++++++++++++++++++++++++++++
3 files changed, 40 insertions(+), 2 deletions(-)
new file (100755): t/t4006-diff-mode.sh

diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -4,6 +4,7 @@
 #include "cache.h"
 #include "strbuf.h"
 #include "diff.h"
+#include "diffcore.h" /* just for MAX_SCORE */
 
 static const char *pickaxe = NULL;
 static int line_termination = '\n';
@@ -77,6 +78,7 @@ int main(int ac, const char **av) {
 			if (status == 'R' || status == 'C') {
 				two_paths = 1;
 				sscanf(cp, "%d", &score);
+				score = score * MAX_SCORE / 100;
 				if (line_termination) {
 					cp = strchr(cp,
 						    inter_name_termination);
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -517,7 +517,8 @@ static void diff_flush_raw(struct diff_f
 	switch (p->status) {
 	case 'C': case 'R':
 		two_paths = 1;
-		sprintf(status, "%c%1d", p->status, p->score);
+		sprintf(status, "%c%03d", p->status,
+			(int)(0.5 + p->score * 100.0/MAX_SCORE));
 		break;
 	default:
 		two_paths = 0;
@@ -750,7 +751,8 @@ static void diff_resolve_rename_copy(voi
 			if (!p->status)
 				p->status = 'R';
 		}
-		else if (memcmp(p->one->sha1, p->two->sha1, 20))
+		else if (memcmp(p->one->sha1, p->two->sha1, 20) ||
+			 p->one->mode != p->two->mode)
 			p->status = 'M';
 		else
 			/* this is a "no-change" entry */
diff --git a/t/t4006-diff-mode.sh b/t/t4006-diff-mode.sh
new file mode 100755
--- /dev/null
+++ b/t/t4006-diff-mode.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Test mode change diffs.
+
+'
+. ./test-lib.sh
+
+test_expect_success \
+    'setup' \
+    'echo frotz >rezrov &&
+     git-update-cache --add rezrov &&
+     tree=`git-write-tree` &&
+     echo $tree'
+
+test_expect_success \
+    'chmod' \
+    'chmod +x rezrov &&
+     git-update-cache rezrov &&
+     git-diff-cache $tree >current'
+
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+sed -e 's/\(:100644 100755\) \('"$_x40"'\) \2 /\1 X X /' <current >check
+echo ":100644 100755 X X M	rezrov" >expected
+
+test_expect_success \
+    'verify' \
+    'diff -u expected check'
+
+test_done
+
------------------------------------------------

[PATCH] Test case portability fix.

[Junio C Hamano]

This is the remainder of testcase fix by Mark Allen to make them
work on his Darwin box.  I was using "xargs -r" (GNU) where it
was not needed, sed -ne '/^\(author\|committer\)/s|>.*|>|p'
where some sed does not know what to do with '\|', and also
"cmp - file" to compare standard input with a file, which his
cmp does not support.

Author: Mark Allen <mrallen1@yahoo.com>
Author-Date: 
Signed-off-by: Junio C Hamano <junkio@cox.net>
---

t/t0000-basic.sh                 |    2 +-
t/t0110-environment-names-old.sh |    6 ++----
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -84,7 +84,7 @@ do
 done
 test_expect_success \
     'adding various types of objects with git-update-cache --add.' \
-    'find path* ! -type d -print0 | xargs -0 -r git-update-cache --add'
+    'find path* ! -type d -print0 | xargs -0 git-update-cache --add'
 
 # Show them and see that matches what we expect.
 test_expect_success \
diff --git a/t/t0110-environment-names-old.sh b/t/t0110-environment-names-old.sh
--- a/t/t0110-environment-names-old.sh
+++ b/t/t0110-environment-names-old.sh
@@ -86,8 +86,7 @@ committer A U Thor <author@example.xz>
 EOF
 test_expect_success \
     'verify old AUTHOR variables were used correctly in commit' \
-    'sed -ne '\''/^\(author\|committer\)/s|>.*|>|p'\'' current |
-     cmp - expected'
+    'sed -ne '\''/^\(author\)/s|>.*|>|p'\'' -e'\''/^\(committer\)/s|>.*|>|p'\''\    current > out && cmp out expected'
 
 unset GIT_DIR
 test_expect_success \
@@ -128,7 +127,6 @@ committer R O Htua <rohtua@example.xz>
 EOF
 test_expect_success \
     'verify new AUTHOR variables were used correctly in commit.' \
-    'sed -ne '\''/^\(author\|committer\)/s|>.*|>|p'\'' current |
-     cmp - expected'
+    'sed -ne '\''/^\(author\)/s|>.*|>|p'\'' -e'\''/^\(committer\)/s|>.*|>|p'\''\    current > out && cmp out expected'
 
 test_done
------------------------------------------------

Re: [PATCH] Test case portability fix.

[Linus Torvalds]

On Wed, 25 May 2005, Junio C Hamano wrote:
>
> This is the remainder of testcase fix by Mark Allen to make them
> work on his Darwin box.  I was using "xargs -r" (GNU) where it
> was not needed, sed -ne '/^\(author\|committer\)/s|>.*|>|p'
> where some sed does not know what to do with '\|', and also
> "cmp - file" to compare standard input with a file, which his
> cmp does not support.
> 
> Author: Mark Allen <mrallen1@yahoo.com>
> Author-Date: 

Btw, do this as

	From: Mark Allen <mrallen1@yahoo.com>

at the top of the email body, and my patch-application scripts will
automatically do the right thing.

The Author-date thing you might as well drop for now..

		Linus

Re: [PATCH] Test case portability fix.

[Junio C Hamano]

Sorry, I think I sent an stale copy from my draft box by
accident and you already have the exactly same patch.

About the "From: " thing, I understood.

Re: change of git-diff-tree and symlinks

[Jochen Roemling]

Hello Kay,

I'm planning to use cogito/git for tracking development of my (PHP 
based) website. Although this is the first time in my life I'm using 
something that smells like a SCM, it seems to work great. The only thing 
lacking is a working gitweb installation.
I downloaded the gitweb.cgi script from 
kernel.org/pub/software/scm/gitweb, but it dates already May 23rd.

In your mail below from May 25 you state that there have been quite some 
changes to git-diff-tree:

Kay Sievers wrote:

>Hi,
>I'm catching up with gitweb.cgi to parse the changed output. Works fine
>so far and is really much easier to parse. Here is something that does
>not work anymore. See the difference between:
>
>   http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6
>   http://ehlo.org/~kay/gitweb.cgi?p=linux/hotplug/udev.git;a=commit;h=49cedafaf893bfe348eb7598227f1a11ae24bfd6
>
>On my box is the lates git with the adapted gitweb.cgi. Here is the raw
>output of the old git-diff-tree:
>  kay@dhcp-188:~/src/udev> /home/kay/src/cogito/git-diff-tree -r 49cedafaf893bfe348eb7598227f1a11ae24bfd6 17f2b1a7e0d10334af7f9622848788add125dea8
>  *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/cciss!c0d0/device
>  *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/rd!c0d0/device
>  *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/sda/device
>  *120000->100644 blob 1c776568bdc9dc750addd0885dded6b008a44460->1c776568bdc9dc750addd0885dded6b008a44460 test/sys/bus/pci/devices/0000:00:09.0
>  *120000->100644 blob e000c77614a23ad57fed284bd007ed7c1cb7872e->e000c77614a23ad57fed284bd007ed7c1cb7872e test/sys/bus/pci/devices/0000:00:1e.0
>  ...
>
>The new one shows simply nothing.
>Shouldn't it print the mode changes like the old one?
>
>  
>
and that might be the reason why I'm getting "nothing" when I'm clicking 
on a "commitdiff" link.
After investigating further, I found out that the object type ("blob" in 
the example above) is no longer printed by git-diff-tree.
Also there is a new parameter '-p' that allows the output of a patch 
directly instead of creating it with the help of temporary files as you 
do it in gitweb.
Unfortunately my perl knowledge is really poor and I didn't get it to 
work. After I changed the call to git-diff-tree to use the '-p' switch, 
I'm getting a "403 Forbidden - Reading diff-tree failed" message, but 
the same command on the command line works perfectly well. I don't know 
what is wrong there.

Could you please post a downloadable current release of gitweb to 
kernel.org that works with the current git? If not, please tell me what 
git release is installed at kernel.org so that I can use that branch to 
get it to work with gitweb.

And as a last side note it would be helpfull if you could include a 
release number in the gitweb footer as you did in the very beginning so 
that we know if something changed and there is new stuff to explore...

... keep up the great work!

Jochen

Re: change of git-diff-tree and symlinks

[Kay Sievers]

On Sun, May 29, 2005 at 04:16:53PM +0200, Jochen Roemling wrote:
> I'm planning to use cogito/git for tracking development of my (PHP 
> based) website. Although this is the first time in my life I'm using 
> something that smells like a SCM, it seems to work great. The only thing 
> lacking is a working gitweb installation.
> I downloaded the gitweb.cgi script from 
> kernel.org/pub/software/scm/gitweb, but it dates already May 23rd.
> 
> In your mail below from May 25 you state that there have been quite some 
> changes to git-diff-tree:
> 
> Kay Sievers wrote:
> >
> >The new one shows simply nothing.
> >Shouldn't it print the mode changes like the old one?
> >
> and that might be the reason why I'm getting "nothing" when I'm clicking 
> on a "commitdiff" link.

The kernel.org servers use Cogito which needs a new release. So the
version on kernel.org is always a working version with the current
release of Cogito as long as we don't get a git-core to provide the
binaries.

> After investigating further, I found out that the object type ("blob" in 
> the example above) is no longer printed by git-diff-tree.

The format has changed, just get the latest version of gitweb from my box
instead.

> Also there is a new parameter '-p' that allows the output of a patch 
> directly instead of creating it with the help of temporary files as you 
> do it in gitweb.

Sure, gitweb did the diffs before git was able to do that. And it does
not really matter who is creating the temp-files, the cgi or the
git-binaries. :)
If things are stable some day, I may switch that over and maybe even
support the nice rename detection.

> Could you please post a downloadable current release of gitweb to 
> kernel.org that works with the current git? If not, please tell me what 
> git release is installed at kernel.org so that I can use that branch to 
> get it to work with gitweb.

My latest version is here:
  ftp://ehlo.org/gitweb.cgi

> And as a last side note it would be helpfull if you could include a 
> release number in the gitweb footer as you did in the very beginning so 
> that we know if something changed and there is new stuff to explore...

It is in the html-source, right at the top.

Thanks,
Kay

Re: change of git-diff-tree and symlinks

[Jochen Roemling]

[resending in plain-text]

Kay Sievers wrote:

>The kernel.org servers use Cogito which needs a new release. So the
>version on kernel.org is always a working version with the current
>release of Cogito as long as we don't get a git-core to provide the
>binaries.
>  
>
Ah yes, I remember the discussion about that.
Sebastian, could you include a matching gitweb.cgi into your 
Debian-Package?

And what about something like:

if (-e '/etc/gitweb.conf') { requite '/etc/gitweb.conf'; }

after the variable settings? The included file would then set 
$projectroot and friends to the local values so that I don't have to 
patch gitweb everytime I upgrade it. Since perl has no plain "include" 
and the require doesn't really work for me (it seems to include the 
file, but the variables stay the same), I leave it up to you (I would 
have been so proud to provide you a patch, but obviously my perl 
knowledge is worse than I thought).

>My latest version is here:
>  ftp://ehlo.org/gitweb.cgi
>  
>
Thanks for pointing that out. I had the impression, on ehlo.org is only 
a installed version, not a downloadable one.

>>And as a last side note it would be helpfull if you could include a 
>>release number in the gitweb footer as you did in the very beginning so 
>>that we know if something changed and there is new stuff to explore...
>>    
>>
>
>It is in the html-source, right at the top.
>  
>
Good hiding-place....

Jochen

Re: change of git-diff-tree and symlinks

[Sebastian Kuzminsky]

Jochen Roemling <jochen@roemling.net> wrote:
> Sebastian, could you include a matching gitweb.cgi into your 
> Debian-Package?

I'm planning to package gitweb as a separate Debian package.  I'll try
to get to it next week.


-- 
Sebastian Kuzminsky

Re: change of git-diff-tree and symlinks

[Junio C Hamano]

>>>>> "KS" == Kay Sievers <kay.sievers@vrfy.org> writes:

>> Also there is a new parameter '-p' that allows the output of a patch 
>> directly instead of creating it with the help of temporary files as you 
>> do it in gitweb.

KS> Sure, gitweb did the diffs before git was able to do that. And it does
KS> not really matter who is creating the temp-files, the cgi or the
KS> git-binaries. :)

I agree that you are doing the right thing here.  In a scripted
environment it usually is more sensible not to use '-p' unless
generating the diff is the _only_ thing you are doing.  '-p' is
primarily for people who ends up using the bare plumbing
interactively.

KS> If things are stable some day, I may switch that over and maybe even
KS> support the nice rename detection.

Sorry for being the primary culprit for the disruption ;-).

You do not have to use '-p' to be able to use rename detection
nor any other diffcore stuff.  If you are processing
"git-diff-tree -r" output right now, you can already process
"git-diff-tree -r -B -M -S'foo' -Oorder" output just fine and
leave your downstream processing the same as is.

Re: change of git-diff-tree and symlinks

[Petr Baudis]

Dear diary, on Sun, May 29, 2005 at 05:06:56PM CEST, I got a letter
where Kay Sievers <kay.sievers@vrfy.org> told me that...
> On Sun, May 29, 2005 at 04:16:53PM +0200, Jochen Roemling wrote:
> > I'm planning to use cogito/git for tracking development of my (PHP 
> > based) website. Although this is the first time in my life I'm using 
> > something that smells like a SCM, it seems to work great. The only thing 
> > lacking is a working gitweb installation.
> > I downloaded the gitweb.cgi script from 
> > kernel.org/pub/software/scm/gitweb, but it dates already May 23rd.
> > 
> > In your mail below from May 25 you state that there have been quite some 
> > changes to git-diff-tree:
> > 
> > Kay Sievers wrote:
> > >
> > >The new one shows simply nothing.
> > >Shouldn't it print the mode changes like the old one?
> > >
> > and that might be the reason why I'm getting "nothing" when I'm clicking 
> > on a "commitdiff" link.
> 
> The kernel.org servers use Cogito which needs a new release. So the
> version on kernel.org is always a working version with the current
> release of Cogito as long as we don't get a git-core to provide the
> binaries.

Tomorrow evening. I'd even do it now but I feel the changes need a bit
of testing yet.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: change of git-diff-tree and symlinks

[Sebastian Kuzminsky]

Jochen Roemling <jochen@roemling.net> wrote:
> Sebastian, could you include a matching gitweb.cgi into your 
> Debian-Package?


Looks like gitweb's already packaged for Debian.  Andres Salomon is
doing it:

    http://marc.theaimsgroup.com/?l=git&m=111661740226054&w=2




-- 
Sebastian Kuzminsky