Skip to content

Less error swallowing when installing gems#5475

Merged
deivid-rodriguez merged 2 commits intomasterfrom
install-missing-dirs
Apr 16, 2022
Merged

Less error swallowing when installing gems#5475
deivid-rodriguez merged 2 commits intomasterfrom
install-missing-dirs

Conversation

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez commented Apr 16, 2022

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

I'm working on some changes to fileutils to not swallow meaningful errors from FileUtils.rm_f and FileUtils.rm_rf at ruby/fileutils#58.

Trying that PR against RubyGems tests made some errors (mostly likely typos where FileUtils.rm_f instead of FileUtils.rm_rf was used).

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

Use the proper call so that the folders are actually removed, and fix the issues caused by that.

While debugging those errors, I also removed one rescue that felt unnecessary and helped me track down the original culprit of the error.

Make sure the following tasks are checked

We were trying to remove directories using `FileUtils.rm_f` which is
unexpected and does not remove anything. Changing to `FileUtils.rm_rf`
actually removes the directories properly. That itself showed other
issues:

* One test was actually removing the gem package it was about to
  install, since it was living in the cache folder. To fix that, avoid
  removing the cache folder, and only make sure the other directories
  are created automatically, which seems enough.

* Another test was actually removing an incorrect directory. Change it
  to remove the right one (the one that's asserted later to have been
  created).
@deivid-rodriguez deivid-rodriguez changed the title Fix some test issues Less error swallowing when installing gems Apr 16, 2022
@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

Thanks @simi!

@deivid-rodriguez deivid-rodriguez merged commit 504272e into master Apr 16, 2022
@deivid-rodriguez deivid-rodriguez deleted the install-missing-dirs branch April 16, 2022 13:58
deivid-rodriguez added a commit that referenced this pull request Apr 20, 2022
Less error swallowing when installing gems

(cherry picked from commit 504272e)
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.

2 participants