From 706007ead9e97afbd0603cb8e4519b64dd325178 Mon Sep 17 00:00:00 2001 From: Nicholas Burlett Date: Sat, 21 Mar 2015 19:25:02 -0700 Subject: [PATCH 1/3] Incrementally regenerate missing destination file Addresses the third point of #3591, in which the incremental regenerator doesn't notice that destination files have gone missing. --- lib/jekyll/regenerator.rb | 32 ++++++++++++++++++++++++-------- test/test_regenerator.rb | 36 ++++++++++++++++++++++++++++++++++++ test/test_site.rb | 19 +++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/lib/jekyll/regenerator.rb b/lib/jekyll/regenerator.rb index 76d720af..efe3c14c 100644 --- a/lib/jekyll/regenerator.rb +++ b/lib/jekyll/regenerator.rb @@ -18,16 +18,21 @@ module Jekyll def regenerate?(document) case document when Post, Page - document.asset_file? || document.data['regenerate'] || - modified?(site.in_source_dir(document.relative_path)) + return ( document.asset_file? || + document.data['regenerate'] || + source_modified_or_dest_missing?( + site.in_source_dir(document.relative_path), + document.destination(@site.dest)) ) when Document - !document.write? || document.data['regenerate'] || modified?(document.path) + return ( !document.write? || + document.data['regenerate'] || + source_modified_or_dest_missing?( + document.path, + document.destination(@site.dest)) ) else - if document.respond_to?(:path) - modified?(document.path) - else - true - end + source_path = document.respond_to?(:path) ? document.path : nil + dest_path = document.respond_to?(:destination) ? document.destination(@site.dest) : nil + return source_modified_or_dest_missing?(source_path, dest_path) end end @@ -67,6 +72,17 @@ module Jekyll @cache = {} end + + # Checks if the source has been modified or the + # destination is missing + # + # returns a boolean + def source_modified_or_dest_missing?(source_path, dest_path) + source_modified = source_path ? modified?(source_path) : true + dest_missing = dest_path ? !File.exist?(dest_path) : false + return source_modified || dest_missing + end + # Checks if a path's (or one of its dependencies) # mtime has changed # diff --git a/test/test_regenerator.rb b/test/test_regenerator.rb index 87312333..7dd39743 100644 --- a/test/test_regenerator.rb +++ b/test/test_regenerator.rb @@ -36,14 +36,50 @@ class TestRegenerator < JekyllUnitTest @regenerator.regenerate?(@document) @regenerator.regenerate?(@asset_file) + # we need to create the destinations for these files, + # because regenerate? checks if the destination exists + [@page, @post, @document, @asset_file].each do |item| + if item.respond_to?(:destination) + dest = item.destination(@site.dest) + FileUtils.mkdir_p(File.dirname(dest)) + FileUtils.touch(dest) + end + end @regenerator.write_metadata @regenerator = Regenerator.new(@site) + # these should pass, since nothing has changed, and the + # loop above made sure the desinations exist assert !@regenerator.regenerate?(@page) assert !@regenerator.regenerate?(@post) assert !@regenerator.regenerate?(@document) end + should "regenerate if destination missing" do + # Process files + @regenerator.regenerate?(@page) + @regenerator.regenerate?(@post) + @regenerator.regenerate?(@document) + @regenerator.regenerate?(@asset_file) + + @regenerator.write_metadata + @regenerator = Regenerator.new(@site) + + # make sure the files don't actually exist + [@page, @post, @document, @asset_file].each do |item| + if item.respond_to?(:destination) + dest = item.destination(@site.dest) + File.unlink(dest) unless !File.exist?(dest) + end + end + + # while nothing has changed, the output files were not + # generated, so they still need to be regenerated + assert @regenerator.regenerate?(@page) + assert @regenerator.regenerate?(@post) + assert @regenerator.regenerate?(@document) + end + should "always regenerate asset files" do assert @regenerator.regenerate?(@asset_file) end diff --git a/test/test_site.rb b/test/test_site.rb index 7888f330..db0e490f 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -495,6 +495,25 @@ class TestSite < JekyllUnitTest assert_equal mtime3, mtime4 # no modifications, so remain the same end + should "regnerate files that have had their destination deleted" do + contacts_html = @site.pages.find { |p| p.name == "contacts.html" } + @site.process + + source = @site.in_source_dir(contacts_html.path) + dest = File.expand_path(contacts_html.destination(@site.dest)) + mtime1 = File.stat(dest).mtime.to_i # first run must generate dest file + + # simulate file modification by user + File.unlink dest + refute File.exist?(dest) + + sleep 1 + @site.process + assert File.exist?(dest) + mtime2 = File.stat(dest).mtime.to_i + refute_equal mtime1, mtime2 # must be regenerated + end + end end From 8f4194eea522ca4a721e30b138ffd3c5ae5b1e18 Mon Sep 17 00:00:00 2001 From: Nicholas Burlett Date: Tue, 24 Mar 2015 21:21:37 -0700 Subject: [PATCH 2/3] Clean up regeneration missing-destination checks Use easier-to-follow checks for missing-destinations in the regenerator. --- lib/jekyll/regenerator.rb | 27 +++++++++++++-------------- test/test_site.rb | 6 +++--- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/jekyll/regenerator.rb b/lib/jekyll/regenerator.rb index efe3c14c..7bff5327 100644 --- a/lib/jekyll/regenerator.rb +++ b/lib/jekyll/regenerator.rb @@ -18,21 +18,19 @@ module Jekyll def regenerate?(document) case document when Post, Page - return ( document.asset_file? || - document.data['regenerate'] || - source_modified_or_dest_missing?( - site.in_source_dir(document.relative_path), - document.destination(@site.dest)) ) + document.asset_file? || document.data['regenerate'] || + source_modified_or_dest_missing?( + site.in_source_dir(document.relative_path), document.destination(@site.dest) + ) when Document - return ( !document.write? || - document.data['regenerate'] || - source_modified_or_dest_missing?( - document.path, - document.destination(@site.dest)) ) + !document.write? || document.data['regenerate'] || + source_modified_or_dest_missing?( + document.path, document.destination(@site.dest) + ) else source_path = document.respond_to?(:path) ? document.path : nil dest_path = document.respond_to?(:destination) ? document.destination(@site.dest) : nil - return source_modified_or_dest_missing?(source_path, dest_path) + source_modified_or_dest_missing?(source_path, dest_path) end end @@ -78,9 +76,7 @@ module Jekyll # # returns a boolean def source_modified_or_dest_missing?(source_path, dest_path) - source_modified = source_path ? modified?(source_path) : true - dest_missing = dest_path ? !File.exist?(dest_path) : false - return source_modified || dest_missing + modified?(source_path) || ((dest_path and !File.exist?(dest_path)) or false) end # Checks if a path's (or one of its dependencies) @@ -90,6 +86,9 @@ module Jekyll def modified?(path) return true if disabled? + # objects that don't have a path are always regenerated + return true if path.nil? + # Check for path in cache if cache.has_key? path return cache[path] diff --git a/test/test_site.rb b/test/test_site.rb index db0e490f..23fe9f7c 100644 --- a/test/test_site.rb +++ b/test/test_site.rb @@ -505,11 +505,11 @@ class TestSite < JekyllUnitTest # simulate file modification by user File.unlink dest - refute File.exist?(dest) + refute File.file?(dest) - sleep 1 + sleep 1 # sleep for 1 second, since mtimes have 1s resolution @site.process - assert File.exist?(dest) + assert File.file?(dest) mtime2 = File.stat(dest).mtime.to_i refute_equal mtime1, mtime2 # must be regenerated end From 1f8157022a4a401d1a508c7896369815f7a9e682 Mon Sep 17 00:00:00 2001 From: Nicholas Burlett Date: Wed, 25 Mar 2015 08:51:58 -0700 Subject: [PATCH 3/3] Clean up destination modified check Clean up the destination modified check in `source_modified_or_dest_missing?` to be easier to read. Note that it can now return `nil` instead of `false` for an unmodified `source_path` and a `nil` `dest_path`, but in a discussion on 706007ead9e97afbd0603cb8e4519b64dd325178 we decided that was okay. --- lib/jekyll/regenerator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll/regenerator.rb b/lib/jekyll/regenerator.rb index 7bff5327..37107350 100644 --- a/lib/jekyll/regenerator.rb +++ b/lib/jekyll/regenerator.rb @@ -76,7 +76,7 @@ module Jekyll # # returns a boolean def source_modified_or_dest_missing?(source_path, dest_path) - modified?(source_path) || ((dest_path and !File.exist?(dest_path)) or false) + modified?(source_path) || (dest_path and !File.exist?(dest_path)) end # Checks if a path's (or one of its dependencies)