From caa3cc8868206f8109e0d633efb09d31e94ef635 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Fri, 7 Sep 2018 21:54:23 +0900 Subject: [PATCH] Do not add parent directory to file system monitoring `EventedFileUpdateChecker` will search the parent directory if the specified directory does not exist. Since `test/mailers/previews` is included in the watch target by default, if there is no test directory (e.g. using `rspec`), the Rails root directory will be included in the watch target. ``` $ rails new app $ cd app $ ./bin/rails r "p Rails.application.reloaders.last.send(:directories_to_watch).include?(Rails.root)" false $ rm -rf test $ ./bin/rails r "p Rails.application.reloaders.last.send(:directories_to_watch).include?(Rails.root)" true ``` This causes `node_modules` to be included in watch target. Adding parent directories to watch target may include unexpected directories. In order to avoid this, fixed that parents of nonexistent directories are not added to the watch targets, instead checking that the directory exists when checking changes. Related to #32700. [Matthew Draper & Yuji Yaginuma] --- .../evented_file_update_checker.rb | 24 ++++++++++++++-- .../test/evented_file_update_checker_test.rb | 28 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/evented_file_update_checker.rb b/activesupport/lib/active_support/evented_file_update_checker.rb index 97e982eb056d0..1ef8584c52db6 100644 --- a/activesupport/lib/active_support/evented_file_update_checker.rb +++ b/activesupport/lib/active_support/evented_file_update_checker.rb @@ -52,7 +52,10 @@ def initialize(files, dirs = {}, &block) @pid = Process.pid @boot_mutex = Mutex.new - if (@dtw = directories_to_watch).any? + dtw = directories_to_watch + @dtw, @missing = dtw.partition(&:exist?) + + if @dtw.any? # Loading listen triggers warnings. These are originated by a legit # usage of attr_* macros for private attributes, but adds a lot of noise # to our test suite. Thus, we lazy load it and disable warnings locally. @@ -75,6 +78,19 @@ def updated? @updated.make_true end end + + if @missing.any?(&:exist?) + @boot_mutex.synchronize do + appeared, @missing = @missing.partition(&:exist?) + shutdown! + + @dtw += appeared + boot! + + @updated.make_true + end + end + @updated.true? end @@ -96,6 +112,10 @@ def boot! Listen.to(*@dtw, &method(:changed)).start end + def shutdown! + Listen.stop + end + def changed(modified, added, removed) unless updated? @updated.make_true if (modified + added + removed).any? { |f| watching?(f) } @@ -123,7 +143,7 @@ def watching?(file) end def directories_to_watch - dtw = (@files + @dirs.keys).map { |f| @ph.existing_parent(f) } + dtw = @files.map(&:dirname) + @dirs.keys dtw.compact! dtw.uniq! diff --git a/activesupport/test/evented_file_update_checker_test.rb b/activesupport/test/evented_file_update_checker_test.rb index a557608986f21..b2d5eb94c23a4 100644 --- a/activesupport/test/evented_file_update_checker_test.rb +++ b/activesupport/test/evented_file_update_checker_test.rb @@ -76,6 +76,34 @@ def touch(files) Process.wait(pid) end + + test "updated should become true when nonexistent directory is added later" do + Dir.mktmpdir do |dir| + watched_dir = File.join(dir, "app") + unwatched_dir = File.join(dir, "node_modules") + not_exist_watched_dir = File.join(dir, "test") + + Dir.mkdir(watched_dir) + Dir.mkdir(unwatched_dir) + + checker = new_checker([], watched_dir => ".rb", not_exist_watched_dir => ".rb") { } + + FileUtils.touch(File.join(watched_dir, "a.rb")) + wait + assert_predicate checker, :updated? + assert checker.execute_if_updated + + Dir.mkdir(not_exist_watched_dir) + wait + assert_predicate checker, :updated? + assert checker.execute_if_updated + + FileUtils.touch(File.join(unwatched_dir, "a.rb")) + wait + assert_not_predicate checker, :updated? + assert_not checker.execute_if_updated + end + end end class EventedFileUpdateCheckerPathHelperTest < ActiveSupport::TestCase