Re: Unexpected pass for t6120-describe.sh on cygwin

From: Michael J Gruber <git@grubix.eu>
Date: Sep 10, 2017, 12:27 UTC
Message-ID: <947699e6-c206-7818-3780-c4367e0996f0@grubix.eu>
In-Reply-To: <7c3db153-2a56-f27d-af71-e4b61f1252a1@ramsayjones.plus.com>

Ramsay Jones venit, vidit, dixit 09.09.2017 15:13:

Show 22 quoted lines

> Hi Adam,
> 
> I ran the test-suite on the 'pu' branch last night (simply because
> that was what I had built at the time!), which resulted in a PASS,
> but t6120 was showing a 'TODO passed' for #52.
> 
> This is a test introduced by Michael's 'mg/name-rev-tests-with-short-stack'
> branch, which uses 'ulimit -s' to try and force a stack overflow.
> Unfortunately, 'ulimit -s' seems to have no effect on cygwin. I created
> a test program (see below) to eat up the stack and tried running it
> with various ulimit values (128, 12, 8), but it always seg-faulted
> at the same stack-frame. (after using approx 2MB stack space).
> 
> So, it looks like all ULIMIT_STACK_SIZE tests need to be disabled
> on cygwin. I also wonder about the ULIMIT_FILE_DESCRIPTORS tests,
> but haven't looked into it.
> 
> Given that 'ulimit' is a bash built-in, this may also be a problem
> on MinGW and Git-For-Windows, but I can't test on those platforms.
> 
> Unfortunately, I can't spend more time on git today, hence this
> heads up! ;-)

Thanks for the note. We have this in t/test-lib.sh:

run_with_limited_cmdline () {
        (ulimit -s 128 && "$@")
}

test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'

This apparantly expects "ulimit -s" to fail on platforms that don't support it, so set the prereq accordingly. I moved the following to t/test-lib.sh:

run_with_limited_stack () {
        (ulimit -s 128 && "$@")
}

test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'

Same things as above. Two things to note:
- Those requisites could be the same, also they are used in different ways.
- "ulimit -s" returning success without doing anything means that, all
along, we ran the existing tests when we didn't mean to (on Win), and
they succeeded for the wrong reason, which we did not notice.

So, I guess, short of testing the effect of "ulimit -s" with another expensive test, it's best to simply set these prerequisites based on "uname -s".

Show 32 quoted lines

> ATB,
> Ramsay Jones
> 
> -- >8 --
> diff --git a/test.c b/test.c
> new file mode 100644
> index 0000000..bcbb805
> --- /dev/null
> +++ b/test.c
> @@ -0,0 +1,21 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <inttypes.h>
> +
> +void test(uint64_t count)
> +{
> +	int i, junk[1024];
> +
> +	for (i = 0; i < 1024; i++)
> +		junk[i] = count;
> +	i = junk[count % 1024];
> +	printf("%" PRIuMAX "\n", (uintmax_t)count);
> +	fflush(stdout);
> +	test(count + 1);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	test(0);
> +	return 0;
> +}
>