On Thu, Aug 04 2022, Victoria Dye via GitGitGadget wrote:
> From: Victoria Dye <vdye@xxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Victoria Dye <vdye@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxx/
