[PATCH v2] test-lib: don't use ulimit in test prerequisites on cygwin

Ramsay Jones· Sep 14, 2017, 17:24 UTC · lore

On cygwin (and MinGW), the 'ulimit' built-in bash command does not have the desired effect of limiting the resources of new processes, at least for the stack and file descriptors. However, it always returns success and leads to several test prerequisites being erroneously set to true.

Add a check for cygwin and MinGW to the prerequisite expressions, using a 'test_have_prereq !MINGW,!CYGWIN' clause, to guard against using ulimit. This affects the prerequisite expressions for the ULIMIT_STACK_SIZE, CMDLINE_LIMIT and ULIMIT_FILE_DESCRIPTORS prerequisites.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Junio,

Johannes suggested the improvement to this patch (namely using the 'test_have_prereq !MINGW,!CYGWIN' sub-expression), which makes it somewhat more compact and obvious.

Thanks Johannes!

ATB, Ramsay Jones

 t/t1400-update-ref.sh |  5 ++++-
 t/t6120-describe.sh   |  1 -
 t/t7004-tag.sh        |  1 -
 t/test-lib.sh         | 10 ++++++++--
 4 files changed, 12 insertions(+), 5 deletions(-)

Show changes to 4 files +12 −5

t/t1400-update-ref.sh, t/t6120-describe.sh, t/t7004-tag.sh, t/test-lib.sh

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index dc98b4bc6..664a3a4e4 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1253,7 +1253,10 @@ run_with_limited_open_files () {
 	(ulimit -n 32 && "$@")
 }
 
-test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
+test_lazy_prereq ULIMIT_FILE_DESCRIPTORS '
+	test_have_prereq !MINGW,!CYGWIN &&
+	run_with_limited_open_files true
+'
 
 test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating branches does not burst open file limit' '
 (
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index dd6dd9df9..3d45dc295 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -279,7 +279,6 @@ test_expect_success 'describe ignoring a borken submodule' '
 	grep broken out
 '
 
-# we require ulimit, this excludes Windows
 test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '
 	i=1 &&
 	while test $i -lt 8000
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 5bf5ace56..b545c33f8 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1863,7 +1863,6 @@ test_expect_success 'version sort with very long prerelease suffix' '
 	git tag -l --sort=version:refname
 '
 
-# we require ulimit, this excludes Windows
 test_expect_success ULIMIT_STACK_SIZE '--contains and --no-contains work in a deep repo' '
 	>expect &&
 	i=1 &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 83f5d3dd2..32f9b2001 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1170,13 +1170,19 @@ run_with_limited_cmdline () {
 	(ulimit -s 128 && "$@")
 }
 
-test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'
+test_lazy_prereq CMDLINE_LIMIT '
+	test_have_prereq !MINGW,!CYGWIN &&
+	run_with_limited_cmdline true
+'
 
 run_with_limited_stack () {
 	(ulimit -s 128 && "$@")
 }
 
-test_lazy_prereq ULIMIT_STACK_SIZE 'run_with_limited_stack true'
+test_lazy_prereq ULIMIT_STACK_SIZE '
+	test_have_prereq !MINGW,!CYGWIN &&
+	run_with_limited_stack true
+'
 
 build_option () {
 	git version --build-options |

-- 
2.14.0

Jonathan Nieder· Sep 14, 2017, 18:24 UTC · re: Ramsay Jones · lore

Re: [PATCH v2] test-lib: don't use ulimit in test prerequisites on cygwin

Ramsay Jones wrote:

Show 17 quoted lines

> On cygwin (and MinGW), the 'ulimit' built-in bash command does not have
> the desired effect of limiting the resources of new processes, at least
> for the stack and file descriptors. However, it always returns success
> and leads to several test prerequisites being erroneously set to true.
>
> Add a check for cygwin and MinGW to the prerequisite expressions, using
> a 'test_have_prereq !MINGW,!CYGWIN' clause, to guard against using ulimit.
> This affects the prerequisite expressions for the ULIMIT_STACK_SIZE,
> CMDLINE_LIMIT and ULIMIT_FILE_DESCRIPTORS prerequisites.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  t/t1400-update-ref.sh |  5 ++++-
>  t/t6120-describe.sh   |  1 -
>  t/t7004-tag.sh        |  1 -
>  t/test-lib.sh         | 10 ++++++++--
>  4 files changed, 12 insertions(+), 5 deletions(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thank you.

v2test-lib: don't use ulimit in test prerequisites on cygwin