listen should not listen on paths loaded from gems #26955
Comments
|
cc @fxn |
Yes, they should, otherwise you would have to require manually all controller, models and helpers defined in engines. I guess to fix this we would have to remove all items in the autoload path that are not inside the application. |
|
I can provide a fix along that path. Is there a better way to check whether or not a path is inside the app apart from |
|
I don't think so. That is the best API that I remember. |
|
We should be able to get the paths for all the gems from rubygems, then exclude those... maybe with some consultation with bundler too. |
|
I have a patch here https://github.com/radiospiel/rails/pull/1 which I verified to work correctly. However, I have trouble building tests since the code detects Rails' root via defined?(::Rails) && Rails.root. Any ideas? |
|
Let's think a bit about this. If you put something in When you want to spare the |
|
Ah, I don't mean this approach is direct in the current dependencies.rb, maybe the implementation needs some adjustment (have not checked the code). |
|
@fxn I agree that maybe things should be moved into autoload_once_paths. However, in the meantime I also found out that listen is called both on the auto_load path (for ruby code), but also on some paths that stem from i18n, and this is in a quite simple application. I have no idea if in other scenarios this would be even invoked from more places. On the other hand you would not expect things to change outside of the current app regardless of how those were loaded (i18n or ruby autoloading), hence I would suggest to fix evented_file_update_checker.rb to only check for updates from inside the app directory in the first place - and this is what https://github.com/radiospiel/rails/pull/1/files is doing. Moving things from auto_load to auto_load_once would be a change with a much broader surface and certainly out of scope of my original report. (Not that I would disagree with it) |
FWIW, I disagree, with the sole exception of gem files |
Mathew, I am afraid I don’t quite catch that. Can you explain further, me at github: https://github.com/radiospiel On 7 Nov 2016, at 2:24, Matthew Draper wrote:
|
|
I do expect files outside of the application's root directory to change. The only files I think it is reasonable to expect not to change are files that belong to installed gems. |
|
@matthewd if that is the expectation then my original complaint is moot - and in that case feel free to close this issue. However, I can't envision a scenario in which a source file's change should affect an applications behaviour if it is not inside the application nor inside an installed gem. |
|
I agree with @matthewd. The key point here is which is a better reasonabe default for watchers? We all would agree that watching stuff in gems is not a good default. It is OK that the use case in which you edit a gem to debug something is not supported out of the box, for example. But there are use cases for wanting to watch stuff outside of the application, like developing an engine that is declared as a dependency via And that also forces us to put our finger on defining what "stuff in gems" exactly means. Because that engine may be a gem, and it is being loaded by Bundler. |
|
I would be open to reconsider this behaviour though. That is, if you want to watch stuff outside of |
|
Could this be a configuration setting via config/environments/development.rb maybe, which lets users define additional directories that should be watched? Coming back to @matthewd suggestion above we could also only include paths inside Rails.root or loaded from the Gemfile via a "path:" gem file location. |
|
My suggestion is that we include all paths except those inside That'll do what we want for installed gems (exclude them) and path gems (include them). It doesn't get git gems right -- we should really exclude those (if they're bundler-managed, and not using a local override): that's where bundler could give us more information. |
|
Should we really care about git gems? When building a gem locally you would not include it via git but via path anyways, I guess... Ah, but then Bundler probably does not install git gems inside Gem.path, I see. |
|
Updated preview here: https://github.com/radiospiel/rails/pull/1 |
|
Any comments on my PR suggestion? |
|
The ones present in the thread at the moment. |
|
@radiospiel that looks good to me. Submit it as an actual PR? |
|
I've added the fix proposed by @radiospiel as a monkey patch and so far so good. There is no processes started outside the app directory. The number of active processes is down to ~15 from ~200 per app. In case someone ends up reading thread from Google while looking for a solution - this worked for me:
module ActiveSupport
class EventedFileUpdateChecker
private
def directories_to_watch
dtw = (@files + @dirs.keys).map { |f| @ph.existing_parent(f) }
dtw.compact!
dtw.uniq!
normalized_gem_paths = Gem.path.map { |path| File.join path, "" }
dtw = dtw.reject do |path|
normalized_gem_paths.any? { |gem_path| path.to_s.starts_with?(gem_path) }
end
@ph.filter_out_descendants(dtw)
end
end
end |
|
This issue has been automatically marked as stale because it has not been commented on for at least three months. |
|
Fixed by 8d50b45 |
I stumbled across this when I tracked the reason why I have so many fsevent_watch processes running: after starting a rails server in development mode I see a lot of fsevent_watch processes watching directories that are in gems. Since these are not supposed to change anyways they should not be watched:
The culprit here seems to be in railsties' lib/rails/application.rb file, which loads all the ActiveSupport::Dependencies.autoload_paths
as paths to watch. In my case config.watchable_files and config.watchable_dirs is empty.
I guess for the reason that also autoload_paths should not include paths from inside gems, but maybe I am wrong there.
In any case fixing this could also help with #26158 and rails/spring-watcher-listen#13
Expected behavior
Watchers should only start for the app directories itself.
Actual behavior
Watchers start for a number of directories outside of the app directory itself.
System configuration
Rails version: 5.0.0.1
Ruby version: 2.3.1
OS: Mac OS 10.11
The text was updated successfully, but these errors were encountered: