tidy: run clang-tidy in quiet mode #25713

fanquake commented at 3:30 PM on July 26, 2022: member

This should cut the amount of output by atleast half.

/opt/homebrew/opt/llvm/bin/clang-tidy --use-color -p=bitcoin bitcoin/src/wallet/test/fuzz/coinselection.cpp
278 warnings generated.
Suppressed 278 warnings (278 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

becomes

/opt/homebrew/opt/llvm/bin/clang-tidy --use-color -p=/bitcoin -quiet bitcoin/src/wallet/test/fuzz/coinselection.cpp
278 warnings generated.

See https://clang.llvm.org/extra/clang-tidy/.

--quiet Run clang-tidy in quiet mode. This suppresses printing statistics about ignored warnings and warnings treated as errors if the respective options are specified.

However the run-clang-tidy option is -quiet. See https://github.com/llvm/llvm-project/blob/f28c006a5895fc0e329fe15fead81e37457cb1d1/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py#L244.

in ci/test/06_script_b.sh:39 in be5f2efecc outdated

  35 | @@ -36,7 +36,7 @@ fi
  36 |  
  37 |  if [ "${RUN_TIDY}" = "true" ]; then
  38 |    export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/src/"
  39 | -  CI_EXEC run-clang-tidy "${MAKEJOBS}"
  40 | +  CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}"

MarcoFalke commented at 3:46 PM on July 26, 2022:

nit what about discarding anything but errors?

  ( CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}" ) | grep "error or so"

jamesob commented at 4:02 PM on July 26, 2022:

This sounds great.

fanquake commented at 4:25 PM on July 26, 2022:

Taken this. Passing -C5 should provide enough additional context as well.

jamesob commented at 4:02 PM on July 26, 2022: member

ACK https://github.com/bitcoin/bitcoin/pull/25713/commits/be5f2efecc1ad7548053259d0a0c0bc3d34f714d

But I like Marco's suggestion as well.

fanquake force-pushed on Jul 26, 2022

in ci/test/06_script_b.sh:40 in 39a35cfb25 outdated

  35 | @@ -36,7 +36,7 @@ fi
  36 |  
  37 |  if [ "${RUN_TIDY}" = "true" ]; then
  38 |    export P_CI_DIR="${BASE_BUILD_DIR}/bitcoin-$HOST/src/"
  39 | -  CI_EXEC run-clang-tidy "${MAKEJOBS}"
  40 | +  ( CI_EXEC run-clang-tidy -quiet "${MAKEJOBS}" ) | grep -C5 "error"

MarcoFalke commented at 4:31 PM on July 26, 2022:

It would be good to test this to see at least one output example and check that the | doesn't eat the exit code

fanquake commented at 8:43 AM on July 27, 2022:

Pushed up a commit that should error

fanquake commented at 8:51 AM on July 27, 2022:

Error output is showing:

clang-tidy-14 --use-color -p=/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu -quiet /tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoin-cli.cpp
/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/bitcoin-cli.cpp:70:17: error: redundant string initialization [readability-redundant-string-init,-warnings-as-errors]
    std::string blah = "";
                ^~~~~~~~~
                blah

MarcoFalke commented at 3:55 PM on July 27, 2022:

Yeah, but the exit code is 0 (green), when it used to be 1 (red)

dongcarl commented at 4:57 PM on July 27, 2022:

Perhaps we should use set -eo pipefail for these scripts?

fanquake commented at 3:32 PM on July 29, 2022:

Perhaps we should use set -eo pipefail for these scripts?

Looks like that will acheive what we want

fanquake force-pushed on Jul 27, 2022

tidy: run clang-tidy in quiet mode 3fe58a98d3

fanquake force-pushed on Jul 29, 2022

jamesob commented at 3:33 PM on July 29, 2022: member

Tidy failure with the new code looks good:

fanquake force-pushed on Jul 29, 2022

fanquake commented at 3:40 PM on July 29, 2022: member

Tidy failure with the new code looks good:

Pushed to drop the failing commit.

jamesob commented at 3:43 PM on July 29, 2022: member

ACK https://github.com/bitcoin/bitcoin/pull/25713/commits/3fe58a98d3406fc62f6dbbd8c0d58caefc69c9ae

in ci/test/06_script_b.sh:38 in 3fe58a98d3

  34 | @@ -35,8 +35,9 @@ if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then
  35 |  fi
  36 |  
  37 |  if [ "${RUN_TIDY}" = "true" ]; then
  38 | +  set -eo pipefail

MarcoFalke commented at 7:27 AM on July 30, 2022:

time to rewrite this stuff to python? :smiling_face_with_tear:

MarcoFalke merged this on Jul 30, 2022

MarcoFalke closed this on Jul 30, 2022

fanquake deleted the branch on Jul 30, 2022

sidhujag referenced this in commit a6270b6b0c on Aug 1, 2022

bitcoin locked this on Jul 30, 2023