On 11/7/22 1:25 PM, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> 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.
> @@ -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
