0.16.0 breaks Sinatra config_file about dry-configurable HOT 9 CLOSED

Hey!

I'll have a look. Definitely seems like some kind of conflict between Zeitwerk and Sinatra. Interestingly, if you load Zeitwerk after Sinatra, the test passes.

Let me see if I find the root cause, then we might follow in the affected repo.

from dry-configurable.

Thanks @fxn, but I already opened an issue here sinatra/sinatra#1826 Do let me know if you'd rather have that closed for now, if not I'll leave it open.

from dry-configurable.

Hi @DawidJanczak, I'd definitely appreciate a reproduction in code, I'm not particularly familiar with details like this from Sinatra.

from dry-configurable.

Hope this helps @timriley : https://github.com/DawidJanczak/dry-configurable-sinatra

Perhaps let's start with the branch that's working, if you checkout configurable-0.15 (https://github.com/DawidJanczak/dry-configurable-sinatra/compare/configurable-0.15) and run bundle followed by APP_ENV=test bundle exec rspec it will work just fine.

Now if you check out main and again run bundle followed by APP_ENV=test bundle exec rspec you will notice it's failing. If however you remove the dry/configurable require it's working again.

I have to say this is one bizarre situation! I'm almost certain zeitwerk is at fault here, but I don't know enough about it to comment.

from dry-configurable.

@DawidJanczak This is extremely weird. I'm officially flummoxed. The two gems don't seem like they should be interacting at all here. dry-configurable certainly doesn't do anything outside of its own Dry::Configurable namespace.

I tried changing the require "dry/configurable" line in your reproduction to some more specific requires, like "dry/configurable/config" and "dry/configurable/settings". Neither of them cause the issue, so whatever it is must be coming from the dry/configurable.rb file specifically (which does possibly hint at a Zeitwerk-related issue).

I was chatting with @solnic and he mentioned he might check with @rkh for his thoughts here from a Sinatra perspective.

from dry-configurable.

I did one further test. I updated the Gemfile to point to a local checkout of dry-configurable (current main branch). And then I tried commenting out various parts in lib/dry/configurable.rb to see what was triggering the actual error.

If I comment out these three lines from the file, then the error goes away:

1. require "zeitwerk"
2. require "dry/core"
3. loader.setup

Bringing any one of those lines back on its own will bring back the error (with the exception of bringing back loader.setup on its own because in that case it complains about a missing Zeitwerk::Loader constant instead, which makes sense).

from dry-configurable.

OK, I boiled it down further.

If you remove dry-configurable from the Gemfile and replace it with zeitwerk:

# frozen_string_literal: true

source 'https://rubygems.org'

gem 'zeitwerk'
gem 'sinatra'
gem 'sinatra-contrib'

group :development, :test do
  gem 'rack-test'
  gem 'rspec'
end

And change app.rb so it requires zeitwerk only:

# frozen_string_literal: true

require "zeitwerk"

require 'sinatra'
require 'sinatra/config_file'

config_file '../config.yml'

get '/' do
  p Sinatra::Base.environment
  p settings.foo
end

Then the tests still fail in the same way:

❯ APP_ENV=test bundle exec rspec
:test
F

Failures:

  1) Main app GET / succeeds
     Failure/Error: p settings.foo

     NoMethodError:
       undefined method `foo' for Sinatra::Application:Class

         p settings.foo
                   ^^^^
     # ./lib/app.rb:14:in `block in <top (required)>'
     # ./spec/app_spec.rb:16:in `block (3 levels) in <top (required)>'

Finished in 0.00588 seconds (files took 0.12422 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/app_spec.rb:15 # Main app GET / succeeds

This makes me think this is a problem between sinatra/sinatra-contrib and zeitwerk only.

My feeling is you happened to see it through dry-configurable because it was the first gem upgrade that introduced zeitwerk into your bundle (dry-configurable 0.15.0 did not use zeitwerk, that only came with dry-configurable 0.16.0).

from dry-configurable.

I've also determined that this comes from Zeitwerk's replacement of Kernel#require. If you bundle open zeitwerk and comment out everything inside its lib/zeitwerk/kernel.rb, then the rests begin passing again.

I'm sure this is only the first step of several to figuring out what's going on here, but hopefully it helps :)

from dry-configurable.

Thanks @timriley, that is really helpful! I'll test this later and file an issue with Sinatra or Zeitwerk :)

from dry-configurable.