git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] Make 'cvs ci' lockless in git-cvsserver by using git-update-ref

From
APAndy Parkins <andyparkins@gmail.com>
Date
Feb 27, 2007, 12:48 UTC
Message-ID
<200702271248.59652.andyparkins@gmail.com>
In-Reply-To
<200702210908.59579.andyparkins@gmail.com>

This makes "ci" codepath lockless by following the usual "remember the tip, do your thing, then compare and swap at the end" update pattern using update-ref. Incidentally, by updating the code that reads where the tip of the head is to use show-ref, it makes it safe to use in a repository whose refs are pack-pruned.

I noticed that other parts of the program are not yet pack-refs safe, but tried to keep the changes to the minimum.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
This patch is actually yours (with one extra removal of lock file reference
that you'd missed, and a change of shortlog), but I don't know how to send
an email that comes from me but attributes authorship to you.
 git-cvsserver.perl |   43 ++++++++++++++++---------------------------
 1 files changed, 16 insertions(+), 27 deletions(-)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 84520e7..8e12f81 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1031,36 +1031,35 @@ sub req_ci
         exit;
     }
 
-    my $lockfile = "$state->{CVSROOT}/refs/heads/$state->{module}.lock";
-    unless ( sysopen(LOCKFILE,$lockfile,O_EXCL|O_CREAT|O_WRONLY) )
-    {
-        $log->warn("lockfile '$lockfile' already exists, please try again");
-        print "error 1 Lock file '$lockfile' already exists, please try again\n";
-        exit;
-    }
-
     # Grab a handle to the SQLite db and do any necessary updates
     my $updater = GITCVS::updater->new($state->{CVSROOT}, $state->{module}, $log);
     $updater->update();
 
     my $tmpdir = tempdir ( DIR => $TEMP_DIR );
     my ( undef, $file_index ) = tempfile ( DIR => $TEMP_DIR, OPEN => 0 );
-    $log->info("Lock successful, basing commit on '$tmpdir', index file is '$file_index'");
+    $log->info("Lockless commit start, basing commit on '$tmpdir', index file is '$file_index'");
 
     $ENV{GIT_DIR} = $state->{CVSROOT} . "/";
     $ENV{GIT_INDEX_FILE} = $file_index;
 
+    # Remember where the head was at the beginning.
+    my $parenthash = `git show-ref -s refs/heads/$state->{module}`;
+    chomp $parenthash;
+    if ($parenthash !~ /^[0-9a-f]{40}$/) {
+	    print "error 1 pserver cannot find the current HEAD of module";
+	    exit;
+    }
+
     chdir $tmpdir;
 
     # populate the temporary index based
-    system("git-read-tree", $state->{module});
+    system("git-read-tree", $parenthash);
     unless ($? == 0)
     {
 	die "Error running git-read-tree $state->{module} $file_index $!";
     }
     $log->info("Created index '$file_index' with for head $state->{module} - exit status $?");
 
-
     my @committedfiles = ();
 
     # foreach file specified on the command line ...
@@ -1095,8 +1094,6 @@ sub req_ci
         {
             # fail everything if an up to date check fails
             print "error 1 Up to date check failed for $filename\n";
-            close LOCKFILE;
-            unlink($lockfile);
             chdir "/";
             exit;
         }
@@ -1139,16 +1136,12 @@ sub req_ci
     {
         print "E No files to commit\n";
         print "ok\n";
-        close LOCKFILE;
-        unlink($lockfile);
         chdir "/";
         return;
     }
 
     my $treehash = `git-write-tree`;
-    my $parenthash = `cat $ENV{GIT_DIR}refs/heads/$state->{module}`;
     chomp $treehash;
-    chomp $parenthash;
 
     $log->debug("Treehash : $treehash, Parenthash : $parenthash");
 
@@ -1165,8 +1158,6 @@ sub req_ci
     {
         $log->warn("Commit failed (Invalid commit hash)");
         print "error 1 Commit failed (unknown reason)\n";
-        close LOCKFILE;
-        unlink($lockfile);
         chdir "/";
         exit;
     }
@@ -1179,14 +1170,17 @@ sub req_ci
 		{
 			$log->warn("Commit failed (update hook declined to update ref)");
 			print "error 1 Commit failed (update hook declined)\n";
-			close LOCKFILE;
-			unlink($lockfile);
 			chdir "/";
 			exit;
 		}
 	}
 
-    print LOCKFILE $commithash;
+	if (system(qw(git update-ref -m), "cvsserver ci",
+			"refs/heads/$state->{module}", $commithash, $parenthash)) {
+		$log->warn("update-ref for $state->{module} failed.");
+		print "error 1 Cannot commit -- update first\n";
+		exit;
+	}
 
     $updater->update();
 
@@ -1215,12 +1209,7 @@ sub req_ci
         }
     }
 
-    close LOCKFILE;
-    my $reffile = "$ENV{GIT_DIR}refs/heads/$state->{module}";
-    unlink($reffile);
-    rename($lockfile, $reffile);
     chdir "/";
-
     print "ok\n";
 }
 
-- 
1.5.0.2.778.gdcb06
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
Previous: Andy ParkinsNext: Jakub Narebski
Message 7 of 39 in “Unresolved issues”
  1. Junio C HamanoFeb 20, 2007
  2. Andy ParkinsFeb 20, 2007
  3. Use git-update-ref to update a ref during commit in git-cvsserverAndy Parkins, Feb 20, 2007
  4. Nicolas PitreFeb 20, 2007
  5. Junio C HamanoFeb 21, 2007
  6. Andy ParkinsFeb 21, 2007
  7. 1/2 Make 'cvs ci' lockless in git-cvsserver by using git-update-refAndy Parkins, Feb 27, 2007
  8. Jakub NarebskiFeb 27, 2007
  9. Nicolas PitreFeb 27, 2007
  10. Junio C HamanoFeb 27, 2007
  11. Andy ParkinsFeb 28, 2007
  12. Junio C HamanoFeb 28, 2007
  13. 2/2 cvsserver: Remove trailing "\n" from commithash in checkin functionAndy Parkins, Feb 27, 2007
  14. Junio C HamanoFeb 27, 2007
  15. Andy ParkinsFeb 28, 2007
  16. Martin LanghoffFeb 27, 2007
  17. Linus TorvaldsFeb 20, 2007
  18. Junio C HamanoFeb 20, 2007
  19. Linus TorvaldsFeb 21, 2007
  20. Junio C HamanoFeb 21, 2007
  21. Johannes SchindelinFeb 21, 2007
  22. Linus TorvaldsFeb 21, 2007
  23. David LangFeb 21, 2007
  24. Johannes SchindelinFeb 21, 2007
  25. Nicolas PitreFeb 21, 2007
  26. Linus TorvaldsFeb 21, 2007
  27. Robin RosenbergFeb 21, 2007
  28. Theodore TsoFeb 21, 2007
  29. Martin WaitzFeb 21, 2007
  30. Johannes SchindelinFeb 21, 2007
  31. Brian GernhardtFeb 21, 2007
  32. Shawn O. PearceFeb 21, 2007
  33. git-status: do not be totally useless in a read-only repository.Junio C Hamano, Feb 22, 2007
  34. update-index: do not die too early in a read-only repository.Junio C Hamano, Feb 22, 2007
  35. Julian PhillipsFeb 26, 2007
  36. Junio C HamanoFeb 26, 2007
  37. Julian PhillipsFeb 26, 2007
  38. Junio C HamanoFeb 26, 2007
  39. Johannes SchindelinFeb 27, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.