Skip to content

Commit

Permalink
Remove dependency on slab
Browse files Browse the repository at this point in the history
  • Loading branch information
taiki-e committed Nov 22, 2024
1 parent e94677a commit 2dfc652
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 59 deletions.
2 changes: 0 additions & 2 deletions .deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ 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 mut restore = restore::Manager::new(!args.remove_dev_deps);
let metadata =
Metadata::new(args.manifest_path.as_deref(), &cargo, cargo_version, &args, &restore)?;
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");
}
Expand Down
16 changes: 6 additions & 10 deletions src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())?;
}
}
Expand Down Expand Up @@ -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(())
}
Expand Down
6 changes: 3 additions & 3 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Metadata {
cargo: &OsStr,
mut cargo_version: u32,
args: &Args,
restore: &restore::Manager,
restore: &mut restore::Manager,
) -> Result<Self> {
let stable_cargo_version =
cargo::version(cmd!("rustup", "run", "stable", "cargo")).map(|v| v.minor).unwrap_or(0);
Expand Down Expand Up @@ -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) => {
Expand Down
58 changes: 18 additions & 40 deletions src/restore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::{
};

use anyhow::Result;
use slab::Slab;

use crate::{fs, term};

Expand All @@ -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<Mutex<Slab<File>>>,
files: Arc<Mutex<Vec<File>>>,
}

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 || {
Expand All @@ -34,40 +33,33 @@ impl Manager {
}

/// Registers the given path if `needs_restore` is `true`.
pub(crate) fn register(&self, text: impl Into<String>, path: impl Into<PathBuf>) -> Handle<'_> {
pub(crate) fn register(&self, text: impl Into<String>, path: impl Into<PathBuf>) {
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<String>,
path: impl Into<PathBuf>,
) -> Handle<'_> {
pub(crate) fn register_always(&self, text: impl Into<String>, path: impl Into<PathBuf>) {
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:#}");
}
Expand All @@ -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,
Expand All @@ -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:#}");
}
}
}

0 comments on commit 2dfc652

Please sign in to comment.