Re: t0027 racy?
- From
Jeff King <peff@peff.net>
- Date
- Aug 9, 2016, 13:27 UTC
- Message-ID
- <20160809132744.kjzmkgt2qiugeolj@sigill.intra.peff.net>
- In-Reply-To
- <20160809125958.GA1501@tb-raspi>
On Tue, Aug 09, 2016 at 12:59:58PM +0000, Torsten Bögershausen wrote:
Show 19 quoted lines
> 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>&1would 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