fix(#4886) more: panics if file is not readable

[Ludmuterol]

fix for #4886 (comment)
i just changed the unwrap for the File::open to a match and i hope it actually is a UUsageError i am supposed to return here. I chose UsageError since the code above mine chose UsageError for a similar case

output of the preinstalled more on my linux: more: cannot open foo: Permission denied
output of this more: ./more: cannot open 'foo': permission denied Try './more --help' for more information.

[cakebaker]

Thanks for your PR :)

I wouldn't use a UUsageError in this case because the user didn't do anything wrong.

And can you please add a test to tests/by-util/test_more.rs to ensure we won't regress in the future?

[Ludmuterol]

:) Sure! If I understand correctly i only need to add a file into the fixtures folder (in the correct sub folder) to be able to use it? i did so and changed it to the 244 perms and now every test panics...

failures:

---- test_more::test_more_dir_arg stdout ----
thread 'test_more::test_more_dir_arg' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', tests/common/util.rs:1134:76

---- test_more::test_valid_arg stdout ----
thread 'test_more::test_valid_arg' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', tests/common/util.rs:1134:76

---- test_more::test_more_no_arg stdout ----
thread 'test_more::test_more_no_arg' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', tests/common/util.rs:1134:76


failures:
    test_more::test_more_dir_arg
    test_more::test_more_no_arg
    test_more::test_valid_arg

i get that comes from recursive copy where it returns an error since it is not allowed to copy.
my idea would be to change the perms in the test, but then i'd need to do that OperatingSystem independent. std::fs::Permissions only supports readonly atm and we want to make it write only. I could go for a Unix solution: https://doc.rust-lang.org/std/os/unix/fs/trait.PermissionsExt.html but then that wont work on other OS's. any suggestions?

[cakebaker]

You won't need fixtures in this case. And a Unix solution is fine. For inspiration you might want to have a look at the tests for cp: https://github.com/uutils/coreutils/blob/main/tests/by-util/test_cp.rs . Hope that helps :)

[Ludmuterol]

(base) tiberius@pop-os:~/Desktop/coreutils/target/release$ touch foo
(base) tiberius@pop-os:~/Desktop/coreutils/target/release$ chmod 244 foo
(base) tiberius@pop-os:~/Desktop/coreutils/target/release$ more foo
more: cannot open foo: Permission denied
(base) tiberius@pop-os:~/Desktop/coreutils/target/release$ ./more foo
./more: cannot open 'foo': permission denied
                                            (base) tiberius@pop-os:~/Desktop/coreutils/target/release$ 

Changed to USimpleError and added a test case.
i have a weird newline(?) at the end of the output tho.. is that expected or an error on my part? (or even an error of my shell, etc..)
and i cant see any new characters i try to input unless i completely restart the shell. (although executing commands does work i only cant see them)

my quick research and my dad say the echo is missing/not happening.

[sylvestre]

please replace the screenshot by text: it is terrible for search and accessibility

[Ludmuterol]

should i add a "skip test on not unix" inside my newly added test?

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/tail-2/inotify-dir-recreate is no longer failing!

[sylvestre]

On windows, fails with:

error[E0433]: failed to resolve: could not find `unix` in `os`
 --> tests\by-util\test_more.rs:4:14
  |
4 | use std::os::unix::fs::PermissionsExt;
  |              ^^^^ could not find `unix` in `os`

warning: unused import: `std::io::Write`
 --> tests\by-util\test_du.rs:9:5

[cakebaker]

i have a weird newline(?) at the end of the output tho.. is that expected or an error on my part? (or even an error of my shell, etc..)
and i cant see any new characters i try to input unless i completely restart the shell. (although executing commands does work i only cant see them)

You forgot to disable "raw mode" before returning the error :)

should i add a "skip test on not unix" inside my newly added test?

Yes, please, otherwise the CICD will fail when running it on Windows. You probably also have to use is_terminal() to make it work on the CICD, like it is done in the other functions in test_more.rs.

[Ludmuterol]

Ah yes I missed that :) thank you!

[cakebaker]

Thanks :)