(+cc: Jonathan Tan, partial clone expert)
Xin Li wrote:
> Subject: fetch: allow adding a filter after initial clone.
nit: should be no period at the end of the subject line
> Signed-off-by: Xin Li <delphij@xxxxxxxxxx>
> Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Let's see whether I still stand by that. ;-)
[...]
> --- a/setup.c
> +++ b/setup.c
> @@ -13,6 +13,9 @@ static int work_tree_config_is_bogus;
> static struct startup_info the_startup_info;
> struct startup_info *startup_info = &the_startup_info;
>
> +static int verify_repository_format_eligibility(const struct repository_format *,
> + struct strbuf *, int);
Odd wrapping, but after Junio's suggestion this goes away.
[...]
> + warning("unable to upgrade repository format from %d to %d: %s",
> + repo_fmt.version, target_version, err.buf);
Same wrapping nit: this should use a tab to line up right after the
parenthesis (Git uses 8-space tabs).
[...]
> --- a/t/t1090-sparse-checkout-scope.sh
> +++ b/t/t1090-sparse-checkout-scope.sh
> @@ -63,7 +63,6 @@ test_expect_success 'in partial clone, sparse checkout only fetches needed blobs
> git -C server commit -m message &&
>
> test_config -C client core.sparsecheckout 1 &&
> - test_config -C client extensions.partialclone origin &&
Nice.
> echo "!/*" >client/.git/info/sparse-checkout &&
> echo "/a" >>client/.git/info/sparse-checkout &&
> git -C client fetch --filter=blob:none origin &&
> diff --git a/t/t2404-worktree-config.sh b/t/t2404-worktree-config.sh
> index 286121d8de..9536d10919 100755
> --- a/t/t2404-worktree-config.sh
> +++ b/t/t2404-worktree-config.sh
> @@ -23,8 +23,10 @@ test_expect_success 'config --worktree without extension' '
> '
>
> test_expect_success 'enable worktreeConfig extension' '
> + git config core.repositoryformatversion 1 &&
> git config extensions.worktreeConfig true &&
Yes, makes sense. Does this patch need it, or could this go in a
separate patch?
> - test_cmp_config true extensions.worktreeConfig
> + test_cmp_config true extensions.worktreeConfig &&
> + test_cmp_config 1 core.repositoryformatversion
This (both the existing code and the modified version) is strange: we
just set the config, so why are we checking it?
[...]
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -999,7 +999,6 @@ fetch_filter_blob_limit_zero () {
> test_config -C "$SERVER" uploadpack.allowfilter 1 &&
>
> git clone "$URL" client &&
> - test_config -C client extensions.partialclone origin &&
>
Nice.
[...]
> --- a/t/t5702-protocol-v2.sh
> +++ b/t/t5702-protocol-v2.sh
> @@ -348,7 +348,6 @@ test_expect_success 'partial fetch' '
> rm -rf client "$(pwd)/trace" &&
> git init client &&
> SERVER="file://$(pwd)/server" &&
> - test_config -C client extensions.partialClone "$SERVER" &&
>
> GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
> fetch --filter=blob:none "$SERVER" master:refs/heads/other &&
This test is a bit unusual, since it's fetching by URL instead of from
a remote. Looks like it comes from
commit ba95710a3bdcb2a80495b1d93a0e482dd69905e1
Author: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
Date: Thu May 3 16:46:56 2018 -0700
{fetch,upload}-pack: support filter in protocol v2
Jonathan, thoughts about this one? Is making it set
extensions.partialClone implicitly like the change above a good
change?
With the nits above and the bits Junio described addressed, this would
indeed be
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks.
