Re: [PATCH] cvsserver: fix legacy cvs client and branch rev issues

[Frank Lichtenheld <frank@lichtenheld.de>]

Hi.

On Sat, Jun 16, 2007 at 07:50:06PM +0100, Dirk Koopman wrote:
> Early cvs clients don't cause state->{args} to be initialised,
> so force this to occur.
> Some revision checking code assumes that revisions will be
> recognisably numeric to perl, Branches are not, because they
> have more decimal points (eg 1.2.3.4 instead of just 1.2). 
> ---
>  git-cvsserver.perl |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index 5cbf27e..0a4b75e 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -1813,11 +1813,14 @@ sub req_annotate
>  # the second is $state->{files} which is everything after it.
>  sub argsplit
>  {
> +    $state->{args} = [];        # need this here because later code depends on it
> +                                # and for some reason earlier versions of CVS don't
> +                                # satisfy the next condition on plain 'cvs update'
> +
>      return unless( defined($state->{arguments}) and ref $state->{arguments} eq "ARRAY" );
>  
>      my $type = shift;
>  
> -    $state->{args} = [];
>      $state->{files} = [];
>      $state->{opt} = {};
>  

I just would move all the initializations up there. And I think the
comment is really unnecessary. Will prepare a replacement patch.

> @@ -1906,11 +1909,13 @@ sub argsfromdir
>  
>      # push added files
>      foreach my $file (keys %{$state->{entries}}) {
> -	if ( exists $state->{entries}{$file}{revision} &&
> -		$state->{entries}{$file}{revision} == 0 )
> -	{
> -	    push @gethead, { name => $file, filehash => 'added' };
> -	}
> +        # remember that revisions could be on branches 1.2.3.4[.5.6..]
> +        # not just a recogisable "numeric" 1.2
> +        if ( exists $state->{entries}{$file}{revision} &&
> +             !$state->{entries}{$file}{revision} )
> +        {
> +            push @gethead, { name => $file, filehash => 'added' };
> +        }
>      }
>  
>      if ( scalar(@{$state->{args}}) == 1 )

Hmm, I don't see how you could have a problem with that since cvsserver
doesn't support branches and never generates any revision numbers in
that format?

There is probably much more code out there in cvsserver that does assume
that revision is always a simple integer.

And again that comment is a but much IMHO.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/