Subject: Re: [PATCH] transport-helper: re-examine object dir after fetching

From: Junio C Hamano <[email-masked]>
Date: Wed, 24 Jan 2024 11:31:09 -0800

Jeff King <[email-masked]> writes:

> Thanks, I was able to reproduce it. Besides using the v0 protocol, two
> key elements are that the server is http and the use of --depth.
>
> The patch below explains what's going on and should fix it. I prepared
> the patch on top of 'master', but it can also be applied directly on
> 61c7711cfe or on v2.21.0, modulo some minor textual conflicts in the
> test script (modern t5551 has some more tests, and no longer calls
> stop_httpd manually).

Thanks.

The usual "one liner fix that requires two-page explanation"
that only you can produce ;-).

> ...
>      So everything works, but mostly due to luck. Whereas in a fetch
>      with --depth, we skip step 2 entirely, and thus the out-of-date
>      cache is still in place for step 3, giving us the wrong answer.
>
> So the test works with a small "--depth 1" fetch, which makes sure that
> we don't store the pack from the other side, and that we don't trigger
> the accidental cache invalidation. And of course it forces the use of
> v0 along with using the http protocol.
>
> Signed-off-by: Jeff King <[email-masked]>
> ---
>  t/t5551-http-fetch-smart.sh | 18 ++++++++++++++++++
>  transport-helper.c          |  3 +++
>  2 files changed, 21 insertions(+)
>
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index e069737b80..a623a1058c 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -733,4 +733,22 @@ test_expect_success 'no empty path components' '
>  	! grep "//" log
>  '
>  
> +test_expect_success 'tag following always works over v0 http' '
> +	upstream=$HTTPD_DOCUMENT_ROOT_PATH/tags &&
> +	git init "$upstream" &&
> +	(
> +		cd "$upstream" &&
> +		git commit --allow-empty -m base &&
> +		git tag not-annotated &&
> +		git tag -m foo annotated
> +	) &&
> +	git init tags &&
> +	git -C tags -c protocol.version=0 \
> +		fetch --depth 1 $HTTPD_URL/smart/tags \
> +		refs/tags/annotated:refs/tags/annotated &&
> +	git -C "$upstream" for-each-ref refs/tags >expect &&
> +	git -C tags for-each-ref >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_done
> diff --git a/transport-helper.c b/transport-helper.c
> index e34a8f47cf..07e42e239a 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -17,6 +17,7 @@
>  #include "refspec.h"
>  #include "transport-internal.h"
>  #include "protocol.h"
> +#include "packfile.h"
>  
>  static int debug;
>  
> @@ -432,6 +433,8 @@ static int fetch_with_fetch(struct transport *transport,
>  			warning(_("%s unexpectedly said: '%s'"), data->name, buf.buf);
>  	}
>  	strbuf_release(&buf);
> +
> +	reprepare_packed_git(the_repository);
>  	return 0;
>  }