Use trap(1) to show the testsuite log

[alejandro-colomar]

Otherwise, 'cat testsuite.log' isn't run, since 'set -e' aborts the script earlier.

[alejandro-colomar]

I've tested it manually, by running these same commands in a docker container.

[ikerexxe]

Reading the logs directly in Github WebUI is very painful. Can you upload them? You have an example at https://github.com/shadow-maint/shadow/blob/master/.github/workflows/runner.yml#L102. This will provide all the logs in a compressed archive in the checks section of the PR.

By the way, I think we are already using v4 for this action, so use that one instead of v3.

[alejandro-colomar]

Reading the logs directly in Github WebUI is very painful. Can you upload them? You have an example at https://github.com/shadow-maint/shadow/blob/master/.github/workflows/runner.yml#L102. This will provide all the logs in a compressed archive in the checks section of the PR.

By the way, I think we are already using v4 for this action, so use that one instead of v3.

Ahh, so there's a way to see cmocka logs? I was quite frustrated in the past when cmocka reported a test failure because I didn't know how to see the logs, and had to manually reproduce the entire build inside a docker container and then cat the log file. :D

But, I don't think that's working as intended. See for example:

• https://github.com/shadow-maint/shadow/actions/runs/7966498776/job/21747847052#step:3:2424
  Since the previous step,

  shadow/.github/workflows/runner.yml

  Line 98 in 1af6b68

  - name: Build container

  , has failed, the following step isn't run. So, the logs seem to be stored only when they succeed, which... well, is another case of the same problem. :)

(BTW, that PR is expected to fail. The reason is the broken strtoi(3bsd) from libbsd. So, if you want to test a patch for a fix, that would be a good PR to test it.)

[alejandro-colomar]

Reading the logs directly in Github WebUI is very painful.

BTW, this reminds me of wanting shell scripts that I can run locally to produce the same stuff that we have on github, and safely. When the string-to-numeric hardening patches are applied, this might be my next project. :)

And BTW, here's the CI/CD script I wrote for the Linux man-pages on my server. It might be useful:

$ ssh www cat /srv/src/alx/linux/man-pages/man-pages.git/hooks/post-update
#!/bin/bash

set -uo pipefail;

cd /home/alx/src/linux/man-pages/man-pages/.bare.git/;

unset $(git rev-parse --local-env-vars);
git fetch srv >/dev/null;

export LANG=C.utf8;

test "$1" = "refs/heads/main" \
&& (
	cd /home/alx/src/linux/man-pages/man-pages/main/;

	git reset srv/main --hard >/dev/null;

	nohup sh <<__EOF__ >/dev/null 2>&1 &
		make build-book _LMB=/srv/www/share/dist/man-pages/git/HEAD/man-pages-HEAD.pdf;
__EOF__
)

test "$1" = "refs/heads/contrib" \
&& (
	set -Ee;

	cd /home/alx/src/linux/man-pages/man-pages/contrib/;

	git reset srv/contrib --hard >/dev/null;

	make_opts='';
	make_opts="$make_opts -j4";
	make_opts="$make_opts -Otarget";
	make_opts="$make_opts --no-print-directory";
	make_opts="$make_opts DISTNAME=man-pages-contrib";

	make_target()
	{
		make $make_opts "$@" 2>&1 \
		| sed '/bashrc.*PS1/d';
	}

	make_target lint;
	make_target build-html;
	make_target build-pdf;
	make_target build-ps;
	make_target build-catman;
	make_target build-ex;
	make_target check;
	make_target dist;
)

[ikerexxe]

Ahh, so there's a way to see cmocka logs? I was quite frustrated in the past when cmocka reported a test failure because I didn't know how to see the logs, and had to manually reproduce the entire build inside a docker container and then cat the log file. :D

I think I never added cmocka logs to the reporting, so that kind of makes sense.

But, I don't think that's working as intended. See for example:

Something must have changed because it was working even when something failed. I'll take a look at it.

[ikerexxe]

Ok, so it seems it's not possible to get the logs when a command in docker build fails. I know there have been some changes around it with BuildKit, but maybe it never worked.

So, as I see it there are two options: either we move all the steps to scripts, or we move them to the Github workflow. Any other idea?

