fix updating of installed crates manifest on custom sparse registry (#2178)

* Add either v1.15.0 to binstalk-registry Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Add `SourceType::Sparse` Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Add `Registry::crate_source` Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Use `Registry::crate_siouy Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Add support for `Source::Sparse` Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Optimize `CratesToml::append_to_file` Bulkify remove Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix typing in cargo_crates_v1.rs and optimize append_to_file Use `Vec::reserve_exact ` in `append_to_file` to avoid unnecessary allocation Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix Registry::url Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix unused variable in `Registry::crate_source` Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix fmt in cargo_crates_v1.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix fmt in binstalk-registry/src/lib.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix fmt in cargo_crates_v1.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Refactor: Extract new fn CratesToml::add_crate Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix `<Source as Display>::fmt` impl for `Source::Sprase` Add `/` to the end of the url if it doesn't have one Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix crate_version_source.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix lifetime of `CrateToml::add_crate` API Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix lifetime of `CratesToml<'::add_crate` API Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * cargo fmt crate_version_source.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix `Registery::crate_source` Match sparse/git crates.io registry to standardrised `CrateSource::cratesio_registry()`. Also optimize it to avoid unnecessary `.to_string()` and `Url::parse` for sparse registry. Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix `GitRegistry::url` ret type Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix `SparseRegistery::url` return type Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Fix typing in `Registry::crate_source` Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Rm unused import in sparse_registry.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Rm unused import in git_registry.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * cargo fmt binstalk-registry/src/lib.rs Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> * Rm unused dep either from binstalk-registry Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> --------- Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>
2025-06-07 11:16:37 +00:00 · 2025-06-06 14:13:05 +10:00 · 2025-06-06 14:13:05 +10:00 · 25a52038ef
commit 25a52038ef
parent e8c9cc3599
7 changed files with 101 additions and 56 deletions
--- a/crates/binstalk-manifests/src/cargo_crates_v1.rs
+++ b/crates/binstalk-manifests/src/cargo_crates_v1.rs
@ -37,7 +37,7 @@ pub struct CratesToml<'a> {
    v1: Vec<(String, Cow<'a, [CompactString]>)>,
 }

-impl CratesToml<'_> {
+impl<'v1> CratesToml<'v1> {
    pub fn default_path() -> Result<PathBuf, CratesTomlParseError> {
        Ok(cargo_home()?.join(".crates.toml"))
    }
@ -68,9 +68,14 @@ impl CratesToml<'_> {
    }

    pub fn remove(&mut self, name: &str) {
+        self.remove_all(&[name]);
+    }
+
+    /// * `sorted_names` - must be sorted
+    pub fn remove_all(&mut self, sorted_names: &[&str]) {
        self.v1.retain(|(s, _bin)| {
            s.split_once(' ')
-                .map(|(crate_name, _rest)| crate_name != name)
+                .map(|(crate_name, _rest)| sorted_names.binary_search(&crate_name).is_err())
                .unwrap_or_default()
        });
    }
@ -106,53 +111,52 @@ impl CratesToml<'_> {
        self.write_to_file(&mut file)
    }

