Fix edge case resulting in a crash when using zeitwerk inside a nested bundle exec invocation
#4062
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deivid-rodriguez
force-pushed
the
zeitwerk
branch
3 times, most recently
from
a47a1ce
to
0a79968
Compare
deivid-rodriguez
changed the title
zeitwerk inside a nested bundle exec invocation
deivid-rodriguez
force-pushed
the
zeitwerk
branch
3 times, most recently
from
ee83fbc
to
d7a9b31
Compare
Only thing we need to reset is settings, and only because settings are loaded relative to the `Gemfile`, so they need to be reset if the configured or passed `Gemfile` is not standard and will resolve to a different root. Resetting more stuff causes issues under some circumstances, specifically when dependencies further tweak `Kernel.require`.
deivid-rodriguez
force-pushed
the
zeitwerk
branch
from
d7a9b31
to
43960ea
Compare
deivid-rodriguez
added a commit
that referenced
this pull request
Dec 7, 2020
Fix edge case resulting in a crash when using `zeitwerk` inside a nested `bundle exec` invocation (cherry picked from commit a9c77de)
deivid-rodriguez
added a commit
that referenced
this pull request
Dec 7, 2020
Fix edge case resulting in a crash when using `zeitwerk` inside a nested `bundle exec` invocation (cherry picked from commit a9c77de)
deivid-rodriguez
added a commit
that referenced
this pull request
Dec 7, 2020
Fix edge case resulting in a crash when using `zeitwerk` inside a nested `bundle exec` invocation (cherry picked from commit a9c77de)
deivid-rodriguez
added a commit
that referenced
this pull request
Dec 7, 2020
Fix edge case resulting in a crash when using `zeitwerk` inside a nested `bundle exec` invocation (cherry picked from commit a9c77de)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was the end-user or developer problem that led to this PR?
See #3270 for the realworld problem. It's a combination of using dependencies that monkeypatch
Kernel.require, and using several levels of nestedbundle execs.When bundler's CLI is initialized, we check whether a custom
Gemfileis configured, and in that case we invalidate the whole bundler environment so that the whole thing is reloaded using the newGemfile. When we are inside a nestedbundle exec, the original Gemfile is explicitly configured so that further subprocesses always use it, so we fall into this situation.The problem is that we're resetting too much, and that can cause dependencies that further decorate
Kernel.requireto lose their decorations.What is your fix for the problem, implemented in this PR?
The fix is to only reset what we really need to reset here, namely, the settings so that they are reloaded using the new
Gemfileas a base.Fixes #3270.
Make sure he following tasks are checked