From 9e97efa5f2b7f5877043fbbc65595291833df1f9 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 8 Nov 2022 09:00:57 -0500 Subject: [PATCH 1/3] scalar: diagnose should not retry When 672196a3073 (scalar-diagnose: use 'git diagnose --mode=all', 2022-08-12) updated 'scalar diagnose' to run 'git diagnose' as a subprocess, it was passed through the run_git() caller. Upstream, this caller does not repeat, but in microsoft/git we have a retry loop specifically for cases where 'git config' commands fail due to concurrency. If the underlying 'git diagnose' command fails, then the failure is repeated. Remove this retry loop for this case. Signed-off-by: Derrick Stolee --- scalar.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scalar.c b/scalar.c index 5a576392d2e0fa..9309fde68c8862 100644 --- a/scalar.c +++ b/scalar.c @@ -930,6 +930,8 @@ static int cmd_diagnose(int argc, const char **argv) setup_enlistment_directory(argc, argv, usage, options, &diagnostics_root); strbuf_addstr(&diagnostics_root, "/.scalarDiagnostics"); + /* Here, a failure should not repeat itself. */ + git_retries = 1; res = run_git("diagnose", "--mode=all", "-s", "%Y%m%d_%H%M%S", "-o", diagnostics_root.buf, NULL); From 064bc3ea8ee2e7da88d148dd6549bf4f8477148b Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 8 Nov 2022 09:08:34 -0500 Subject: [PATCH 2/3] fixup! scalar diagnose: include shared cache info When this loop was adapted into diagnose.c, it gained an error response when failing to read a file. This started causing consistent failures for users with a shared object cache, demonstrating a gap in our testing support. The error message itself is confusing: error: could not read '[...]/info': Permission denied fatal: unable to create diagnostics archive [...].zip: Permission denied The "Permission denied" is a red herring for both of these. The second one is due to a die_errno() even though it's not related to that filename. The first is actually due to trying to _read the contents of the directory_. Since we previously did not error out in this case, we never noticed this problem in the past. Now, it is causing 'scalar diagnose' to fail. The fix is to add the filename to the info directory path and read from that file. Be careful that the entry is actually a file. Signed-off-by: Derrick Stolee --- diagnose.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/diagnose.c b/diagnose.c index 52c4df114d5d04..09515e39bb80a3 100644 --- a/diagnose.c +++ b/diagnose.c @@ -322,6 +322,8 @@ int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode) } if (shared_cache) { + size_t path_len; + strbuf_reset(&buf); strbuf_addf(&path, "%s/pack", shared_cache); strbuf_reset(&buf); @@ -336,6 +338,8 @@ int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode) strbuf_reset(&path); strbuf_addf(&path, "%s/info", shared_cache); + path_len = path.len; + if (is_directory(path.buf)) { DIR *dir = opendir(path.buf); struct dirent *e; @@ -343,9 +347,16 @@ int create_diagnostics_archive(struct strbuf *zip_path, enum diagnose_mode mode) while ((e = readdir(dir))) { if (!strcmp(".", e->d_name) || !strcmp("..", e->d_name)) continue; + if (e->d_type == DT_DIR) + continue; strbuf_reset(&buf); strbuf_addf(&buf, "--add-virtual-file=info/%s:", e->d_name); + + strbuf_setlen(&path, path_len); + strbuf_addch(&path, '/'); + strbuf_addstr(&path, e->d_name); + if (strbuf_read_file(&buf, path.buf, 0) < 0) { res = error_errno(_("could not read '%s'"), path.buf); goto diagnose_cleanup; From cf052e5ce24887b2d9928967ab8eae1f907f292a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 18 Oct 2022 16:15:33 -0400 Subject: [PATCH 3/3] Makefile: force -O0 when compiling with SANITIZE=leak Cherry pick commit d3775de0 (Makefile: force -O0 when compiling with SANITIZE=leak, 2022-10-18), as otherwise the leak checker at GitHub Actions CI seems to fail with a false positive. [Backported to v2.37.0 for easier merging e.g. into Git for Windows] Signed-off-by: Jeff King Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index b8548e77d1cf78..b0c067b3e97337 100644 --- a/Makefile +++ b/Makefile @@ -1351,6 +1351,7 @@ BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS endif ifneq ($(filter leak,$(SANITIZERS)),) BASIC_CFLAGS += -DSUPPRESS_ANNOTATED_LEAKS +BASIC_CFLAGS += -O0 SANITIZE_LEAK = YesCompiledWithIt endif ifneq ($(filter address,$(SANITIZERS)),)