Re: [PATCH 21/44] t5704: send object-format capability with SHA-256

From: Martin Ågren <martin.agren@gmail.com>
Date: May 16, 2020, 11:02 UTC
Message-ID: <CAN0heSqf8E9cXvJcDJmx_YU5Eiwnqre-g3MwU3UMe40SioCNPA@mail.gmail.com>
In-Reply-To: <20200513005424.81369-22-sandals@crustytoothpaste.net>

On Wed, 13 May 2020 at 02:56, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 6 quoted lines

>
> When we speak protocol v2 in this test, we must pass the object-format
> header if the algorithm is not SHA-1.  Otherwise, git upload-pack fails
> because the hash algorithm doesn't match and not because we've failed to
> speak the protocol correctly.  Pass the header so that our assertions
> test what we're really interested in.

Show 25 quoted lines

> +# If we don't print the object format, we'll fail for a spurious reason: the
> +# mismatched object format.
> +print_object_format () {
> +       local algo=$(test_oid algo) &&
> +       if test "$algo" != "sha1"
> +       then
> +               packetize "object-format=$algo"
> +       fi
> +}
> +
>  test_expect_success 'extra delim packet in v2 ls-refs args' '
>         {
>                 packetize command=ls-refs &&
> +               print_object_format &&
>                 printf 0001 &&
>                 # protocol expects 0000 flush here
>                 printf 0001
> @@ -21,6 +32,7 @@ test_expect_success 'extra delim packet in v2 ls-refs args' '
>  test_expect_success 'extra delim packet in v2 fetch args' '
>         {
>                 packetize command=fetch &&
> +               print_object_format &&
>                 printf 0001 &&
>                 # protocol expects 0000 flush here
>                 printf 0001

So we need to pass this capability for the SHA-256 tests to run ok. But if we start passing "object-format=sha1" unconditionally at this point in the series, the tests will fail:

  error: 'grep expected flush after ls-refs arguments err' didn't find
a match in:
  fatal: unknown capability 'object-format=sha1'

That is, we don't yet actually implement "object-format" handling. So this will still fail with SHA-256 ("unknown capability"), just that once the implementation is in place, the SHA-256 tests will pass (as will the normal SHA-1 runs). Do I understand that correctly?

Or put differently, by the end of the series, we can do this:

diff --git a/t/t5704-protocol-violations.sh b/t/t5704-protocol-violations.sh
index 47e78932b9..22993812e2 100755
--- a/t/t5704-protocol-violations.sh
+++ b/t/t5704-protocol-violations.sh
@@ -6,14 +6,11 @@ communications if the other side says something
unexpected. We are mostly
 making sure that we do not segfault or otherwise behave badly.'
 . ./test-lib.sh

-# If we don't print the object format, we'll fail for a spurious reason: the
-# mismatched object format.
+# If we don't print the object format, we might fail for a spurious reason:
+# the mismatched object format.
 print_object_format () {
        local algo=$(test_oid algo) &&
-       if test "$algo" != "sha1"
-       then
-               packetize "object-format=$algo"
-       fi
+       packetize "object-format=$algo"
 }

 test_expect_success 'extra delim packet in v2 ls-refs args' '

Should we? (And if we do, we might as well drop this function and inline
the whole thing, IMHO.)


Martin