diff options
| -rwxr-xr-x | ci/lint/04_install.sh | 3 | ||||
| -rw-r--r-- | src/bench/wallet_ismine.cpp | 5 | ||||
| -rw-r--r-- | test/lint/README.md | 2 | ||||
| -rw-r--r-- | test/lint/test_runner/src/main.rs | 128 |
4 files changed, 126 insertions, 12 deletions
diff --git a/ci/lint/04_install.sh b/ci/lint/04_install.sh index 476417d04b..47cb8864c2 100755 --- a/ci/lint/04_install.sh +++ b/ci/lint/04_install.sh @@ -10,10 +10,11 @@ export PATH=$PWD/ci/retry:$PATH ${CI_RETRY_EXE} apt-get update # Lint dependencies: +# - automake pkg-config libtool (for lint_includes_build_config) # - curl/xz-utils (to install shellcheck) # - git (used in many lint scripts) # - gpg (used by verify-commits) -${CI_RETRY_EXE} apt-get install -y curl xz-utils git gpg +${CI_RETRY_EXE} apt-get install -y automake pkg-config libtool curl xz-utils git gpg PYTHON_PATH="/python_build" if [ ! -d "${PYTHON_PATH}/bin" ]; then diff --git a/src/bench/wallet_ismine.cpp b/src/bench/wallet_ismine.cpp index 261c95c7c8..3f922e18a5 100644 --- a/src/bench/wallet_ismine.cpp +++ b/src/bench/wallet_ismine.cpp @@ -2,6 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#if defined(HAVE_CONFIG_H) +#include <config/bitcoin-config.h> +#endif // HAVE_CONFIG_H #include <bench/bench.h> #include <interfaces/chain.h> #include <key.h> @@ -11,9 +14,9 @@ #include <util/translation.h> #include <validationinterface.h> #include <wallet/context.h> +#include <wallet/test/util.h> #include <wallet/wallet.h> #include <wallet/walletutil.h> -#include <wallet/test/util.h> namespace wallet { static void WalletIsMine(benchmark::Bench& bench, bool legacy_wallet, int num_combo = 0) diff --git a/test/lint/README.md b/test/lint/README.md index 1fba41d9ea..1bfcb75327 100644 --- a/test/lint/README.md +++ b/test/lint/README.md @@ -19,7 +19,7 @@ test runner To run all the lint checks in the test runner outside the docker, use: ```sh -( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && cargo run ) +( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && RUST_BACKTRACE=1 cargo run ) ``` #### Dependencies diff --git a/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index 1dc79e97bd..b97e822484 100644 --- a/test/lint/test_runner/src/main.rs +++ b/test/lint/test_runner/src/main.rs @@ -4,7 +4,7 @@ use std::env; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; use std::process::ExitCode; @@ -34,17 +34,30 @@ fn get_git_root() -> PathBuf { PathBuf::from(check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap()) } +/// Return all subtree paths +fn get_subtrees() -> Vec<&'static str> { + vec![ + "src/crc32c", + "src/crypto/ctaes", + "src/leveldb", + "src/minisketch", + "src/secp256k1", + ] +} + +/// Return the pathspecs to exclude all subtrees +fn get_pathspecs_exclude_subtrees() -> Vec<String> { + get_subtrees() + .iter() + .map(|s| format!(":(exclude){}", s)) + .collect() +} + fn lint_subtree() -> LintResult { // This only checks that the trees are pure subtrees, it is not doing a full // check with -r to not have to fetch all the remotes. let mut good = true; - for subtree in [ - "src/crypto/ctaes", - "src/secp256k1", - "src/minisketch", - "src/leveldb", - "src/crc32c", - ] { + for subtree in get_subtrees() { good &= Command::new("test/lint/git-subtree-check.sh") .arg(subtree) .status() @@ -82,6 +95,102 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted. } } +fn lint_includes_build_config() -> LintResult { + let config_path = "./src/config/bitcoin-config.h.in"; + let include_directive = "#include <config/bitcoin-config.h>"; + if !Path::new(config_path).is_file() { + assert!(Command::new("./autogen.sh") + .status() + .expect("command error") + .success()); + } + let defines_regex = format!( + r"^\s*(?!//).*({})", + check_output(Command::new("grep").args(["undef ", "--", config_path])) + .expect("grep failed") + .lines() + .map(|line| { + line.split("undef ") + .nth(1) + .unwrap_or_else(|| panic!("Could not extract name in line: {line}")) + }) + .collect::<Vec<_>>() + .join("|") + ); + let print_affected_files = |mode: bool| { + // * mode==true: Print files which use the define, but lack the include + // * mode==false: Print files which lack the define, but use the include + let defines_files = check_output( + git() + .args([ + "grep", + "--perl-regexp", + if mode { + "--files-with-matches" + } else { + "--files-without-match" + }, + &defines_regex, + "--", + "*.cpp", + "*.h", + ]) + .args(get_pathspecs_exclude_subtrees()) + .args([ + // These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds + // these cppflags manually. + ":(exclude)src/crypto/sha256_arm_shani.cpp", + ":(exclude)src/crypto/sha256_avx2.cpp", + ":(exclude)src/crypto/sha256_sse41.cpp", + ":(exclude)src/crypto/sha256_x86_shani.cpp", + ]), + ) + .expect("grep failed"); + git() + .args([ + "grep", + if mode { + "--files-without-match" + } else { + "--files-with-matches" + }, + include_directive, + "--", + ]) + .args(defines_files.lines()) + .status() + .expect("command error") + .success() + }; + let missing = print_affected_files(true); + if missing { + return Err(format!( + r#" +^^^ +One or more files use a symbol declared in the bitcoin-config.h header. However, they are not +including the header. This is problematic, because the header may or may not be indirectly +included. If the indirect include were to be intentionally or accidentally removed, the build could +still succeed, but silently be buggy. For example, a slower fallback algorithm could be picked, +even though bitcoin-config.h indicates that a faster feature is available and should be used. + +If you are unsure which symbol is used, you can find it with this command: +git grep --perl-regexp '{}' -- file_name + "#, + defines_regex + )); + } + let redundant = print_affected_files(false); + if redundant { + return Err(r#" +^^^ +None of the files use a symbol declared in the bitcoin-config.h header. However, they are including +the header. Consider removing the unused include. + "# + .to_string()); + } + Ok(()) +} + fn lint_doc() -> LintResult { if Command::new("test/lint/check-doc.py") .status() @@ -123,6 +232,7 @@ fn main() -> ExitCode { let test_list: Vec<(&str, LintFn)> = vec![ ("subtree check", lint_subtree), ("std::filesystem check", lint_std_filesystem), + ("build config includes check", lint_includes_build_config), ("-help=1 documentation check", lint_doc), ("lint-*.py scripts", lint_all), ]; @@ -134,7 +244,7 @@ fn main() -> ExitCode { // chdir to root before each lint test env::set_current_dir(&git_root).unwrap(); if let Err(err) = lint_fn() { - println!("{err}\n^---- Failure generated from {lint_name}!"); + println!("{err}\n^---- ⚠️ Failure generated from {lint_name}!"); test_failed = true; } } |