Skip to content

Lazily load fileutils and tmpdir #616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 18, 2021
Merged

Lazily load fileutils and tmpdir #616

merged 1 commit into from
Feb 18, 2021

Conversation

deivid-rodriguez
Copy link
Contributor

This change removes some fileutils redefintion warnings that happen when you boot a rails application that uses spring on a system with a default fileutils version, and a higher version of the fileutils gem available as well. This can happen, for example, on ruby 2.6.

Repro instructions are here: ruby/fileutils#22 (comment).

The short explanation of why this happens is that spring loads without using bundler, so the fileutils require uses rubygems version of Kernel.require which automatically activates the latest version of the corresponding gem if a require argument matches a default gem (like fileutils). Then rails itself also requires fileutils, but at that point bundler is already loaded, and bundler does not change Kernel.require at all, hence the default version of fileutils is chosen this time.

By lazily loading fileutils, we make sure that bundler is always loaded before fileutils, and thus the version that ends up being required is consistent.

The explanation of why also lazily loading tmpdir is because tmpdir also loads fileutils.

@deivid-rodriguez
Copy link
Contributor Author

CI is terribly slow on TravisCI now. Should we migrate to a different CI?

@deivid-rodriguez
Copy link
Contributor Author

Rebased now that #617 was merged!

@rafaelfranca rafaelfranca merged commit 577cf01 into rails:master Feb 18, 2021
@deivid-rodriguez deivid-rodriguez deleted the lazily_load_tmpdir_and_fileutils branch February 18, 2021 09:45
@deivid-rodriguez
Copy link
Contributor Author

Thanks a lot!

@pjg
Copy link

pjg commented Jun 11, 2021

How about cutting a patch release with this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants