ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: "Eregon (Benoit Daloze)" <noreply@ruby-lang.org>
To: ruby-core@ruby-lang.org
Subject: [ruby-core:109768] [Ruby master Bug#18784] `FileUtils.rm_f` and `FileUtils.rm_rf` should not mask exceptions
Date: Mon, 29 Aug 2022 12:58:36 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-99006.20220829125836.7174@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-18784.20220516090409.7174@ruby-lang.org

Issue #18784 has been updated by Eregon (Benoit Daloze).


I'd suggest having the warning for $VERBOSE=false too (and so no warning for $VERBOSE=nil).
When such an error happens it means something is going wrong, and I'd imagine it's a lot more helpful for anyone using `rm_rf` and getting a bug report if they also see the file which could not be removed.
It's pretty rare to use `-v`/`$VERBOSE=true`, so only in `$VERBOSE=true` means losing most of the value.

The exception would be even better IMO, maybe we can first warn and then in the next version actually raise?

----------------------------------------
Bug #18784: `FileUtils.rm_f` and `FileUtils.rm_rf` should not mask exceptions
https://bugs.ruby-lang.org/issues/18784#change-99006

* Author: deivid (David Rodríguez)
* Status: Open
* Priority: Normal
* Assignee: mame (Yusuke Endoh)
* ruby -v: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
In recent times, I've been having issues with these methods because they don't let you know when some issue happened while trying to remove the given folders/files.

IMO most users expect all pre-existing folders/files that are passed to these methods to be actually removed by the methods. Instead, when this happens, errors are silently swallowed and normally the result is that you will get some other issue further down the road, making the problem hard to debug.

The current workaround I'm using is to double check whether the files still exist after the method, and raise a custom error if they do, but I still can't see the original problem, so issues are similarly hard to debug.

This is also a deviation from how `rm -rf` and `rm -f` work, since these tools finish with a failure exit code when they fail to remove the given files. Given that `fileutils` is intended to mimic shell functionality, I think this is just a bug.

I think the intention of the `force` flag here is to:
* Don't prompt for confirmation.
* Ignore given arguments that are not files that already exist.

But any issue other than that should not be swallowed, and in general I think the method should succeed if and only if the given list of file names does not exist after the methods are done.

I think this is in line with the following note I get when I run `man rm`, but also suggests that this is not the standard behavior of "historical implementations"

> COMPATIBILITY
>
> The rm utility differs from historical implementations in that the -f option only masks attempts to remove non-existent files instead of masking a large variety of errors.

I implemented this at https://github.com/ruby/fileutils/pull/58, but treating this as a bug. I can also implement a more conservative for approach for users that might be using `FileUtils.rm_rf` or `FileUtils.rm_f` but don't really care if the files are removed or not.

Alternative proposals would be `FileUtils.rm_rf(force: strict)`, or `FileUtils.strict_rm_rf`, but to be honest, if this is considered a breaking change, I would ship it as a new major version, and let users update their code to swallow errors themselves if they need to.

Happy to hear any feedback!



-- 
https://bugs.ruby-lang.org/

  parent reply	other threads:[~2022-08-29 12:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-16  9:04 [ruby-core:108565] [Ruby master Bug#18784] `FileUtils.rm_f` and `FileUtils.rm_rf` should not mask exceptions deivid
2022-07-19  7:50 ` [ruby-core:109240] " mame (Yusuke Endoh)
2022-07-19 10:15 ` [ruby-core:109245] " deivid
2022-07-21 13:11 ` [ruby-core:109285] " mame (Yusuke Endoh)
2022-07-21 14:12 ` [ruby-core:109287] " deivid
2022-07-21 14:53 ` [ruby-core:109288] " deivid
2022-07-26 12:46 ` [ruby-core:109324] " mame (Yusuke Endoh)
2022-07-26 14:27 ` [ruby-core:109326] " deivid
2022-08-26  4:08 ` [ruby-core:109701] " mame (Yusuke Endoh)
2022-08-26  5:34 ` [ruby-core:109703] " koic (Koichi ITO)
2022-08-26  7:16 ` [ruby-core:109704] " deivid
2022-08-26  8:17 ` [ruby-core:109705] " deivid
2022-08-27 11:18 ` [ruby-core:109732] " mame (Yusuke Endoh)
2022-08-27 11:24 ` [ruby-core:109733] " deivid
2022-08-27 11:40 ` [ruby-core:109734] " mame (Yusuke Endoh)
2022-08-27 13:16 ` [ruby-core:109735] " Eregon (Benoit Daloze)
2022-08-29 11:36 ` [ruby-core:109761] " mame (Yusuke Endoh)
2022-08-29 11:39 ` [ruby-core:109762] " mame (Yusuke Endoh)
2022-08-29 11:47 ` [ruby-core:109763] " Eregon (Benoit Daloze)
2022-08-29 12:02 ` [ruby-core:109765] " deivid
2022-08-29 12:58 ` Eregon (Benoit Daloze) [this message]
2022-08-30  7:37 ` [ruby-core:109783] " mame (Yusuke Endoh)
2022-08-30  8:14 ` [ruby-core:109784] " deivid
2022-08-30 10:45 ` [ruby-core:109787] " deivid
2022-09-22 23:14 ` [ruby-core:110014] " mame (Yusuke Endoh)
2022-09-23 12:55 ` [ruby-core:110044] " Dan0042 (Daniel DeLorme)
2022-11-07 11:27 ` [ruby-core:110637] " mame (Yusuke Endoh)
2022-11-14  3:09 ` [ruby-core:110739] " mame (Yusuke Endoh)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-99006.20220829125836.7174@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).