[2/2] ci(github): also mark up compile errors

Commit Message

Johannes Schindelin June 9, 2022, 11:32 a.m. UTC

From: Johannes Schindelin <[e-mail]>

When GCC produces those helpful errors, we will want to present them in
the GitHub workflow runs in the most helpful manner. To that end, we
want to use workflow commands to render errors and warnings:
https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions

In the previous commit, we ensured that grouping is used for the build
in all jobs, and this allows us to piggy-back onto the `group` function
to transmogrify the output.

Note: If `set -o pipefail` was available, we could do this in a little
more elegant way. But since some of the steps are run using `dash`, we
have to do a little `{ ...; echo $? >exit.status; } | ...` dance.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 ci/lib.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Ævar Arnfjörð Bjarmason June 9, 2022, 1:47 p.m. UTC | #1

On Thu, Jun 09 2022, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[e-mail]>
>
> When GCC produces those helpful errors, we will want to present them in
> the GitHub workflow runs in the most helpful manner. To that end, we
> want to use workflow commands to render errors and warnings:
> https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions

The docs you're linking to state:

	::warning file={name},line={line},endLine={endLine},title={title}::{message}

But here...

> +		sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2 line=\3::\1/'

You seem to omit the comma, and the CI output itself seems to note the
filename as "compat/win32/syslog.c line=53#L1".

I haven't tested, but is this the issue you noted in the CL as "the only
downside"? I.e. the link to the source code is nonsensical in that CI
output, it links to the diff of the PR itself.

But the GH docs say "associate the message with a particular file in
your repository.", so it would seem that there should be a way to link
to the file at that revision, not only if it was altered in the given
commit.

On the "sed" one-liner, at least GCC supports emitting JSON error
output:
https://stackoverflow.com/questions/36657869/how-do-i-dump-gcc-warnings-into-a-structured-format

You don't fill in "column" now, but if you used that presumably that
would be easy, and more useful.

It seems clang also supports it, but not any easily machine-readable
format:
https://clang.llvm.org/docs/UsersManual.html#cmdoption-fdiagnostics-format

Junio C Hamano June 10, 2022, 4:30 p.m. UTC | #2

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> +		sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2 line=\3::\1/'

https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message

seems to use comma as field separator, not SP, for files and lines.
We'll see if they are equivalent without getting documented soon, as
I will be adding this to my tree for 'seen' today.  We should throw
a build failure in to see its effect ;-)

Patch

diff --git a/ci/lib.sh b/ci/lib.sh
index 2f6d9d26e40..b747e34842c 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -29,8 +29,14 @@  else
 		set +x
 		begin_group "$1"
 		shift
-		"$@"
-		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/'
+		res=$(cat exit.status)
+		rm exit.status
 		end_group
 		return $res
 	}