Don't use require_relative in active_record_migrations.rb #6
Comments
|
Why is mixing When you're able to reproduce the problem I'll be able to help you detecting why your configurations file is being loaded more than once. Regular Ruby makes no distinction with regards to files being loaded multiple times whether they are required with require or require_relative... |
|
|
The offending scenario, as I imagined, would go along these lines: Looking at There's no reason to use |
|
There's no Maybe some other file in your project is requiring |
|
By the way, I've released 4.1.5.1 early this morning... It shouldn't make any difference but maybe you'd like to know... |
"No real necessity", to put it more strictly. :)
Yeah, okay. I've always figured using the You're not using it very consistently, though: its
I'm pretty sure they don't.
I'd be happy to, but it's proprietary. I might find out more tomorrow.
I know, thanks. |
You're right, I should update the version line to use require_relative as well. There are other places that I find require_relative confusing, specially if it starts with '..', so I prefer regular require in such cases. In theory, if you have many directories in your load path, require_relative should be faster as it doesn't have to look-up on each possible path although I guess this wouldn't make any noticable performance difference for most projects. |
|
It's pretty easy to detect who is loading this file. Just |
|
Thanks for the tip. :) Check this out: |
|
Looking at
Unless you have any further suggestions, I guess we'll have to work around this on our side. Thanks for the help anyway. |
|
Humm... it makes sense. I'll check with ruby-core mailing list if this could be detected somehow in the Ruby side to avoid issues like that. In the meanwhile I'll see if I can find some time later today to replace the require_relative with require and explain the reason when symlinks are used... |
|
Also found this ticket: https://bugs.ruby-lang.org/issues/5721 |
|
I wonder if |
|
It's not mostly academical just because it doesn't fix your current application since it should fix Ruby from now on if possible. I believe the proper way is to always point to the real path if possible. This is probably much easier to implement and guarantee it would always work. Let's see what they think about it once I discuss this behavior with them. |
Hm, yeah, that should work. |
|
Sorry but I'm not going to release a new version only due to this change but you can feel free to point to master in your Gemfile if you want. |
|
Thanks, and no problem! |
|
Would you mind explaining further how to replicate the issue? I tried this but didn't experience any troubles... Maybe this is a bug in MRI 1.9.3? mkdir a
ln -s a b
echo "require_relative 'b'" > a/a.rb
echo "p 'b loaded'" > a/b.rb
ruby -I . -e 'require "b/a"'I can't open a bug report in ruby-core unless I'm able to reproduce the problem. |
|
Taking a closer look at your stacktrace it seems the problem seems to be introduced by ActiveSupport::Dependencies. It overrides Object#require and seems to be causing the bug. Most likely this bug is specific to your project and how AS is used there. Ideally in production require should not be touched by ActiveSupport so most likely your application is still buggy. This is only a work-around on the real issue but you should expect more problems in your application side. |
Apparently, just loading ActiveRecord (which we use) drags ActiveSupport::Dependencies with it: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/base.rb#L4 Anyway, AS doesn't seem to be the problem. Try this: mkdir a
ln -s a b
echo "require_relative 'b'" > a/a.rb
echo "p 'b loaded'" > a/b.rb
echo "$: << File.expand_path('../b', __FILE__); require 'a'; require 'b'" > c.rb
ruby c.rbAnd if I add |
|
Great, I'm going to sleep now and will try this tomorrow morning and discuss with Ruby core team if I can reproduce it. Thank you! |
|
I've just created a ticket for it: https://bugs.ruby-lang.org/issues/10222 Thank you for the instructions on how to reproduce it. |
|
Thanks! |
In our staging environment, it causes the
ActiveRecordMigrations.load_taskscall to re-evaluateconfigurations.rband thus revert to the default configuration (so loading of the Rakefile fails with something like "db/config.yml not found").IIUC, because
tasks/new_migration.rakehasrequire 'active_record_migrations/configurations', andload_tasksloads that file.For some reason, that doesn't happen on my (development) machine, but mixing
requireandrequire_relativeis bad either way.The text was updated successfully, but these errors were encountered: