This diff adds a check to clang-tidy for adding the braces automatically.
Configuration files are added to the libraries (leveldb, secp256k1,
univalue) to avoid formatting them.
Depends on D5138.
Details
Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project - Commits
- rSTAGING40a99e1f0ed3: Automatically add missing braces
rABC40a99e1f0ed3: Automatically add missing braces
Add some missing brace somewhere in a source file if D5120 is not landed yet.
cmake -GNinja .. ninja
Check that the files are updated with braces at build time.
Reset the file to have missing braces, then:
cmake -GNinja .. -DENABLE_CLANG_TIDY=OFF ninja
Check that clang-tidy is not run.
Run the Gitian builds.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Build Bitcoin-ABC / Diffs / Diff Testing failed.
Snippet of first build failure:
[22:26:53] : [Step 1/1] -- Looking for getifaddrs
[22:26:53] : [Step 1/1] -- Looking for getifaddrs - found
[22:26:53] : [Step 1/1] -- Looking for freeifaddrs
[22:26:53] : [Step 1/1] -- Looking for freeifaddrs - found
[22:26:53] : [Step 1/1] -- Performing Test HAVE_SYS_GETRANDOM
[22:26:53] : [Step 1/1] -- Performing Test HAVE_SYS_GETRANDOM - Success
[22:26:53] : [Step 1/1] -- Performing Test HAVE_SYSCTL_ARND
[22:26:53] : [Step 1/1] -- Performing Test HAVE_SYSCTL_ARND - Failed
[22:26:53] : [Step 1/1] -- Performing Test CHAR_EQUALS_INT8
[22:26:53] : [Step 1/1] -- Performing Test CHAR_EQUALS_INT8 - Failed
[22:26:53] : [Step 1/1] -- Performing Test HAVE_LARGE_FILE_SUPPORT
[22:26:54] : [Step 1/1] -- Performing Test HAVE_LARGE_FILE_SUPPORT - Success
[22:26:54] : [Step 1/1] -- Performing Test HAVE_FUNC_ATTRIBUTE_VISIBILITY
[22:26:54] : [Step 1/1] -- Performing Test HAVE_FUNC_ATTRIBUTE_VISIBILITY - Success
[22:26:54] : [Step 1/1] -- Performing Test HAVE_FUNC_ATTRIBUTE_DLLEXPORT
[22:26:54] : [Step 1/1] -- Performing Test HAVE_FUNC_ATTRIBUTE_DLLEXPORT - Failed
[22:26:54] : [Step 1/1] -- Checking prototype __fdelt_warn for FDELT_PROTOTYPE_LONG_UNSIGNED_INT - False
[22:26:54] : [Step 1/1] -- Looking for EVP_MD_CTX_new
[22:26:54] : [Step 1/1] -- Looking for EVP_MD_CTX_new - found
[22:26:54]W: [Step 1/1] CMake Error at cmake/modules/DoOrFail.cmake:4 (message):
[22:26:54]W: [Step 1/1] Failed to find program
[22:26:54]W: [Step 1/1] [clang-tidy;clang-tidy-8;clang-tidy-9;clang-tidy-10], please make sure that
[22:26:54]W: [Step 1/1] it is installed and reachable through the system PATH.
[22:26:54]W: [Step 1/1] Call Stack (most recent call first):
[22:26:54]W: [Step 1/1] cmake/modules/ClangTidy.cmake:5 (find_program_or_fail)
[22:26:54]W: [Step 1/1] src/crypto/CMakeLists.txt:19 (include)
[22:26:54]W: [Step 1/1]
[22:26:54]W: [Step 1/1]
[22:26:54] : [Step 1/1] -- Configuring incomplete, errors occurred!
[22:26:54] : [Step 1/1] See also "/home/teamcity/buildAgent/work/c4a5708f2bae7929/build/native/CMakeFiles/CMakeOutput.log".
[22:26:54] : [Step 1/1] See also "/home/teamcity/buildAgent/work/c4a5708f2bae7929/build/native/CMakeFiles/CMakeError.log".
[22:26:54] : [Step 1/1] [56/400] Building CXX object src/leveldb/CMakeFiles/leveldb.dir/port/port_posix.cc.o
[22:26:54] : [Step 1/1] ../src/leveldb/port/port_posix.cc: In function 'bool leveldb::port::HasAcceleratedCRC32C()':
[22:26:54] : [Step 1/1] ../src/leveldb/port/port_posix.cc:60:15: warning: 'ecx' may be used uninitialized in this function [-Wmaybe-uninitialized]
[22:26:54] : [Step 1/1] return (ecx & (1 << 20)) != 0;
[22:26:54] : [Step 1/1] ~~~~~^~~~~~~~~~~~
[22:26:54] : [Step 1/1] [102/400] Generating bitcoin_hu.qm
[22:26:54] : [Step 1/1] Removed plural forms as the target language has less forms.
[22:26:54] : [Step 1/1] If this sounds wrong, possibly the target language is not set or recognized.
[22:26:54] : [Step 1/1] [126/400] Generating bitcoin_pl.qm
[22:26:54] : [Step 1/1] Removed plural forms as the target language has less forms.
[22:26:54] : [Step 1/1] If this sounds wrong, possibly the target language is not set or recognized.
[22:26:54] : [Step 1/1] [137/400] Generating bitcoin_ru.qm
[22:26:54] : [Step 1/1] Removed plural forms as the target language has less forms.
[22:26:54] : [Step 1/1] If this sounds wrong, possibly the target language is not set or recognized.
[22:26:54] : [Step 1/1] [149/400] Generating bitcoin_tr.qm
[22:26:54] : [Step 1/1] Removed plural forms as the target language has less forms.
[22:26:54] : [Step 1/1] If this sounds wrong, possibly the target language is not set or recognized.
[22:26:54] : [Step 1/1] [185/400] Linking CXX static library src/zmq/libzmq.a
[22:26:54] : [Step 1/1] FAILED: native/CMakeCache.txt
[22:26:54] : [Step 1/1] cd /home/teamcity/buildAgent/work/c4a5708f2bae7929/build && /home/teamcity/buildAgent/work/c4a5708f2bae7929/build/config/run_native_cmake.sh
[22:26:54] : [Step 1/1] ninja: build stopped: subcommand failed.
[22:26:54] : [Step 1/1] *** Output of /tmp/sanitizer_logs/*.log.* ***
[22:26:54]W: [Step 1/1] ++ print_sanitizers_log
[22:26:54]W: [Step 1/1] ++ for log in "${SAN_LOG_DIR}"/*.log.*
[22:26:54]W: [Step 1/1] ++ echo '*** Output of /tmp/sanitizer_logs/*.log.* ***'
[22:26:54]W: [Step 1/1] ++ cat '/tmp/sanitizer_logs/*.log.*'
[22:26:54]W: [Step 1/1] cat: '/tmp/sanitizer_logs/*.log.*': No such file or directory
[22:26:54]W: [Step 1/1] Process exited with code 1
[22:26:54]E: [Step 1/1] Process exited with code 1 (Step: Command Line)| src/CMakeLists.txt | ||
|---|---|---|
| 581 ↗ | (On Diff #15907) | This is clearly not going to scale if we need to add it to every single target. |
Comment Actions
Several improvements:
- Use an exclusion mechanism instead of inclusion to avoid duplication
- Forward the clang-tidy option to the native build, so the native source files get formatted
- Rebase on top of D5121 to avoid formatting generated files
- Fix CONTRIBUTING.md, improve OSX instructions
| CONTRIBUTING.md | ||
|---|---|---|
| 130 ↗ | (On Diff #15926) | Add a line break. |
| 148 ↗ | (On Diff #15926) | It's be great if we gave a people a way to do thing either with brew or npm, but not a mixture of both. |
| cmake/modules/ClangTidy.cmake | ||
| 23 ↗ | (On Diff #15926) | It would be preferable to use a .clang-tidy file instead of forcing anyone to know the build system internal soup to be able to do anything with clang-tidy. With some luck, it even fix your problem with generated files, because they'll be in the build directory instead of the source directory. |
| src/CMakeLists.txt | ||
| 202 ↗ | (On Diff #15926) | That is really ugly. Having a clang-tidy file in these directories that disable the check that fails would be more beneficial. |
| CONTRIBUTING.md | ||
|---|---|---|
| 148 ↗ | (On Diff #15926) | There is no npm package for clang-tidy, and no brew formulae for clang-format-8 :(. |
| cmake/modules/ClangTidy.cmake | ||
| 23 ↗ | (On Diff #15926) | This was the first thing I tried but I faced some limitations and finally I gave up. There is one last possibility that I didn't explore: finding a rule that is green for the whole codebase, and which we want to enforce. Doing so will allow to use the files, as there will always be one rule activated. I'll see if there is such a rule, and update the diff. |
Comment Actions
Separate clang-tidy integration in D5138 and rebase on top.
This diff now simply enable the rule and adds the config files
for disabling it in the libraries.