use the manifest path instead of just the member name when using `pre…

…pare --bin` (#255)

* use the manifest path instead of just the member name when using --bin with prepare

* apply suggestions

* apply clippy suggestions to the new tests

src/skeleton/mod.rs

@@ -6,8 +6,10 @@ use crate::skeleton::target::{Target, TargetKind};
 use crate::OptimisationProfile;
 use anyhow::Context;
 use cargo_manifest::Product;
+use cargo_metadata::Metadata;
 use fs_err as fs;
 use globwalk::GlobWalkerBuilder;
+use pathdiff::diff_paths;
 use serde::{Deserialize, Serialize};
 use std::path::{Path, PathBuf};
 
@@ -49,9 +51,9 @@ impl Skeleton {
 
         // Read relevant files from the filesystem
         let config_file = read::config(&base_path)?;
-        let mut manifests = read::manifests(&base_path, metadata)?;
+        let mut manifests = read::manifests(&base_path, &metadata)?;
         if let Some(member) = member {
-            ignore_all_members_except(&mut manifests, member);
+            ignore_all_members_except(&mut manifests, &metadata, member);
         }
 
         let mut lock_file = read::lockfile(&base_path)?;
@@ -313,18 +315,40 @@ fn extract_cargo_metadata(path: &Path) -> Result<cargo_metadata::Metadata, anyho
 }
 
 /// If the top-level `Cargo.toml` has a `members` field, replace it with
-/// a list consisting of just the specified member.
+/// a list consisting of just the path to the package.
 ///
 /// Also deletes the `default-members` field because it does not play nicely
-/// with a modified `members` field and has no effect on cooking the final receipe.
-fn ignore_all_members_except(manifests: &mut [ParsedManifest], member: String) {
+/// with a modified `members` field and has no effect on cooking the final recipe.
+fn ignore_all_members_except(
+    manifests: &mut [ParsedManifest],
+    metadata: &Metadata,
+    member: String,
+) {
     let workspace_toml = manifests
         .iter_mut()
         .find(|manifest| manifest.relative_path == std::path::PathBuf::from("Cargo.toml"));
 
     if let Some(workspace) = workspace_toml.and_then(|toml| toml.contents.get_mut("workspace")) {
         if let Some(members) = workspace.get_mut("members") {
-            *members = toml::Value::Array(vec![toml::Value::String(member)]);
+            let workspace_root = &metadata.workspace_root;
+            let workspace_packages = metadata.workspace_packages();
+
+            if let Some(pkg) = workspace_packages
+                .into_iter()
+                .find(|pkg| pkg.name == member)
+            {
+                // Make this a relative path to the workspace, and remove the `Cargo.toml` child.
+                let member_cargo_path = diff_paths(pkg.manifest_path.as_os_str(), workspace_root);
+                let member_workspace_path = member_cargo_path
+                    .as_ref()
+                    .and_then(|path| path.parent())
+                    .and_then(|dir| dir.to_str());
+
+                if let Some(member_path) = member_workspace_path {
+                    *members =
+                        toml::Value::Array(vec![toml::Value::String(member_path.to_string())]);
+                }
+            }
         }
         if let Some(workspace) = workspace.as_table_mut() {
             workspace.remove("default-members");

src/skeleton/read.rs

@@ -39,7 +39,7 @@ pub(super) fn config<P: AsRef<Path>>(base_path: &P) -> Result<Option<String>, an
 
 pub(super) fn manifests<P: AsRef<Path>>(
     base_path: &P,
-    metadata: Metadata,
+    metadata: &Metadata,
 ) -> Result<Vec<ParsedManifest>, anyhow::Error> {
     let mut packages: BTreeMap<PathBuf, BTreeSet<Target>> = metadata
         .workspace_packages()

tests/skeletons.rs

@@ -1517,6 +1517,166 @@ impl CargoWorkspace {
     }
 }
 
+/// See https://github.com/LukeMathWalker/cargo-chef/issues/232.
+#[test]
+pub fn workspace_bin_nonstandard_dirs() {
+    // Arrange
+    let project = CargoWorkspace::new()
+        .manifest(
+            ".",
+            r#"
+[workspace]
+members = [
+    "crates/client/project_a",
+    "crates/client/project_b",
+    "crates/server/*",
+    "vendored/project_e",
+    "project_f",
+]        
+"#,
+        )
+        .bin_package(
+            "crates/client/project_a",
+            r#"
+[package]
+name = "project_a"
+version = "0.1.0"
+edition = "2018"
+
+[dependencies]
+uuid = { version = "=0.8.0", features = ["v4"] }
+"#,
+        )
+        .bin_package(
+            "crates/client/project_b",
+            r#"
+[package]
+name = "project_b"
+version = "0.1.0"
+edition = "2018"
+
+[dependencies]
+uuid = { version = "=0.8.0", features = ["v4"] }
+"#,
+        )
+        .bin_package(
+            "crates/server/project_c",
+            r#"
+[package]
+name = "project_c"
+version = "0.1.0"
+edition = "2018"
+
+[dependencies]
+uuid = { version = "=0.8.0", features = ["v4"] }
+"#,
+        )
+        .bin_package(
+            "crates/server/project_d",
+            r#"
+[package]
+name = "project_d"
+version = "0.1.0"
+edition = "2018"
+
+[dependencies]
+uuid = { version = "=0.8.0", features = ["v4"] }
+"#,
+        )
+        .bin_package(
+            "vendored/project_e",
+            r#"
+[package]
+name = "project_e"
+version = "0.1.0"
+edition = "2018"
+
+[dependencies]
+uuid = { version = "=0.8.0", features = ["v4"] }
+"#,
+        )
+        .bin_package(
+            "project_f",
+            r#"
+[package]
+name = "project_f"
+version = "0.1.0"
+edition = "2018"
+
+[dependencies]
+uuid = { version = "=0.8.0", features = ["v4"] }
+"#,
+        )
+        .build();
+
+    fn manifest_content_dirs(skeleton: &Skeleton) -> Vec<String> {
+        // This is really ugly... sorry.
+        skeleton
+            .manifests
+            .first()
+            .unwrap()
+            .contents
+            .split('=')
+            .last()
+            .unwrap()
+            .replace(['[', ']', '"'], "")
+            .trim()
+            .split(',')
+            .map(|w| w.trim().to_string())
+            .collect()
+    }
+
+    // Act
+    let path = project.path();
+    let all = Skeleton::derive(&path, None).unwrap();
+    assert_eq!(
+        manifest_content_dirs(&all),
+        vec![
+            "crates/client/project_a",
+            "crates/client/project_b",
+            "crates/server/*",
+            "vendored/project_e",
+            "project_f"
+        ]
+    );
+
+    let project_a = Skeleton::derive(&path, Some("project_a".into())).unwrap();
+    assert_eq!(
+        manifest_content_dirs(&project_a),
+        vec!["crates/client/project_a"]
+    );
+
+    let project_b = Skeleton::derive(&path, Some("project_b".into())).unwrap();
+    assert_eq!(
+        manifest_content_dirs(&project_b),
+        vec!["crates/client/project_b"]
+    );
+
+    let project_c = Skeleton::derive(&path, Some("project_c".into())).unwrap();
+    assert_eq!(
+        manifest_content_dirs(&project_c),
+        vec!["crates/server/project_c"]
+    );
+
+    let project_d = Skeleton::derive(&path, Some("project_d".into())).unwrap();
+    assert_eq!(
+        manifest_content_dirs(&project_d),
+        vec!["crates/server/project_d"]
+    );
+
+    let project_e = Skeleton::derive(&path, Some("project_e".into())).unwrap();
+    assert_eq!(
+        manifest_content_dirs(&project_e),
+        vec!["vendored/project_e"]
+    );
+
+    let project_f = Skeleton::derive(&path, Some("project_f".into())).unwrap();
+    assert_eq!(manifest_content_dirs(&project_f), vec!["project_f"]);
+
+    // TODO: If multiple binaries are valid in `cargo chef prepare`, then testing
+    // with multiple binaries is probably a good idea here!
+}
+
 struct BuiltWorkspace {
     directory: TempDir,
 }