install: print directory name instead of file name if mkstemp fails

[VexedUXR]

Printing the file name doesn't make sense since mkstemp failing means that the file wasn't created.

before:

$ install existing_file /nonexistent/file
install: /nonexistent/INS@5TLQxk: No such file or directory

after:

$ install existing_file /nonexistent/file
install: /nonexistent: No such file or directory

[jlduran]

This looks good, maybe add something like:

diff --git a/usr.bin/xinstall/tests/install_test.sh b/usr.bin/xinstall/tests/install_test.sh
index b35706521ec3..c7fe1decfd95 100755
--- a/usr.bin/xinstall/tests/install_test.sh
+++ b/usr.bin/xinstall/tests/install_test.sh
@@ -45,6 +45,13 @@ copy_to_nonexistent_body() {
        copy_to_nonexistent_with_opts
 }
 
+atf_test_case copy_to_nonexistent_dir
+copy_to_nonexistent_dir_body() {
+        printf 'test\n123\r456\r\n789\0z' >testf
+        atf_check -s not-exit:0 -e match:"install: /nonexistent: No such file or directory" \
+            install testf /nonexistent/testf
+}
+
 atf_test_case copy_to_nonexistent_safe
 copy_to_nonexistent_safe_body() {
        copy_to_nonexistent_with_opts -S
@@ -506,6 +513,7 @@ set_optional_exec_body()
 atf_init_test_cases() {
        atf_add_test_case copy_to_empty
        atf_add_test_case copy_to_nonexistent
+       atf_add_test_case copy_to_nonexistent_dir
        atf_add_test_case copy_to_nonexistent_safe
        atf_add_test_case copy_to_nonexistent_comparing
        atf_add_test_case copy_to_nonexistent_safe_comparing

[VexedUXR]

Sure, that looks reasonable

[VexedUXR]

Okay, committed but I've changed it a little

[jlduran]

Comparing this file with Darwin's, the fix still seems ok.

[bsdimp]

Also, bonus points for adding a test.

[jlduran]

Knowing the directory that failed is much more important, I think.

Yes, I was essentially pondering between using dirname(tempfile) (this patch) or to_name, to match the output produced in NetBSD/macOS/Linux.

[jlduran]

Sorry, I changed my mind. I would prefer to use to_name, and match the output of the aforementioned operating systems:

With dirname(tempfile):

% touch testf
% install testf nonexistent/
install: nonexistent: No such file or directory

With to_name:

% touch testf
% install testf nonexistent/
install: nonexistent/: No such file or directory

In my opinion, the original trailing slash should be printed.

[!NOTE]
I still believe the right thing to do is what is currently done: express clearly to the user our intentions to create a temporary file—as stated in the FILES section of install(1)—and failed (also matching OpenBSD's output). However, this is arguably more helpful to the end user. Given this is what Linux does, and GNU/Linux is the new POSIX, let's proceed with the change?

[VexedUXR]

Using to_name would make it so the output of

$ install foo nonexistent/bar

becomes:

install: nonexistent/bar: No such file or directory

which IMO doesn't make much sense. I'm expecting install to create bar, so why is it complaining about bar not existing?

About printing tempfile; When I first encountered this, I thought "why is install looking for files in my target directory? and why is the file name so odd?". Then I noticed that it looked like a mktemp(3) string, having a look at the manpage, sure enough. IMO it makes more sense to print the nonexistent directory name rather than the name of a file in said nonexistent directory.

I will add that NetBSD prints the function's name before the failure message, so the message is a bit clearer.

install: nonexistent/bar: open: No such file or directory

linux's output is also clearer

install: cannot create regular file 'nonexistent/bar': No such file or directory

My opinion is still that install: nonexistent: No such file or directory is clearer.

Anyways, I won't die on this hill. If others agree with printing to_name, then sure.

[jlduran]

No problem, Warner said that just printing the directory name is more important. I was just trying to find an edge case. At least having a test case clearly expresses the project's desired output. Thank you!

[bsdimp]

Well, I'm not averse to better error messages...

[jlduran]

It is fine as is. I'm bike-shedding on this pull request, even thinking about appending a slash to indicate it is a directory.