Re: [PATCH v2] transport-helper: enforce atomic in push_refs_with_push

[Jonathan Nieder <jrnieder@gmail.com>]

SZEDER Gábor wrote:

> I expected that this will eventually happen after Travis CI's default
> Linux image recently changed from Ubuntu 14.04 to 16.04; explanation
> in the commit message below.
>
> With that patch issues like this could be caught earlier, while they
> are only in 'pu' but not yet in 'next'.  But do we really want to do
> that, is that the right tradeoff?  Dunno...  Adding a dedicated CI job
> just to check that there are no 'for' loop initial declarations seems
> kind of excessive, even if it only builds but doesn't run the test
> suite.  And I don't know whether there are any other undesired ("too
> new") constructs that GCC 4.8 would catch but later compilers quietly
> accept.

This makes sense to me.  Not really for the 'for' loop declaration
aspect: for that, I'd want some more specialized tool that allows
turning on such a check specifically.  But more because Ubuntu trusty
is still a platform that some people use (though hopefully not for
long), so it's helpful as a representative old platform to see if we
break the build on it.

[...]
> [2] On Travis CI 'make test' alone would take about 9 minutes in this
>     new job (without running httpd, Subversion, and P4 tests).  For
>     comparison, starting the job and building Git with GCC 4.8 takes
>     only about 2 minutes.

Nice.  In an ideal world there would be some kind of "make fasttest"
that runs some fast subset of tests.  But this seems pretty safe.

> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  .travis.yml               |  4 ++++
>  ci/run-build-and-tests.sh | 17 +++++++++++++----
>  2 files changed, 17 insertions(+), 4 deletions(-)

Thanks.

[...]
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -11,9 +11,9 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";;
>  esac
>  
>  make
> -make test
> -if test "$jobname" = "linux-gcc"
> -then
> +case "$jobname" in
> +linux-gcc)
> +	make test
>  	export GIT_TEST_SPLIT_INDEX=yes
>  	export GIT_TEST_FULL_IN_PACK_ARRAY=true
>  	export GIT_TEST_OE_SIZE=10
> @@ -21,7 +21,16 @@ then
>  	export GIT_TEST_COMMIT_GRAPH=1
>  	export GIT_TEST_MULTI_PACK_INDEX=1
>  	make test
> -fi
> +	;;
> +linux-gcc-4.8)
> +	# Don't run the tests; we only care about whether Git can be
> +	# built with GCC 4.8, as it errors out on some undesired (C99)
> +	# constructs that newer compilers seem to quietly accept.
> +	;;
> +*)
> +	make test
> +	;;
> +esac

Does what it says on the tin.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>