Re: [PATCH] cvsimport: strip all inappropriate tag strings

From: Alex Vandiver <alex@chmrr.net>
Date: Sep 5, 2012, 06:44 UTC
Message-ID: <1346827469.1137.22.camel@umgah.localdomain>
In-Reply-To: <1346819164-8116-1-git-send-email-ktdreyer@ktdreyer.com>

On Tue, 2012-09-04 at 22:26 -0600, Ken Dreyer wrote:

Show 32 quoted lines

> When importing CVS tags, strip all the inappropriate strings from the
> tag names as we translate them to git tag names.
>
> [snip]
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 8d41610..0dc598d 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -889,7 +889,25 @@ sub commit {
>  		$xtag =~ s/\s+\*\*.*$//; # Remove stuff like ** INVALID ** and ** FUNKY **
>  		$xtag =~ tr/_/\./ if ( $opt_u );
>  		$xtag =~ s/[\/]/$opt_s/g;
> -		$xtag =~ s/\[//g;
> +
> +		# See ref.c for these rules.
> +		# Tag cannot end with a '/' - this is already handled above.
> +		# Tag cannot contain bad chars. See bad_ref_char in ref.c.
> +		$xtag =~ s/[ ~\^:\\\*\?\[]//g;
> +		# Tag cannot contain '..'.
> +		$xtag =~ s/\.\.//g;
> +		# Tag cannot contain '@{'.
> +		$xtag =~ s/\@{//g;
> +		# Tag cannot end with '.lock'.
> +		$xtag =~ s/(?:\.lock)+$//;
> +		# Tag cannot begin or end with '.'.
> +		$xtag =~ s/^\.+//;
> +		$xtag =~ s/\.+$//;
> +		# Tag cannot consist of a single '.' - already handled above.
> +		# Tag cannot be empty.
> +		if ($xtag eq '') {
> +			return;
> +		}

Unfortunately, this isn't quite sufficient. Consider the case of a tag named "foo.lock." The .lock rule doesn't match, because it's not at the end of the string -- but after s/\.+$// runs, it _is_ at the end, and hence invalid. A similar problem exists with a tag named "a.@{.b", given the ordering of @{ and .. removal.

Something like the following would suffice:

    1 while $xtag =~ s/
               (?: \.\.        # Tag cannot contain '..'.
               |   \@{         # Tag cannot contain '@{'.
               |   \.lock $    # Tag cannot end with '.lock'.
               | ^ \.          # Tag cannot begin...
               |   \. $        # ...or end with '.'
               )//xg;

 - Alex