threads / patch / 8408

patchUse git-tag in git-cvsimport

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

## tl;dr

7 messages between Jun 3, 2007 and Jun 6, 2007. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Elvis Pranskevichus· Jun 3, 2007, 06:56 UTC · lore

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(-)
Show changes to git-cvsimport.perl +2 −24
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
Junio C Hamano· Jun 3, 2007, 08:37 UTC · re: Elvis Pranskevichus · lore

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

Elvis Pranskevichus <el@prans.net> writes:
Show 11 quoted lines
> 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.

Show 21 quoted lines
>  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?
Elvis Pranskevichus· Jun 4, 2007, 23:55 UTC · re: Junio C Hamano · lore

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

> hoi :)
Hi =)
Show 5 quoted lines
> 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
Martin Waitz· Jun 3, 2007, 22:53 UTC · re: Elvis Pranskevichus · lore

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

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
Junio C Hamano· Jun 4, 2007, 00:01 UTC · re: Martin Waitz · lore

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

Martin Waitz <tali@admingilde.org> writes:
Show 8 quoted lines
> 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?
Martin Waitz· Jun 4, 2007, 07:18 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Jun 6, 2007, 20:41 UTC · re: Martin Waitz · lore

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

Martin Waitz <tali@admingilde.org> writes:
Show 11 quoted lines
> 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.

← back to recent threads