`touch no-file/` fails with a sub optimal error

[sylvestre]

$ touch no-file/
touch: cannot touch 'no-file/': Other

GNU:

$ touch no-file/
touch: setting times of 'no-file/': No such file or directory

We should probably mix the two and have

touch: cannot touch 'no-file/': No such file or directory

The code is in:

coreutils/src/uu/touch/src/touch.rs

Line 88 in 1bb23cd

show!(e.map_err_context(|| format!("cannot touch '{}'", path.display())));

@miDeb Any idea how to implement that? thanks :)

[tertsdiepraam]

The current uutils behaviour is:

touch: cannot touch 'no-file/': Is a directory

[sylvestre]

not great either :)

[goodhoko]

May I give this a shot?

[tertsdiepraam]

Yeah, go ahead!

[goodhoko]

I checked out main and ran cargo run touch no-file/ and got

touch: cannot touch 'no-file/': No such file or directory

It seems this issue has been already addressed in #2408. The code that handled this was later removed in 0cfaaec#diff-9565d8fad7b29837b3b468a61a809dc187fb351a72421ffe4b7c4879a8445b27R85 but the test from the PR is still in place and is passing.

@tertsdiepraam

The current uutils behaviour is: touch: cannot touch 'no-file/': Is a directory

How (platform, version of uutils) are you getting this behaviour? I couldn't reproduce it (macos, built from main).

[tertsdiepraam]

Maybe it depends on whether or not no-file/ exists as a directory? I.e. before and after mkdir no-file

[goodhoko]

@tertsdiepraam I did try these scenarios and they all seem to behave as expected:

$ touch non-existent
# no output, creates a file named "non-existent"

$ touch non-existent/
touch: cannot touch 'non-existent/': No such file or directory

$ touch existing-dir
# no output, touches the dir

$ touch existing-dir/
# no output, touches the dir

these behaviors are consistent across my default touch implementation (BSD), GNU's and uutils'. Except the slight wording differences: BSD's leaves out the cannot touch part and GNU's replaces it with setting times of as described in the issue description.

Maybe it depends on whether or not no-file/ exists as a directory? I.e. before and after mkdir no-file

The case when no-file actually exists as directory doesn't and AFAIK shouldn't produce any error at all.

Anyway, the uutils' implementation seems to behave just this issue proposes. Am I missing something?

[tertsdiepraam]

Very curious, I'm still getting the same result, but maybe it's me who's missing something :) I'm on Linux and on the main branch.

❯ cargo run -- no-file/
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `/home/terts/Programming/coreutils/target/debug/touch no-file/`
/home/terts/Programming/coreutils/target/debug/touch: cannot touch 'no-file/': Is a directory

The case when no-file actually exists as directory doesn't and AFAIK shouldn't produce any error at all.

Oh yeah, that's true. Thanks for pointing that out!

[goodhoko]

Very curious, I'm still getting the same result, but maybe it's me who's missing something :) I'm on Linux and on the main branch.

Curious indeed! Seems like maybe the platform is the culprit here. Gimme some more time to try on linux as well. And also to find out where exactly these messages originate.

[chipbuster]

The issue appears to originate in this block. The Error returned by File::create is OS-dependent, and so is the message.

On Linux, printing the error gives Os { code: 21, kind: IsADirectory, message: "Is a directory" }
On MacOS, it gives Os { code: 2, kind: NotFound, message: "No such file or directory" }
on Windows, it gives Os { code: 123, kind: InvalidFilename, message: "The filename, directory name, or volume label syntax is incorrect." }

The last part of the error message on each platform appears to be just the message field of the Error (I'm guessing generated by map_err_context, but I'm not 100% sure).

[tertsdiepraam]

Thanks for investigating this!

(I'm guessing generated by map_err_context, but I'm not 100% sure).

Yes that is correct

[chipbuster]

It seems to me that there might need to be some custom error message rules around this, since modifying the implementation of map_err_context feel like too broad of a change for this.

I'm thinking something like take the error from line 119 and then write a function that interprets it based on the OS and outputs the appropriate error message. Would that be an acceptable solution?

(Also, I'm not tied to writing this---if @goodhoko still wants to work on this, I would be happy to let them).