| Message ID | 20200513005424.81369-22-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256 part 2/3: protocol functionality | expand |
On Wed, 13 May 2020 at 02:56, brian m. carlson <sandals@crustytoothpaste.net> wrote: > > 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. > +# 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
On 2020-05-16 at 11:02:48, Martin Ågren wrote: > 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? Yes, that's correct. > 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.) We certainly can. I'll move this later on in the series so that we can simplify the code.
diff --git a/t/t5704-protocol-violations.sh b/t/t5704-protocol-violations.sh index 950cfb21fe..47e78932b9 100755 --- a/t/t5704-protocol-violations.sh +++ b/t/t5704-protocol-violations.sh @@ -6,9 +6,20 @@ 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. +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
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. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- t/t5704-protocol-violations.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)