On March 11, 2020 6:06 PM, Junio C Hamano wrote:
> "Randall S. Becker" <rsbecker@xxxxxxxxxxxxx> writes:
>
> > This situation still occurs at 2.26.0-rc0. As above, this results from
> > a text compare to a platform-specific message that should not be used.
> > To hack around it, a possible fix (which I don't like) could be as follows:
>
> Martin's fix 4605a730 (t1091: don't grep for `strerror()` string,
> 2020-03-08) was merged to 'next' on the 9th and then down to 'master'
> today, and will be in the final (unless there is some issues with it, which I do
> not think will be the case).
>
> Thanks.
>
> -- >8 --
> From: Martin Ågren <martin.agren@xxxxxxxxx>
> Subject: [PATCH] t1091: don't grep for `strerror()` string
>
> We grep for "File exists" in stderr of the failing `git sparse-checkout` to make
> sure that it failed for the right reason. We expect the string to show up there
> since we call `strerror(errno)` in `unable_to_lock_message()` in lockfile.c.
>
> On the NonStop platform, this fails because the error string is "File already
> exists", which doesn't match our grepping.
>
> See 9042140097 ("test-dir-iterator: do not assume errno values",
> 2019-07-30) for a somewhat similar fix. There, we patched a test helper,
> which meant we had access to `errno` and could investigate it better in the
> test helper instead of just outputting the numerical value and evaluating it in
> the test script. The current situation is different, since (short of modifying the
> lockfile machinery, e.g., to be more
> verbose) we don't have more than the output from `strerror()` available.
>
> Except we do: We prefix `strerror(errno)` with `_("Unable to create
> '%s.lock': ")`. Let's grep for that part instead. It verifies that we were indeed
> unable to create the lock file. (If that fails for some other reason than the file
> existing, we really really should expect other tests to fail as well.)
I'm hoping that that error message matches. Will test it as soon as it is final is out there.
> An alternative fix would be to loosen the expression a bit and grep for
> "File.* exists" instead. There would be no guarantee that some other
> implementation couldn't come up with another error string, That is, that
> could be the first move in an endless game of whack-a-mole. Of course, it
> could also take us from "99" to "100" percent of the platforms and we'd
> never have this problem again. But since we have another way of addressing
> this, let's not even try the "loosen it up a bit" strategy.
>
> Reported-by: Randall S. Becker <rsbecker@xxxxxxxxxxxxx>
> Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx>
> Acked-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> t/t1091-sparse-checkout-builtin.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-
> builtin.sh
> index b4c9c32a03..44a91205d6 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -305,7 +305,7 @@ test_expect_success 'fail when lock is taken' '
> test_when_finished rm -rf repo/.git/info/sparse-checkout.lock &&
> touch repo/.git/info/sparse-checkout.lock &&
> test_must_fail git -C repo sparse-checkout set deep 2>err &&
> - test_i18ngrep "File exists" err
> + test_i18ngrep "Unable to create .*\.lock" err
> '
>
> test_expect_success '.gitignore should not warn about cone mode' '
> --
> 2.26.0-rc1-6-ga56d361f66
This still really helps stabilize things for the NonStop platform.
Thanks!
Randall
