From 5bb5d12949ef6b73448e849cc5ae1e0f2aa11159 Mon Sep 17 00:00:00 2001
From: Jiahao XU <Jiahao_XU@outlook.com>
Date: Sun, 12 Jun 2022 16:37:53 +1000
Subject: [PATCH] Optimize `fetch_crate_cratesio` using `ManifestVisitor`

and `download_tar_based_and_visit`.

By using these two items, we avoid any I/O altogether.
Everything happens in memory, thus there will be no i/o related errors
or cost.

This commit does not regress anything because
`helpers::load_manifest_path` calls `Manifest::from_path_with_metadata`,
which read in the whole `Cargo.toml` file at once.

Thus this commit would not cause any OOM when the the original code
would not.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
---
 src/drivers/cratesio.rs | 25 +++++++++----------------
 src/main.rs             |  9 +++++----
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/src/drivers/cratesio.rs b/src/drivers/cratesio.rs
index 82b5044d..861b49cd 100644
--- a/src/drivers/cratesio.rs
+++ b/src/drivers/cratesio.rs
@@ -1,19 +1,20 @@
 use std::path::{Path, PathBuf};
 use std::time::Duration;
 
+use cargo_toml::Manifest;
 use crates_io_api::AsyncClient;
 use log::debug;
 use url::Url;
 
-use super::find_version;
-use crate::{helpers::*, BinstallError, TarBasedFmt};
+use super::{find_version, ManifestVisitor};
+use crate::{helpers::*, BinstallError, Meta, TarBasedFmt};
 
 /// Fetch a crate Cargo.toml by name and version from crates.io
 pub async fn fetch_crate_cratesio(
     name: &str,
     version_req: &str,
     temp_dir: &Path,
-) -> Result<PathBuf, BinstallError> {
+) -> Result<Manifest<Meta>, BinstallError> {
     // Fetch / update index
     debug!("Looking up crate information");
 
@@ -59,22 +60,14 @@ pub async fn fetch_crate_cratesio(
 
     debug!("Fetching crate from: {crate_url} and extracting Cargo.toml from it");
 
-    let crate_dir: PathBuf = format!("{name}-{version_name}").into();
-    let crate_path = temp_dir.join(&crate_dir);
+    let manifest_dir_path: PathBuf = format!("{name}-{version_name}").into();
 
-    let cargo_toml = crate_dir.join("Cargo.toml");
-    let src = crate_dir.join("src");
-    let main = src.join("main.rs");
-    let bin = src.join("bin");
-
-    download_and_extract_with_filter(
+    download_tar_based_and_visit(
         Url::parse(&crate_url)?,
         TarBasedFmt::Tgz,
         &temp_dir,
-        move |path: &Path| path == cargo_toml || path == main || path.starts_with(&bin),
+        ManifestVisitor::new(manifest_dir_path),
     )
-    .await?;
-
-    // Return crate directory
-    Ok(crate_path)
+    .await?
+    .load_manifest()
 }
diff --git a/src/main.rs b/src/main.rs
index 5b89353c..b95a7111 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -210,13 +210,14 @@ async fn entry() -> Result<()> {
     // Fetch crate via crates.io, git, or use a local manifest path
     // TODO: work out which of these to do based on `opts.name`
     // TODO: support git-based fetches (whole repo name rather than just crate name)
-    let manifest_path = match opts.manifest_path.clone() {
-        Some(p) => p,
+    let manifest = match opts.manifest_path.clone() {
+        Some(manifest_path) => {
+            debug!("Reading manifest: {}", manifest_path.display());
+            load_manifest_path(manifest_path.join("Cargo.toml"))?
+        }
         None => fetch_crate_cratesio(&opts.name, &opts.version, temp_dir.path()).await?,
     };
 
-    debug!("Reading manifest: {}", manifest_path.display());
-    let manifest = load_manifest_path(manifest_path.join("Cargo.toml"))?;
     let package = manifest.package.unwrap();
 
     let is_plain_version = semver::Version::from_str(&opts.version).is_ok();