-    pub fn append_to_file<'a, Iter>(file: &mut File, iter: Iter) -> Result<(), CratesTomlParseError>
-    where
-        Iter: IntoIterator<Item = &'a CrateInfo>,
-    {
-        fn inner(
-            file: &mut File,
-            iter: &mut dyn Iterator<Item = &CrateInfo>,
-        ) -> Result<(), CratesTomlParseError> {
-            let mut c1 = CratesToml::load_from_reader(&mut *file)?;
-
-            for metadata in iter {
+    pub fn add_crate(&mut self, metadata: &'v1 CrateInfo) {
        let name = &metadata.name;
        let version = &metadata.current_version;
        let source = Source::from(&metadata.source);

-                c1.remove(name);
-                c1.v1.push((
+        self.v1.push((
            format!("{name} {version} ({source})"),
            Cow::borrowed(&metadata.bins),
        ));
    }

+    pub fn append_to_file(
+        file: &mut File,
+        crates: &[CrateInfo],
+    ) -> Result<(), CratesTomlParseError> {
+        let mut c1 = CratesToml::load_from_reader(&mut *file)?;
+
+        let mut crate_names: Vec<_> = crates
+            .iter()
+            .map(|metadata| metadata.name.as_str())
+            .collect();
+        crate_names.sort_unstable();
+        c1.remove_all(&crate_names);
+
+        c1.v1.reserve_exact(crates.len());
+
+        for metadata in crates {
+            c1.add_crate(metadata);
+        }
+
        file.rewind()?;
        c1.write_to_file(file)?;

        Ok(())
    }

-        inner(file, &mut iter.into_iter())
-    }
-
-    pub fn append_to_path<'a, Iter>(
+    pub fn append_to_path(
        path: impl AsRef<Path>,
-        iter: Iter,
-    ) -> Result<(), CratesTomlParseError>
-    where
-        Iter: IntoIterator<Item = &'a CrateInfo>,
-    {
+        crates: &[CrateInfo],
+    ) -> Result<(), CratesTomlParseError> {
        let mut file = create_if_not_exist(path.as_ref())?;
-        Self::append_to_file(&mut file, iter)
+        Self::append_to_file(&mut file, crates)
    }

-    pub fn append<'a, Iter>(iter: Iter) -> Result<(), CratesTomlParseError>
-    where
-        Iter: IntoIterator<Item = &'a CrateInfo>,
-    {
-        Self::append_to_path(Self::default_path()?, iter)
+    pub fn append(crates: &[CrateInfo]) -> Result<(), CratesTomlParseError> {
+        Self::append_to_path(Self::default_path()?, crates)
    }

    /// Return BTreeMap with crate name as key and its corresponding version
--- a/crates/binstalk-manifests/src/cargo_crates_v1/crate_version_source.rs
+++ b/crates/binstalk-manifests/src/cargo_crates_v1/crate_version_source.rs
@ -1,4 +1,8 @@
-use std::{borrow::Cow, fmt, str::FromStr};
+use std::{
+    borrow::Cow,
+    fmt::{self, Write as _},
+    str::FromStr,
+};

 use binstalk_types::maybe_owned::MaybeOwned;
 use compact_str::CompactString;
@ -30,6 +34,7 @@ impl From<&CrateInfo> for CrateVersionSource {
                Git => Source::Git(url),
                Path => Source::Path(url),
                Registry => Source::Registry(url),
+                Sparse => Source::Sparse(url),
            },
        }
    }
@ -40,6 +45,7 @@ pub enum Source<'a> {
    Git(MaybeOwned<'a, Url>),
    Path(MaybeOwned<'a, Url>),
    Registry(MaybeOwned<'a, Url>),
+    Sparse(MaybeOwned<'a, Url>),
 }

 impl<'a> From<&'a CrateSource> for Source<'a> {
@ -52,6 +58,7 @@ impl<'a> From<&'a CrateSource> for Source<'a> {
            Git => Self::Git(url),
            Path => Self::Path(url),
            Registry => Self::Registry(url),
+            Sparse => Self::Sparse(url),
        }
    }
 }
@ -125,6 +132,15 @@ impl fmt::Display for Source<'_> {
            Source::Git(url) => write!(f, "git+{url}"),
            Source::Path(url) => write!(f, "path+{url}"),
            Source::Registry(url) => write!(f, "registry+{url}"),
+            Source::Sparse(url) => {
+                let url = url.as_str();
+                write!(f, "sparse+{url}")?;
+                if url.ends_with("/") {
+                    Ok(())
+                } else {
+                    f.write_char('/')
+                }
+            }
        }
    }
 }
--- a/crates/binstalk-registry/src/git_registry.rs
+++ b/crates/binstalk-registry/src/git_registry.rs
@ -1,4 +1,4 @@
-use std::{fmt::Display, io, path::PathBuf, sync::Arc};
+use std::{io, path::PathBuf, sync::Arc};

 use binstalk_downloader::remote::Client;
 use binstalk_types::cargo_toml_binstall::Meta;
@ -73,7 +73,7 @@ impl GitRegistry {
        }))
    }