[alejandro-colomar]

Ok, so it seems it's not possible to get the logs when a command in docker build fails. I know there have been some changes around it with BuildKit, but maybe it never worked.

Except that we can do a trap(1) in the Dockerfile, since each RUN is basically a script.

So, as I see it there are two options: either we move all the steps to scripts, or we move them to the Github workflow. Any other idea?

I like stderr. If we can do both, then that's great. If we need to chose, I'd like to at least have stderr, which can be redirected to anything.

[alejandro-colomar]

v1b changes:

• Rebase to master

$ git range-diff gh/trap^..gh/trap shadow/master..trap
1:  17573002 = 1:  f9f8dc8e .github/workflows/runner.yml: Use trap(1) to show the testsuite log

[alejandro-colomar]

It ain't the most beautiful thing in the world, but it works:

$ cat Dockerfile 
FROM debian
RUN bash -c "trap 'echo foo >&2' ERR; false;"

$ sudo docker build -f Dockerfile .
Sending build context to Docker daemon  28.67kB
Step 1/2 : FROM debian
 ---> 2a033a8c6371
Step 2/2 : RUN bash -c "trap 'echo foo >&2' ERR; false;"
 ---> Running in 262b18fe86de
foo
The command '/bin/sh -c bash -c "trap 'echo foo >&2' ERR; false;"' returned a non-zero code: 1

[alejandro-colomar]

v2 changes:

• trap(1) to see the cmocka logs in the Docker builds [Cc: @ikerexxe ]

$ git range-diff shadow/master gh/trap trap 
1:  f9f8dc8e = 1:  f9f8dc8e .github/workflows/runner.yml: Use trap(1) to show the testsuite log
-:  -------- > 2:  eff32030 share/containers/: Specify one argument per line
-:  -------- > 3:  0dbd1b77 share/containers/: trap(1) to see the cmocka logs

I've also pushed those two commits to

• String-to-numeric hardening #893

to test them.

---

That run has already finished, and it looks good to me:

• https://github.com/shadow-maint/shadow/actions/runs/8079452916/job/22073889934?pr=893

[alejandro-colomar]

I'll merge it already. It shouldn't be risky, given it only changes code running tests. And it might be useful to have this working in stable branches, in case we need to debug any problems. Thanks!

[alejandro-colomar]

v2b changes:

• Rebase to master

$ git range-diff master..gh/trap shadow/master..trap 
1:  f9f8dc8e = 1:  fe8caf4c .github/workflows/runner.yml: Use trap(1) to show the testsuite log
2:  eff32030 = 2:  ce2486f3 share/containers/: Specify one argument per line
3:  0dbd1b77 = 3:  ef77b580 share/containers/: trap(1) to see the cmocka logs

[alejandro-colomar]

v2c changes:

• Tags [@ikerexxe , @hallyn ]
• Use some consistency in the commit Subject lines.

$ git range-diff shadow/master gh/trap trap 
1:  fe8caf4c ! 1:  eb5fcb4b .github/workflows/runner.yml: Use trap(1) to show the testsuite log
    @@ Metadata
     Author: Alejandro Colomar <[email protected]>
     
      ## Commit message ##
    -    .github/workflows/runner.yml: Use trap(1) to show the testsuite log
    +    .github/workflows/runner.yml: trap(1) to see the testsuite log
     
         Otherwise, 'cat testsuite.log' isn't run, since 'set -e' aborts the
         script earlier.
     
    +    Reviewed-by: "Serge E. Hallyn" <[email protected]>
    +    Cc: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## .github/workflows/runner.yml ##
2:  ce2486f3 ! 2:  6c143c29 share/containers/: Specify one argument per line
    @@ Metadata
      ## Commit message ##
         share/containers/: Specify one argument per line
     
    +    Reviewed-by: "Serge E. Hallyn" <[email protected]>
    +    Cc: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## share/containers/alpine.dockerfile ##
3:  ef77b580 ! 3:  816b4fd7 share/containers/: trap(1) to see the cmocka logs
    @@ Metadata
      ## Commit message ##
         share/containers/: trap(1) to see the cmocka logs
     
    +    Reviewed-by: "Serge E. Hallyn" <[email protected]>
         Cc: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>