ci: Ignore internal errors of snapshot compilers #1437

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:231106-ci changing 1 files +15 −1

hebasto commented at 10:04 PM on November 6, 2023: member

It was discussed on today's IRC meeting.
in ci/ci.sh:87 in d14484f81a outdated
```
  82 | @@ -83,7 +83,19 @@ esac
  83 |      --host="$HOST" $EXTRAFLAGS
  84 |  
  85 |  # We have set "-j<n>" in MAKEFLAGS.
  86 | -make
  87 | +build_exit_code=0
  88 | +make || build_exit_code=$?
```
real-or-random commented at 10:20 AM on November 7, 2023:

nit: This should be simpler

make build_exit_code=$?
hebasto commented at 10:42 AM on November 7, 2023:

nit: This should be simpler

Doesn't set -eux in the top of the script force exiting when make fails?

real-or-random commented at 10:47 AM on November 7, 2023:

Nevermind.
in ci/ci.sh:91 in d14484f81a outdated
```
  87 | +build_exit_code=0
  88 | +make || build_exit_code=$?
  89 | +if [ $build_exit_code -ne 0 ]; then
  90 | +    case $CC in
  91 | +        *snapshot*)
  92 | +            make 2>&1 >/dev/null | grep "internal compiler error:"
```
real-or-random commented at 10:22 AM on November 7, 2023:

Regarding make 2>&1 >/dev/null:

I think it would be more reliable to run make only once. (Perhaps gcc-snapshot crashes in one run but not in another.)

One way could be: Change the first make to make | tee make.log and then use grep on make.log.

real-or-random commented at 10:24 AM on November 7, 2023:

Regarding grep "internal compiler error:":

Could also additionally grep for "PLEASE submit a bug report". This is what clang will output when it crashes.

hebasto commented at 11:32 AM on November 7, 2023:

Thanks! Done.

hebasto commented at 11:32 AM on November 7, 2023:

Sure. Done.
real-or-random commented at 10:25 AM on November 7, 2023: contributor

Concept ACK, thanks for the quick response
hebasto force-pushed on Nov 7, 2023
hebasto force-pushed on Nov 7, 2023
hebasto commented at 11:31 AM on November 7, 2023: member
Updated d14484f81a0799917ce5799a78b2a9fdb92711f8 -> 5a8db10c59974a4a9b0a00d2954e6a5cd3fd055f (pr1437.01 -> pr1437.03, diff).

Addressed @real-or-random's comments:
- #1437 (review)
- #1437 (review)

in ci/ci.sh:93 in 5a8db10c59 outdated

  88 | +make > make.log 2>&1 || build_exit_code=$?
  89 | +cat make.log
  90 | +if [ $build_exit_code -ne 0 ]; then
  91 | +    case $CC in
  92 | +        *snapshot*)
  93 | +            grep -e "internal compiler error:" -e "PLEASE submit a bug report" make.log

real-or-random commented at 11:41 AM on November 7, 2023:

nit:

            # Ignore internal compiler errors in gcc-snapshot and clang-snapshot
            grep -e "internal compiler error:" -e "PLEASE submit a bug report" make.log

hebasto commented at 12:27 PM on November 7, 2023:

Done.

real-or-random approved
real-or-random commented at 11:42 AM on November 7, 2023: contributor

ACK mod nit and CI results
hebasto force-pushed on Nov 7, 2023
hebasto commented at 12:27 PM on November 7, 2023: member

Updated 5a8db10c59974a4a9b0a00d2954e6a5cd3fd055f -> 61575fd5a647ff6ca0b0b4111b21dc1e0a12d5ae (pr1437.03 -> pr1437.04, diff).

Addressed @real-or-random's comment.
real-or-random commented at 12:10 AM on November 8, 2023: contributor

Real failures are still reported as real failures: https://github.com/bitcoin-core/secp256k1/actions/runs/6791773753/job/18463903460?pr=1440

But for some of them, CC is not set (and we let configure detect the right CC) https://github.com/bitcoin-core/secp256k1/actions/runs/6791773753/job/18463894482?pr=1440#step:3:780

Perhaps just add a default substitution in case CC is not set. This should be good enough. The snapshots are only used if we ask for them explicitly by setting CC. We could also read CC from the CC= line in config.log, but that sounds like overkill.
ci: Ignore internal errors in snapshot compilers 8185e72d29
hebasto force-pushed on Nov 8, 2023
hebasto commented at 5:56 PM on November 8, 2023: member

But for some of them, CC is not set...

Solved by using the same approach as used in other places of the ci.sh script.
real-or-random approved
real-or-random commented at 7:11 PM on November 8, 2023: contributor

ACK 8185e72d299bc77de9c06cc92fd1988676df3bc1

See https://github.com/real-or-random/secp256k1/actions/runs/6802798679/job/18496698030 for a real build failure on top of this PR
real-or-random merged this on Nov 8, 2023
real-or-random closed this on Nov 8, 2023
real-or-random commented at 7:52 PM on November 8, 2023: contributor

Merging this, we shouldn't hold this up because a broken CI is very annoying.
hebasto deleted the branch on Nov 8, 2023

Contributors

hebasto

real-or-random