fxn / zeitwerk

Efficient and thread-safe code loader for Ruby
MIT License
1.97k stars 117 forks source link

Zeitwerk made Loader#ignored_glob_patterns private? #254

Closed aarona closed 1 year ago

aarona commented 1 year ago

I'm maintaining a Rails application which requires an upgrade due to a security flaw (Rails 6.0.5.1 -> 6.1.7). When I try to run the server/console I'm getting the following error:

`block in configure_autoload!': private method `ignored_glob_patterns' called for #<Zeitwerk::Loader:0x000056118256d170> (NoMethodError)

Aside from different environments, we also support different "flavors" of our software and this is getting called whenever the app reload!s so I think the person before me is resetting what's to be ignored and reloading all the files in case something has changed. We want to ignore some code if its not applicable to the configured features for the current system. Those (now private) methods are being called like so:

Rails.autoloaders.each do |autoloader|
  # autloader.class => Zeitwek::Loader
  autoloader.ignored_glob_patterns.clear
  autoloader.ignored_paths.clear
  autoloader.ignore(*Feature.skipped_namespaces.map { |n| Rails.root.join('**', n) })
end

This was working before I started upgrading. (zeitwerk 2.6.0 -> currenly 2.6.6). I saw the change log that you're trying to make the API a little stricter. However, is there a public method that can use to clear the ignored patterns/paths? Any help is much appreciated.

Just before I submit this I figured out I can call send(:ignore_glob_patterns).clear and do the same thing for ignored_paths as well so this fixes my issue but obviously I'd like to know if there is a better way to do this with your software.

fxn commented 1 year ago

Those methods were undocumented, and marked internally as private. It is private interface that cannot be used, simply. You cannot open the source and manipulate the internal state of the loader as you please!

The purpose of an internal interface is to implement state management or logic that relies on the fact that nobody but the loader is using it.

Please do not use private interfaces, instead, open a ticket and explain your use case.

OK. As documeneted, glob patterns are expanded on every reload. Could you help me understand why does your application need to do that with a concrete example?

aarona commented 1 year ago

I completely agree with you re: not accessing private members. Like I said, this was code that was working before I needed to upgrade (which included upgrading zietwerk) but currently, the Rails app doesn't boot unless I do that hack.

I will bring this up in our next stand up on Monday and see if I can track down who wrote it and what their goals were and get back to you on this. From what I can tell, it seems that they wanted to clear out any ignored globs/paths and then prevent the app from reloading files that have features that might be unsupported depending on what flavor setting the application is running on.

I'll get back to you on this and hopefully we can figure out a good way to implement zeitwerk correctly in this regard.

fxn commented 1 year ago

There are some things to take into account for that meeting:

I'd need to understand your use case better.

fxn commented 1 year ago

To make my point more clear, the point is not only if the API is private or not. The point is to know if what you have to accomplish is logically possible at all.

The public interface is there for users to be able to do what can be done, when it can be done. It is your gate to correctness.

To the point that after this ticket I believe the loader should raise if you want to ignore after setup. Your code should raise. I was kind of soft on this restriction, but this example suggests to me it should be in place.

If your use case can be supported logically, we'll find an API for it.

aarona commented 1 year ago

Ok, we figured out how to get what we needed without needing to call those private methods. Thank you for your patience with this.