-    pub fn url(&self) -> impl Display + '_ {
+    pub fn url(&self) -> &GitUrl {
        &self.0.url
    }

--- a/crates/binstalk-registry/src/lib.rs
+++ b/crates/binstalk-registry/src/lib.rs
@ -7,7 +7,11 @@ use binstalk_downloader::{
    download::DownloadError,
    remote::{Client, Error as RemoteError},
 };
-use binstalk_types::cargo_toml_binstall::Meta;
+use binstalk_types::{
+    cargo_toml_binstall::Meta,
+    crate_info::{CrateSource, SourceType},
+    maybe_owned::MaybeOwned,
+};
 use cargo_toml_workspace::cargo_toml::{Error as CargoTomlError, Manifest};
 use compact_str::CompactString;
 use leon::{ParseError, RenderError};
@ -80,7 +84,7 @@ pub enum RegistryError {
    CargoManifest(#[from] Box<CargoTomlError>),

    #[error("Failed to parse url: {0}")]
-    UrlParse(#[from] url::ParseError),
+    UrlParse(#[from] UrlParseError),

    #[error(transparent)]
    Download(#[from] DownloadError),
@ -195,6 +199,38 @@ impl Registry {
            }
        }
    }
+
+    /// Get url of the regsitry
+    pub fn url(&self) -> Result<MaybeOwned<'_, Url>, UrlParseError> {
+        match self {
+            #[cfg(feature = "git")]
+            Registry::Git(registry) => {
+                Url::parse(&registry.url().to_string()).map(MaybeOwned::Owned)
+            }
+            Registry::Sparse(registry) => Ok(MaybeOwned::Borrowed(registry.url())),
+        }
+    }
+
+    /// Get crate source of this registry
+    pub fn crate_source(&self) -> Result<CrateSource, UrlParseError> {
+        let registry = self.url()?;
+        let source_type = match self {
+            #[cfg(feature = "git")]
+            Registry::Git(_) => SourceType::Git,
+            Registry::Sparse(_) => SourceType::Sparse,
+        };
+
+        Ok(match (registry.as_str(), source_type) {
+            ("https://index.crates.io/", SourceType::Sparse)
+            | ("https://github.com/rust-lang/crates.io-index", SourceType::Git) => {
+                CrateSource::cratesio_registry()
+            }
+            _ => CrateSource {
+                source_type,
+                url: MaybeOwned::Owned(registry.into_owned()),
+            },
+        })
+    }
 }

 impl fmt::Display for Registry {
--- a/crates/binstalk-registry/src/sparse_registry.rs
+++ b/crates/binstalk-registry/src/sparse_registry.rs
@ -1,5 +1,3 @@
-use std::fmt::Display;
-
 use binstalk_downloader::remote::{Client, Error as RemoteError};
 use binstalk_types::cargo_toml_binstall::Meta;
 use cargo_toml_workspace::cargo_toml::Manifest;
@ -30,7 +28,7 @@ impl SparseRegistry {
        }
    }

-    pub fn url(&self) -> impl Display + '_ {
+    pub fn url(&self) -> &Url {
        &self.url
    }

--- a/crates/binstalk-types/src/crate_info.rs
+++ b/crates/binstalk-types/src/crate_info.rs
@ -65,6 +65,7 @@ pub enum SourceType {
    Git,
    Path,
    Registry,
+    Sparse,
 }

 #[derive(Clone, Debug, Serialize, Deserialize)]
--- a/crates/binstalk/src/ops/resolve.rs
+++ b/crates/binstalk/src/ops/resolve.rs
@ -496,17 +496,7 @@ impl PackageInfo {
                        .fetch_crate_matched(client, &name, version_req),
                )
                .await?,
-                {
-                    let registry = format!("{}", opts.registry);
-                    if registry == "https://index.crates.io/" {
-                        CrateSource::cratesio_registry()
-                    } else {
-                        CrateSource {
-                            source_type: SourceType::Registry,
-                            url: MaybeOwned::Owned(Url::parse(&registry)?),
-                        }
-                    }
-                },
+                opts.registry.crate_source()?,
            ),
        };