Skip to content

Commit c50e1c3

Browse files
fanquakevijaydasmp
authored andcommitted
Merge bitcoin#28076: util: Replace std::filesystem with util/fs.h
bbbbdb0 ci: Add filesystem lint check (MarcoFalke) fada2f9 refactor: Replace <filesystem> with <util/fs.h> (MarcoFalke) Pull request description: Using `std::filesystem` is problematic: * There is a `fs` namespace wrapper for it. So having two ways to achieve the same is confusing. * Not using the `fs` wrapper is dangerous and buggy, because it disables known bugs by deleting problematic functions. Fix all issues by removing use of it and adding a linter to avoid using it again in the future. ACKs for top commit: TheCharlatan: ACK bbbbdb0 fanquake: ACK bbbbdb0 🦀 Tree-SHA512: 0e2d49742b08eb2635e6fce41485277cb9c40fe20b81017c391d3472a43787db1278a236825714ca1e41c9d2f59913865cfb0c649e3c8ab1fb598c849f80c660
1 parent 45af4f9 commit c50e1c3

8 files changed

Lines changed: 119 additions & 0 deletions

File tree

ci/lint/04_install.sh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@ if [ -z "${SKIP_PYTHON_INSTALL}" ]; then
3333
python3 --version
3434
fi
3535

36+
export LINT_RUNNER_PATH="/lint_test_runner"
37+
if [ ! -d "${LINT_RUNNER_PATH}" ]; then
38+
${CI_RETRY_EXE} apt-get install -y cargo
39+
(
40+
cd ./test/lint/test_runner || exit 1
41+
cargo build
42+
mkdir -p "${LINT_RUNNER_PATH}"
43+
mv target/debug/test_runner "${LINT_RUNNER_PATH}"
44+
)
45+
fi
46+
3647
# NOTE: BUMP ALSO contrib/containers/ci/ci-slim.Dockerfile
3748
${CI_RETRY_EXE} pip3 install codespell==2.2.1
3849
${CI_RETRY_EXE} pip3 install flake8==5.0.4

ci/lint/06_script.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ test/lint/git-subtree-check.sh src/crypto/ctaes
2727
test/lint/git-subtree-check.sh src/secp256k1
2828
test/lint/git-subtree-check.sh src/minisketch
2929
test/lint/git-subtree-check.sh src/leveldb
30+
RUST_BACKTRACE=1 "${LINT_RUNNER_PATH}/test_runner"
3031
test/lint/check-doc.py
3132
test/lint/all-lint.py
3233

ci/lint/docker-entrypoint.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ export LC_ALL=C
55
# of the mounted bitcoin src dir.
66
git config --global --add safe.directory /bitcoin
77

8+
export LINT_RUNNER_PATH="/lint_test_runner"
9+
810
if [ -z "$1" ]; then
911
LOCAL_BRANCH=1 bash -ic "./ci/lint/06_script.sh"
1012
else

src/fs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ static inline path PathFromString(const std::string& string)
184184
* already exists or is a symlink to an existing directory.
185185
* This is a temporary workaround for an issue in libstdc++ that has been fixed
186186
* upstream [PR101510].
187+
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
187188
*/
188189
static inline bool create_directories(const std::filesystem::path& p)
189190
{

test/lint/README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,14 @@ docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
1717
After building the container once, you can simply run the last command any time you
1818
want to lint.
1919

20+
test runner
21+
===========
22+
23+
To run the checks in the test runner outside the docker, use:
24+
25+
```sh
26+
( cd ./test/lint/test_runner/ && cargo fmt && cargo clippy && cargo run )
27+
```
2028

2129
check-doc.py
2230
============

test/lint/test_runner/Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/lint/test_runner/Cargo.toml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Copyright (c) The Bitcoin Core developers
2+
# Distributed under the MIT software license, see the accompanying
3+
# file COPYING or https://opensource.org/license/mit/.
4+
5+
[package]
6+
name = "test_runner"
7+
version = "0.1.0"
8+
edition = "2021"
9+
10+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
11+
12+
[dependencies]

test/lint/test_runner/src/main.rs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright (c) The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or https://opensource.org/license/mit/.
4+
5+
use std::env;
6+
use std::path::PathBuf;
7+
use std::process::Command;
8+
use std::process::ExitCode;
9+
10+
use String as LintError;
11+
12+
/// Return the git command
13+
fn git() -> Command {
14+
Command::new("git")
15+
}
16+
17+
/// Return stdout
18+
fn check_output(cmd: &mut std::process::Command) -> Result<String, LintError> {
19+
let out = cmd.output().expect("command error");
20+
if !out.status.success() {
21+
return Err(String::from_utf8_lossy(&out.stderr).to_string());
22+
}
23+
Ok(String::from_utf8(out.stdout)
24+
.map_err(|e| format!("{e}"))?
25+
.trim()
26+
.to_string())
27+
}
28+
29+
/// Return the git root as utf8, or panic
30+
fn get_git_root() -> String {
31+
check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap()
32+
}
33+
34+
fn lint_std_filesystem() -> Result<(), LintError> {
35+
let found = git()
36+
.args([
37+
"grep",
38+
"std::filesystem",
39+
"--",
40+
"./src/",
41+
":(exclude)src/util/fs.h",
42+
])
43+
.status()
44+
.expect("command error")
45+
.success();
46+
if found {
47+
Err(r#"
48+
^^^
49+
Direct use of std::filesystem may be dangerous and buggy. Please include <util/fs.h> and use the
50+
fs:: namespace, which has unsafe filesystem functions marked as deleted.
51+
"#
52+
.to_string())
53+
} else {
54+
Ok(())
55+
}
56+
}
57+
58+
fn main() -> ExitCode {
59+
let test_list = [("std::filesystem check", lint_std_filesystem)];
60+
61+
let git_root = PathBuf::from(get_git_root());
62+
63+
let mut test_failed = false;
64+
for (lint_name, lint_fn) in test_list {
65+
// chdir to root before each lint test
66+
env::set_current_dir(&git_root).unwrap();
67+
if let Err(err) = lint_fn() {
68+
println!("{err}\n^---- Failure generated from {lint_name}!");
69+
test_failed = true;
70+
}
71+
}
72+
if test_failed {
73+
ExitCode::FAILURE
74+
} else {
75+
ExitCode::SUCCESS
76+
}
77+
}

0 commit comments

Comments
 (0)