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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Aug 04 2022, Victoria Dye via GitGitGadget wrote:

> From: Victoria Dye <vdye@github.com>
>
> If a directory added to the 'scalar diagnose' archiver does not exist, warn
> and return 0 from 'add_directory_to_archiver()' rather than failing with a
> fatal error. This handles a failure edge case where the '.git/logs' has not
> yet been created when running 'scalar diagnose', but extends to any
> situation where a directory may be missing in the '.git' dir.
>
> Now, when a directory is missing a warning is captured in the diagnostic
> logs. This provides a user with more complete information than if 'scalar
> diagnose' simply failed with an error.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
>  contrib/scalar/scalar.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
> index 04046452284..b9092f0b612 100644
> --- a/contrib/scalar/scalar.c
> +++ b/contrib/scalar/scalar.c
> @@ -266,14 +266,20 @@ static int add_directory_to_archiver(struct strvec *archiver_args,
>  					  const char *path, int recurse)
>  {
>  	int at_root = !*path;
> -	DIR *dir = opendir(at_root ? "." : path);
> +	DIR *dir;
>  	struct dirent *e;
>  	struct strbuf buf = STRBUF_INIT;
>  	size_t len;
>  	int res = 0;
>  
> -	if (!dir)
> +	dir = opendir(at_root ? "." : path);
> +	if (!dir) {
> +		if (errno == ENOENT) {

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

It might be valuable to note in some file in the archive such oddities
we find, but warning() won't give us that.

> +			warning(_("could not archive missing directory '%s'"), path);

In any case, this should be e.g.:

	warning_errno(_("could not archive directory '%s'"), path);

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

> +			return 0;
> +		}
>  		return error_errno(_("could not open directory '%s'"), path);
> +	}
>  
>  	if (!at_root)
>  		strbuf_addf(&buf, "%s/", path);

1. https://lore.kernel.org/git/220610.86ilp9s1x7.gmgdl@evledraar.gmail.com/