Re: [PATCH] fetch: do not pass ref-prefixes for fetch by exact SHA1

> When v2.18.0-rc0~10^2~1 (refspec: consolidate ref-prefix generation
> logic, 2018-05-16) factored out the ref-prefix generation code for
> reuse, it left out the 'if (!item->exact_sha1)' test in the original
> ref-prefix generation code. As a result, fetches by SHA-1 generate
> ref-prefixes as though the SHA-1 being fetched were an abbreviated ref
> name:
>
>  $ GIT_TRACE_PACKET=1 bin-wrappers/git -c protocol.version=2 \
>         fetch origin 12039e008f9a4e3394f3f94f8ea897785cb09448
> [...]
>  packet:        fetch> ref-prefix 12039e008f9a4e3394f3f94f8ea897785cb09448
>  packet:        fetch> ref-prefix refs/12039e008f9a4e3394f3f94f8ea897785cb09448
>  packet:        fetch> ref-prefix refs/tags/12039e008f9a4e3394f3f94f8ea897785cb09448
>  packet:        fetch> ref-prefix refs/heads/12039e008f9a4e3394f3f94f8ea897785cb09448
>  packet:        fetch> ref-prefix refs/remotes/12039e008f9a4e3394f3f94f8ea897785cb09448
>  packet:        fetch> ref-prefix refs/remotes/12039e008f9a4e3394f3f94f8ea897785cb09448/HEAD
>  packet:        fetch> 0000
>
> If there is another ref name on the command line or the object being
> fetched is already available locally, then that's mostly harmless.
> But otherwise, we error out with
>
>  fatal: no matching remote head
>
> since the server did not send any refs we are interested in.  Filter
> out the exact_sha1 refspecs to avoid this.
>
> This patch adds a test to check this behavior that notices another
> behavior difference between protocol v0 and v2 in the process.  Add a
> NEEDSWORK comment to clear it up.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Here's the change described in
> https://public-inbox.org/git/20180531010739.GB36515@aiede.svl.corp.google.com/
> as a proper patch.
>
> Thoughts of all kinds welcome, as always.
>
>  refspec.c             |  2 ++
>  refspec.h             |  4 ++++
>  t/t5516-fetch-push.sh | 19 +++++++++++++++++++
>  3 files changed, 25 insertions(+)
>
> diff --git a/refspec.c b/refspec.c
> index c59a4ccf1e..ada7854f7a 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -202,6 +202,8 @@ void refspec_ref_prefixes(const struct refspec *rs,
>                 const struct refspec_item *item = &rs->items[i];
>                 const char *prefix = NULL;
>
> +               if (item->exact_sha1)
> +                       continue;
>                 if (rs->fetch == REFSPEC_FETCH)
>                         prefix = item->src;
>                 else if (item->dst)
> diff --git a/refspec.h b/refspec.h
> index 01b700e094..3a9363887c 100644
> --- a/refspec.h
> +++ b/refspec.h
> @@ -42,6 +42,10 @@ void refspec_clear(struct refspec *rs);
>  int valid_fetch_refspec(const char *refspec);
>
>  struct argv_array;
> +/*
> + * Determine what <prefix> values to pass to the peer in ref-prefix lines
> + * (see Documentation/technical/protocol-v2.txt).
> + */
>  void refspec_ref_prefixes(const struct refspec *rs,
>                           struct argv_array *ref_prefixes);
>
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index f4d28288f0..a5077d8b7c 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1121,6 +1121,25 @@ test_expect_success 'fetch exact SHA1' '
>         )
>  '
>
> +test_expect_success 'fetch exact SHA1 in protocol v2' '
> +       mk_test testrepo heads/master hidden/one &&
> +       git push testrepo master:refs/hidden/one &&
> +       git -C testrepo config transfer.hiderefs refs/hidden &&
> +       check_push_result testrepo $the_commit hidden/one &&
> +
> +       mk_child testrepo child &&
> +       git -C child config protocol.version 2 &&
> +
> +       # make sure $the_commit does not exist here
> +       git -C child repack -a -d &&
> +       git -C child prune &&
> +       test_must_fail git -C child cat-file -t $the_commit &&
> +
> +       # fetching the hidden object succeeds by default
> +       # NEEDSWORK: should this match the v0 behavior instead?
> +       git -C child fetch -v ../testrepo $the_commit:refs/heads/copy
> +'
> +
>  for configallowtipsha1inwant in true false
>  do
>         test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
> --
> 2.17.1.1185.g55be947832
>