[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35

test branch puppet-dev at googlegroups.com
Wed Jul 14 10:34:11 UTC 2010


The following commit has been merged in the upstream branch:
commit a6b52bb01b0c73d36d6b40c263e7542cb5cfbfff
Author: David Schmitt <david at dasz.at>
Date:   Tue May 18 16:33:47 2010 +0200

    Avoid trying to lock on non-files
    
    This is not supported on windows and makes little sense on POSIX

diff --git a/lib/puppet/util/file_locking.rb b/lib/puppet/util/file_locking.rb
index 101ece3..f1f02be 100644
--- a/lib/puppet/util/file_locking.rb
+++ b/lib/puppet/util/file_locking.rb
@@ -5,6 +5,7 @@ module Puppet::Util::FileLocking
 
     # Create a shared lock for reading
     def readlock(file)
+        raise ArgumentError, "%s is not a file" % file unless !File.exists?(file) or File.file?(file)
         Puppet::Util.sync(file).synchronize(Sync::SH) do
             File.open(file) { |f|
                 f.lock_shared { |lf| yield lf }
@@ -18,6 +19,7 @@ module Puppet::Util::FileLocking
         unless FileTest.directory?(File.dirname(file))
             raise Puppet::DevError, "Cannot create %s; directory %s does not exist" % [file, File.dirname(file)]
         end
+        raise ArgumentError, "%s is not a file" % file unless !File.exists?(file) or File.file?(file)
         tmpfile = file + ".tmp"
 
         unless mode
diff --git a/spec/unit/util/file_locking.rb b/spec/unit/util/file_locking.rb
index 70003fa..35e7f78 100755
--- a/spec/unit/util/file_locking.rb
+++ b/spec/unit/util/file_locking.rb
@@ -26,18 +26,23 @@ describe Puppet::Util::FileLocking do
     end
 
     describe "when acquiring a read lock" do
+        before do
+            File.stubs(:exists?).with('/file').returns true
+            File.stubs(:file?).with('/file').returns true
+        end
+        
         it "should use a global shared mutex" do
-            sync = mock 'sync'
-            sync.expects(:synchronize).with(Sync::SH)
-            Puppet::Util.expects(:sync).with("/file").returns sync
+            @sync = mock 'sync'
+            @sync.expects(:synchronize).with(Sync::SH).once
+            Puppet::Util.expects(:sync).with('/file').returns @sync
 
             Puppet::Util::FileLocking.readlock '/file'
         end
 
         it "should use a shared lock on the file" do
-            sync = mock 'sync'
-            sync.expects(:synchronize).yields
-            Puppet::Util.expects(:sync).with("/file").returns sync
+            @sync = mock 'sync'
+            @sync.stubs(:synchronize).yields
+            Puppet::Util.expects(:sync).with('/file').returns @sync
 
             fh = mock 'filehandle'
             File.expects(:open).with("/file").yields fh
@@ -47,6 +52,22 @@ describe Puppet::Util::FileLocking do
             Puppet::Util::FileLocking.readlock('/file') { |l| result = l }
             result.should == "locked_fh"
         end
+
+        it "should only work on regular files" do
+            File.expects(:file?).with('/file').returns false
+            proc { Puppet::Util::FileLocking.readlock('/file') }.should raise_error(ArgumentError)
+        end
+
+        it "should create missing files" do
+            @sync = mock 'sync'
+            @sync.stubs(:synchronize).yields
+            Puppet::Util.expects(:sync).with('/file').returns @sync
+
+            File.expects(:exists?).with('/file').returns false
+            File.expects(:open).with('/file').once
+
+            Puppet::Util::FileLocking.readlock('/file') 
+        end
     end
 
     describe "when acquiring a write lock" do
@@ -54,10 +75,14 @@ describe Puppet::Util::FileLocking do
             @sync = mock 'sync'
             Puppet::Util.stubs(:sync).returns @sync
             @sync.stubs(:synchronize).yields
+            File.stubs(:file?).with('/file').returns true
+            File.stubs(:exists?).with('/file').returns true
         end
 
         it "should fail if the parent directory does not exist" do
             FileTest.expects(:directory?).with("/my/dir").returns false
+            File.stubs(:file?).with('/my/dir/file').returns true
+            File.stubs(:exists?).with('/my/dir/file').returns true
 
             lambda { Puppet::Util::FileLocking.writelock('/my/dir/file') }.should raise_error(Puppet::DevError)
         end
@@ -111,5 +136,21 @@ describe Puppet::Util::FileLocking do
                 f.print "foo"
             end
         end
+
+        it "should only work on regular files" do
+            File.expects(:file?).with('/file').returns false
+            proc { Puppet::Util::FileLocking.writelock('/file') }.should raise_error(ArgumentError)
+        end
+
+        it "should create missing files" do
+            @sync = mock 'sync'
+            @sync.stubs(:synchronize).yields
+            Puppet::Util.expects(:sync).with('/file').returns @sync
+
+            File.expects(:exists?).with('/file').returns false
+            File.expects(:open).with('/file', 'w', 0600).once
+
+            Puppet::Util::FileLocking.writelock('/file') 
+        end
     end
 end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list