Fix bundler/setup unintendedly writing to the filesystem
#6814
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
fix-git-cache
branch
from
0ba5fda
to
ae41177
Compare
|
Thank you for doing this! I like the new test case. Were the other test failures random or from this change/fix? |
Previously, when sorting and comparing git Gemfile vs lockfile sources during `bundler/setup` to figure out whether we need to re-resolve or not, we would try to find the default branch if nothing more specific was specified in the Gemfile. If the git cache has been deleted thought, that would fail. The error would still be swallowed (and the branch would simply not be displayed), but trying to clone would still generate the side effect of creating the parent folder for the clone. That could affect non-writable systems that don't expect `bundler/setup` to write to the filesystem at all. To fix this, override `Bundler::Source::Git#identifier` to use exclusively static information, so it does not even try to clone the repo nor generate any side effects.
deivid-rodriguez
force-pushed
the
fix-git-cache
branch
from
ae41177
to
582eb2e
Compare
|
@metaskills They were directly related to this change. I pushed one more tweak that I expect to fix them 🤞. |
|
YUP! Looks green now. I really appreciate this followup. Is there anything I can do to help test/QA/etc? |
You can try to checkout related branch and add to path. |
|
Thanks! Confirmed working from my end. |
simi
approved these changes
Jul 12, 2023
|
@simi !!!! How are you doing? Nice to see you around. |
All good. :) |
deivid-rodriguez
pushed a commit
that referenced
this pull request
Jul 12, 2023
Fix `bundler/setup` unintendedly writing to the filesystem (cherry picked from commit 8e10600)
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?
After #6786,
bundler/setupwould write to the filesystem when dealing with git sources.What is your fix for the problem, implemented in this PR?
When sorting and comparing git Gemfile vs lockfile sources during
bundler/setupto figure out whether we need to re-resolve or not, we would try to find the default branch if nothing more specific was specified in the Gemfile.If the git cache has been deleted though, that would fail.
The error would still be swallowed (and the branch would simply not be displayed), but trying to clone would still generate the side effect of creating the parent folder for the clone.
That could affect non-writable systems that don't expect
bundler/setupto write to the filesystem at all.To fix this, override
Bundler::Source::Git#identifierto use exclusively static information, so it does not even try to clone the repo nor generate any side effects.This is essentially a rework of the original attempt to fix #6743, but without the problems discussed in that PR.
Fixes #6813.
Make sure the following tasks are checked