This changes the condition checking of handlebucket, as well as
moves it (and remove_backup) into a separate module.  It
additionally refactors common code out of handlebucket into
a separate (now private) method.

All spec tests still pass; however, test/ral/type/file.rb:696 fails.

Signed-off-by: Steven Jenkins <[email protected]>
---
 lib/puppet/type/file.rb    |  126 +-------------------------------------------
 lib/puppet/util/backups.rb |  110 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 124 deletions(-)
 create mode 100644 lib/puppet/util/backups.rb

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 55d4ec7..657c7ab 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -7,11 +7,13 @@ require 'puppet/network/handler'
 require 'puppet/util/diff'
 require 'puppet/util/checksums'
 require 'puppet/network/client'
+require 'puppet/util/backups'
 
 module Puppet
     newtype(:file) do
         include Puppet::Util::MethodHelper
         include Puppet::Util::Checksums
+        include Puppet::Util::Backups
         @doc = "Manages local files, including setting ownership and
             permissions, creation of both files and directories, and
             retrieving entire files from remote servers.  As Puppet matures, it
@@ -391,103 +393,6 @@ module Puppet
             @stat = nil
         end
 
-        # Deal with backups.
-        def handlebackup(file = nil)
-            # let the path be specified
-            file ||= self[:path]
-            # if they specifically don't want a backup, then just say
-            # we're good
-            unless FileTest.exists?(file)
-                return true
-            end
-
-            unless self[:backup]
-                return true
-            end
-
-            case File.stat(file).ftype
-            when "directory"
-                if self[:recurse]
-                    # we don't need to backup directories when recurse is on
-                    return true
-                else
-                    backup = self.bucket || self[:backup]
-                    case backup
-                    when Puppet::Network::Client.client(:Dipper)
-                        notice "Recursively backing up to filebucket"
-                        require 'find'
-                        Find.find(self[:path]) do |f|
-                            if File.file?(f)
-                                sum = backup.backup(f)
-                                self.notice "Filebucketed %s to %s with sum 
%s" %
-                                    [f, backup.name, sum]
-                            end
-                        end
-
-                        return true
-                    when String
-                        newfile = file + backup
-                        # Just move it, since it's a directory.
-                        if FileTest.exists?(newfile)
-                            remove_backup(newfile)
-                        end
-                        begin
-                            bfile = file + backup
-
-                            # Ruby 1.8.1 requires the 'preserve' addition, but
-                            # later versions do not appear to require it.
-                            FileUtils.cp_r(file, bfile, :preserve => true)
-                            return true
-                        rescue => detail
-                            # since they said they want a backup, let's error 
out
-                            # if we couldn't make one
-                            self.fail "Could not back %s up: %s" %
-                                [file, detail.message]
-                        end
-                    else
-                        self.err "Invalid backup type %s" % backup.inspect
-                        return false
-                    end
-                end
-            when "file"
-                backup = self.bucket || self[:backup]
-                case backup
-                when Puppet::Network::Client.client(:Dipper)
-                    sum = backup.backup(file)
-                    self.notice "Filebucketed to %s with sum %s" %
-                        [backup.name, sum]
-                    return true
-                when String
-                    newfile = file + backup
-                    if FileTest.exists?(newfile)
-                        remove_backup(newfile)
-                    end
-                    begin
-                        # FIXME Shouldn't this just use a Puppet object with
-                        # 'source' specified?
-                        bfile = file + backup
-
-                        # Ruby 1.8.1 requires the 'preserve' addition, but
-                        # later versions do not appear to require it.
-                        FileUtils.cp(file, bfile, :preserve => true)
-                        return true
-                    rescue => detail
-                        # since they said they want a backup, let's error out
-                        # if we couldn't make one
-                        self.fail "Could not back %s up: %s" %
-                            [file, detail.message]
-                    end
-                else
-                    self.err "Invalid backup type %s" % backup.inspect
-                    return false
-                end
-            when "link"; return true
-            else
-                self.notice "Cannot backup files of type %s" % 
File.stat(file).ftype
-                return false
-            end
-        end
-
         def initialize(hash)
             # Used for caching clients
             @clients = {}
