[PATCH] New files in git weren't being downloaded during CVS update

If a repository was checked out via git-cvsserver and then later a new
file is added to the git repository via some other method; a CVS update
wasn't fetching the new file.

It would be reported as a new file as
 A some/dir/newfile.c
but would never appear in the directory.

The problem (I think) is that when git-cvsserver detected a new file, it
was issuing the new file message then skipping the actual file send part
and moving to the next file its list.  In fact only an updated file
would be transmitted.

The fix is to make the added file section identical to the udpated file
section.  This additionally makes git-cvsserver behave like a
traditional CVS server and will now output
 U some/dir/newfile.c
for an added file.

Signed-off-by: Andy Parkins <redacted>
---
This is in response to Simon Schubert's suggestion that T_ADDED is an
inappropriate category for a remotely added file.  Instead this treats
remotely added files the same as remotely changed files.

 git-cvsserver.perl |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index a33a876..501c182 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl

@@ -879,11 +879,11 @@ sub req_update
 	    elsif ( !defined($wrev) || $wrev == 0 )
 	    {
 	        $log->info("Tell the client the file will be added");
-		print "MT text A \n";
-                print "MT fname $filename\n";
-                print "MT newline\n";
-		next;
-
+			print "MT +updated\n";
+			print "MT text U \n";
+			print "MT fname $filename\n";
+			print "MT newline\n";
+			print "MT -updated\n";
 	    }
 	    else {
                 $log->info("Updating '$filename' $wrev");

-- 
1.5.0.rc1.gf4b6c

On Sunday 2007 January 21 14:25, Andy Parkins wrote:

This is in response to Simon Schubert's suggestion that T_ADDED is an
inappropriate category for a remotely added file.  Instead this treats
remotely added files the same as remotely changed files.

I'm still concerned that this fix is not right.  Having thought more about it, 
I suspect that this breaks the other "added-file" case, where the file is 
added locally but not remotely.

There are three key cases:
 1. File present locally and remotely.  Modified remotely.  Response "U"
 2. File present locally only.  Response should be "A"
 3. File present remotely only.  Response should be "U"

I think the real problem is that both 2 and 3 are being handled in the same 
place.  Hence, my patch, which has fixed case 3; will have broken case 2.

I need a bit of confirmation from Martin; but I suspect that the correct fix 
is something like this:

    elsif ( (!defined($wrev) || $wrev == 0) && !defined($meta->{revision})
    {
        $log->info("Tell the client the file will be added");

If I'm correct, this would only run the added section when there is no 
matching revision in the repository - this would be case 2.  Then case 3 
would be handled as the else to this if, which handles every other case.

My testing of this works, but I'd like confirmation that I'm right in this 
thinking?  Patch to follow...


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

If a repository was checked out via git-cvsserver and then later a new
file is added to the git repository via some other method; a CVS update
wasn't fetching the new file.

It would be reported as a new file as
 A some/dir/newfile.c
but would never appear in the directory.

The problem seems to be that git-cvsserver was treating these two cases
identically, as "A" type results.

1. New file in repository
2. New file locally

In fact, traditionally, case 1 is treated as a "U" result, and case 2
only is treated as an "A" result.  "A", should just report that the file
is added locally and then skip that file during an update as their is
(of course) nothing to send.

In both these cases there is no working revision, so the checking for
"is there no working revision" will return true.  The test for case 2
needs refining to say "if there is no working revision and no upstream
revision".  This patch does just that, leaving case 1 to be handled by
the normal "U" handler.

I've also updated the log message to more accurately describe the
operation.  i.e. that "A" means that content is scheduled for addition;
not that it actually has been added.

Signed-off-by: Andy Parkins <redacted>
---
 git-cvsserver.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index a33a876..2479f71 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl

@@ -876,9 +876,9 @@ sub req_update
                 print "MT newline\n";
 		next;
 	    }
-	    elsif ( !defined($wrev) || $wrev == 0 )
+	    elsif ( (!defined($wrev) || $wrev == 0) && (!defined($meta->{revision}) || $meta->{revision} == 0) )
 	    {
-	        $log->info("Tell the client the file will be added");
+	        $log->info("Tell the client the file is scheduled for addition");
 		print "MT text A \n";
                 print "MT fname $filename\n";
                 print "MT newline\n";

-- 
1.5.0.rc2.g3f8f2

If a repository was checked out via git-cvsserver and then later a new
file is added to the git repository via some other method; a CVS update
wasn't fetching the new file.

It would be reported as a new file as
 A some/dir/newfile.c
but would never appear in the directory.

The problem seems to be that git-cvsserver was treating these two cases
identically, as "A" type results.

1. New file in repository
2. New file locally

In fact, traditionally, case 1 is treated as a "U" result, and case 2
only is treated as an "A" result.  "A", should just report that the file
is added locally and then skip that file during an update as there is
(of course) nothing to send.

In both these cases there is no working revision, so the checking for
"is there no working revision" will return true.  The test for case 2
needs refining to say "if there is no working revision and no upstream
revision".  This patch does just that, leaving case 1 to be handled by
the normal "U" handler.

I've also updated the log message to more accurately describe the
operation.  i.e. that "A" means that content is scheduled for addition;
not that it actually has been added.

Signed-off-by: Andy Parkins <redacted>
---

Apologies; this one is the real replacement.  The previous patch didn't
include the correction to the update log message which would potentially
use an undefined variable when adding new content.

 git-cvsserver.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index a33a876..e18e901 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl

@@ -876,9 +876,9 @@ sub req_update
                 print "MT newline\n";
 		next;
 	    }
-	    elsif ( !defined($wrev) || $wrev == 0 )
+	    elsif ( (!defined($wrev) || $wrev == 0) && (!defined($meta->{revision}) || $meta->{revision} == 0) )
 	    {
-	        $log->info("Tell the client the file will be added");
+	        $log->info("Tell the client the file is scheduled for addition");
 		print "MT text A \n";
                 print "MT fname $filename\n";
                 print "MT newline\n";

@@ -886,7 +886,7 @@ sub req_update
 
 	    }
 	    else {
-                $log->info("Updating '$filename' $wrev");
+                $log->info("Updating '$filename' to ".$meta->{revision});
                 print "MT +updated\n";
                 print "MT text U \n";
                 print "MT fname $filename\n";

-- 
1.5.0.rc2.g3f8f2

Andy Parkins wrote:

This is nice but probably does not handle a collision (cvs output: file was added by a third party) well and will probably overwrite the locally added file.  however it is a step in the right direction.  from skimming the code I couldn't find collision handling anyways.

cheers
  simon

-- 
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \