Merge pull request #147 from jmcconnell26/ISSUE-16-RemoveUseOfPackageSet

ISSUE-16 - Remove use of cargo::core:

cargo-geiger/src/cli.rs

@@ -8,16 +8,11 @@
 // TODO: Investigate how cargo-clippy is implemented. Is it using syn?  Is is
 // using rustc? Is it implementing a compiler plugin?
 
-use crate::args::FeaturesArgs;
 use crate::Args;
 
 // TODO: Consider making this a lib.rs (again) and expose a full API, excluding
 // only the terminal output..? That API would be dependent on cargo.
-use cargo::core::package::PackageSet;
-use cargo::core::registry::PackageRegistry;
-use cargo::core::resolver::ResolveOpts;
-use cargo::core::{Package, PackageId, PackageIdSpec, Resolve, Workspace};
-use cargo::ops;
+use cargo::core::Workspace;
 use cargo::util::{self, important_paths, CargoResult};
 use cargo::Config;
 use cargo_metadata::{CargoOpt, Metadata, MetadataCommand};
@@ -88,15 +83,6 @@ pub fn get_krates(cargo_metadata: &Metadata) -> CargoResult<Krates> {
         .build_with_metadata(cargo_metadata.clone(), |_| ())?)
 }
 
-pub fn get_registry<'a>(
-    config: &'a Config,
-    package: &Package,
-) -> CargoResult<PackageRegistry<'a>> {
-    let mut registry = PackageRegistry::new(config)?;
-    registry.add_sources(Some(package.package_id().source_id()))?;
-    Ok(registry)
-}
-
 pub fn get_workspace(
     config: &Config,
     manifest_path: Option<PathBuf>,
@@ -108,37 +94,6 @@ pub fn get_workspace(
     Workspace::new(&root, config)
 }
 
-pub fn resolve<'a, 'cfg>(
-    args: &FeaturesArgs,
-    package_id: PackageId,
-    registry: &mut PackageRegistry<'cfg>,
-    workspace: &'a Workspace<'cfg>,
-) -> CargoResult<(PackageSet<'a>, Resolve)> {
-    let dev_deps = true; // TODO: Review this.
-    let uses_default_features = !args.no_default_features;
-    let opts = ResolveOpts::new(
-        dev_deps,
-        &args.features.clone(),
-        args.all_features,
-        uses_default_features,
-    );
-    let prev = ops::load_pkg_lockfile(workspace)?;
-    let resolve = ops::resolve_with_previous(
-        registry,
-        workspace,
-        &opts,
-        prev.as_ref(),
-        None,
-        &[PackageIdSpec::from_package_id(package_id)],
-        true,
-    )?;
-    let packages = ops::get_resolved_packages(
-        &resolve,
-        PackageRegistry::new(workspace.config())?,
-    )?;
-    Ok((packages, resolve))
-}
-
 // TODO: Make a wrapper type for canonical paths and hide all mutable access.
 
 #[cfg(test)]
@@ -195,29 +150,6 @@ mod cli_tests {
         assert!(krates_result.is_ok());
     }
 
-    #[rstest]
-    fn get_registry_test() {
-        let config = Config::default().unwrap();
-        let workspace = Workspace::new(
-            &important_paths::find_root_manifest_for_wd(config.cwd()).unwrap(),
-            &config,
-        )
-        .unwrap();
-        let package = workspace.current().unwrap();
-
-        let registry_result = get_registry(&config, &package);
-
-        assert!(registry_result.is_ok());
-        let registry = registry_result.unwrap();
-
-        let package_ids = vec![package.package_id()];
-        let package_set_result = registry.get(&package_ids);
-        assert!(package_set_result.is_ok());
-        let package_set = package_set_result.unwrap();
-
-        assert_eq!(package_set.sources().len(), 1);
-    }
-
     #[rstest]
     fn get_workspace_test() {
         let config = Config::default().unwrap();
@@ -233,19 +165,4 @@ mod cli_tests {
 
         assert_eq!(package.package_id().name(), "cargo-geiger");
     }
-
-    #[rstest]
-    fn resolve_test() {
-        let args = FeaturesArgs::default();
-        let config = Config::default().unwrap();
-        let manifest_path: Option<PathBuf> = None;
-        let workspace = get_workspace(&config, manifest_path).unwrap();
-        let package = workspace.current().unwrap();
-        let mut registry = get_registry(&config, &package).unwrap();
-
-        let resolve_cargo_result =
-            resolve(&args, package.package_id(), &mut registry, &workspace);
-
-        assert!(resolve_cargo_result.is_ok());
-    }
 }

cargo-geiger/src/format.rs

@@ -6,7 +6,7 @@ pub mod table;
 mod display;
 mod parse;
 
-use cargo::core::dependency::DepKind;
+use cargo_metadata::DependencyKind;
 use std::fmt;
 use std::str::{self, FromStr};
 use strum_macros::EnumIter;
@@ -79,11 +79,12 @@ impl fmt::Display for FormatError {
     }
 }
 
