diff --git a/.deny.toml b/.deny.toml index c230c73..3b7c8c7 100644 --- a/.deny.toml +++ b/.deny.toml @@ -22,7 +22,6 @@ build.allow-build-scripts = [ { name = "proc-macro2" }, # via serde_derive via cargo-config2 { name = "serde_json" }, { name = "serde" }, - { name = "slab" }, { name = "windows_aarch64_gnullvm" }, # via ctrlc & same-file & termcolor { name = "windows_aarch64_msvc" }, # via ctrlc & same-file & termcolor { name = "windows_i686_gnu" }, # via ctrlc & same-file & termcolor @@ -33,7 +32,6 @@ build.allow-build-scripts = [ { name = "windows_x86_64_msvc" }, # via ctrlc & same-file & termcolor ] build.bypass = [ - { name = "autocfg", allow-globs = ["tests/wrap_ignored"] }, # via slab # Import libraries are necessary because raw-dylib (requires 1.71+ for x86, 1.65+ for others) is not available on MSRV of them. { name = "windows_aarch64_gnullvm", allow-globs = ["lib/*.a"] }, # via ctrlc & same-file & termcolor { name = "windows_aarch64_msvc", allow-globs = ["lib/*.lib"] }, # via ctrlc & same-file & termcolor diff --git a/.github/.cspell/project-dictionary.txt b/.github/.cspell/project-dictionary.txt index e5285ff..5b98493 100644 --- a/.github/.cspell/project-dictionary.txt +++ b/.github/.cspell/project-dictionary.txt @@ -1,4 +1,3 @@ -autocfg binstall qpmember subcrate diff --git a/Cargo.toml b/Cargo.toml index 36dd97e..4805916 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,8 +31,7 @@ ctrlc = { version = "3.4.4", features = ["termination"] } lexopt = "0.3" same-file = "1.0.1" serde_json = "1" -slab = "0.4.4" -termcolor = "1.1" +termcolor = "1" toml_edit = "0.22.7" [dev-dependencies] diff --git a/src/context.rs b/src/context.rs index af4090e..1ed8717 100644 --- a/src/context.rs +++ b/src/context.rs @@ -49,9 +49,14 @@ impl Context { .unwrap_or(0); // if `--remove-dev-deps` flag is off, restore manifest file. - let restore = restore::Manager::new(!args.remove_dev_deps); - let metadata = - Metadata::new(args.manifest_path.as_deref(), &cargo, cargo_version, &args, &restore)?; + let mut restore = restore::Manager::new(!args.remove_dev_deps); + let metadata = Metadata::new( + args.manifest_path.as_deref(), + &cargo, + cargo_version, + &args, + &mut restore, + )?; if metadata.cargo_version < 41 && args.include_deps_features { bail!("--include-deps-features requires Cargo 1.41 or later"); } diff --git a/src/manifest.rs b/src/manifest.rs index f318c47..703d164 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -104,8 +104,7 @@ pub(crate) fn with(cx: &Context, f: impl FnOnce() -> Result<()>) -> Result<()> { let restore_lockfile = true; let no_dev_deps = cx.no_dev_deps | cx.remove_dev_deps; let no_private = cx.no_private; - let restore_handles = if no_dev_deps || no_private { - let mut restore_handles = Vec::with_capacity(cx.metadata.workspace_members.len()); + if no_dev_deps || no_private { let workspace_root = &cx.metadata.workspace_root; let root_manifest = &workspace_root.join("Cargo.toml"); let mut root_id = None; @@ -134,7 +133,7 @@ pub(crate) fn with(cx: &Context, f: impl FnOnce() -> Result<()>) -> Result<()> { info!("removing dev-dependencies from {}", manifest_path.display()); } remove_dev_deps(&mut doc); - restore_handles.push(cx.restore.register(&manifest.raw, manifest_path)); + cx.restore.register(&manifest.raw, manifest_path); fs::write(manifest_path, doc.to_string())?; } } @@ -170,24 +169,21 @@ pub(crate) fn with(cx: &Context, f: impl FnOnce() -> Result<()>) -> Result<()> { } remove_private_crates(&mut doc, workspace_root, private_crates); } - restore_handles.push(cx.restore.register(orig, manifest_path)); + cx.restore.register(orig, manifest_path); fs::write(manifest_path, doc.to_string())?; } if restore_lockfile { let lockfile = &workspace_root.join("Cargo.lock"); if lockfile.exists() { - restore_handles.push(cx.restore.register(fs::read_to_string(lockfile)?, lockfile)); + cx.restore.register(fs::read_to_string(lockfile)?, lockfile); } } - restore_handles - } else { - vec![] - }; + } f()?; // Restore original Cargo.toml and Cargo.lock. - drop(restore_handles); + cx.restore.restore_all(); Ok(()) } diff --git a/src/metadata.rs b/src/metadata.rs index 157005d..b8dba54 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -54,7 +54,7 @@ impl Metadata { cargo: &OsStr, mut cargo_version: u32, args: &Args, - restore: &restore::Manager, + restore: &mut restore::Manager, ) -> Result { let stable_cargo_version = cargo::version(cmd!("rustup", "run", "stable", "cargo")).map(|v| v.minor).unwrap_or(0); @@ -114,14 +114,14 @@ impl Metadata { cmd.run_with_output()?; } let guard = term::verbose::scoped(false); - let mut handle = restore.register_always(fs::read_to_string(&lockfile)?, lockfile); + restore.register_always(fs::read_to_string(&lockfile)?, lockfile); // Try with stable cargo because if workspace member has // a dependency that requires newer cargo features, `cargo metadata` // with older cargo may fail. cmd = cmd!("rustup", "run", "stable", "cargo"); append_metadata_args(&mut cmd); let json = cmd.read(); - handle.close()?; + restore.restore_last()?; drop(guard); match json { Ok(json) => { diff --git a/src/restore.rs b/src/restore.rs index 9dbc945..92dafaf 100644 --- a/src/restore.rs +++ b/src/restore.rs @@ -7,7 +7,6 @@ use std::{ }; use anyhow::Result; -use slab::Slab; use crate::{fs, term}; @@ -16,12 +15,12 @@ pub(crate) struct Manager { // A flag that indicates restore is needed. needs_restore: bool, /// Information on files that need to be restored. - files: Arc>>, + files: Arc>>, } impl Manager { pub(crate) fn new(needs_restore: bool) -> Self { - let this = Self { needs_restore, files: Arc::new(Mutex::new(Slab::new())) }; + let this = Self { needs_restore, files: Arc::new(Mutex::new(vec![])) }; let cloned = this.clone(); ctrlc::set_handler(move || { @@ -34,40 +33,33 @@ impl Manager { } /// Registers the given path if `needs_restore` is `true`. - pub(crate) fn register(&self, text: impl Into, path: impl Into) -> Handle<'_> { + pub(crate) fn register(&self, text: impl Into, path: impl Into) { if !self.needs_restore { - return Handle(None); + return; } - self.register_always(text.into(), path.into()) + self.register_always(text.into(), path.into()); } /// Registers the given path regardless of the value of `needs_restore`. - pub(crate) fn register_always( - &self, - text: impl Into, - path: impl Into, - ) -> Handle<'_> { + pub(crate) fn register_always(&self, text: impl Into, path: impl Into) { let mut files = self.files.lock().unwrap(); - let entry = files.vacant_entry(); - let key = entry.key(); - entry.insert(File { text: text.into(), path: path.into() }); - - Handle(Some((self, key))) + files.push(File { text: text.into(), path: path.into() }); } - fn restore(&self, key: usize) -> Result<()> { + // This takes `&mut self` instead of `&self` to prevent misuse in multi-thread contexts. + pub(crate) fn restore_last(&mut self) -> Result<()> { let mut files = self.files.lock().unwrap(); - if let Some(file) = files.try_remove(key) { + if let Some(file) = files.pop() { file.restore()?; } Ok(()) } - fn restore_all(&self) { + pub(crate) fn restore_all(&self) { let mut files = self.files.lock().unwrap(); if !files.is_empty() { - for (_, file) in mem::take(&mut *files) { + for file in mem::take(&mut *files) { if let Err(e) = file.restore() { error!("{e:#}"); } @@ -76,6 +68,12 @@ impl Manager { } } +impl Drop for Manager { + fn drop(&mut self) { + self.restore_all(); + } +} + struct File { /// The original text of this file. text: String, @@ -91,23 +89,3 @@ impl File { fs::write(&self.path, &self.text) } } - -#[must_use] -pub(crate) struct Handle<'a>(Option<(&'a Manager, usize)>); - -impl Handle<'_> { - pub(crate) fn close(&mut self) -> Result<()> { - if let Some((manager, key)) = self.0.take() { - manager.restore(key)?; - } - Ok(()) - } -} - -impl Drop for Handle<'_> { - fn drop(&mut self) { - if let Err(e) = self.close() { - error!("{e:#}"); - } - } -}