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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 14, 2017, 12:54 UTC
Message-ID: <alpine.DEB.2.21.1.1709141440510.4132@virtualbox>
In-Reply-To: <b57731ed-4640-d27f-f7b2-6c70af8dd400@ramsayjones.plus.com>

Hi Ramsay,

On Wed, 13 Sep 2017, Ramsay Jones wrote:

> So, I finally got around to testing ulimit with open files and
> found that it does not limit them on cygwin (it actually fails
> on the hard limit of 3200).

Thank you!

> Having seen Johannes email earlier, I took a chance and added MinGW to
> this patch as well. (Hopefully this won't cause any screams!) ;-)

It earns you my gratitude instead.

If you want to see the difference between the MSYS2 runtime (Git for Windows flavor) and the Cygwin runtime, the patches applied on top of the Cygwin runtime's source code can be seen here:

https://github.com/git-for-windows/MSYS2-packages/tree/master/msys2-runtime

The current overall diff can be inspected here:

https://github.com/git-for-windows/msys2-runtime/compare/aca9144e65ba...874e2c8efeed

The changes are not *all* that extensive, revolving more around the Windows <-> POSIX path conversion (I am actually no longer certain that the characterisation of "POSIX path" is correct, think about VMS still being considered POSIX...).

There is also some stuff about environment variables, but that's about it.

Most importantly, the core code (such as ulimit functionality) seems not to be changed at all IIAC.

Meaning: I agree with your assessment to enable the workaround also for
`uname -s` starting with MINGW.

One suggestion:

Show 19 quoted lines

> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index dc98b4bc6..49415074f 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -1253,7 +1253,16 @@ 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 '
> +	case $(uname -s) in
> +	CYGWIN*|MINGW*)
> +		false
> +		;;
> +	*)
> +		run_with_limited_open_files true
> +		;;
> +	esac
> +'

It would be more semantically meanginful to do this instead:

-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 +'

In other words, *spell out* that we want neither MINGW nor CYGWIN as prerequisites before testing ulimit with that shell function.

It would also be a little more succinct.

Ciao, Dscho