Skip to content
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

Fix fileutils double load when using bundler/inline #3991

Merged
merged 1 commit into from Oct 14, 2020

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Oct 2, 2020

Description:

In the following conditions:

  • Using gemfile(true, &block) (inline Gemfile and install dependencies on the fly).
  • Specifying a gem that depends of fileutils.
  • Having both a default version of fileutils and a higher version installed on your system (happens by default on ruby 2.6).

Then you'll get a lot of redefinition warnings for FileUtils constants because two different versions of fileutils are required.

This PR fixes the problem by avoiding requiring fileutils too early. To do this, we do the following:

  • Remove top-level fileutils requires from rubygems DependencyInstaller.
  • Vendor tmpdir code inside bundler, and change its require of fileutils to instead require the version of fileutils vendored inside bundler.

With the above changes, warnings are gone since a consistent version of fileutils is used.

Fixes #3213.

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

We need inline gemfiles to not load `fileutils`. If it does, a require
of `fileutils` after a `gemfile(true)` block might cause redefinition
warnings because the initial require will use "rubygems version of
require", which "upgrades default gems", whereas the second require will
use the default version of `fileutils`, since bundler "undoes" rubygems
require monkeypatches.

Similar to other default gem issues, the solution is a mix of:

* Lazily loading `fileutils` as much as possible.
* Vendor `tmpdir` inside `bundler`. Since `tmpdir` loads `fileutils`
  internally, by vendoring it we can change this require to instead use
  bundler's vendored version of fileutils, which is namespaced so it
  doesn't cause any redefinition issues.
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review October 12, 2020 18:33
@deivid-rodriguez deivid-rodriguez merged commit 6ebda1d into master Oct 14, 2020
@deivid-rodriguez deivid-rodriguez deleted the fileutils_double_load branch October 14, 2020 17:05
deivid-rodriguez added a commit that referenced this pull request Dec 7, 2020
Fix fileutils double load when using `bundler/inline`

(cherry picked from commit 6ebda1d)
deivid-rodriguez added a commit that referenced this pull request Dec 7, 2020
Fix fileutils double load when using `bundler/inline`

(cherry picked from commit 6ebda1d)
deivid-rodriguez added a commit that referenced this pull request Dec 7, 2020
Fix fileutils double load when using `bundler/inline`

(cherry picked from commit 6ebda1d)
deivid-rodriguez added a commit that referenced this pull request Dec 7, 2020
Fix fileutils double load when using `bundler/inline`

(cherry picked from commit 6ebda1d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When installing Ruby's default gems from rubygems, both gems load.
2 participants