Re: cvsserver bug

Previous thread: [BUG] 'stg add FILE' when FILE is a symlink to dir adds dir contents by Tomash Brechko on Wednesday, April 11, 2007 - 8:54 am. (3 messages)

Next thread: [PATCH 1/2] simple random data generator for tests by Nicolas Pitre on Wednesday, April 11, 2007 - 10:33 am. (4 messages)
From: Daniel Barkalow
Subject: cvsserver bug
Date: Wednesday, April 11, 2007 - 9:10 am

It seems like git-cvsserver doesn't know the CVS special case that, if the 
client has removed the file from the working directory (but not called 
"cvs remove"), this means to revert it to the server's version. I think 
that the condition around line 843 needs to exclude this case, and it 
needs to get to line 892 instead, but I can't even fake perl to fix it.

	-Daniel
*This .sig left intentionally blank*
-

From: Frank Lichtenheld
Date: Wednesday, April 11, 2007 - 12:43 pm

I can confirm the bug. Will look into it a bit and see whether I can
come up with a fix.

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

From: Frank Lichtenheld
Date: Wednesday, April 11, 2007 - 1:38 pm

Only send a modified response if the client sent a
"Modified" entry. This fixes the case where the
file was locally deleted on the client without
being removed from CVS. In this case the client
will only have sent the Entry for the file but nothing
else.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
 git-cvsserver.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

 We really, really need a test suite for cvsserver...
 I've tested this as good for regressions as I could
 think of but am still unsure about it.

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 68aa752..25816c5 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -843,6 +843,7 @@ sub req_update
         if ( defined ( $wrev )
              and defined($meta->{revision})
              and $wrev == $meta->{revision}
+             and defined($state->{entries}{$filename}{modified_hash})
              and not exists ( $state->{opt}{C} ) )
         {
             $log->info("Tell the client the file is modified");
-- 
1.5.1

-

From: Junio C Hamano
Date: Wednesday, April 11, 2007 - 2:09 pm

Thanks.

-

From: Junio C Hamano
Date: Wednesday, April 11, 2007 - 2:36 pm

This would make the modified response go away, but would it
cause a fresh re-checkout to happen?

-

From: Daniel Barkalow
Date: Wednesday, April 11, 2007 - 2:39 pm

It falls through into the appropriate case, evidently. At least, it seems 
to resolve my issue.

	-Daniel
*This .sig left intentionally blank*
-

From: Frank Lichtenheld
Date: Wednesday, April 11, 2007 - 2:42 pm

Yeah, the rest of the code handles this case correctly if
it actually gets the chance to do so.

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

From: Martin Langhoff
Date: Wednesday, April 11, 2007 - 2:52 pm

Ack. With this, git-cvsserver matches cvs's behaviour. The only
difference is that cvs also spits out

  E cvs update: warning: $path was lost

cheers,


martin
-- 
-----------------------------------------------------------------------
Martin @ Catalyst .Net .NZ  Ltd, PO Box 11-053, Manners St,  Wellington
WEB: http://catalyst.net.nz/           PHYS: Level 2, 150-154 Willis St
OFFICE: +64(4)916-7224  UK: 0845 868 5733 ext 7224  MOB: +64(21)364-017
      Make things as simple as possible, but no simpler - Einstein
-----------------------------------------------------------------------
-

From: Daniel Barkalow
Date: Wednesday, April 11, 2007 - 3:02 pm

My cvs client says:

cvs update: warning: $path unexpectedly disappeared

I'm not sure if something suitable is generated client-side in general, 
but mine seems to.

	-Daniel
*This .sig left intentionally blank*
-

Previous thread: [BUG] 'stg add FILE' when FILE is a symlink to dir adds dir contents by Tomash Brechko on Wednesday, April 11, 2007 - 8:54 am. (3 messages)

Next thread: [PATCH 1/2] simple random data generator for tests by Nicolas Pitre on Wednesday, April 11, 2007 - 10:33 am. (4 messages)