Skip to content

Commit 628139c

Browse files
2bndy52bndy5
committed
more nitpicky review concerns from coderabbit AI
1 parent 5c4e623 commit 628139c

9 files changed

Lines changed: 52 additions & 68 deletions

File tree

‎benchmark/benchmark.nu‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export def summarize [] {
151151
let data = open benchmark.json
152152
let results = $data | get results | reject "times" "exit_codes"
153153
let summary = open --raw benchmark.md
154-
let summary_file = if ($env | get --optional GITHUB_STEP_SUMMARY | is-not-empty) {
154+
if ($env | get --optional GITHUB_STEP_SUMMARY | is-not-empty) {
155155
$"\n# Results\n\n($summary)" | save --append $env.GITHUB_STEP_SUMMARY
156156
} else {
157157
$"# Results\n\n($summary)" | save --force benchmark.md

‎clang-installer/src/downloader/hashing.rs‎

Lines changed: 41 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
use sha2::digest::{Digest, OutputSizeUser, generic_array::ArrayLength};
2+
13
use super::DownloadError;
4+
use crate::progress_bar::ProgressBar;
25

3-
use std::{fs, io::Read, path::Path};
6+
use std::{fs, io::Read, num::NonZero, path::Path};
47

58
/// Represents the supported hash algorithms for file integrity checking.
69
///
@@ -20,79 +23,62 @@ pub enum HashAlgorithm {
2023
}
2124

2225
impl HashAlgorithm {
26+
fn hash_file<H>(mut hasher: H, file_path: &Path, expected: &str) -> Result<(), DownloadError>
27+
where
28+
H: Digest + OutputSizeUser,
29+
<H as OutputSizeUser>::OutputSize: std::ops::Add,
30+
<<H as OutputSizeUser>::OutputSize as std::ops::Add>::Output: ArrayLength<u8>,
31+
{
32+
let mut file_reader = fs::OpenOptions::new().read(true).open(file_path)?;
33+
let file_size = file_reader.metadata()?.len();
34+
let mut progress_bar =
35+
ProgressBar::new(NonZero::new(file_size), "Verifying file integrity");
36+
let mut buf = [0u8; 1024];
37+
loop {
38+
let bytes_read = file_reader.read(&mut buf)?;
39+
if bytes_read == 0 {
40+
break;
41+
}
42+
progress_bar.inc(bytes_read as u64)?;
43+
hasher.update(&buf[..bytes_read]);
44+
}
45+
progress_bar.finish()?;
46+
let actual = format!("{:x}", hasher.finalize());
47+
if actual == *expected {
48+
Ok(())
49+
} else {
50+
Err(DownloadError::HashMismatch {
51+
expected: expected.to_owned(),
52+
actual,
53+
})
54+
}
55+
}
56+
2357
/// Verify a given file (located at `file_path`) against the expected checksum value.
2458
///
2559
/// This method reads the file in chunks (of 1024 bytes) to compute the hash,
2660
/// thus no extraneous memory is allocated when reading the file's entire contents.
2761
///
2862
/// Note, a progress bar is displayed to stdout.
2963
pub fn verify(&self, file_path: &Path) -> Result<(), DownloadError> {
30-
let mut file_reader = fs::OpenOptions::new().read(true).open(file_path)?;
31-
let mut buf = [0u8; 1024];
3264
match self {
3365
HashAlgorithm::Sha256(expected) => {
3466
use sha2::{Digest, Sha256};
3567

36-
let mut hasher = Sha256::new();
37-
loop {
38-
let bytes_read = file_reader.read(&mut buf)?;
39-
if bytes_read == 0 {
40-
break;
41-
}
42-
hasher.update(&buf[..bytes_read]);
43-
}
44-
let actual = format!("{:x}", hasher.finalize());
45-
if actual == *expected {
46-
Ok(())
47-
} else {
48-
Err(DownloadError::HashMismatch {
49-
expected: expected.clone(),
50-
actual,
51-
})
52-
}
68+
let hasher = Sha256::new();
69+
Self::hash_file(hasher, file_path, expected)
5370
}
5471
HashAlgorithm::Blake2b256(expected) => {
5572
use blake2::{Blake2b, Digest, digest::consts::U32};
5673

57-
let mut hasher = Blake2b::<U32>::new();
58-
loop {
59-
let bytes_read = file_reader.read(&mut buf)?;
60-
if bytes_read == 0 {
61-
break;
62-
}
63-
hasher.update(&buf[..bytes_read]);
64-
}
65-
let actual = format!("{:x}", hasher.finalize());
66-
if actual == *expected {
67-
Ok(())
68-
} else {
69-
Err(DownloadError::HashMismatch {
70-
expected: expected.clone(),
71-
actual,
72-
})
73-
}
74+
let hasher = Blake2b::<U32>::new();
75+
Self::hash_file(hasher, file_path, expected)
7476
}
7577
HashAlgorithm::Sha512(expected) => {
7678
use sha2::{Digest, Sha512};
7779

78-
let mut hasher = Sha512::new();
79-
80-
loop {
81-
let bytes_read = file_reader.read(&mut buf)?;
82-
if bytes_read == 0 {
83-
break;
84-
}
85-
hasher.update(&buf[..bytes_read]);
86-
}
87-
let actual = format!("{:x}", hasher.finalize());
88-
if actual == *expected {
89-
Ok(())
90-
} else {
91-
Err(DownloadError::HashMismatch {
92-
expected: expected.clone(),
93-
actual,
94-
})
95-
}
80+
let hasher = Sha512::new();
81+
Self::hash_file(hasher, file_path, expected)
9682
}
9783
}
9884
}

‎clang-installer/src/downloader/native_packages/mod.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,6 @@ pub async fn try_install_package(
8181
return Ok(None);
8282
} else {
8383
for mgr in os_pkg_managers {
84-
if !mgr.is_installed() {
85-
log::debug!("Skipping {mgr} package manager as it is not installed.");
86-
continue;
87-
}
8884
log::info!("Trying to install {tool} v{min_version} using {mgr} package manager.");
8985
let pkg_name = mgr.get_package_name(tool);
9086
if mgr.is_installed_package(&pkg_name, Some(min_version)) {

‎clang-installer/src/downloader/native_packages/windows.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ impl PackageManager for WindowsPackageManager {
4343
Command::new(self.as_str())
4444
.arg("--version")
4545
.output()
46-
.is_ok()
46+
.is_ok_and(|output| output.status.success())
4747
}
4848

4949
fn list_managers() -> Vec<impl PackageManager + Display>
@@ -170,7 +170,7 @@ impl PackageManager for WindowsPackageManager {
170170
// is already installed (with a newer version). So use `--force` to reinstall the specified version.
171171
cmd.arg("--version").arg(version.to_string()).arg("--force");
172172
}
173-
let output = cmd.output().map_err(PackageManagerError::Io)?;
173+
let output = cmd.output()?;
174174
if output.status.success() {
175175
Ok(())
176176
} else {

‎clang-installer/src/downloader/pypi.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,12 @@ impl LinuxLibC {
185185
LinuxLibC::Musl {
186186
version: pkg_musl_version,
187187
} => Self::get_musl_version()
188-
.map(|sys_musl_version| *pkg_musl_version >= sys_musl_version)
188+
.map(|sys_musl_version| sys_musl_version >= *pkg_musl_version)
189189
.unwrap_or(false),
190190
LinuxLibC::Glibc {
191191
version: pkg_glibc_version,
192192
} => Self::get_glibc_version()
193-
.map(|sys_glibc_version| *pkg_glibc_version >= sys_glibc_version)
193+
.map(|sys_glibc_version| sys_glibc_version >= *pkg_glibc_version)
194194
.unwrap_or(false),
195195
}
196196
}

‎clang-installer/src/downloader/static_dist.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ impl StaticDistDownloader {
168168
}
169169
let sha512_cache_path = cache_path
170170
.join("static_dist")
171-
.join(format!("{tool}-{ver_str}.sha512").as_str());
171+
.join(format!("{tool}-{ver_str}.sha512"));
172172
if sha512_cache_path.exists() {
173173
log::info!(
174174
"Using cached SHA512 checksum for {tool} version {ver_str} from {:?}",

‎clang-installer/src/progress_bar.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ impl ProgressBar {
8080

8181
/// Renders the progress bar based on the current state.
8282
///
83-
/// This should only be invoked once after [`Self::new()`].
83+
/// This should be invoked once after [`Self::new()`] to render the initial 0% state.
8484
/// Subsequent updates should be made using [`Self::inc()`], which will call this method internally.
8585
pub fn render(&mut self) -> Result<()> {
8686
let advance_bar = self.total.map(|total| {

‎clang-installer/src/version.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ pub enum GetToolError {
4747
#[error("Failed to parse version: {0}")]
4848
VersionParseError(String),
4949

50-
/// The version requirement does satisfy any known/supported clang version
51-
#[error("The version requirement does satisfy any known/supported clang version")]
50+
/// The version requirement does not satisfy any known/supported clang version
51+
#[error("The version requirement does not satisfy any known/supported clang version")]
5252
UnsupportedVersion,
5353

5454
/// Binary executable in cache has no parent directory.

‎cpp-linter/src/common_fs/file_filter.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ impl FileFilter {
1919
///
2020
/// The `ignore` parameter is a list of glob patterns that should be ignored.
2121
/// These can be explicitly not ignored by prefixing them with `!`.
22+
/// Hidden files/folders (patterns that start with a ".") cannot be explicitly not
23+
/// ignored; they are always ignored.
2224
///
2325
/// The `extensions` parameter is a list of file extensions that should be included.
2426
pub fn new(ignore: &[String], extensions: Vec<String>) -> Self {

0 commit comments

Comments
 (0)