diff options
author | MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> | 2024-02-08 13:58:44 +0100 |
---|---|---|
committer | MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> | 2024-02-20 15:03:23 +0100 |
commit | fa31908ea848488ff842f1b9fce6235bb8855ec7 (patch) | |
tree | ead9e34c075cbbc33242329c8e8f3d335007c705 | |
parent | fa63b0e351dee4782ee19ad46603957ef8d020eb (diff) | |
download | bitcoin-fa31908ea848488ff842f1b9fce6235bb8855ec7.tar.xz |
lint: Check for missing or redundant bitcoin-config.h includes
-rwxr-xr-x | ci/lint/04_install.sh | 3 | ||||
-rw-r--r-- | test/lint/test_runner/src/main.rs | 107 |
2 files changed, 108 insertions, 2 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/test/lint/test_runner/src/main.rs b/test/lint/test_runner/src/main.rs index 3207538e6f..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; @@ -45,6 +45,14 @@ fn get_subtrees() -> Vec<&'static str> { ] } +/// 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. @@ -87,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() @@ -128,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), ]; |