[PATCH] Use git-tag in git-cvsimport

[PATCH] Use git-tag in git-cvsimport

[Elvis Pranskevichus]

Currently git-cvsimport tries to create tag objects directly via git-mktag
in a very broken way, e.g the stuff it writes into the tagger field of
the tag object doesn't really resemble the GIT_COMMITTER_IDENT. This makes
gitweb and possibly other tools that try to interpret tag objects to be
confused about tag date and authorship.

Fix this by calling git-tag instead. This also has a nice side effect of
not creating the tag object but only the lightweight tag as that's the only
thing CVS has anyways.

Signed-off-by: Elvis Pranskevichus <el@prans.net>
---
 git-cvsimport.perl |   26 ++------------------------
 1 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index f68afe7..d5ca66b 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -771,31 +771,9 @@ sub commit {
 		$xtag =~ s/\s+\*\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **
 		$xtag =~ tr/_/\./ if ( $opt_u );
 		$xtag =~ s/[\/]/$opt_s/g;
-		
-		my $pid = open2($in, $out, 'git-mktag');
-		print $out "object $cid\n".
-		    "type commit\n".
-		    "tag $xtag\n".
-		    "tagger $author_name <$author_email>\n"
-		    or die "Cannot create tag object $xtag: $!\n";
-		close($out)
-		    or die "Cannot create tag object $xtag: $!\n";
-
-		my $tagobj = <$in>;
-		chomp $tagobj;
-
-		if ( !close($in) or waitpid($pid, 0) != $pid or
-		     $? != 0 or $tagobj !~ /^[0123456789abcdef]{40}$/ ) {
-		    die "Cannot create tag object $xtag: $!\n";
-	        }
-		
-
-		open(C,">$git_dir/refs/tags/$xtag")
+
+		system("git-tag $xtag $cid") == 0
 			or die "Cannot create tag $xtag: $!\n";
-		print C "$tagobj\n"
-			or die "Cannot write tag $xtag: $!\n";
-		close(C)
-			or die "Cannot write tag $xtag: $!\n";
 
 		print "Created tag '$xtag' on '$branch'\n" if $opt_v;
 	}
-- 
1.5.2

Re: [PATCH] Use git-tag in git-cvsimport

[Junio C Hamano]

Elvis Pranskevichus <el@prans.net> writes:

> Currently git-cvsimport tries to create tag objects directly via git-mktag
> in a very broken way, e.g the stuff it writes into the tagger field of
> the tag object doesn't really resemble the GIT_COMMITTER_IDENT. This makes
> gitweb and possibly other tools that try to interpret tag objects to be
> confused about tag date and authorship.
>
> Fix this by calling git-tag instead. This also has a nice side effect of
> not creating the tag object but only the lightweight tag as that's the only
> thing CVS has anyways.
>
> Signed-off-by: Elvis Pranskevichus <el@prans.net>

This sounds very sane, although I have not thought through the
possible ramifications.

>  git-cvsimport.perl |   26 ++------------------------
>  1 files changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index f68afe7..d5ca66b 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -771,31 +771,9 @@ sub commit {
>  		$xtag =~ s/\s+\*\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **
>  		$xtag =~ tr/_/\./ if ( $opt_u );
>  		$xtag =~ s/[\/]/$opt_s/g;
> - ...
> +
> +		system("git-tag $xtag $cid") == 0
>  			or die "Cannot create tag $xtag: $!\n";
> - ...
>  
>  		print "Created tag '$xtag' on '$branch'\n" if $opt_v;
>  	}
> -- 
> 1.5.2

Other than that I would write the "system" in a slightly newer
style, i.e.

	system('git-tag', $xtag, $cid)

I do not think of any obvious downside, either in the code nor
the change to use unannotated tag.

Anybody on the list see downsides with this?

Re: [PATCH] Use git-tag in git-cvsimport

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

hoi :)

On Sun, Jun 03, 2007 at 02:56:36AM -0400, Elvis Pranskevichus wrote:
> Fix this by calling git-tag instead. This also has a nice side effect of
> not creating the tag object but only the lightweight tag as that's the only
> thing CVS has anyways.

but lightweight tags are not fetched by default.
And only leaving them in the repository that actually did the cvsimport
makes them much less valuable.

> +
> +		system("git-tag $xtag $cid") == 0

I suggest adding something like -m "import label from CVS".

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Use git-tag in git-cvsimport

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> writes:

> hoi :)
>
> On Sun, Jun 03, 2007 at 02:56:36AM -0400, Elvis Pranskevichus wrote:
>> Fix this by calling git-tag instead. This also has a nice side effect of
>> not creating the tag object but only the lightweight tag as that's the only
>> thing CVS has anyways.
>
> but lightweight tags are not fetched by default.

Are you sure about that?

Re: [PATCH] Use git-tag in git-cvsimport

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]

hoi :)

On Sun, Jun 03, 2007 at 05:01:03PM -0700, Junio C Hamano wrote:
> Martin Waitz <tali@admingilde.org> writes:
> > but lightweight tags are not fetched by default.
> 
> Are you sure about that?

not any more now that you questioned it ;-)

But at least there is a hook script which refuses to receive
un-annotated tags and I always considered those tags to be temporary
tags in the local repository.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Use git-tag in git-cvsimport

[Elvis Pranskevichus]

> hoi :)

Hi =)

> On Sun, Jun 03, 2007 at 05:01:03PM -0700, Junio C Hamano wrote:
> > Martin Waitz <tali@admingilde.org> writes:
> > > but lightweight tags are not fetched by default.
> > 
> > Are you sure about that?

> not any more now that you questioned it ;-)

Last time I checked, unannotated tags are git-cloned and git-fetched just 
fine. I'm not sure about the exact definition and behaviour of lightweight 
tags, though. 

CVS just doesn't attach any valuable info to the tags, it's just a point in 
time.

> But at least there is a hook script which refuses to receive
> un-annotated tags and I always considered those tags to be temporary
> tags in the local repository.

Well, there's no mention about that in the docs. And I don't think that the 
notion of git losing valid objects along the way is the good one =)

Anyways, the patch wasn't about the tag type change. As I mentioned it's just 
a side effect. The main point is to fix the cvsimport tag breakage. I've 
tested that change on a few big (and messy) CVS repos, and it works just 
fine.

Cheers,
-- 

                 Elvis

Re: [PATCH] Use git-tag in git-cvsimport

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> writes:

> On Sun, Jun 03, 2007 at 05:01:03PM -0700, Junio C Hamano wrote:
>> Martin Waitz <tali@admingilde.org> writes:
>> > but lightweight tags are not fetched by default.
>> 
>> Are you sure about that?
>
> not any more now that you questioned it ;-)
>
> But at least there is a hook script which refuses to receive
> un-annotated tags and I always considered those tags to be temporary
> tags in the local repository.

I'll take the patch to 'next' and see if anybody screams.  We
can add the '-m "Label from CVS"' bit easily if annotated tags
turns out to be easier for people before it graduates to
'master', although I suspect we probably do not have to.