Trying to fix a bug where files other than site.pp do not get noticed for reparsing

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

6 files changed, 146 insertions, 79 deletions

diff --git a/lib/puppet/loadedfile.rb b/lib/puppet/loadedfile.rb
index c0b00c597..b0e408475 100755
--- a/lib/puppet/loadedfile.rb
+++ b/lib/puppet/loadedfile.rb
@@ -6,7 +6,7 @@ require 'puppet'
 module Puppet
     class NoSuchFile < Puppet::Error; end
     class LoadedFile
-        attr_reader :file
+        attr_reader :file, :statted
 
         # Provide a hook for setting the timestamp during testing, so we don't
         # have to depend on the granularity of the filesystem.
@@ -20,20 +20,19 @@ module Puppet
         )
 
         # Determine whether the file has changed and thus whether it should
-        # be reparsed
+        # be reparsed.
         def changed?
-            # Don't actually stat the file more often than filetimeout.
-            if Time.now - @statted >= Puppet[:filetimeout]
-                tmp = stamp()
-
-                if tmp == @tstamp
-                    return false
-                else
-                    @tstamp = tmp
-                    return true
-                end
-            else
+            tmp = stamp()
+
+            # We use a different internal variable than the stamp method
+            # because it doesn't keep historical state and we do -- that is,
+            # we will always be comparing two timestamps, whereas
+            # stamp() just always wants the latest one.
+            if tmp == @tstamp
                 return false
+            else
+                @tstamp = tmp
+                return @tstamp
             end
         end
 
@@ -41,20 +40,25 @@ module Puppet
         def initialize(file)
             @file = file
             unless FileTest.exists?(@file)
-                raise Puppet::NoSuchFile, "Can not use a non-existent file for parsing"
+                raise Puppet::NoSuchFile,
+                    "Can not use a non-existent file for parsing"
             end
+            @statted = 0
             @tstamp = stamp()
         end
 
-        def to_s
-            @file
+        # Retrieve the filestamp, but only refresh it if we're beyond our
+        # filetimeout
+        def stamp
+            if @stamp.nil? or (Time.now.to_i - @statted >= Puppet[:filetimeout])
+                @statted = Time.now.to_i
+                @stamp = File.stat(@file).ctime
+            end
+            return @stamp
         end
 
-        private
-
-        def stamp
-            @statted = Time.now
-            return File.stat(@file).ctime
+        def to_s
+            @file
         end
     end
 end
diff --git a/lib/puppet/parser/grammar.ra b/lib/puppet/parser/grammar.ra
index 623ca0cab..3a1a78e50 100644
--- a/lib/puppet/parser/grammar.ra
+++ b/lib/puppet/parser/grammar.ra
@@ -661,20 +661,23 @@ def initialize
     #end
 end
 
+# Add a new file to be checked when we're checking to see if we should be
+# reparsed.
+def newfile(*files)
+    files.each do |file|
+        unless file.is_a? Puppet::LoadedFile
+            file = Puppet::LoadedFile.new(file)
+        end
+        @files << file
+    end
+end
+
 def on_error(token,value,stack)
     #on '%s' at '%s' in\n'%s'" % [token,value,stack]
     #error = "line %s: parse error after '%s'" %
     #    [@lexer.line,@lexer.last]
     error = "Syntax error at '%s'" % [value]
 
-    #if Puppet[:debug]
-        #puts stack.inspect
-        #puts stack.class
-    #end
-    #if @lexer.file
-    #    error += (" in '%s'" % @lexer.file)
-    #end
-
     except = Puppet::ParseError.new(error)
     except.line = @lexer.line
     if @lexer.file
@@ -705,33 +708,27 @@ def parse(string = nil)
         # and this is a framework error
         except.line ||= @lexer.line
         except.file ||= @lexer.file
-        #if Puppet[:debug]
-        #    puts except.stack
-        #end
         raise except
     rescue Puppet::DevError => except
         except.line ||= @lexer.line
         except.file ||= @lexer.file
-        #if Puppet[:debug]
-        #    puts except.stack
-        #end
         raise except
     rescue => except
         error = Puppet::DevError.new(except.message)
         error.line = @lexer.line
         error.file = @lexer.file
         error.set_backtrace except.backtrace
