Re: t0027 racy?

[Jeff King <peff@peff.net>]

On Tue, Aug 09, 2016 at 12:59:58PM +0000, Torsten Bögershausen wrote:

> Thanks for the explanation, so there are 2 chances for a race.
> 
> I assume that the suggested "touch" will fix race#2 in most cases.
> In my understanding, the change of the file size will be more reliable:
>  
> 
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 2860d2d..9933a9b 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -120,6 +120,7 @@ commit_chk_wrnNNO () {
>                 cp $f $fname &&
>                 printf Z >>"$fname" &&
>                 git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
> +               printf Z >>"$fname" &&
>                 git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1
>         done
> -------------------
> Does anybody agree ?

I think the mtime change is reliable. We know that the mtime on the file
will be greater than or equal to the index mtime (because it happened
afterwards), so git will always look at the on-disk contents.

With your change, "git commit" will also always re-read the file from
disk, because it actually has new content (and you provide the filename
on the command line, so it is stage-and-commit, not just
"commit-the-index").

So either is fine.

> And, by the way, the convert warning may be issued twice, once in
> "git add" and once in "git commit".

Yes, but you only save it from "git commit", so we can ignore what
happens from "add" here. But that's why I wondered if:

  git -c core.autocrlf=$crlf add $fname >"${pfx}_$f.err" 2>&1

would make more sense. We _know_ that we have to do convert_to_git() in
that step because the content is changed. And then you can ignore the
warnings from "git commit" (which are racy), or you can simply commit as
a whole later, as some other loops do.

But like Dscho, I do not actually understand what this test is checking.
The function is called commit_chk_wrnNNO(), so perhaps you really are
interested in what "commit" has to say. But IMHO that is not an
interesting test. We know that if it has to read the content from disk,
it will call convert_to_git(), which is the exact same code path used by
"git add". So I do not understand what it is accomplishing to make a
commit at all here.

-Peff