Merge pull request #226 from NobodyXu/fix/metafile-update

Fix race condition when updating metafile

src/binstall.rs

@@ -1,6 +1,7 @@
+use std::collections::BTreeSet;
 use std::path::PathBuf;
 
-use crate::{DesiredTargets, PkgOverride};
+use crate::{metafiles, DesiredTargets, PkgOverride};
 
 mod resolve;
 pub use resolve::*;
@@ -16,3 +17,11 @@ pub struct Options {
     pub cli_overrides: PkgOverride,
     pub desired_targets: DesiredTargets,
 }
+
+/// MetaData required to update MetaFiles.
+pub struct MetaData {
+    pub bins: BTreeSet<String>,
+    pub cvs: metafiles::CrateVersionSource,
+    pub version_req: String,
+    pub target: String,
+}

src/binstall/install.rs

@@ -1,18 +1,18 @@
-use std::{collections::BTreeSet, path::PathBuf, process, sync::Arc};
+use std::{path::PathBuf, process, sync::Arc};
 
 use cargo_toml::Package;
 use log::{debug, error, info};
 use miette::{miette, IntoDiagnostic, Result, WrapErr};
 use tokio::{process::Command, task::block_in_place};
 
-use super::{Options, Resolution};
+use super::{MetaData, Options, Resolution};
 use crate::{bins, fetchers::Fetcher, *};
 
 pub async fn install(
     resolution: Resolution,
     opts: Arc<Options>,
     jobserver_client: LazyJobserverClient,
-) -> Result<()> {
+) -> Result<Option<MetaData>> {
     match resolution {
         Resolution::Fetch {
             fetcher,
@@ -37,13 +37,15 @@ pub async fn install(
                 .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?;
 
             if !opts.dry_run {
-                install_from_source(package, target, jobserver_client).await
+                install_from_source(package, target, jobserver_client)
+                    .await
+                    .map(|_| None)
             } else {
                 info!(
                     "Dry-run: running `cargo install {} --version {} --target {target}`",
                     package.name, package.version
                 );
-                Ok(())
+                Ok(None)
             }
         }
     }
@@ -56,7 +58,7 @@ async fn install_from_package(
     version: String,
     bin_path: PathBuf,
     bin_files: Vec<bins::BinFile>,
-) -> Result<()> {
+) -> Result<Option<MetaData>> {
     // Download package
     if opts.dry_run {
         info!("Dry run, not downloading package");
@@ -90,7 +92,7 @@ async fn install_from_package(
 
     if opts.dry_run {
         info!("Dry run, not proceeding");
-        return Ok(());
+        return Ok(None);
     }
 
     info!("Installing binaries...");
@@ -106,25 +108,12 @@ async fn install_from_package(
             }
         }
 
-        let bins: BTreeSet<String> = bin_files.into_iter().map(|bin| bin.base_name).collect();
-
-        debug!("Writing .crates.toml");
-        metafiles::v1::CratesToml::append(&cvs, bins.clone())?;
-
-        debug!("Writing .crates2.json");
-        metafiles::v2::Crates2Json::append(
-            &cvs,
-            metafiles::v2::CrateInfo {
-                version_req: Some(version),
-                bins,
-                profile: "release".into(),
+        Ok(Some(MetaData {
+            bins: bin_files.into_iter().map(|bin| bin.base_name).collect(),
+            cvs,
+            version_req: version,
             target: fetcher.target().to_string(),
-                rustc: format!("{} {}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")),
-                ..Default::default()
-            },
-        )?;
-
-        Ok(())
+        }))
     })
 }
 

src/main.rs

@@ -12,7 +12,7 @@ use log::{debug, error, info, warn, LevelFilter};
 use miette::{miette, Result, WrapErr};
 use simplelog::{ColorChoice, ConfigBuilder, TermLogger, TerminalMode};
 use tempfile::TempDir;
-use tokio::{runtime::Runtime, task::JoinError};
+use tokio::{runtime::Runtime, task::block_in_place};
 
 use cargo_binstall::{binstall, *};
 
@@ -330,9 +330,35 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
             .collect()
     };
 
