Re: [PATCH] scalar reconfigure -a: remove stale `scalar.repo` entries
From: Derrick Stolee <hidden>
Date: 2022-11-07 20:15:27
On 11/7/22 1:25 PM, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <redacted> Every once in a while, a Git for Windows installation fails because the attempt to reconfigure a Scalar enlistment failed because it was deleted manually without removing the corresponding entries in the global Git config.
Not actually important to this patch: does it actually fail, or does it just report a warning message (which _looks_ like a failure)?
In f5f0842d0b5 (scalar: let 'unregister' handle a deleted enlistment directory gracefully, 2021-12-03), we already taught `scalar delete` to handle the case of a manually deleted enlistment gracefully. This patch adds the same graceful handling to `scalar reconfigure --all`. This patch is best viewed with `--color-moved`.
Thanks, that did help, even if it's a small change.
quoted hunk ↗ jump to hunk
@@ -638,8 +656,22 @@ static int cmd_reconfigure(int argc, const char **argv) strbuf_reset(&gitdir); if (chdir(dir) < 0) { - warning_errno(_("could not switch to '%s'"), dir); - res = -1; + struct strbuf buf = STRBUF_INIT; + + if (errno != ENOENT) { + warning_errno(_("could not switch to '%s'"), dir); + res = -1; + continue; + } + + strbuf_addstr(&buf, dir); + if (remove_deleted_enlistment(&buf)) + res = error(_("could not remove stale " + "scalar.repo '%s'"), dir); + else + warning(_("removing stale scalar.repo '%s'"), + dir); + strbuf_release(&buf);
Looks good.
+test_expect_success '`reconfigure -a` removes stale config entries' ' + git init stale/src && + scalar register stale && + scalar list >scalar.repos && + grep stale scalar.repos && + rm -rf stale && + scalar reconfigure -a && + scalar list >scalar.repos && + ! grep stale scalar.repos +'
In his reply, Taylor mentioned it would be good to check that we still have the correct list of scalar.repos value. I think that might be critical since one "solution" to this problem of a stale repo could be to remove all scalar.repos values (and this test would currently pass). Please use a `test_cmp` against scalar.repos. Something like this works: test_expect_success '`reconfigure -a` removes stale config entries' ' git init stale/src && scalar register stale && scalar list >scalar.repos && grep stale scalar.repos && grep -v stale scalar.repos >expect && rm -rf stale && scalar reconfigure -a && scalar list >scalar.repos && test_cmp expect scalar.repos ' Thanks, -Stolee