-pub fn get_kind_group_name(dep_kind: DepKind) -> Option<&'static str> {
+pub fn get_kind_group_name(dep_kind: DependencyKind) -> Option<&'static str> {
     match dep_kind {
-        DepKind::Build => Some("[build-dependencies]"),
-        DepKind::Development => Some("[dev-dependencies]"),
-        DepKind::Normal => None,
+        DependencyKind::Build => Some("[build-dependencies]"),
+        DependencyKind::Development => Some("[dev-dependencies]"),
+        DependencyKind::Normal => None,
+        _ => panic!("Unrecognised Dependency Kind"),
     }
 }
 
@@ -103,15 +104,15 @@ mod format_tests {
     #[rstest]
     fn get_kind_group_name_test() {
         assert_eq!(
-            get_kind_group_name(DepKind::Build),
+            get_kind_group_name(DependencyKind::Build),
             Some("[build-dependencies]")
         );
 
         assert_eq!(
-            get_kind_group_name(DepKind::Development),
+            get_kind_group_name(DependencyKind::Development),
             Some("[dev-dependencies]")
         );
 
-        assert_eq!(get_kind_group_name(DepKind::Normal), None);
+        assert_eq!(get_kind_group_name(DependencyKind::Normal), None);
     }
 }

cargo-geiger/src/format/table/handle_text_tree_line.rs

@@ -8,7 +8,7 @@ use super::total_package_counts::TotalPackageCounts;
 use super::TableParameters;
 use super::{table_row, table_row_empty};
 
-use cargo::core::dependency::DepKind;
+use cargo_metadata::DependencyKind;
 use std::collections::HashSet;
 
 pub struct HandlePackageParameters<'a> {
@@ -18,7 +18,7 @@ pub struct HandlePackageParameters<'a> {
 }
 
 pub fn handle_text_tree_line_extra_deps_group(
-    dep_kind: DepKind,
+    dep_kind: DependencyKind,
     table_lines: &mut Vec<String>,
     tree_vines: String,
 ) {
@@ -162,12 +162,18 @@ mod handle_text_tree_line_tests {
     #[rstest(
         input_dep_kind,
         expected_kind_group_name,
-        case(DepKind::Build, Some(String::from("[build-dependencies]"))),
-        case(DepKind::Development, Some(String::from("[dev-dependencies]"))),
-        case(DepKind::Normal, None)
+        case(
+            DependencyKind::Build,
+            Some(String::from("[build-dependencies]"))
+        ),
+        case(
+            DependencyKind::Development,
+            Some(String::from("[dev-dependencies]"))
+        ),
+        case(DependencyKind::Normal, None)
     )]
     fn handle_text_tree_line_extra_deps_group_test(
-        input_dep_kind: DepKind,
+        input_dep_kind: DependencyKind,
         expected_kind_group_name: Option<String>,
     ) {
         let mut table_lines = Vec::<String>::new();

cargo-geiger/src/graph.rs

@@ -61,7 +61,7 @@ pub fn build_graph<'a>(
         &config_host,
         &args.deps_args,
         &args.target_args,
-    )?;
+    );
     let cfgs = get_cfgs(config, &args.target_args.target, &workspace)?;
 
     let mut graph = Graph {
@@ -88,7 +88,7 @@ pub fn build_graph<'a>(
             &graph_configuration,
             &mut graph,
             &mut pending_packages,
-        )?;
+        );
     }
 
     Ok(graph)
@@ -126,7 +126,7 @@ fn add_package_dependencies_to_graph(
     graph_configuration: &GraphConfiguration,
     graph: &mut Graph,
     pending_packages: &mut Vec<cargo_metadata::PackageId>,
-) -> CargoResult<()> {
+) {
     let index = graph.nodes[&package_id];
     let package = cargo_metadata_parameters
         .krates
@@ -177,15 +177,13 @@ fn add_package_dependencies_to_graph(
             );
         }
     }
-
-    Ok(())
 }
 
 fn build_graph_prerequisites<'a>(
     config_host: &'a InternedString,
     deps_args: &'a DepsArgs,
     target_args: &'a TargetArgs,
-) -> CargoResult<(ExtraDeps, Option<&'a str>)> {
+) -> (ExtraDeps, Option<&'a str>) {
     let extra_deps = if deps_args.all_deps {
         ExtraDeps::All
     } else if deps_args.build_deps {
@@ -202,7 +200,7 @@ fn build_graph_prerequisites<'a>(
         Some(target_args.target.as_deref().unwrap_or(&config_host))
     };
 
-    Ok((extra_deps, target))
+    (extra_deps, target)
 }
 
 #[cfg(test)]
