Re: [PATCH] scalar reconfigure -a: remove stale `scalar.repo` entries

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 7, 2022, 19:03 UTC
Message-ID: <Y2lWc2gmPaTfi4k5@nand.local>
In-Reply-To: <pull.1407.git.1667845501422.gitgitgadget@gmail.com>

On Mon, Nov 07, 2022 at 06:25:01PM +0000, Johannes Schindelin via GitGitGadget wrote:

Show 11 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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.
>
> 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`.

Makes sense, thanks.

Show 13 quoted lines

> @@ -166,6 +166,17 @@ test_expect_success 'scalar reconfigure' '
>  	test true = "$(git -C one/src config core.preloadIndex)"
>  '
>
> +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

Arguably this test would be slightly improved if we checked not only for '!grep stale scalar.repos', but that the correct set of repositories *does* appear in the list.

But presumably we have plenty of other tests which do so already, so we're not lacking any coverage here by omitting it.

This patch looks good to me, will queue.

Thanks, Taylor