okay, last try on sources hopefully -- no failures, and basic functionality all seems to be working

git-svn-id: https://reductivelabs.com/svn/puppet/library/trunk@439 980ebf18-57e1-0310-9a29-db15c13687c0

7 files changed, 117 insertions, 172 deletions

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 8766460ea..dcb9adc36 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -54,7 +54,7 @@ class Transaction
                 begin
                     # use an array, so that changes can return more than one
                     # event if they want
-                    events = [change.forward].flatten
+                    events = [change.forward].flatten.reject { |e| e.nil? }
                     #@@changed.push change.state.parent
                 rescue => detail
                     Puppet.err("%s failed: %s" % [change,detail])
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 2e0fd3a63..40c276a99 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -322,7 +322,8 @@ class Type < Puppet::Element
             name = stateklass.name
             if @validstates.include?(name) 
                 if @validstates[name] != stateklass
-                    raise "Redefining state %s(%s) in %s" % [name,stateklass,self]
+                    raise Puppet::Error.new("Redefining state %s(%s) in %s" %
+                        [name,stateklass,self])
                 else
                     # it's already there, so don't bother
                 end
@@ -531,7 +532,7 @@ class Type < Puppet::Element
         else
             @managed = false
             states.each { |state|
-                if state.should
+                if state.should and ! state.class.unmanaged
                     @managed = true
                 end
             }
@@ -784,10 +785,6 @@ class Type < Puppet::Element
             @evalcount = 0
         end
         @@retrieved[self] += 1
-        if self.name =~ /e\/dav_fs.load/ and @@retrieved[self] > 1
-            Puppet.notice "%s(%s) %s" %
-                [@@retrieved[self], @evalcount, self.path.join(":")]
-        end
         # if we're a metaclass and we've already evaluated once...
         #if self.metaclass and @evalcount > 0
         #    return
@@ -807,19 +804,13 @@ class Type < Puppet::Element
         #end
 
         # this only operates on states, not states + children
-        #self.retrieve
-        #unless self.insync?
+        # it's important that we call retrieve() on the type instance,
+        # not directly on the state, because it allows the type to override
+        # the method, like pfile does
+        self.retrieve
 
         # states() is a private method, returning an ordered list