@@ -668,33 +573,6 @@ module Puppet
             Puppet::FileServing::Metadata.search(path, :links => self[:links], 
:recurse => (self[:recurse] == :remote ? true : self[:recurse]), :recurselimit 
=> self[:recurselimit], :ignore => self[:ignore])
         end
 
-        # Remove the old backup.
-        def remove_backup(newfile)
-            if self.class.name == :file and self[:links] != :follow
-                method = :lstat
-            else
-                method = :stat
-            end
-            old = File.send(method, newfile).ftype
-
-            if old == "directory"
-                raise Puppet::Error,
-                    "Will not remove directory backup %s; use a filebucket" %
-                    newfile
-            end
-
-            info "Removing old backup of type %s" %
-                File.send(method, newfile).ftype
-
-            begin
-                File.unlink(newfile)
-            rescue => detail
-                puts detail.backtrace if Puppet[:trace]
-                self.err "Could not remove old backup: %s" % detail
-                return false
-            end
-        end
-
         # Remove any existing data.  This is only used when dealing with
         # links or directories.
         def remove_existing(should)
diff --git a/lib/puppet/util/backups.rb b/lib/puppet/util/backups.rb
new file mode 100644
index 0000000..89b0505
--- /dev/null
+++ b/lib/puppet/util/backups.rb
@@ -0,0 +1,110 @@
+module Puppet::Util::Backups
+
+        # Deal with backups.
+        def handlebackup(file = nil)
+            # let the path be specified
+            file ||= self[:path]
+            # if they specifically don't want a backup, then just say
+            # we're good
+            unless FileTest.exists?(file)
+                return true
+            end
+
+            unless self[:backup]
+                return true
+            end
+
+            backup = self.bucket || self[:backup]
+            case File.stat(file).ftype
+            when "directory"
+                # we don't need to backup directories when recurse is on
+               return true if self[:recurse]
+
+               if self.bucket
+                    notice "Recursively backing up to filebucket"
+                    require 'find'
+                    Find.find(self[:path]) do |f|
+                        if File.file?(f)
+                            sum = backup.backup(f)
+                            self.notice "Filebucketed %s to %s with sum %s" %
+                                [f, backup.name, sum]
+                        end
+                    end
+
+                    return true
+               elsif self[:backup]
+                   handlebackuplocal(file)
+                else
+                    self.err "Invalid backup type %s" % backup.inspect
+                    return false
+                end
+            when "file"
+               if self.bucket
+                    sum = backup.backup(file)
+                    self.notice "Filebucketed to %s with sum %s" %
+                        [backup.name, sum]
+                    return true
+                elsif self[:backup]
+                   handlebackuplocal(file)
+                else
+                    self.err "Invalid backup type %s" % backup.inspect
+                    return false
+                end
+            when "link"; return true
+            else
+                self.notice "Cannot backup files of type %s" % 
File.stat(file).ftype
+                return false
+            end
+        end
+
+private
+
+       def remove_backup(newfile)
+            if self.class.name == :file and self[:links] != :follow
+                method = :lstat
+            else
+                method = :stat
+            end
+            old = File.send(method, newfile).ftype
+
+            if old == "directory"
+                raise Puppet::Error,
+                    "Will not remove directory backup %s; use a filebucket" %
+                    newfile
+            end
+
+            info "Removing old backup of type %s" %
+                File.send(method, newfile).ftype
+
+            begin
+                File.unlink(newfile)
+            rescue => detail
+                puts detail.backtrace if Puppet[:trace]
+                self.err "Could not remove old backup: %s" % detail
+                return false
+            end
+        end
+
+       def handlelocalbackup(file)
+            newfile = file + backup
+            if FileTest.exists?(newfile)
+                remove_backup(newfile)
+            end
+
+             begin
+               bfile = file + backup
+
+               # Ruby 1.8.1 requires the 'preserve' addition, but
+               # later versions do not appear to require it.
+               # N.B. cp_r works on both files and directories
+               FileUtils.cp_r(file, bfile, :preserve => true)
+               return true
+              rescue => detail
+                # since they said they want a backup, let's error out
+                # if we couldn't make one
+                self.fail "Could not back %s up: %s" %
+                    [file, detail.message]
+              end
+       end
+
+end
-- 
1.6.3.3


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to