There was a critical design flaw in the link recursion work I did previously, and fixing it required a decently large reorganization. Everything is much, much cleaner now.

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

4 files changed, 131 insertions, 54 deletions

diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index a50f24f75..89e20c067 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1651,8 +1651,8 @@ class Type < Puppet::Element
         # the other states matter.
         changes = []
         if @states.include?(:ensure) and ! @states[:ensure].insync?
-            #self.info "ensuring %s from %s" %
-            #    [@states[:ensure].should, @states[:ensure].is]
+            self.info "ensuring %s from %s" %
+                [@states[:ensure].should, @states[:ensure].is]
             changes = [Puppet::StateChange.new(@states[:ensure])]
         # Else, if the 'ensure' state is correctly absent, then do
         # nothing
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index d77da1942..afe02e879 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -279,7 +279,7 @@ module Puppet
             # We specifically look in @parameters here, because 'linkmaker' isn't
             # a valid attribute for subclasses, so using 'self[:linkmaker]' throws
             # an error.
-            if @parameters.include?(:linkmaker) and
+            if @parameters.include?(:linkmaker) and @parameters[:linkmaker] == true and
                 args.include?(:source) and ! FileTest.directory?(args[:source])
                 klass = Puppet.type(:symlink)
 
@@ -398,7 +398,8 @@ module Puppet
             end
 
             # are we at the end of the recursion?
-            if recurse == 0
+            #if recurse == 0
+            unless self.recurse?
                 self.info "finished recursing"
                 return
             end
@@ -408,7 +409,7 @@ module Puppet
             end
 
             self.localrecurse(recurse)
-            if @states.include?(:ensure) and @states[:ensure].should =~ /^#{File::SEPARATOR}/
+            if @states.include? :target
                 self.linkrecurse(recurse)
             end
             if @states.include?(:source)
@@ -416,9 +417,19 @@ module Puppet
             end
         end
 
+        def recurse?
+            return false unless @parameters.include?(:recurse)
+
+            if @parameters[:recurse].value == true or @parameters[:recurse].value > 0
+                return true
+            else
+                return false
+            end
+        end
+
         # Build a recursive map of a link source
         def linkrecurse(recurse)
-            target = @states[:ensure].should
+            target = @states[:target].should
 
             method = :lstat
             if self[:links] == :follow
@@ -439,6 +450,10 @@ module Puppet
                 return
             end
 
+            # Now that we know our corresponding target is a directory,
+            # change our type
+            self[:ensure] = :directory
+
             unless FileTest.readable? target
                 self.notice "Cannot manage %s: permission denied" % self.name
                 return
@@ -453,19 +468,21 @@ module Puppet
 
             added = []
             children.each do |file|
-                longname = File.join(@states[:ensure].should, file)
-
-                # Files know to create directories when recursion
-                # is enabled and we're making links
-                args = {
-                    :recurse => recurse,
-                    :ensure => longname
-                }
+                Dir.chdir(target) do
+                    longname = File.join(target, file)
+
+                    # Files know to create directories when recursion
+                    # is enabled and we're making links
+                    args = {
+                        :recurse => recurse,
+                        :ensure => longname
+                    }
 
-                if child = self.newchild(file, true, args)
-                    unless @children.include?(child)
-                        self.push child
-                        added.push file
+                    if child = self.newchild(file, true, args)
+                        unless @children.include?(child)
+                            self.push child
+                            added.push file
+                        end
                     end
                 end
             end
@@ -759,6 +776,7 @@ module Puppet
     require 'puppet/type/pfile/uid'
     require 'puppet/type/pfile/group'
     require 'puppet/type/pfile/mode'
+    require 'puppet/type/pfile/target'
     require 'puppet/type/pfile/type'
 end
 # $Id$
diff --git a/lib/puppet/type/pfile/ensure.rb b/lib/puppet/type/pfile/ensure.rb
index 9f04202cb..4287c9382 100755
--- a/lib/puppet/type/pfile/ensure.rb
+++ b/lib/puppet/type/pfile/ensure.rb
@@ -73,40 +73,37 @@ module Puppet
             return :directory_created
         end
 