-        changes << states().each { |state|
-            @@retrieved[state] += 1
-            #if self.name =~ /e\/dav_fs.load/
-            #    Puppet.notice "%s %s" % [@@retrieved[state], state.path]
-            #end
-            #unless @@retrieved[state] > 0
-                state.retrieve
-            #end
-        }.find_all { |state|
+        changes << states().find_all { |state|
             ! state.insync?
         }.collect { |state|
             Puppet::StateChange.new(state)
@@ -852,6 +843,9 @@ class Type < Puppet::Element
         if changes.length > 0
             Puppet.info "%s: %s change(s)" %
                 [self.name, changes.length]
+            #changes.each { |change|
+            #    Puppet.debug "change: %s" % change.state.name
+            #}
         end
         return changes.flatten
     end
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 30e46a6d0..5a3392016 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -45,9 +45,6 @@ module Puppet
 
 
             def sync
-                if defined? @synced
-                    Puppet.err "We've already been synced?"
-                end
                 event = nil
                 begin
                     case @should
@@ -68,7 +65,6 @@ module Puppet
                         [@should, detail]
                     raise error
                 end
-                @synced = true
                 return event
             end
         end
@@ -79,6 +75,8 @@ module Puppet
             @name = :checksum
             @event = :file_modified
 
+            @unmanaged = true
+
             def should=(value)
                 @checktype = value
                 state = Puppet::Storage.state(self)
@@ -90,9 +88,10 @@ module Puppet
                     else
                         #Puppet.debug "Found checksum for %s but not of type %s" %
                         #    [@parent[:path],@checktype]
-                        @should = nil
+                        @should = -1
                     end
-                #else
+                else
+                    @should = -1
                     #Puppet.debug "No checksum for %s" % @parent[:path]
                 end
             end
@@ -110,7 +109,7 @@ module Puppet
 
                 sum = ""
                 case @checktype
-                when "md5":
+                when "md5", "md5lite":
                     if FileTest.directory?(@parent[:path])
                         #Puppet.info "Cannot MD5 sum directory %s" %
                         #    @parent[:path]
@@ -123,7 +122,19 @@ module Puppet
                     else
                         begin
                             File.open(@parent[:path]) { |file|
-                                sum = Digest::MD5.hexdigest(file.read)
+                                text = nil
+                                if @checktype == "md5"
+                                    text = file.read
+                                else
+                                    text = file.read(512)
+                                end
+                                if text.nil?
+                                    Puppet.info "Not checksumming empty file %s" %
+                                        @parent.name
+                                    sum = 0
+                                else
+                                    sum = Digest::MD5.hexdigest(text)
+                                end
                             }
                         rescue Errno::EACCES => detail
                             Puppet.notice "Cannot checksum %s: permission denied" %
@@ -135,27 +146,6 @@ module Puppet
                             @parent.delete(self.class.name)
                         end
                     end
-                when "md5lite":
-                    if FileTest.directory?(@parent[:path])
-                        #Puppet.info "Cannot MD5 sum directory %s" %
-                        #    @parent[:path]
-
-                        # because we cannot sum directories, just delete ourselves
-                        # from the file
-                        # is/should so we won't sync
-                        return
-                    else
-                        File.open(@parent[:path]) { |file|
-                            text = file.read(512)
-                            if text.nil?
-                                Puppet.info "Not checksumming empty file %s" %
-                                    @parent.name
-                                sum = 0
-                            else
-                                sum = Digest::MD5.hexdigest(text)
-                            end
-                        }
-                    end
                 when "timestamp","mtime":
                     sum = File.stat(@parent[:path]).mtime.to_s
                 when "time":
@@ -180,14 +170,18 @@ module Puppet
 
                 if @is == -1
                     self.retrieve
-                    Puppet.debug "%s(%s): after refresh, is '%s'" %
-                        [self.class.name,@parent.name,@is]
+                    #Puppet.debug "%s(%s): after refresh, is '%s'" %
+                    #    [self.class.name,@parent.name,@is]
 
                     # if we still can't retrieve a checksum, it means that
                     # the file still doesn't exist
                     if @is == -1
-                        Puppet.warning "File %s does not exist -- cannot checksum" %
-                            @parent.name
+                        # if they're copying, then we won't worry about the file
+                        # not existing yet
+                        unless @parent.state(:copy) or @parent.state(:create)
+                            Puppet.warning "File %s does not exist -- cannot checksum" %
+                                @parent.name
+                        end
                         return nil
                     end
                 end
@@ -234,7 +228,7 @@ module Puppet
                     end
                     Puppet.debug "Replacing checksum %s with %s" %
                         [state[@parent.name][@checktype],@is]
-                    Puppet.debug "@is: %s; @should: %s" % [@is,@should]
+                    #Puppet.debug "@is: %s; @should: %s" % [@is,@should]
                     result = true
                 else
                     Puppet.debug "Creating checksum %s for %s of type %s" %
@@ -422,11 +416,11 @@ module Puppet
                 if @is == -1
                     @parent.stat(true)
                     self.retrieve
-                    Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
+                    #Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
                 unless @parent.stat
-                    Puppet.err "PFile '%s' does not exist; cannot chown" %
+                    Puppet.err "File '%s' does not exist; cannot chown" %
                         @parent[:path]
                 end
 
@@ -498,13 +492,13 @@ module Puppet
                 if @is == -1
                     @parent.stat(true)
                     self.retrieve
-                    Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
+                    #Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
                 unless @parent.stat
-                    Puppet.err "PFile '%s' does not exist; cannot chmod" %
+                    Puppet.err "File '%s' does not exist; cannot chmod" %
                         @parent[:path]
-                    return
+                    return nil
                 end
 
                 unless defined? @fixed
@@ -566,10 +560,10 @@ module Puppet
                 # we probably shouldn't actually modify the 'should' value
                 # but i don't see a good way around it right now
                 # mmmm, should
-                if defined? @should
-                else
-                    @parent.delete(self.name)
-                end
+                #if defined? @should
+                #else
+                #    @parent.delete(self.name)
+                #end
             end
 
             def should=(value)
@@ -632,13 +626,13 @@ module Puppet
                 if @is == -1
                     @parent.stat(true)
                     self.retrieve
-                    Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
+                    #Puppet.debug "%s: after refresh, is '%s'" % [self.class.name,@is]
                 end
 
                 unless @parent.stat
-                    Puppet.err "PFile '%s' does not exist; cannot chgrp" %
+                    Puppet.err "File '%s' does not exist; cannot chgrp" %
                         @parent[:path]
-                    return
+                    return nil
                 end
 
                 begin
@@ -661,10 +655,6 @@ module Puppet
             def retrieve
                 sum = nil
                 if sum = @parent.state(:checksum)
-                    if @parent.name =~ /e\/dav_fs.load/
-                        puts caller
-                        Puppet.notice "Ah: %s" % @parent.name
-                    end
                     if sum.is
                         if sum.is == -1
                             sum.retrieve
@@ -705,21 +695,15 @@ module Puppet
             end
 
             def sync
-                if @is.nil?
-                    Puppet.err "@is is nil"
-                end
                 if @is == -1
                     self.retrieve # try again
                     if @is == @should
                         return nil
-                    else
-                        Puppet.err "@is: %s; @should: %s" % [@is, @should]
                     end
                 end
                 @backed = false
                 bak = @parent[:backup] || ".puppet-bak"
 
-                Puppet.notice "@is: %s; @should: %s" % [@is, @should]
                 # try backing ourself up before we overwrite
                 if FileTest.file?(@parent.name)
                     if bucket = @parent[:filebucket]
@@ -745,6 +729,8 @@ module Puppet
                     @backed = true
                 end
 
+                #Puppet.notice "@is: %s; @should: %s" % [@is,@should]
+                #Puppet.err "@is: %s; @should: %s" % [@is,@should]
                 # okay, we've now got whatever backing up done we might need
                 # so just copy the files over
                 if @local
@@ -814,8 +800,8 @@ module Puppet
                 #Puppet::State::PFileSource,
             @states = [
                 Puppet::State::PFileCreate,
-                Puppet::State::PFileCopy,
                 Puppet::State::PFileChecksum,
+                Puppet::State::PFileCopy,
                 Puppet::State::PFileUID,
                 Puppet::State::PFileGroup,
                 Puppet::State::PFileMode,
@@ -1059,6 +1045,7 @@ module Puppet
             # if recursion is turned off, then this whole thing is pretty easy
             def paramsource=(source)
                 @parameters[:source] = source
+                @arghash.delete(:source)
                 @source = source
 
                 # verify we support the proto
@@ -1088,54 +1075,14 @@ module Puppet
                     return
                 end
 
-#                    # okay, we now have the whole source tree in memory, being modelled
-#                    # now we just need to compare it with what we have, to see
-#                    # if we're missing any files
-#
-#                    # we're assuming that 'paramrecurse=' has already been called
-#
-#                    if FileTest.directory?(@source)
-#                        mkchilds = @sourceobj.reject { |schild|
-#                            schild.is_a?(Puppet::State)
-#                        }.collect { |schild|
-#                            File.basename(schild.name)
-#                        }
-#
-#                        @children.each { |child|
-#                            name = File.basename(child.name)
-#                            if mkchilds.include?(name)
-#                                mkchilds.delete(name)
-#                            end
-#                        }
-#
-#                        # okay, now we know which ones we still need to make
-#                        mkchilds.each { |child|
-#                            Puppet.notice "Making non-existent file %s" % child
-#                            child = self.newchild(child)
-#                            self.push child
-#                        }
-#                    end
-#
-#                    @srcbase = @source
-#
-#                    # now, the sourceobj models the entire tree at once
-#                    # and we've already recursed through what exists locally
-#                end
-#
-#                # Check whether we'll be creating the file or whether it already
-#                # exists.  The root of the destination tree will cause the
-#                # recursive creation of all of the objects, and then all the
-#                # children of the tree will just pull existing objects
-#                unless @sourceobj = self.newsource(@source)
-#                    return
-#                end
-
                 # okay, now we've got the object; retrieve its values, so we
                 # can make them our 'should' values
                 @sourceobj.retrieve
 
                 @@pinparams.each { |state|
                     next if state == :checksum
+                    next if Process.uid != 0 and state == :owner
+                    next if Process.uid != 0 and state == :group
                     unless @states.include?(state)
                         # this copies the source's 'is' value to our 'should'
                         # but doesn't override existing settings
@@ -1157,7 +1104,6 @@ module Puppet
                     sourcesum = @sourceobj.state(:checksum)
                     destsum = @states[:checksum]
 
-                    begin
                     unless destsum.checktype == sourcesum.checktype
                         Puppet.warning(("Source file '%s' checksum type %s is " +
                             "incompatible with destination file '%s' checksum " +
@@ -1184,10 +1130,6 @@ module Puppet
                         end
                         checktype = "md5"
                     end
-                    rescue => detail
-                        Puppet.err detail
-                        exit
-                    end
                 elsif @sourceobj.state(:checksum)
                     checktype = @sourceobj.state(:checksum).checktype
                     self[:checksum] = checktype
@@ -1331,6 +1273,10 @@ module Puppet
                 if @stat.nil? or refresh == true
                     begin
                         @stat = File.stat(self.name)
+                    rescue Errno::ENOENT => error
+                        #Puppet.debug "Failed to stat %s: No such file or directory" %
+                        #    [self.name]
+                        @stat = nil
                     rescue => error
                         Puppet.debug "Failed to stat %s: %s" %
                             [self.name,error]
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index 20e3a234d..716f6336c 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -20,6 +20,10 @@ class State < Puppet::Element
 
     @virtual = true
 
+    class << self
+        attr_accessor :unmanaged
+    end
+
     #---------------------------------------------------------------
     # which event gets generated if this state change happens; not currently
     # called
@@ -77,6 +81,9 @@ class State < Puppet::Element
     def insync?
         #debug "%s value is '%s', should be '%s'" %
         #    [self,self.is.inspect,self.should.inspect]
+        unless defined? @should
+            return true
+        end
         self.is == self.should
     end
     #---------------------------------------------------------------
diff --git a/test/other/tc_transactions.rb b/test/other/tc_transactions.rb
index a1976e12b..06dc8d8e2 100644
--- a/test/other/tc_transactions.rb
+++ b/test/other/tc_transactions.rb
@@ -5,14 +5,30 @@ if __FILE__ == $0
 end
 
 require 'puppet'
+require 'puppettest'
 require 'test/unit'
 
 # $Id$
 
 class TestTransactions < Test::Unit::TestCase
+    def cycle(comp)
+        assert_nothing_raised {
+            trans = comp.evaluate
+        }
+        events = nil
+        assert_nothing_raised {
+            events = trans.evaluate.collect { |e|
+                e.event
+            }
+        }
+        return events
+    end
+
     def setup
+        Puppet::Type.allclear
+        @@tmpfiles = []
         Puppet[:loglevel] = :debug if __FILE__ == $0
-
+        Puppet[:statefile] = "/var/tmp/puppetstate"
         @groups = %x{groups}.chomp.split(/ /)
         unless @groups.length > 1
             p @groups
@@ -21,36 +37,35 @@ class TestTransactions < Test::Unit::TestCase
     end
 
     def teardown
-        assert_nothing_raised() {
-            Puppet::Type.allclear
+        Puppet::Type.allclear
+        @@tmpfiles.each { |file|
+            if FileTest.exists?(file)
+                system("chmod -R 755 %s" % file)
+                system("rm -rf %s" % file)
+            end
         }
-
+        @@tmpfiles.clear
+        system("rm -f %s" % Puppet[:statefile])
         print "\n\n" if Puppet[:debug]
     end
 
-    def newfile
+    def newfile(hash = {})
+        tmpfile = PuppetTestSuite.tempfile()
+        File.open(tmpfile, "w") { |f| f.puts rand(100) }
+        @@tmpfiles.push tmpfile
+        hash[:name] = tmpfile
         assert_nothing_raised() {
-            cfile = File.join($puppetbase,"examples/root/etc/configfile")
-            unless Puppet::Type::PFile.has_key?(cfile)
-                Puppet::Type::PFile.new(
-                    :path => cfile,
-                    :check => [:mode, :owner, :group]
-                )
-            end
-            return Puppet::Type::PFile[cfile]
+            return Puppet::Type::PFile.new(hash)
         }
     end
 
     def newservice
         assert_nothing_raised() {
-            unless Puppet::Type::Service.has_key?("sleeper")
-                Puppet::Type::Service.new(
-                    :name => "sleeper",
-                    :path => File.join($puppetbase,"examples/root/etc/init.d"),
-                    :check => [:running]
-                )
-            end
-            return Puppet::Type::Service["sleeper"]
+            return Puppet::Type::Service.new(
+                :name => "sleeper",
+                :path => File.join($puppetbase,"examples/root/etc/init.d"),
+                :check => [:running]
+            )
         }
     end
 
@@ -69,7 +84,7 @@ class TestTransactions < Test::Unit::TestCase
         return comp
     end
 
-    def test_filetrans
+    def test_filerollback
         transaction = nil
         file = newfile()
         states = {}
@@ -80,10 +95,14 @@ class TestTransactions < Test::Unit::TestCase
             file.retrieve
         }
 
-        check.each { |state|
-            states[state] = file[state]
+        assert_nothing_raised() {
+            check.each { |state|
+                assert(file[state])
+                states[state] = file[state]
+            }
         }
 
+
         component = newcomp("file",file)
         assert_nothing_raised() {
             file[:group] = @groups[1]
@@ -110,7 +129,7 @@ class TestTransactions < Test::Unit::TestCase
 
     def test_servicetrans
         transaction = nil
-        service = newservice
+        service = newservice()
         service[:check] = [:running]
 
         component = newcomp("service",service)
@@ -196,16 +215,6 @@ class TestTransactions < Test::Unit::TestCase
         # restarts
         # XXX i don't have a good way to retrieve this information...
         #assert_equal(1,transaction.triggercount(sub))
-
-        # now set everything back to how it was
-        assert_nothing_raised() {
-            service[:running] = 0
-            service.sync
-            check.each { |state|
-                file[state] = states[state]
-            }
-            file.sync
-        }
     end
 
     def test_twocomps
@@ -268,16 +277,6 @@ class TestTransactions < Test::Unit::TestCase
         # XXX this doesn't work, because the sub is being triggered in
         # a contained transaction, not this one
         #assert_equal(1,transaction.triggercount(sub))
-
-        # now set everything back to how it was
-        assert_nothing_raised() {
-            service[:running] = 0
-            service.sync
-            check.each { |state|
-                file[state] = states[state]
-            }
-            file.sync
-        }
     end
 
 end
diff --git a/test/types/tc_exec.rb b/test/types/tc_exec.rb
index 877078d31..387902e8d 100755
--- a/test/types/tc_exec.rb
+++ b/test/types/tc_exec.rb
@@ -170,9 +170,6 @@ class TestExec < Test::Unit::TestCase
         [file,cmd].each { |obj|
             comp.push obj
         }
-        assert_nothing_raised {
-            trans = comp.evaluate
-        }
         events = nil
         assert_nothing_raised {
             trans = comp.evaluate
@@ -189,8 +186,6 @@ class TestExec < Test::Unit::TestCase
         }
         assert_nothing_raised {
             trans = comp.evaluate
-        }
-        assert_nothing_raised {
             events = trans.evaluate.collect { |event|
                 event.event
             }
diff --git a/test/types/tc_service.rb b/test/types/tc_service.rb
index b2e0f4770..7a4799b24 100644
--- a/test/types/tc_service.rb
+++ b/test/types/tc_service.rb
@@ -31,6 +31,10 @@ class TestService < Test::Unit::TestCase
         }
     end
 
+    def teardown
+        Puppet::Type.allclear
+    end
+
     def test_process_start
         # start it
         assert_nothing_raised() {