Re: [PATCH v6+ 4/7] scalar: implement `scalar diagnose`

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 10, 2022, 02:08 UTC
Message-ID: <220610.86ilp9s1x7.gmgdl@evledraar.gmail.com>
In-Reply-To: <20220528231118.3504387-5-gitster@pobox.com>

On Sat, May 28 2022, Junio C Hamano wrote:

Show 13 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> [...]
> The `diagnose` command is the culmination of this hard-won knowledge: it
> gathers the installed hooks, the config, a couple statistics describing
> the data shape, among other pieces of information, and then wraps
> everything up in a tidy, neat `.zip` archive.
> [...]
> +	if ((res = add_directory_to_archiver(&archiver_args, ".git", 0)) ||
> +	    (res = add_directory_to_archiver(&archiver_args, ".git/hooks", 0)) ||
> +	    (res = add_directory_to_archiver(&archiver_args, ".git/info", 0)) ||
> +	    (res = add_directory_to_archiver(&archiver_args, ".git/logs", 1)) ||
> +	    (res = add_directory_to_archiver(&archiver_args, ".git/objects/info", 0)))
> +		goto diagnose_cleanup;

Noticed on top of some local changes I have to not add a .git/hooks (the --no-template topic), but this fails to diagnose any repo that doesn't have these paths, which are optional, either because a user could have manually removed them, or used --template=.

although I don't think there's a way to create that sort of repo with the scalar tooling, it doesn't seem to forward that option, but I didn't look deeply.

So, no big deal, but it would be nice to have that fixed. Is there a reason for why this mere addition of various stuff for diagnosis goes straight to an opendir() and error on failure, as opposed to doing an lstat() etc. first?