df: better error message when executed in a chroot without /proc #3601

[lendandgit]

No description provided.

[sylvestre]

Please fix the warning:

error: useless use of `format!`
   --> src/uu/df/src/df.rs:451:25
    |
451 |                         format!("cannot read table of mounted file systems"),
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"cannot read table of mounted file systems".to_string()`
    |
    = note: `-D clippy::useless-format` implied by `-D warnings`

[lendandgit]

Thank you for pointing out the warning. I have fixed it in the new commit.

[tertsdiepraam]

Looks great, thanks! I just have one suggestion for a little bit of further cleanup.

[tertsdiepraam]

I think this could be:

use uucore::error::FromIo;
// --- snip ---
let filesystems = get_all_filesystems(&opt)
    .map_err_context(|| "cannot read table of mounted file systems".into())?;

See the docs for UIoError.

Sidenote: if you have any ideas about how to make this feature more discoverable/intuitive, I would be happy to hear it, because first-time contributors often miss this.

[lendandgit]

I made the requested changes in the new commit, thanks for the suggestion.

It would be useful for first-time contributors to explicitly recommend this way of error construction in the documentation.

[tertsdiepraam]

Nice! Is it possible to add a test for this?

[tertsdiepraam]

I'm wondering about this case, because it's currently missing a context. Do we know that GNU's error in this case is?

[lendandgit]

I tested the Rust and GNU versions of stat in the same environment I used to test df (chroot without proc).
While the Rust version fails, the GNU version does not but displays the expected output. For this reason I was not able to produce the error with GNU's stat.

I could only find GNU's error message in the sources:

https://github.com/coreutils/coreutils/blob/93e099e4c3b659b2e329f655fbdc73fdf594a66e/src/stat.c#L961

[tertsdiepraam]

Interesting, let's use that error message in stat then too

[lendandgit]

To add a test I would need a way to make /proc inaccessible to df. Is that possible?

[tertsdiepraam]

To add a test I would need a way to make /proc inaccessible to df. Is that possible?

Hmm, yeah I guess that's not really possible. At least not with how the tests are currently set up.

[sylvestre]

@tertsdiepraam is it ok with you?