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

From

Junio C Hamano <gitster@pobox.com>

Date

Jul 27, 2019, 16:11 UTC

Message-ID

<xmqq4l37mp8d.fsf@gitster-ct.c.googlers.com>

In-Reply-To

<20190727084348.GO20404@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

> Junio,
>
> On Thu, Jul 18, 2019 at 05:22:34PM +0200, SZEDER Gábor wrote:
>> Subject: [PATCH] travis-ci: build with GCC 4.8 as well
>
> This patch conflicts with topic 'js/trace2-json-schema', and the
> current conflict resolution in 'pu' is not quite correct.

Thanks.

diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index cdd2913440..ec38bf379a 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -14,6 +14,8 @@ make
 make test
 if test "$jobname" = "linux-gcc"
 then
+	make -C t/trace_schema_validator
+	export GIT_TRACE2_EVENT=$(mktemp)
 	export GIT_TEST_SPLIT_INDEX=yes
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
 	export GIT_TEST_OE_SIZE=10
@@ -21,6 +23,10 @@ then
 	export GIT_TEST_COMMIT_GRAPH=1
 	export GIT_TEST_MULTI_PACK_INDEX=1
 	make test
+	t/trace_schema_validator/trace_schema_validator \
+		--trace2_event_file=${GIT_TRACE2_EVENT} \
+		--schema_file=t/trace_schema_validator/strict_schema.json \
+		--progress=10000
 fi
 
 check_unignored_build_artifacts

i.e. they want to run an extra make in that validator directory,
export another environment variable, and then run the validator
*after* running the normal "make test", in linux-gcc job.

>> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
>> index cdd2913440..ff0ef7f08e 100755
>> --- 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
>
> This 'make test' here is important, but the confict resolution
> accidentally removed it.

Right.  Thanks for spotting.

I can see in "git diff pu~2 pu~1" that indeed the first 'make test'
that we want to run without any of these environment variables is
lost in the merge.  We want to run two tests, with or without these
environment variables, and the validator wants to piggyback on the
second one.

Will fix in the meantime, but I was expecting that this "validator
in CI" business to be redone differently, perhaps with a different
validator implementation and either in a separate job or just part
of an existing job but trace enabled only for some subset of the
tests and/or only for new tests specifically written for trace
coverage, so after that happens this may turn into a moot point.

The change the merge brings in to the file now reads like this.

Thanks, again.

diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index ff0ef7f08e..35ff4d3038 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -14,6 +14,9 @@ make
 case "$jobname" in
 linux-gcc)
 	make test
+
+	make -C t/trace_schema_validator
+	export GIT_TRACE2_EVENT=$(mktemp)
 	export GIT_TEST_SPLIT_INDEX=yes
 	export GIT_TEST_FULL_IN_PACK_ARRAY=true
 	export GIT_TEST_OE_SIZE=10
@@ -21,6 +24,11 @@ linux-gcc)
 	export GIT_TEST_COMMIT_GRAPH=1
 	export GIT_TEST_MULTI_PACK_INDEX=1
 	make test
+
+	t/trace_schema_validator/trace_schema_validator \
+		--trace2_event_file=${GIT_TRACE2_EVENT} \
+		--schema_file=t/trace_schema_validator/strict_schema.json \
+		--progress=10000
 	;;
 linux-gcc-4.8)
 	# Don't run the tests; we only care about whether Git can be