ci: Run iwyu on all src files #27571

maflcko commented at 10:57 AM on May 4, 2023: member

This makes it easier to look at the CI output of a file without having to manually add it first to the list.

DrahtBot commented at 10:57 AM on May 4, 2023: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto
Concept ACK	fanquake, jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27636 (kernel: Remove interface_ui, util/system from kernel library by TheCharlatan)
#27576 (kernel: Remove args, chainparams, chainparamsbase from kernel library by TheCharlatan)
#27425 (test: move remaining rand code from util/setup_common to util/random by jonatack)
#27385 (net, refactor: extract Network and BIP155Network logic to node/network by jonatack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Tests on May 4, 2023

DrahtBot added the label CI failed on May 4, 2023

maflcko force-pushed on May 4, 2023

maflcko marked this as a draft on May 4, 2023

fanquake commented at 2:31 PM on May 9, 2023: member

Is this dependant on #27573

maflcko commented at 2:55 PM on May 9, 2023: member

Yes, I can't figure out how to fight bash -c, so I removed it there

fanquake commented at 3:25 PM on May 9, 2023: member

Concept ACK on removing the continual manual updating of this list

maflcko marked this as ready for review on May 10, 2023

maflcko force-pushed on May 10, 2023

fanquake commented at 11:41 AM on May 10, 2023: member

(qt/qrc_bitcoin_locale.cpp has correct #includes/fwd-decls)

Looks like it's mostly working, except GUI stuff is still being dragged in

hebasto commented at 12:31 PM on May 10, 2023: member

(qt/qrc_bitcoin_locale.cpp has correct #includes/fwd-decls)

Looks like it's mostly working, except GUI stuff is still being dragged in

And qt/qrc_bitcoin.cpp as well.

maflcko force-pushed on May 10, 2023

maflcko marked this as a draft on May 10, 2023

maflcko added the label Up for grabs on May 10, 2023

jonatack commented at 10:02 AM on May 11, 2023: contributor

Concept ACK. I've been adding sources files to this list while working on each pull.

DrahtBot added the label Needs rebase on May 11, 2023

maflcko force-pushed on May 12, 2023

DrahtBot removed the label Needs rebase on May 12, 2023

ci: Run iwyu on all src files

This makes it easier to look at the CI output of a file without having
to manually add it first.

ddddf4957b

maflcko force-pushed on May 13, 2023

maflcko removed the label Up for grabs on May 13, 2023

maflcko marked this as ready for review on May 13, 2023

DrahtBot removed the label CI failed on May 13, 2023

maflcko commented at 3:02 PM on May 15, 2023: member

Should be ready for review

in ci/test/06_script_b.sh:49 in ddddf4957b

  41 | @@ -42,51 +42,16 @@ if [ "${RUN_TIDY}" = "true" ]; then
  42 |    set -eo pipefail
  43 |    cd "${BASE_BUILD_DIR}/bitcoin-$HOST/src/"
  44 |    ( run-clang-tidy-16 -quiet "${MAKEJOBS}" ) | grep -C5 "error"
  45 | +  # Filter out files by regex here, because regex may not be
  46 | +  # accepted in src/.bear-tidy-config
  47 | +  # Filter out:
  48 | +  # * qt qrc and moc generated files
  49 | +  # * walletutil (temporarily)

hebasto commented at 2:11 PM on May 16, 2023:

Mind elaborating about skipping walletutil?

maflcko commented at 5:41 PM on May 16, 2023:

because it crashes. See:

# python3 "/include-what-you-use/iwyu_tool.py" -p . ./src/wallet/walletutil.cpp -j 9 -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="$PWD/contrib/devtools/iwyu/bitcoin.core.imp"  
In file included from wallet/walletutil.cpp:5:
In file included from ./wallet/walletutil.h:8:
In file included from ./script/descriptor.h:8:
In file included from ./outputtype.h:9:
In file included from ./script/signingprovider.h:10:
In file included from ./key.h:10:
In file included from ./pubkey.h:10:
In file included from ./hash.h:10:
In file included from ./crypto/common.h:12:
In file included from /usr/lib64/clang/16/include/stdint.h:52:
In file included from /usr/include/stdint.h:26:
In file included from /usr/include/bits/libc-header-start.h:33:
/usr/include/features.h:413:4: warning: _FORTIFY_SOURCE requires compiling with optimization (-O) [-W#warnings]
#  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
   ^
In file included from wallet/walletutil.cpp:5:
In file included from ./wallet/walletutil.h:8:
In file included from ./script/descriptor.h:8:
In file included from ./outputtype.h:9:
In file included from ./script/signingprovider.h:10:
In file included from ./key.h:10:
In file included from ./pubkey.h:10:
In file included from ./hash.h:14:
In file included from ./serialize.h:17:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/memory:78:
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unique_ptr.h:97:16: error: invalid application of 'sizeof' to an incomplete type 'wallet::DescriptorScriptPubKeyMan'
        static_assert(sizeof(_Tp)>0,
                      ^~~~~~~~~~~
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unique_ptr.h:404:4: note: in instantiation of member function 'std::default_delete<wallet::DescriptorScriptPubKeyMan>::operator()' requested here
          get_deleter()(std::move(__ptr));
          ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:151:19: note: in instantiation of member function 'std::unique_ptr<wallet::DescriptorScriptPubKeyMan>::~unique_ptr' requested here
      __pointer->~_Tp();
                  ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:196:2: note: in instantiation of function template specialization 'std::_Destroy_aux<false>::__destroy<std::unique_ptr<wallet::DescriptorScriptPubKeyMan> *>' requested here
        __destroy(__first, __last);
        ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/alloc_traits.h:947:7: note: in instantiation of function template specialization 'std::_Destroy<std::unique_ptr<wallet::DescriptorScriptPubKeyMan> *>' requested here
      _Destroy(__first, __last);
      ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_vector.h:732:7: note: in instantiation of function template specialization 'std::_Destroy<std::unique_ptr<wallet::DescriptorScriptPubKeyMan> *, std::unique_ptr<wallet::DescriptorScriptPubKeyMan>>' requested here
        std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish,
             ^
./wallet/walletutil.h:122:8: note: in instantiation of member function 'std::vector<std::unique_ptr<wallet::DescriptorScriptPubKeyMan>>::~vector' requested here
struct MigrationData
       ^
./wallet/walletutil.h:119:7: note: forward declaration of 'wallet::DescriptorScriptPubKeyMan'
class DescriptorScriptPubKeyMan;
      ^

hebasto approved

hebasto commented at 6:04 PM on May 16, 2023: member

ACK ddddf4957b02c83ed9b6c46b35d8ae1e137889d2

fanquake merged this on May 17, 2023

fanquake closed this on May 17, 2023

maflcko deleted the branch on May 17, 2023

sidhujag referenced this in commit 352ed46608 on May 17, 2023

fanquake referenced this in commit dfe658009d on May 29, 2023

bitcoin locked this on May 16, 2024