Re: [PATCH v2 0/2] ci(GitHub): mark up compile errors, too

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 13, 2022, 17:08 UTC
Message-ID: <xmqq35g88p9y.fsf@gitster.g>
In-Reply-To: <pull.1253.v2.git.1655125988.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  * The curly bracket construct was replaced by a proper subshell, to avoid
>    jumbled output and a race where the exit.status file could be read before
>    it was written.

I do prefer () when making a subshell in a case like this (e.g. upstream of a pipe), so I am happy with this version, but the above is curious.

I am not sure how "jumbled output" is possible, let alone "reading exit.status before it is written". The output goes to sed to be processed either way, nobody else other than "$@" produces such an output from there, and sed would not exit until it finishes reading from the upstream so res=$(cat exit.status) won't kick in before the upstream exits.

Anyway, thanks, will queue.

Show 33 quoted lines

> Johannes Schindelin (2):
>   ci(github): use grouping also in the `win-build` job
>   ci(github): also mark up compile errors
>
>  ci/lib.sh                 | 10 ++++++++--
>  ci/make-test-artifacts.sh |  2 +-
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
>
> base-commit: 3069f2a6f4c38e7e599067d2e4a8e31b4f53e2d3
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1253%2Fdscho%2Fci-mark-up-compile-failures-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1253/dscho/ci-mark-up-compile-failures-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1253
>
> Range-diff vs v1:
>
>  1:  5212c5ec474 = 1:  5212c5ec474 ci(github): use grouping also in the `win-build` job
>  2:  19d6e34f038 ! 2:  34daf06bb71 ci(github): also mark up compile errors
>      @@ ci/lib.sh: else
>       -		"$@"
>       -		res=$?
>       +		# work around `dash` not supporting `set -o pipefail`
>      -+		{
>      ++		(
>       +			"$@" 2>&1
>       +			echo $? >exit.status
>      -+		} |
>      -+		sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2 line=\3::\1/'
>      ++		) |
>      ++		sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
>       +		res=$(cat exit.status)
>       +		rm exit.status
>        		end_group