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

[Alex Vandiver <alex@chmrr.net>]

On Tue, 2012-09-04 at 22:26 -0600, Ken Dreyer wrote:
> 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