+    let mut metadata_vec = Vec::with_capacity(tasks.len());
     for task in tasks {
-        await_task(task).await?;
+        if let Some(metadata) = await_task(task).await? {
+            metadata_vec.push(metadata);
         }
+    }
+
+    block_in_place(|| {
+        debug!("Writing .crates.toml");
+        metafiles::v1::CratesToml::append(
+            metadata_vec
+                .iter()
+                .map(|metadata| (&metadata.cvs, metadata.bins.clone())),
+        )?;
+
+        debug!("Writing .crates2.json");
+        metafiles::v2::Crates2Json::append(metadata_vec.into_iter().map(|metadata| {
+            (
+                metadata.cvs,
+                metafiles::v2::CrateInfo {
+                    version_req: Some(metadata.version_req),
+                    bins: metadata.bins,
+                    profile: "release".into(),
+                    target: metadata.target,
+                    rustc: format!("{} {}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION")),
+                    ..Default::default()
+                },
+            )
+        }))?;
 
         if opts.no_cleanup {
             // Consume temp_dir without removing it from fs.
@@ -344,4 +370,5 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
         }
 
         Ok(())
+    })
 }

src/metafiles/v1.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::{BTreeMap, BTreeSet},
     fs, io,
+    iter::IntoIterator,
     path::{Path, PathBuf},
     str::FromStr,
 };
@@ -44,11 +45,13 @@ impl CratesToml {
         Ok(())
     }
 
-    pub fn append_to_path(
+    pub fn append_to_path<'a, Iter>(
         path: impl AsRef<Path>,
-        cvs: &CrateVersionSource,
-        bins: BTreeSet<String>,
-    ) -> Result<(), CratesTomlParseError> {
+        iter: Iter,
+    ) -> Result<(), CratesTomlParseError>
+    where
+        Iter: IntoIterator<Item = (&'a CrateVersionSource, BTreeSet<String>)>,
+    {
         let mut c1 = match Self::load_from_path(path.as_ref()) {
             Ok(c1) => c1,
             Err(CratesTomlParseError::Io(io_err)) if io_err.kind() == io::ErrorKind::NotFound => {
@@ -56,17 +59,19 @@ impl CratesToml {
             }
             Err(err) => return Err(err),
         };
+        for (cvs, bins) in iter {
             c1.insert(cvs, bins);
+        }
         c1.write_to_path(path.as_ref())?;
 
         Ok(())
     }
 
-    pub fn append(
-        cvs: &CrateVersionSource,
-        bins: BTreeSet<String>,
-    ) -> Result<(), CratesTomlParseError> {
-        Self::append_to_path(Self::default_path()?, cvs, bins)
+    pub fn append<'a, Iter>(iter: Iter) -> Result<(), CratesTomlParseError>
+    where
+        Iter: IntoIterator<Item = (&'a CrateVersionSource, BTreeSet<String>)>,
+    {
+        Self::append_to_path(Self::default_path()?, iter)
     }
 }
 

src/metafiles/v2.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::{BTreeMap, BTreeSet},
     fs, io,
+    iter::IntoIterator,
     path::{Path, PathBuf},
 };
 
@@ -66,11 +67,13 @@ impl Crates2Json {
         Ok(())
     }
 
-    pub fn append_to_path(
+    pub fn append_to_path<Iter>(
         path: impl AsRef<Path>,
-        cvs: &CrateVersionSource,
-        info: CrateInfo,
-    ) -> Result<(), Crates2JsonParseError> {
+        iter: Iter,
+    ) -> Result<(), Crates2JsonParseError>
+    where
+        Iter: IntoIterator<Item = (CrateVersionSource, CrateInfo)>,
+    {
         let mut c2 = match Self::load_from_path(path.as_ref()) {
             Ok(c2) => c2,
             Err(Crates2JsonParseError::Io(io_err)) if io_err.kind() == io::ErrorKind::NotFound => {
@@ -78,14 +81,19 @@ impl Crates2Json {
             }
             Err(err) => return Err(err),
         };
-        c2.insert(cvs, info);
+        for (cvs, info) in iter {
+            c2.insert(&cvs, info);
+        }
         c2.write_to_path(path.as_ref())?;
 
         Ok(())
     }
 
-    pub fn append(cvs: &CrateVersionSource, info: CrateInfo) -> Result<(), Crates2JsonParseError> {
-        Self::append_to_path(Self::default_path()?, cvs, info)
+    pub fn append<Iter>(iter: Iter) -> Result<(), Crates2JsonParseError>
+    where
+        Iter: IntoIterator<Item = (CrateVersionSource, CrateInfo)>,
+    {
+        Self::append_to_path(Self::default_path()?, iter)
     }
 }