@@ -279,14 +277,12 @@ mod graph_tests {
         let config_host = InternedString::new("config_host");
         let target_args = TargetArgs::default();
 
-        let result = build_graph_prerequisites(
+        let (extra_deps, _) = build_graph_prerequisites(
             &config_host,
             &input_deps_args,
             &target_args,
         );
 
-        assert!(result.is_ok());
-        let (extra_deps, _) = result.unwrap();
         assert_eq!(extra_deps, expected_extra_deps);
     }
 
@@ -321,14 +317,12 @@ mod graph_tests {
         let config_host = InternedString::new("default_config_host");
         let deps_args = DepsArgs::default();
 
-        let result = build_graph_prerequisites(
+        let (_, target) = build_graph_prerequisites(
             &config_host,
             &deps_args,
             &input_target_args,
         );
 
-        assert!(result.is_ok());
-        let (_, target) = result.unwrap();
         assert_eq!(target, expected_target);
     }
 }

cargo-geiger/src/main.rs

@@ -19,15 +19,14 @@ mod scan;
 mod tree;
 
 use crate::args::{Args, HELP};
-use crate::cli::{
-    get_cargo_metadata, get_krates, get_registry, get_workspace, resolve,
-};
+use crate::cli::{get_cargo_metadata, get_krates, get_workspace};
 use crate::graph::build_graph;
 use crate::mapping::{CargoMetadataParameters, QueryResolve};
 use crate::scan::scan;
 
 use cargo::core::shell::Shell;
-use cargo::{CliResult, Config};
+use cargo::util::important_paths;
+use cargo::{CliError, CliResult, Config};
 
 const VERSION: Option<&'static str> = option_env!("CARGO_PKG_VERSION");
 
@@ -52,21 +51,22 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
     };
 
     let workspace = get_workspace(config, args.manifest_path.clone())?;
-    let root_package = workspace.current()?;
-    let mut registry = get_registry(config, &root_package)?;
 
-    let cargo_metadata_root_package_id =
-        cargo_metadata.root_package().unwrap().id.clone();
+    let cargo_metadata_root_package_id;
 
-    let (package_set, _) = resolve(
-        &args.features_args,
-        root_package.package_id(),
-        &mut registry,
-        &workspace,
-    )?;
+    if let Some(cargo_metadata_root_package) = cargo_metadata.root_package() {
+        cargo_metadata_root_package_id = cargo_metadata_root_package.id.clone();
+    } else {
+        eprintln!(
+            "manifest path `{}` is a virtual manifest, but this command requires running against an actual package in this workspace",
+            match args.manifest_path.clone() {
+                Some(path) => path,
+                None => important_paths::find_root_manifest_for_wd(config.cwd())?,
+            }.as_os_str().to_str().unwrap()
+        );
 
-    let package_ids = package_set.package_ids().collect::<Vec<_>>();
-    let package_set = registry.get(&package_ids)?;
+        return CliResult::Err(CliError::code(1));
+    }
 
     let graph = build_graph(
         args,
@@ -90,7 +90,6 @@ fn real_main(args: &Args, config: &mut Config) -> CliResult {
         &cargo_metadata_parameters,
         config,
         &graph,
-        &package_set,
         query_resolve_root_package_id,
         &workspace,
     )

cargo-geiger/src/mapping.rs

@@ -72,6 +72,12 @@ pub trait ToCargoCoreDepKind {
     fn to_cargo_core_dep_kind(&self) -> DepKind;
 }
 
+pub trait ToCargoGeigerDependencyKind {
+    fn to_cargo_geiger_dependency_kind(
+        &self,
+    ) -> cargo_geiger_serde::DependencyKind;
+}
+
 pub trait ToCargoGeigerPackageId {
     fn to_cargo_geiger_package_id(
         &self,

cargo-geiger/src/mapping/core.rs

@@ -60,11 +60,11 @@ impl ToCargoMetadataPackageId for PackageId {
 mod core_tests {
     use super::*;
 
-    use crate::cli::{get_registry, get_workspace};
+    use crate::cli::get_workspace;
 
     use cargo::core::registry::PackageRegistry;
     use cargo::core::Workspace;
-    use cargo::Config;
+    use cargo::{CargoResult, Config};
     use cargo_metadata::{CargoOpt, Metadata, MetadataCommand};
     use krates::Builder as KratesBuilder;
     use krates::Krates;
@@ -148,4 +148,13 @@ mod core_tests {
 
         (package, registry, workspace)
     }
+
+    fn get_registry<'a>(
+        config: &'a Config,
+        package: &Package,
+    ) -> CargoResult<PackageRegistry<'a>> {
+        let mut registry = PackageRegistry::new(config)?;
+        registry.add_sources(Some(package.package_id().source_id()))?;
+        Ok(registry)
+    }
 }