On Fri, Dec 02, 2022 at 02:02:55AM +0100, René Scharfe wrote:
> Am 02.12.2022 um 01:06 schrieb Ævar Arnfjörð Bjarmason:
> > Don't hide the exit code from the "git checkout" we run to checkout
> > our attributes file.
> >
> > This fixes cases where we'd have e.g. missed memory leaks under
> > SANITIZE=leak, this code doesn't leak (the relevant "git checkout"
> > leak has been fixed), but in a past version of git we'd continue past
> > this failure under SANITIZE=leak when these invocations had errored
> > out, even under "--immediate".
> >
> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> > ---
> > t/t0027-auto-crlf.sh | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> > index a94ac1eae37..574344a99db 100755
> > --- a/t/t0027-auto-crlf.sh
> > +++ b/t/t0027-auto-crlf.sh
> > @@ -294,11 +294,17 @@ checkout_files () {
> > pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
> > for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
> > do
> > - rm crlf_false_attr__$f.txt &&
> > - if test -z "$ceol"; then
> > - git checkout -- crlf_false_attr__$f.txt
> > + if test -z "$ceol"
> > + then
> > + test_expect_success "setup $f checkout" '
> > + rm crlf_false_attr__$f.txt &&
> > + git checkout -- crlf_false_attr__$f.txt
> > + '
> > else
> > - git -c core.eol=$ceol checkout -- crlf_false_attr__$f.txt
> > + test_expect_success "setup $f checkout with core.eol=$ceol" '
> > + rm crlf_false_attr__$f.txt &&
> > + git -c core.eol=$ceol checkout -- crlf_false_attr__$f.txt
> > + '
>
> That adds five test_expect_success calls. Wouldn't one suffice, for the
> whole for loop, and a "|| return 1"?
>
> One line above the context there's a "git config" call that should also
> be covered, right?
>
> Side note: The checkout commands only differ in their -c parameter.
> They could be unified like this, which might simplify handling their
> return code:
>
> git ${ceol:+-c core.eol=$ceol} checkout -- crlf_false_attr__$f.txt
>
> > fi
> > done
> >
That is a nice one. (I learned about the ${:+} yesterday).
Is it supported by all the shells/os combinations ?
If not, we can still extract the if:
if test -z "$ceol"; then
CEOL="-c core.eol=$ceol"
else
CEOL=""
fi
git $CEOL checkout -- crlf_false_attr__$f.txt &&
The original reasoning (for not looking at the return code) was that
if the checkout fails, the test suite will fail further down anyway.
But checking for failures early is a good thing.
Thanks for cleaning up my mess.
