2db926f49c
facbcd4cef8890ae18976fb53b67ea56b3c04454 log: Use ConstevalFormatString (MarcoFalke)
fae9b60c4ffef38d9725f42f992b1f38765312a3 test: Use LogPrintStr to test m_log_sourcelocations (MarcoFalke)
fa39b1ca63874db8ef8bc16b87e2699e8e1b67be doc: move-only logging warning (MarcoFalke)
Pull request description:
This changes all logging (including the wallet logging) to produce a
`ConstevalFormatString` at compile time, so that the format string can be
validated at compile-time.
I tested with `clang` and found that the compiler will use less than 1% more of time and memory.
When an error is found, the compile-time error depends on the compiler, but it may look similar to:
```
src/util/string.h: In function ‘int main(int, char**)’:
src/bitcoind.cpp:265:5: in ‘constexpr’ expansion of ‘util::ConstevalFormatString<1>(((const char*)"Hi %s %s"))’
src/util/string.h:38:98: in ‘constexpr’ expansion of ‘util::ConstevalFormatString<1>::Detail_CheckNumFormatSpecifiers(std::basic_string_view<char>(((const char*)((util::ConstevalFormatString<1>*)this)->util::ConstevalFormatString<1>::fmt)))’
src/util/string.h:78:34: error: expression ‘<throw-expression>’ is not a constant expression
78 | if (num_params != count) throw "Format specifier count must match the argument count!";
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
This refactor does not change behavior of the compiled executables.
ACKs for top commit:
hodlinator:
re-ACK facbcd4cef8890ae18976fb53b67ea56b3c04454
l0rinc:
ACK facbcd4cef8890ae18976fb53b67ea56b3c04454
ryanofsky:
Code review ACK facbcd4cef8890ae18976fb53b67ea56b3c04454
pablomartin4btc:
re-ACK facbcd4cef8890ae18976fb53b67ea56b3c04454
stickies-v:
Approach ACK and code LGTM facbcd4cef8890ae18976fb53b67ea56b3c04454 modulo a `tinyformat::format_error` concern.
Tree-SHA512: 852f74d360897020f0d0f6e5064edc5e7f7dacc2bec1d5feff22c634a2fcd2eb535aa75be0b7191d9053728be6108484c737154b02d68ad3186a2e5544ba0db8
This folder contains lint scripts.
Running locally
To run linters locally with the same versions as the CI environment, use the included Dockerfile:
DOCKER_BUILDKIT=1 docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
Building the container can be done every time, because it is fast when the result is cached and it prevents issues when the image changes.
test runner
To run all the lint checks in the test runner outside the docker you first need to install the rust toolchain using your package manager of choice or rustup.
Then you can use:
( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && RUST_BACKTRACE=1 cargo run )
If you wish to run individual lint checks, run the test_runner with
--lint=TEST_TO_RUN arguments. If running with cargo run, arguments after
-- are passed to the binary you are running e.g.:
( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --lint=doc --lint=trailing_whitespace )
To see a list of all individual lint checks available in test_runner, use -h
or --help:
( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --help )
Dependencies
| Lint test | Dependency |
|---|---|
lint-python.py |
lief |
lint-python.py |
mypy |
lint-python.py |
pyzmq |
lint-python-dead-code.py |
vulture |
lint-shell.py |
ShellCheck |
lint-spelling.py |
codespell |
py_lint |
ruff |
| markdown link check | mlc |
In use versions and install instructions are available in the CI setup.
Please be aware that on Linux distributions all dependencies are usually available as packages, but could be outdated.
Running the tests
Individual tests can be run by directly calling the test script, e.g.:
test/lint/lint-files.py
check-doc.py
Check for missing documentation of command line options.
commit-script-check.sh
Verification of scripted diffs. Scripted diffs are only assumed to run on the latest LTS release of Ubuntu. Running them on other operating systems might require installing GNU tools, such as GNU sed.
git-subtree-check.sh
Run this script from the root of the repository to verify that a subtree matches the contents of the commit it claims to have been updated to.
Usage: test/lint/git-subtree-check.sh [-r] DIR [COMMIT]
test/lint/git-subtree-check.sh -?
DIRis the prefix within the repository to check.COMMITis the commit to check, if it is not provided, HEAD will be used.-rchecks that subtree commit is present in repository.
To do a full check with -r, make sure that you have fetched the upstream repository branch in which the subtree is
maintained:
- for
src/secp256k1: https://github.com/bitcoin-core/secp256k1.git (branch master) - for
src/leveldb: https://github.com/bitcoin-core/leveldb-subtree.git (branch bitcoin-fork) - for
src/crypto/ctaes: https://github.com/bitcoin-core/ctaes.git (branch master) - for
src/crc32c: https://github.com/bitcoin-core/crc32c-subtree.git (branch bitcoin-fork) - for
src/minisketch: https://github.com/sipa/minisketch.git (branch master)
To do so, add the upstream repository as remote:
git remote add --fetch secp256k1 https://github.com/bitcoin-core/secp256k1.git
lint_ignore_dirs.py
Add list of common directories to ignore when running tests