Re: [PATCH v2 03/10] scalar-diagnose: add directory to archiver more gently
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-08-04 06:22:56
On Thu, Aug 04 2022, Victoria Dye via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Victoria Dye <redacted> 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 [off-list ref] Signed-off-by: Victoria Dye <redacted> --- 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/ (local)