ci: fail iwyu job on compiler errors instead of silently logging them #36235

1. 
   David-Uka commented at 12:58 AM on September 13, 2026: none

   The IWYU invocation ends in 2>&1 || true, added as a TODO when pipefail was introduced in fa99a3ccace. That || true swallows the non-zero exit from a compiler fatal error: (for example a generated header that was not built before IWYU ran), so such errors only reached the raw CI logs and never failed the job, as reported in #35361.

   Dropping the || true lets pipefail propagate the failure, so the job fails loudly instead of passing silently. The recent header-generation fixes (#35468, #36228) mean the current master iwyu job is already free of such errors, so this does not turn CI red today.

   Closes #35361.

2. 
   DrahtBot commented at 12:59 AM on September 13, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36235.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   ACK	maflcko, hebasto

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

3. 
   hebasto commented at 9:31 AM on September 13, 2026: member

   Concept ACK.

   cc @maflcko

4. David-Uka requested review from hebasto on Sep 15, 2026
5. in ci/test/03_test_script.sh:270 in 676a83f935

    265 | +    # common cause is a generated header that was not built before IWYU ran.
    266 | +    # See https://github.com/bitcoin/bitcoin/issues/35361.
    267 | +    iwyu_fatal=$(grep -A1 "fatal error:" /tmp/iwyu_ci.out || true)
    268 | +    if [ -n "${iwyu_fatal}" ]; then
    269 | +      echo "${iwyu_fatal}"
    270 | +      echo "^^^ ⚠️ IWYU hit a compiler error and could not analyse the file above. If a generated header is missing, add its target to GOAL in ci/test/00_setup_env_native_iwyu.sh."
   

   ---

   ---

   

   maflcko commented at 4:01 PM on September 17, 2026:

         echo "^^^ ⚠️ Failure generated from IWYU. IWYU hit a compiler error and could not analyse the file above. If a generated header is missing, add its target to GOAL in ci/test/00_setup_env_native_iwyu.sh."
   

   nit: Could adjust the error message, so that the bot can read (https://github.com/maflcko/DrahtBot/blob/7554887cadb7c8fb614a278feef14ae399d125ce/webhook_features/src/features/ci_status.rs#L188) it too?

6. maflcko approved
7. 
   maflcko commented at 4:01 PM on September 17, 2026: member

   lgtm, seems ugly to grep for "fatal error", but seems better to fail loudly then, instead of silently passing.

8. 
   maflcko commented at 4:09 PM on September 17, 2026: member

   Those errors were only ever printed to the raw CI logs and never failed the job, because iwyu_tool.py's non-zero exit is swallowed by || true. That is how they went unnoticed for so long.

   Actually, I don't think this is true. This was ignored due to pipefail not being set.

   I set that in fa99a3ccace and added the || true as a "TODO".

   So I think the better fix would be to just remove the || true now?

9. David-Uka force-pushed on Sep 21, 2026
10. 
    David-Uka commented at 2:02 PM on September 21, 2026: none

    Good catch, thanks. Dropped the || true so pipefail fails the job.

11. David-Uka requested review from maflcko on Sep 21, 2026
12. in ci/test/03_test_script.sh:260 in bd6fbf7809

     255 | @@ -256,7 +256,7 @@ if [[ "${RUN_IWYU}" == true ]]; then
     256 |               -Xiwyu --check_also='*/interfaces/*\.h' \
     257 |               -Xiwyu --check_also='*/primitives/transaction_identifier\.h' \
     258 |               -Xiwyu --check_also='*/rpc/protocol\.h' \
     259 | -             2>&1 || true
     260 | +             2>&1
     261 |      } | tee /tmp/iwyu_ci.out
    

    ---

    ---

    

    maflcko commented at 2:43 PM on September 21, 2026:

         | tee /tmp/iwyu_ci.out
    

    can remove the {}?

13. 
    David-Uka commented at 3:01 PM on September 21, 2026: none

    Applying just the suggestion leaves the opening { unbalanced (and | tee then needs a \ on the preceding 2>&1). Did you mean to drop the braces entirely and pipe iwyu_tool.py ... 2>&1 straight into tee?

14. 
    maflcko commented at 3:17 PM on September 21, 2026: member

    diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh
    index b8bd0dd..ddc5949 100755
    --- a/ci/test/03_test_script.sh
    +++ b/ci/test/03_test_script.sh
    @@ -243,9 +243,8 @@ if [[ "${RUN_IWYU}" == true ]]; then
     
       run_iwyu() {
         mv "${BASE_BUILD_DIR}/$1" "${BASE_BUILD_DIR}/compile_commands.json"
    -    {
    -      python3 /include-what-you-use/mapgen/iwyu-mapgen-clang-intrin.py --lang imp "$("clang-${IWYU_LLVM_V}" -print-resource-dir)/include" > "${BASE_BUILD_DIR}/clang.intrinsics.imp"
    -      python3 /include-what-you-use/iwyu_tool.py \
    +    python3 /include-what-you-use/mapgen/iwyu-mapgen-clang-intrin.py --lang imp "$("clang-${IWYU_LLVM_V}" -print-resource-dir)/include" > "${BASE_BUILD_DIR}/clang.intrinsics.imp"
    +    python3 /include-what-you-use/iwyu_tool.py \
                  -p "${BASE_BUILD_DIR}" "${MAKEJOBS}" -- \
                  -Xiwyu --cxx17ns \
                  -Xiwyu --mapping_file="${BASE_ROOT_DIR}/contrib/devtools/iwyu/bitcoin.core.imp" \
    @@ -256,8 +255,7 @@ if [[ "${RUN_IWYU}" == true ]]; then
                  -Xiwyu --check_also='*/interfaces/*\.h' \
                  -Xiwyu --check_also='*/primitives/transaction_identifier\.h' \
                  -Xiwyu --check_also='*/rpc/protocol\.h' \
    -             2>&1
    -    } | tee /tmp/iwyu_ci.out
    +             2>&1 | tee /tmp/iwyu_ci.out
         python3 "/include-what-you-use/fix_includes.py" --nosafe_headers < /tmp/iwyu_ci.out
         python3 -c '
     import runpy
    

15. ci, iwyu: fail job on compiler errors instead of ignoring them

    The IWYU invocation ends in "2>&1 || true", added as a TODO when pipefail
    was introduced in fa99a3ccace. That "|| true" swallows the non-zero exit
    from a compiler "fatal error:" (for example a generated header that was
    not built before IWYU ran), so such errors only reached the raw CI logs
    and never failed the job, as reported in #35361.
    
    Drop the "|| true" so pipefail propagates the failure and the job fails
    loudly instead of passing silently. Also drop the now-redundant "{ }"
    grouping so the command pipes straight into tee.
    
    Closes #35361.

    622817f097
16. David-Uka force-pushed on Sep 21, 2026
17. 
    David-Uka commented at 5:51 PM on September 21, 2026: none

    Applied your diff, thanks.

18. David-Uka requested review from maflcko on Sep 21, 2026
19. 
    maflcko commented at 6:25 PM on September 21, 2026: member

    lgtm ACK 622817f097d68cb1a59004ca7c115110d8bf64a0

20. DrahtBot renamed this:
    ci, iwyu: fail job on compiler errors instead of silently logging them
    ci: fail iwyu job on compiler errors instead of silently logging them
    on Sep 21, 2026
21. DrahtBot added the label Tests on Sep 21, 2026
22. DrahtBot added the label CI failed on Sep 22, 2026
23. 
    hebasto commented at 9:38 AM on September 23, 2026: member

    CI fails: https://github.com/bitcoin/bitcoin/actions/runs/35634651503/job/106763289406

24. 
    maflcko commented at 9:52 AM on September 23, 2026: member

    https://github.com/bitcoin/bitcoin/actions/runs/35634651503/job/106763289406?pr=36235#step:11:10912

    include-what-you-use: /usr/lib/llvm-23/include/llvm/Support/Casting.h:656: decltype(auto) llvm::dyn_cast(From*) [with To = clang::UsingShadowDecl; From = const clang::NamedDecl]: Assertion `detail::isPresent(Val) && "dyn_cast on a non-existent value"' failed.
    

    Maybe it should be minimized/bisected/reported/fixed upstream?

25. 
    David-Uka commented at 1:37 PM on September 23, 2026: none

    Localized it: IWYU aborts on a clang::UsingShadowDecl while analyzing the libmultiprocess subtree (src/ipc/libmultiprocess/include/mp/proxy.h and the mp/test TUs) — a single, deterministic abort. Looks like an upstream IWYU/llvm-23 bug rather than our code. I can report it upstream.

    For this PR, would you prefer I exclude the libmultiprocess subtree from the IWYU compile DB (its output is already git restored, so nothing is lost) to keep the job green while the upstream bug is open, or hold this until it's fixed upstream?

26. 
    hebasto commented at 1:58 PM on September 23, 2026: member

    Localized it: IWYU aborts on a clang::UsingShadowDecl while analyzing the libmultiprocess subtree (src/ipc/libmultiprocess/include/mp/proxy.h and the mp/test TUs) — a single, deterministic abort. Looks like an upstream IWYU/llvm-23 bug rather than our code. I can report it upstream.

    It's a generated source file for me:

    $ include-what-you-use -std=c++20 -Ibuild/src/ipc/libmultiprocess/include build/src/ipc/libmultiprocess/test/mp/test/foo.capnp.c++
    include-what-you-use: /usr/lib/llvm-23/include/llvm/Support/Casting.h:656: decltype(auto) llvm::dyn_cast(From*) [with To = clang::UsingShadowDecl; From = const clang::NamedDecl]: Assertion `detail::isPresent(Val) && "dyn_cast on a non-existent value"' failed.
    Aborted (core dumped)
    

27. 
    hebasto commented at 1:59 PM on September 23, 2026: member

    ... would you prefer I exclude the libmultiprocess subtree from the IWYU compile DB...

    See #36252.

28. 
    David-Uka commented at 2:07 PM on September 23, 2026: none

    Thanks, #36252 is the right fix — it drops the crashing libmultiprocess sources from the IWYU DB. Reviewed and ACK'd it. Once it lands I'll rebase this on top; CI should then be green.

29. 
    maflcko commented at 2:31 PM on September 23, 2026: member

    There should be no need to rebase. A maintainer can just re-run the CI after the fix and it will turn green by itself.

    Edit:

    Looks like an upstream IWYU/llvm-23 bug rather than our code. I can report it upstream.

    Jup, a minimized repro could be reported.

30. 
    hebasto commented at 3:42 PM on September 23, 2026: member

    https://github.com/bitcoin/bitcoin/actions/runs/35634651503/job/106763289406?pr=36235#step:11:10912

    include-what-you-use: /usr/lib/llvm-23/include/llvm/Support/Casting.h:656: decltype(auto) llvm::dyn_cast(From*) [with To = clang::UsingShadowDecl; From = const clang::NamedDecl]: Assertion `detail::isPresent(Val) && "dyn_cast on a non-existent value"' failed.
    

    Maybe it should be minimized/bisected/reported/fixed upstream?

    See https://github.com/include-what-you-use/include-what-you-use/issues/2120.

31. fanquake marked this as a draft on Sep 24, 2026
32. 
    hebasto commented at 6:24 PM on September 26, 2026: member

    https://github.com/bitcoin/bitcoin/actions/runs/35634651503/job/106763289406?pr=36235#step:11:10912

    include-what-you-use: /usr/lib/llvm-23/include/llvm/Support/Casting.h:656: decltype(auto) llvm::dyn_cast(From*) [with To = clang::UsingShadowDecl; From = const clang::NamedDecl]: Assertion `detail::isPresent(Val) && "dyn_cast on a non-existent value"' failed.
    

    Maybe it should be minimized/bisected/reported/fixed upstream?

    See include-what-you-use/include-what-you-use#2120.

    The issue has been fixed upstream and backported to the clang_23 branch.

    Re-running the "iwyu" CI job now.

33. DrahtBot removed the label CI failed on Sep 28, 2026
34. hebasto approved
35. 
    hebasto commented at 11:07 AM on September 28, 2026: member

    ACK 622817f097d68cb1a59004ca7c115110d8bf64a0.

36. 
    maflcko commented at 11:38 AM on September 28, 2026: member

    ci-only patch with two acks, rfm?

37. fanquake marked this as ready for review on Sep 29, 2026
38. fanquake merged this on Sep 29, 2026
39. fanquake closed this on Sep 29, 2026

    ---

Contributors

David-UkaDrahtBothebastomaflcko

Review Requested
maflcko