-        # Symlinks.  We pretty much have to match just about anything,
-        # in order to match relative links.  Somewhat ugly, but eh, it
-        # works.
-        newvalue(/./) do
-            Dir.chdir(File.dirname(@parent[:path])) do
-                target = self.should
-                unless FileTest.exists?(target)
-                    self.debug "Not linking to non-existent '%s'" % target
-                    nil # Grrr, can't return
+
+        newvalue(:link) do
+            if state = @parent.state(:target)
+                state.retrieve
+
+                if state.linkmaker
+                    self.set_directory
                 else
-                    if FileTest.directory?(target) and @parent[:recurse]
-                        # If we're pointing to a directory and recursion is
-                        # enabled, then make a directory instead of a link.
-                        self.set_directory
-                    else
-                        Puppet::Util.asuser(@parent.asuser()) do
-                            mode = @parent.should(:mode)
-                            if mode
-                                Puppet::Util.withumask(000) do
-                                    File.symlink(self.should, @parent[:path])
-                                end
-                            else
-                                File.symlink(self.should, @parent[:path])
-                            end
-                        end
-
-                        # We can't use "return" here because we're in an anonymous
-                        # block.
-                        :link_created
-                    end
+                    state.sync
                 end
+            else
+                self.fail "Cannot create a symlink without a target"
             end
         end
 
+        # Symlinks.
+        newvalue(/./) do
+            # This code never gets executed.  We need the regex to support
+            # specifying it, but the work is done in the 'symlink' code block.
+        end
+
+        munge do |value|
+            value = super(value)
+
+            return value if value.is_a? Symbol
+
+            @parent[:target] = value
+
+            return :link
+        end
+
         # Check that we can actually create anything
         def check
             basedir = File.dirname(@parent[:path])
@@ -123,14 +120,9 @@ module Puppet
         end
 
         def retrieve
+
             if stat = @parent.stat(false)
-                # If we're a link, set the 'is' value to the destination
-                # of the link
-                if stat.ftype == "link"
-                    @is = File.readlink(@parent[:path])
-                else
-                    @is = stat.ftype.intern
-                end
+                @is = stat.ftype.intern
             else
                 if self.should == :false
                     @is = :false
diff --git a/lib/puppet/type/pfile/target.rb b/lib/puppet/type/pfile/target.rb
new file mode 100644
index 000000000..efcb31a82
--- /dev/null
+++ b/lib/puppet/type/pfile/target.rb
@@ -0,0 +1,67 @@
+module Puppet
+    Puppet.type(:file).newstate(:target) do
+        attr_accessor :linkmaker
+
+        desc "The target for creating a link.  Currently, symlinks are the
+            only type supported."
+
+        munge do |value|
+            value
+        end
+
+        def retrieve
+            if @parent.state(:ensure).should == :directory
+                @is = self.should
+                @linkmaker = true
+            else
+                if stat = @parent.stat
+                    if tstat = File.lstat(self.should) and tstat.ftype == "directory" and @parent.recurse?
+                        @parent[:ensure] = :directory
+                        @is = self.should
+                        @linkmaker = true
+                    else
+                        @is = File.readlink(@parent[:path])
+                        @linkmaker = false
+                    end
+                else
+                    @is = :absent
+                end
+            end
+        end
+
+        def sync
+            target = self.should
+
+            if stat = @parent.stat
+                unless stat.ftype == "link"
+                    self.fail "Not replacing non-symlink"
+                end
+                File.unlink(@parent[:path])
+            end
+            Dir.chdir(File.dirname(@parent[:path])) do
+
+                unless FileTest.exists?(target)
+                    self.debug "Not linking to non-existent '%s'" % target
+                    return nil # Grrr, can't return
+                else
+                    Puppet::Util.asuser(@parent.asuser()) do
+                        mode = @parent.should(:mode)
+                        if mode
+                            Puppet::Util.withumask(000) do
+                                File.symlink(target, @parent[:path])
+                            end
+                        else
+                            File.symlink(target, @parent[:path])
+                        end
+                    end
+
+                    # We can't use "return" here because we're in an anonymous
+                    # block.
+                    return :link_created
+                end
+            end
+        end
+    end
+end
+
+# $Id$