Simplify

Remove enum, pull out function and dependent function

Author: illicitonion

src/cargo/ops/cargo_install.rs

@@ -170,103 +170,97 @@ fn install_one(
 
     let dst = root.join("bin").into_path_unlocked();
 
-    let dependency = {
-        if let Some(krate) = krate {
-            let vers = if source_id.is_git() || source_id.is_path() {
-                // We explicitly ignore the version flag for these source types.
-                None
-            } else if let Some(vers_flag) = vers {
-                Some(parse_semver_flag(vers_flag)?.to_string())
-            } else {
-                if source_id.is_registry() {
-                    // Avoid pre-release versions from crate.io
-                    // unless explicitly asked for
-                    Some(String::from("*"))
+    let pkg = {
+        let dep = {
+            if let Some(krate) = krate {
+                let vers = if let Some(vers_flag) = vers {
+                    Some(parse_semver_flag(vers_flag)?.to_string())
                 } else {
-                    None
-                }
-            };
-            Some(Dependency::parse_no_deprecated(
-                krate,
-                vers.as_deref(),
-                source_id,
-            )?)
-        } else {
-            None
-        }
-    };
-
-    let pkg = if source_id.is_git() {
-        let dep_or_list_all_fn = if let Some(dep) = dependency {
-            DependencyOrListAllFn::Dependency(dep)
-        } else {
-            DependencyOrListAllFn::ListAllFn(|git: &mut GitSource<'_>| git.read_packages())
+                    if source_id.is_registry() {
+                        // Avoid pre-release versions from crate.io
+                        // unless explicitly asked for
+                        Some(String::from("*"))
+                    } else {
+                        None
+                    }
+                };
+                Some(Dependency::parse_no_deprecated(
+                    krate,
+                    vers.as_deref(),
+                    source_id,
+                )?)
+            } else {
+                None
+            }
         };
-        select_pkg(
-            &mut GitSource::new(source_id, config)?,
-            dep_or_list_all_fn,
-            config,
-            true,
-        )?
-    } else if source_id.is_path() {
-        let mut src = path_source(source_id, config)?;
-        if !src.path().is_dir() {
-            bail!(
-                "`{}` is not a directory. \
-                 --path must point to a directory containing a Cargo.toml file.",
-                src.path().display()
-            )
-        }
-        if !src.path().join("Cargo.toml").exists() {
-            if from_cwd {
+
+        if source_id.is_git() {
+            let mut source = GitSource::new(source_id, config)?;
+            select_pkg(
+                &mut source,
+                dep,
+                |git: &mut GitSource<'_>| git.read_packages(),
+                config,
+            )?
+        } else if source_id.is_path() {
+            let mut src = path_source(source_id, config)?;
+            if !src.path().is_dir() {
                 bail!(
-                    "`{}` is not a crate root; specify a crate to \
+                    "`{}` is not a directory. \
+                 --path must point to a directory containing a Cargo.toml file.",
+                    src.path().display()
+                )
+            }
+            if !src.path().join("Cargo.toml").exists() {
+                if from_cwd {
+                    bail!(
+                        "`{}` is not a crate root; specify a crate to \
                      install from crates.io, or use --path or --git to \
                      specify an alternate source",
-                    src.path().display()
-                );
-            } else {
-                bail!(
-                    "`{}` does not contain a Cargo.toml file. \
+                        src.path().display()
+                    );
+                } else {
+                    bail!(
+                        "`{}` does not contain a Cargo.toml file. \
                      --path must point to a directory containing a Cargo.toml file.",
-                    src.path().display()
-                )
+                        src.path().display()
+                    )
+                }
             }
-        }
-        src.update()?;
-
-        let dep_or_list_all_fn = if let Some(dep) = dependency {
-            DependencyOrListAllFn::Dependency(dep)
+            select_pkg(
+                &mut src,
+                dep,
+                |path: &mut PathSource<'_>| path.read_packages(),
+                config,
+            )?
         } else {
-            DependencyOrListAllFn::ListAllFn(|path: &mut PathSource<'_>| path.read_packages())
-        };
-        select_pkg(&mut src, dep_or_list_all_fn, config, false)?
-    } else {
-        if !dependency.is_some() {
-            bail!(
-                "must specify a crate to install from \
+            if let Some(dep) = dep {
+                let mut source = map.load(source_id, &HashSet::new())?;
+                if let Ok(Some(pkg)) = installed_exact_package(
+                    dep.clone(),
+                    &mut source,
+                    config,
+                    opts,
+                    root,
+                    &dst,
+                    force,
+                ) {
+                    let msg = format!(
+                        "package `{}` is already installed, use --force to override",
+                        pkg
+                    );
+                    config.shell().status("Ignored", &msg)?;
+                    return Ok(true);
+                }
+                select_dep_pkg(&mut source, dep, config, needs_update_if_source_is_index)?
+            } else {
+                bail!(
+                    "must specify a crate to install from \
                      crates.io, or use --path or --git to \
                      specify alternate source"
-            )
-        }
-        let dep = dependency.unwrap(); // Verified with is_some/bail above.
-        let mut source = map.load(source_id, &HashSet::new())?;
-        if let Ok(Some(pkg)) =
-            installed_exact_package(dep.clone(), &mut source, config, opts, root, &dst, force)
-        {
-            let msg = format!(
-                "package `{}` is already installed, use --force to override",
-                pkg
-            );
-            config.shell().status("Ignored", &msg)?;
-            return Ok(true);
+                )
+            }
         }
-        select_pkg(
-            &mut source,
-            DependencyOrListAllFn::<fn(&mut Box<dyn Source>) -> CargoResult<Vec<Package>>>::Dependency(dep),
-            config,
-            needs_update_if_source_is_index,
-        )?
     };
 
     let git_package = if source_id.is_git() {
@@ -578,12 +572,7 @@ where
     // expensive network call in the case that the package is already installed.
     // If this fails, the caller will possibly do an index update and try again, this is just a
     // best-effort check to see if we can avoid hitting the network.
-    if let Ok(pkg) = select_pkg(
-        source,
-        DependencyOrListAllFn::<fn(&mut T) -> CargoResult<Vec<Package>>>::Dependency(dep),
-        config,
-        false,
-    ) {
+    if let Ok(pkg) = select_dep_pkg(source, dep, config, false) {
         let (_ws, rustc, target) =
             make_ws_rustc_target(&config, opts, &source.source_id(), pkg.clone())?;
         if let Ok(true) = is_installed(&pkg, config, opts, &rustc, &target, root, &dst, force) {
@@ -644,6 +633,7 @@ fn parse_semver_flag(v: &str) -> CargoResult<VersionReq> {
             ),
         }
     } else {
+        println!("DWH: Converting...");
         match v.to_semver() {
             Ok(v) => Ok(VersionReq::exact(&v)),
             Err(e) => {

src/cargo/ops/cargo_uninstall.rs

@@ -88,9 +88,9 @@ fn uninstall_cwd(root: &Filesystem, bins: &[String], config: &Config) -> CargoRe
     let mut src = path_source(source_id, config)?;
     let pkg = select_pkg(
         &mut src,
-        DependencyOrListAllFn::ListAllFn(|path: &mut PathSource<'_>| path.read_packages()),
+        None,
+        |path: &mut PathSource<'_>| path.read_packages(),
         config,
-        true,
     )?;
     let pkgid = pkg.package_id();
     uninstall_pkgid(root, tracker, pkgid, bins, config)

src/cargo/ops/common_for_install_and_uninstall.rs

@@ -519,23 +519,15 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult<PathSour
     Ok(PathSource::new(&path, source_id, config))
 }
 
-/// Either the specific dependency to be selected, or a function which can be executed to list all
-/// packages known in a source so that one can be selected from that list.
-pub enum DependencyOrListAllFn<F> {
-    Dependency(Dependency),
-    ListAllFn(F),
-}
-
 /// Gets a Package based on command-line requirements.
-pub fn select_pkg<T, F>(
+pub fn select_dep_pkg<T>(
     source: &mut T,
-    dep_or_list_all_fn: DependencyOrListAllFn<F>,
+    dep: Dependency,
     config: &Config,
     needs_update: bool,
 ) -> CargoResult<Package>
 where
     T: Source,
-    F: FnMut(&mut T) -> CargoResult<Vec<Package>>,
 {
     // This operation may involve updating some sources or making a few queries
     // which may involve frobbing caches, as a result make sure we synchronize
@@ -546,54 +538,71 @@ where
         source.update()?;
     }
 
-    match dep_or_list_all_fn {
-        DependencyOrListAllFn::Dependency(dep) => {
-            let deps = source.query_vec(&dep)?;
-            match deps.iter().map(|p| p.package_id()).max() {
-                Some(pkgid) => {
-                    let pkg = Box::new(source).download_now(pkgid, config)?;
-                    Ok(pkg)
-                }
-                None => bail!(
-                    "could not find `{}` in {} with version `{}`",
-                    dep.package_name(),
-                    source.source_id(),
-                    dep.version_req(),
-                ),
-            }
+    let deps = source.query_vec(&dep)?;
+    match deps.iter().map(|p| p.package_id()).max() {
+        Some(pkgid) => {
+            let pkg = Box::new(source).download_now(pkgid, config)?;
+            Ok(pkg)
         }
-        DependencyOrListAllFn::ListAllFn(mut list_all) => {
-            let candidates = list_all(source)?;
-            let binaries = candidates
-                .iter()
-                .filter(|cand| cand.targets().iter().filter(|t| t.is_bin()).count() > 0);
-            let examples = candidates
-                .iter()
-                .filter(|cand| cand.targets().iter().filter(|t| t.is_example()).count() > 0);
-            let pkg = match one(binaries, |v| multi_err("binaries", v))? {
+        None => bail!(
+            "could not find `{}` in {} with version `{}`",
+            dep.package_name(),
+            source.source_id(),
+            dep.version_req(),
+        ),
+    }
+}
+
+pub fn select_pkg<T, F>(
+    source: &mut T,
+    dep: Option<Dependency>,
+    mut list_all: F,
+    config: &Config,
+) -> CargoResult<Package>
+where
+    T: Source,
+    F: FnMut(&mut T) -> CargoResult<Vec<Package>>,
+{
+    // This operation may involve updating some sources or making a few queries
+    // which may involve frobbing caches, as a result make sure we synchronize
+    // with other global Cargos
+    let _lock = config.acquire_package_cache_lock()?;
+
+    source.update()?;
+
+    return if let Some(dep) = dep {
+        select_dep_pkg(source, dep, config, false)
+    } else {
+        let candidates = list_all(source)?;
+        let binaries = candidates
+            .iter()
+            .filter(|cand| cand.targets().iter().filter(|t| t.is_bin()).count() > 0);
+        let examples = candidates
+            .iter()
+            .filter(|cand| cand.targets().iter().filter(|t| t.is_example()).count() > 0);
+        let pkg = match one(binaries, |v| multi_err("binaries", v))? {
+            Some(p) => p,
+            None => match one(examples, |v| multi_err("examples", v))? {
                 Some(p) => p,
-                None => match one(examples, |v| multi_err("examples", v))? {
-                    Some(p) => p,
-                    None => bail!(
-                        "no packages found with binaries or \
-                     examples"
-                    ),
-                },
-            };
-            return Ok(pkg.clone());
-
-            fn multi_err(kind: &str, mut pkgs: Vec<&Package>) -> String {
-                pkgs.sort_unstable_by_key(|a| a.name());
-                format!(
-                    "multiple packages with {} found: {}",
-                    kind,
-                    pkgs.iter()
-                        .map(|p| p.name().as_str())
-                        .collect::<Vec<_>>()
-                        .join(", ")
-                )
-            }
-        }
+                None => bail!(
+                    "no packages found with binaries or \
+                         examples"
+                ),
+            },
+        };
+        Ok(pkg.clone())
+    };
+
+    fn multi_err(kind: &str, mut pkgs: Vec<&Package>) -> String {
+        pkgs.sort_unstable_by_key(|a| a.name());
+        format!(
+            "multiple packages with {} found: {}",
+            kind,
+            pkgs.iter()
+                .map(|p| p.name().as_str())
+                .collect::<Vec<_>>()
+                .join(", ")
+        )
     }
 }