Re: gitweb wishlist

From: David Mansfield <david@cobite.com>
Date: May 24, 2005, 03:33 UTC
Message-ID: <4292A08A.5050108@cobite.com>
In-Reply-To: <Pine.LNX.4.58.0505201702170.2206@ppc970.osdl.org>

Hi Linus,

Linus Torvalds wrote:

Show 18 quoted lines

> [ Thomas added to cc, since he seems to have also worked on this ]
> 
> On Fri, 20 May 2005, H. Peter Anvin wrote:
> 
>>Here is my "main" OSS CVS repository; look at the syslinux module.  It 
>>has at least some minor branching.
> 
> 
> Ok, "cvsps" output scares me. I wonder what
> 
> 	WARNING: Invalid PatchSet 775, Tag syslinux-2_12-pre7:
> 	    memdisk/init32.asm:1.3=after, memdisk/Makefile:1.26=before. Treated as 'before'
> 	WARNING: Invalid PatchSet 775, Tag syslinux-2_12-pre7:
> 	    memdisk/init32.asm:1.3=after, memdisk/e820test.c:1.7=before. Treated as 'before'
> 	...
> 
> means..
>

Ok. I'll tell you. It means that the committer uses bad practices in tagging ;-) It generally means that force tag (cvs tag -F <file>) was used on a specific file. Here's the scenario:

cvsps is trying to associate a tag to a specific commit. But in the cvs world this is not always at all possible. If, for example, a commit made and all files are tagged. Now some random file is modified and committed. Then, a bug is found in a file from the previously tagged set, say the file 'memdisk/init32.asm'. The bug is fixed, committed and the tag is MOVED for _just that file_ forward to the new version. Now there is no commit that can be associated with the tag. In this case, cvsps believes this to be a 'FUNKY' tag. There is a more pathological case having to do with 'INVALID' tags... It's enough to make a grown man cry.

Show 27 quoted lines

> Also, your syslinux repo is interesting and shows another thing: doing a
> 
> 	cvsps -g -p separate
> 
> ends badly with
> 
> 	Directing PatchSet 938 to file separate/938.patch
> 	cvs rdiff: failed to read diff file header /tmp/cvso8PswZ for mdiskchk.com,v: end of file
> 	system command returned non-zero exit status: 1: aborting
> 
> which doesn't look very promising and causes an empty diff for
> mdiskck.com. Trying with --cvs-direct shows the reason:
> 
> 	Index: syslinux/sample/mdiskchk.com
> 	===================================================================
> 	RCS file: 
> 	/home/torvalds/src/osscvs/cvsroot/syslinux/sample/mdiskchk.com,v
> 	retrieving revision 1.1
> 	retrieving revision 1.2
> 	diff -u -r1.1 -r1.2
> 	Binary files /tmp/cvsU6MGU0 and /tmp/cvsiskFVR differ
> 
> which shows that anything that bases itself of diffs (ie uses "-g" with
> cvsps) is just doomed to failure, since there's no good way to handle
> binary data. Both Kay's and Thomas' scripts try to do the "-g" thing, 
> that's just not right.
>

I accept patches ;-) Honestly, handling binary data should be trivial I just haven't had the interest, and surprisingly noone else on the internet ever has. The only binary file in the kernel appears to be the logo.gif, according to Ingo.

[ discussion on working around broken handling of binary files in cvsps]

Show 14 quoted lines

> 
> There seems to be two questions:
> 
>  - what to do about branch creation (ie a branch name we haven't seen
>    before): it looks like cvsps doesn't tell you what the _originating_
>    branch was for a new branch (that may be my confusion - maybe you can't
>    create branches off branches in CVS?)
> 
>    For syslinux, it looks like you can always base it on HEAD, or possibly 
>    just the previous patch (which looks like it is always HEAD). The above 
>    pseudo-script will actually do that automatically, simply by virtue of
>    the "git-read-tree -m" at the top of the loop failing when the
>    branchname doesn't exist yet.
>

See attached patch to cvsps.c which displays 'Ancestor branch' when this differs from Branch.

Show 13 quoted lines

>  - whether to bother to create merge entries for when somebody tried to 
>    merge a branch back or forth in CVS. 
> 
>    CVS fundamentally doesn't have the notion of such a thing, and cvsps 
>    can't either. But we could try to guess, based on the commit message, 
>    perhaps.
> 
>    NOTE! Such a "merge" would not have any real GIT merge functionality 
>    what-so-ever. It would just introduce a second parent into the commit, 
>    nothing more.
> 
> Bah. What crud.
>

Hey, a polished turd is only so shiny... cvsps is a 99% solution [to the problem of extracting metatdata from cvs] only and cvs makes the other 1% impossible.

David

--- cvsps.c~ 2003-04-11 10:06:01.000000000 -0400 +++ cvsps.c 2005-05-23 23:26:12.110231536 -0400

@@ -1402,6 +1402,16 @@
 	   tm->tm_hour, tm->tm_min, tm->tm_sec);
     printf("Author: %s\n", ps->author);
     printf("Branch: %s\n", ps->branch);
+    
+    /* check if ancestor was different branch */
+    if (!list_empty(&ps->members)) 
+    {
+	    PatchSetMember * psm = list_entry(ps->members.next, PatchSetMember, link);
+	    const char * abr = psm->pre_rev ? psm->pre_rev->branch : NULL;
+	    if (abr && strcmp(ps->branch, abr) != 0)
+		    printf("Ancestor branch: %s\n", abr);
+    }
+
     printf("Tag: %s %s\n", ps->tag ? ps->tag : "(none)", tag_flag_descr[ps->tag_flags]);
     printf("Log:\n%s\n", ps->descr);
     printf("Members: \n");