Subject: Re: [PATCH v2 03/10] scalar-diagnose: add directory to archiver more gently

From: Junio C Hamano <[email-masked]>
Date: Thu, 04 Aug 2022 10:12:37 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

>> +	dir = opendir(at_root ? "." : path);
>> +	if (!dir) {
>> +		if (errno == ENOENT) {
>
> Per [1]

"Per [1]" somehow sounds more like a reference to an authoritative
source, at least to me.  Every time you use it, I have to see what
it refers to, and after realizing that you used it as a replacement
of "I said it already in [1]" again, it leaves a funny feeling.

> I think this is incorrect or overly strict. Let's not spew
> warnings if the user "rm -rf .git/hooks" or whatever.

The above is doing the right thing even in that situation, doesn't
it?  If there is no ".git/hooks" that is fine.  We get ENOENT, give
a warning to indicate that we found an unusual situation, and return
without failing.  If we got something other than ENOENT, we fail with
error_errno(), because opendir() failed for a reason other than "No
such file or directory".

> You already have an errno, so using *_errno() will add the standard
> information about what the issue is.

Reading the code aloud, slowly, may help.  When errno says ENOENT,
we know opendir() failed because of "No such file or directory",
so "path" was missing.  So let's say 'not archiving a missing directory'".

ENOENT or "No such file or directory" is an implementation detail
that does not help the end user.

The other side, i.e. when the errno is *not* ENOENT, already uses
error_errno().

So, I am puzzled.