-        #if Puppet[:debug]
-        #    puts caller
-        #end
         raise error
     end
 end
 
+# See if any of the files have changed.
 def reparse?
-    @files.detect { |file|
-        file.changed?
-    }
+    if file = @files.detect { |file| file.changed?  }
+        return file.stamp
+    else
+        return false
+    end
 end
 
 def string=(string)
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index ad30e6ed3..26bf8104e 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -89,8 +89,6 @@ module Puppet
                     raise Puppet::DevError, "You must provide code or a manifest"
                 end
 
-                @lastchecked = 0
-
                 if hash.include?(:UseNodes)
                     @usenodes = hash[:UseNodes]
                 else
@@ -386,8 +384,9 @@ module Puppet
                 # Check if the parser should reparse.
                 if @file
                     if defined? @parser
-                        unless @parser.reparse?
-                            @lastchecked = Time.now
+                        if stamp = @parser.reparse?
+                            Puppet.notice "Reloading files"
+                        else
                             return false
                         end
                     end
@@ -401,17 +400,14 @@ module Puppet
                     end
                 end
 
-                if defined? @parser
-                    # If this isn't our first time parsing in this process,
-                    # note that we're reparsing.
-                    Puppet.info "Reloading files"
-                end
                 # should i be creating a new parser each time...?
                 @parser = Puppet::Parser::Parser.new()
                 if @code
                     @parser.string = @code
                 else
                     @parser.file = @file
+                    # Mark when we parsed, so we can check freshness
+                    @parsedate = File.stat(@file).ctime.to_i
                 end
 
                 if @local
@@ -421,10 +417,7 @@ module Puppet
                         @ast = @parser.parse
                     end
                 end
-
-                # Mark when we parsed, so we can check freshness
                 @parsedate = Time.now.to_i
-                @lastchecked = Time.now
             end
 
             # Store the configs into the database.
diff --git a/lib/puppet/parser/parser.rb b/lib/puppet/parser/parser.rb
index 41c119c2c..a79f18b0e 100644
--- a/lib/puppet/parser/parser.rb
+++ b/lib/puppet/parser/parser.rb
@@ -29,7 +29,7 @@ module Puppet
 
     class Parser < Racc::Parser
 
-module_eval <<'..end grammar.ra modeval..idab2f060823', 'grammar.ra', 603
+module_eval <<'..end grammar.ra modeval..id1e49212871', 'grammar.ra', 603
 require 'puppet/parser/functions'
 
 attr_reader :file
@@ -91,20 +91,23 @@ def initialize
     #end
 end
 
+# Add a new file to be checked when we're checking to see if we should be
+# reparsed.
+def newfile(*files)
+    files.each do |file|
+        unless file.is_a? Puppet::LoadedFile
+            file = Puppet::LoadedFile.new(file)
+        end
+        @files << file
+    end
+end
+
 def on_error(token,value,stack)
     #on '%s' at '%s' in\n'%s'" % [token,value,stack]
     #error = "line %s: parse error after '%s'" %
     #    [@lexer.line,@lexer.last]
     error = "Syntax error at '%s'" % [value]
 
-    #if Puppet[:debug]
-        #puts stack.inspect
-        #puts stack.class
-    #end
-    #if @lexer.file
-    #    error += (" in '%s'" % @lexer.file)
-    #end
-
     except = Puppet::ParseError.new(error)
     except.line = @lexer.line
     if @lexer.file
@@ -135,33 +138,27 @@ def parse(string = nil)
         # and this is a framework error
         except.line ||= @lexer.line
         except.file ||= @lexer.file
-        #if Puppet[:debug]
-        #    puts except.stack
-        #end
         raise except
     rescue Puppet::DevError => except
         except.line ||= @lexer.line
         except.file ||= @lexer.file
-        #if Puppet[:debug]
-        #    puts except.stack
-        #end
         raise except
     rescue => except
         error = Puppet::DevError.new(except.message)
         error.line = @lexer.line
         error.file = @lexer.file
         error.set_backtrace except.backtrace
-        #if Puppet[:debug]
-        #    puts caller
-        #end
         raise error
     end
 end
 
+# See if any of the files have changed.
 def reparse?
