From 58376208ddcc2ce8baab430b55f4e14dc94f83c6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 21 Mar 2023 18:50:00 -0400 Subject: [PATCH 1/4] Bump MSRV to 1.66.0 This is what we have elsewhere in coreos/ and I think will be prep for further PRs. Signed-off-by: Colin Walters --- .github/workflows/rust.yml | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7aa2f9c1..9ef9e41b 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -13,7 +13,7 @@ permissions: env: CARGO_TERM_COLOR: always # Pinned toolchain for linting - ACTION_LINTS_TOOLCHAIN: 1.59.0 + ACTION_LINTS_TOOLCHAIN: 1.66.0 jobs: tests-stable: diff --git a/Cargo.toml b/Cargo.toml index a3ce6284..8be7cd48 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ license = "Apache-2.0" version = "0.2.9" authors = ["Colin Walters "] edition = "2021" -rust-version = "1.58.1" +rust-version = "1.66.0" # See https://github.com/cgwalters/cargo-vendor-filterer [package.metadata.vendor-filter] From a3a46cec90419a62811f01666d246a2bf7d654f4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 4 May 2023 17:18:51 -0400 Subject: [PATCH 2/4] Allow clippy::box_default Because this code is right, and what `cargo clippy --fix` wants to do actually breaks compilation. Signed-off-by: Colin Walters --- src/bootupd.rs | 2 ++ src/component.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/bootupd.rs b/src/bootupd.rs index a4992532..0266a736 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -73,9 +73,11 @@ pub(crate) fn get_components() -> Components { } #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] + #[allow(clippy::box_default)] insert_component(&mut components, Box::new(efi::Efi::default())); #[cfg(any(target_arch = "x86_64", target_arch = "powerpc64"))] + #[allow(clippy::box_default)] insert_component(&mut components, Box::new(bios::Bios::default())); components diff --git a/src/component.rs b/src/component.rs index eee5ed93..36992d0a 100644 --- a/src/component.rs +++ b/src/component.rs @@ -77,8 +77,10 @@ pub(crate) trait Component { pub(crate) fn new_from_name(name: &str) -> Result> { let r: Box = match name { #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] + #[allow(clippy::box_default)] "EFI" => Box::new(crate::efi::Efi::default()), #[cfg(any(target_arch = "x86_64", target_arch = "powerpc64"))] + #[allow(clippy::box_default)] "BIOS" => Box::new(crate::bios::Bios::default()), _ => anyhow::bail!("No component {}", name), }; From 087f63beccf556809d7b49332483eb5dc81f0c6c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 4 May 2023 17:21:38 -0400 Subject: [PATCH 3/4] tree-wide: Fix various minor clippy lints The unnecessary borrows one always feels like the most churn for the least value, but let's do it. Signed-off-by: Colin Walters --- src/bios.rs | 6 +++--- src/component.rs | 2 +- src/efi.rs | 6 +++--- src/filetree.rs | 17 ++++++++--------- src/util.rs | 6 +++--- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/bios.rs b/src/bios.rs index 374042c1..7dc7099c 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -62,9 +62,9 @@ impl Bios { // We also add part_gpt because in some cases probing of the partition map can fail such // as in a container, but we always use GPT. #[cfg(target_arch = "x86_64")] - cmd.args(&["--target", "i386-pc"]) - .args(&["--boot-directory", boot_dir.to_str().unwrap()]) - .args(&["--modules", "mdraid1x part_gpt"]) + cmd.args(["--target", "i386-pc"]) + .args(["--boot-directory", boot_dir.to_str().unwrap()]) + .args(["--modules", "mdraid1x part_gpt"]) .arg(device); #[cfg(target_arch = "powerpc64")] diff --git a/src/component.rs b/src/component.rs index 36992d0a..7ac0c7f0 100644 --- a/src/component.rs +++ b/src/component.rs @@ -116,7 +116,7 @@ pub(crate) fn write_update_metadata( let sysroot = openat::Dir::open(sysroot)?; let dir = sysroot.sub_dir(BOOTUPD_UPDATES_DIR)?; let name = component_update_data_name(component); - dir.write_file_with(&name, 0o644, |w| -> Result<_> { + dir.write_file_with(name, 0o644, |w| -> Result<_> { Ok(serde_json::to_writer(w, &meta)?) })?; Ok(()) diff --git a/src/efi.rs b/src/efi.rs index 6806b451..d407788f 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -198,9 +198,9 @@ impl Component for Efi { // TODO - add some sort of API that allows directly setting the working // directory to a file descriptor. let r = std::process::Command::new("cp") - .args(&["-rp", "--reflink=auto"]) + .args(["-rp", "--reflink=auto"]) .arg(&srcdir_name) - .arg(&destdir) + .arg(destdir) .current_dir(format!("/proc/self/fd/{}", src_root.as_raw_fd())) .status()?; if !r.success() { @@ -262,7 +262,7 @@ impl Component for Efi { // Fork off mv() because on overlayfs one can't rename() a lower level // directory today, and this will handle the copy fallback. - Command::new("mv").args(&[&efisrc, &dest_efidir]).run()?; + Command::new("mv").args([&efisrc, &dest_efidir]).run()?; } // Query the rpm database and list the package and build times for all the diff --git a/src/filetree.rs b/src/filetree.rs index 12a2d3a3..a55f8d9b 100644 --- a/src/filetree.rs +++ b/src/filetree.rs @@ -276,7 +276,7 @@ pub(crate) struct ApplyUpdateOptions { pub(crate) fn syncfs(d: &openat::Dir) -> Result<()> { let d = d.sub_dir(".").expect("subdir"); let mut c = std::process::Command::new("sync"); - let c = c.args(&["-f", "."]); + let c = c.args(["-f", "."]); unsafe { c.pre_exec(move || { nix::unistd::fchdir(d.as_raw_fd()).expect("fchdir"); @@ -357,8 +357,8 @@ mod tests { use std::io::Write; fn run_diff(a: &openat::Dir, b: &openat::Dir) -> Result { - let ta = FileTree::new_from_dir(&a)?; - let tb = FileTree::new_from_dir(&b)?; + let ta = FileTree::new_from_dir(a)?; + let tb = FileTree::new_from_dir(b)?; let diff = ta.diff(&tb)?; Ok(diff) } @@ -374,7 +374,7 @@ mod tests { let c = t.path().join("c"); let r = std::process::Command::new("cp") .arg("-rp") - .args(&[a, &c]) + .args([a, &c]) .status()?; if !r.success() { bail!("failed to cp"); @@ -415,9 +415,8 @@ mod tests { skip_removals: true, ..Default::default() }; - test_one_apply(&a, &b, None).context("testing apply (with removals)")?; - test_one_apply(&a, &b, Some(&skip_removals)) - .context("testing apply (skipping removals)")?; + test_one_apply(a, b, None).context("testing apply (with removals)")?; + test_one_apply(a, b, Some(&skip_removals)).context("testing apply (skipping removals)")?; Ok(()) } @@ -496,7 +495,7 @@ mod tests { let newsubp = Path::new(relp).join("subdir"); fs::create_dir_all(b.join(&newsubp))?; fs::write(b.join(&newsubp).join("newgrub.x64"), "newgrub data")?; - fs::remove_file(b.join(&relp).join("shim.x64"))?; + fs::remove_file(b.join(relp).join("shim.x64"))?; { let a = openat::Dir::open(&a)?; let b = openat::Dir::open(&b)?; @@ -516,7 +515,7 @@ mod tests { String::from_utf8(std::fs::read(a.join(&newsubp).join("newgrub.x64"))?)?, "newgrub data" ); - assert!(!a.join(&relp).join("shim.x64").exists()); + assert!(!a.join(relp).join("shim.x64").exists()); Ok(()) } } diff --git a/src/util.rs b/src/util.rs index 16cee818..319796fe 100644 --- a/src/util.rs +++ b/src/util.rs @@ -80,7 +80,7 @@ pub(crate) fn ensure_writable_mount>(p: P) -> Result<()> { return Ok(()); } let status = std::process::Command::new("mount") - .args(&["-o", "remount,rw"]) + .args(["-o", "remount,rw"]) .arg(p) .status()?; if !status.success() { @@ -128,7 +128,7 @@ pub(crate) fn parse_rpm_metadata(stdout: Vec) -> Result { /// files in the EFI system partition, or for grub2-install file pub(crate) fn rpm_query(sysroot_path: &str, path: &Path) -> Result { let mut c = ostreeutil::rpm_cmd(sysroot_path); - c.args(&["-q", "--queryformat", "%{nevra},%{buildtime} ", "-f"]); + c.args(["-q", "--queryformat", "%{nevra},%{buildtime} ", "-f"]); match path.file_name().expect("filename").to_str() { Some("EFI") => { @@ -139,7 +139,7 @@ pub(crate) fn rpm_query(sysroot_path: &str, path: &Path) -> Result { })); } Some("grub2-install") => { - c.arg(&path); + c.arg(path); } _ => { bail!("Unsupported file/directory {:?}", path) From 1f32f947ecc5516e1f72056c9f26e73c16cb17e1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 4 May 2023 17:22:34 -0400 Subject: [PATCH 4/4] filetree: Use `.write_all()` in tests This clippy lint catches a real bug. Signed-off-by: Colin Walters --- src/filetree.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/filetree.rs b/src/filetree.rs index a55f8d9b..fe1c6787 100644 --- a/src/filetree.rs +++ b/src/filetree.rs @@ -437,7 +437,7 @@ mod tests { assert_eq!(diff.count(), 0); { let mut bar = a.write_file("foo/bar", 0o644)?; - bar.write("foobarcontents".as_bytes())?; + bar.write_all("foobarcontents".as_bytes())?; } let diff = run_diff(&a, &b)?; assert_eq!(diff.count(), 1); @@ -456,14 +456,14 @@ mod tests { b.create_dir("foo", 0o755)?; { let mut bar = b.write_file("foo/bar", 0o644)?; - bar.write("foobarcontents".as_bytes())?; + bar.write_all("foobarcontents".as_bytes())?; } let diff = run_diff(&a, &b)?; assert_eq!(diff.count(), 0); test_apply(&pa, &pb).context("testing apply 2")?; { let mut bar2 = b.write_file("foo/bar", 0o644)?; - bar2.write("foobarcontents2".as_bytes())?; + bar2.write_all("foobarcontents2".as_bytes())?; } let diff = run_diff(&a, &b)?; assert_eq!(diff.count(), 1);