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

Show better error when previous installation fails to be removed #5564

Merged
merged 1 commit into from May 27, 2022

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented May 25, 2022

What was the end-user or developer problem that led to this PR?

A while ago we improved our handling of permission issues, but detecting when a preexisting installation of a gem fails to be removed, and raise an error, instead of silently ignoring the issue and crash with a more confusing error somewhere else.

I've seen the new error in the wild, and although better, it doesn't make it clear what the original error was that prevented Bundler from being able to delete the folder.

What is your fix for the problem, implemented in this PR?

My fix is to use a better helper for deleting folder, that does not swallow any errors, and properly report the underlying issue when errors happen.

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez force-pushed the better-rm-rf-error branch 2 times, most recently from e37857d to f161f1a Compare May 25, 2022 09:53
Instead of guessing on the culprit.

We actually have a helper, `Bundler.rm_rf`, with exactly the behavior
that we want:

* Allow the passed folder to not exist.
* No exception swallowing other than that.
@deivid-rodriguez deivid-rodriguez merged commit 79f72c9 into master May 27, 2022
@deivid-rodriguez deivid-rodriguez deleted the better-rm-rf-error branch May 27, 2022 08:26
deivid-rodriguez added a commit that referenced this pull request Jun 1, 2022
Show better error when previous installation fails to be removed

(cherry picked from commit 79f72c9)
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.

None yet

1 participant