-    @files.detect { |file|
-        file.changed?
-    }
+    if file = @files.detect { |file| file.changed?  }
+        return file.stamp
+    else
+        return false
+    end
 end
 
 def string=(string)
@@ -175,7 +172,7 @@ end
 
 # $Id$
 
-..end grammar.ra modeval..idab2f060823
+..end grammar.ra modeval..id1e49212871
 
 ##### racc 1.4.5 generates ###
 
diff --git a/test/language/interpreter.rb b/test/language/interpreter.rb
index 51ee1e5f4..36fd5922b 100755
--- a/test/language/interpreter.rb
+++ b/test/language/interpreter.rb
@@ -349,4 +349,51 @@ class nothernode {}
 
 
     end
+
+    def test_parsedate
+        Puppet[:filetimeout] = 0
+        main = tempfile()
+        sub = tempfile()
+        mainfile = tempfile()
+        subfile = tempfile()
+        count = 0
+        updatemain = proc do
+            count += 1
+            File.open(main, "w") { |f|
+                f.puts "import '#{sub}'
+                    file { \"#{mainfile}\": content => #{count} }
+                    "
+            }
+        end
+        updatesub = proc do
+            count += 1
+            File.open(sub, "w") { |f|
+                f.puts "file { \"#{subfile}\": content => #{count} }
+                "
+            }
+        end
+
+        updatemain.call
+        updatesub.call
+
+        interp = Puppet::Parser::Interpreter.new(
+            :Manifest => main,
+            :Local => true
+        )
+
+        date = interp.parsedate
+
+        # Now update the site file and make sure we catch it
+        sleep 1
+        updatemain.call
+        newdate = interp.parsedate
+        assert(date != newdate, "Parsedate was not updated")
+        date = newdate
+
+        # And then the subfile
+        sleep 1
+        updatesub.call
+        newdate = interp.parsedate
+        assert(date != newdate, "Parsedate was not updated")
+    end
 end
diff --git a/test/other/loadedfile.rb b/test/other/loadedfile.rb
index efb17a1e5..1089402a1 100755
--- a/test/other/loadedfile.rb
+++ b/test/other/loadedfile.rb
@@ -22,9 +22,10 @@ class TestLoadedFile < Test::Unit::TestCase
 
         assert(!file.changed?, "File incorrectly returned changed")
 
-        #sleep(1)
         File.open(path, "w") { |f| f.puts "booness" }
-        file.send("tstamp=".intern, File.stat(path).ctime - 5)
+        #file.tstamp = File.stat(path).ctime - 5
+        new = File.stat(path).ctime - 5
+        file.tstamp = new
 
         assert(file.changed?, "File did not catch change")
     end
@@ -39,10 +40,14 @@ class TestLoadedFile < Test::Unit::TestCase
             file = Puppet::LoadedFile.new(path)
         }
 
-        assert_nothing_raised { file.changed? }
+        assert_nothing_raised {
+            assert(!file.changed?,
+                "File thought it changed immediately")
+        }
 
+        sleep 1
         File.open(path, "w") { |f| f.puts "yay" }
-        file.send("tstamp=".intern, File.stat(path).ctime - 5)
+        #file.tstamp = File.stat(path).ctime - 5
 
         assert(!file.changed?,
             "File was marked as changed too soon")
@@ -50,8 +55,32 @@ class TestLoadedFile < Test::Unit::TestCase
         Puppet[:filetimeout] = 0
         assert(file.changed?,
             "File was not marked as changed soon enough")
+    end
 
+    def test_stamp
+        file = tempfile()
+        File.open(file, "w") { |f| f.puts "" }
+        obj = nil
+        assert_nothing_raised {
+            obj = Puppet::LoadedFile.new(file)
+        }
 
+        # Make sure we don't refresh
+        Puppet[:filetimeout] = 50
+
+        stamp = File.stat(file).ctime
+
+        assert_equal(stamp, obj.stamp)
+
+        sleep 1
+        # Now change the file, and make sure the stamp doesn't update yet
+        File.open(file, "w") { |f| f.puts "" }
+        assert_equal(stamp, obj.stamp,
+            "File prematurely refreshed")
+
+        Puppet[:filetimeout] = 0
+        assert_equal(File.stat(file).ctime, obj.stamp,
+            "File did not refresh")
     end
 end