Merge #7387

7387: Refactor build script specific data r=edwin0cheng a=edwin0cheng

It refactors for separating logic for build script specific data. 

bors r+

Co-authored-by: Edwin Cheng <[email protected]>

4 files changed, 228 insertions, 196 deletions

diff --git a/crates/project_model/src/build_data.rs b/crates/project_model/src/build_data.rs
new file mode 100644
index 000000000..82fcf23ad
--- /dev/null
+++ b/crates/project_model/src/build_data.rs
@@ -0,0 +1,206 @@
+//! Handles build script specific information
+
+use std::{
+    ffi::OsStr,
+    io::BufReader,
+    path::{Path, PathBuf},
+    process::{Command, Stdio},
+};
+
+use anyhow::Result;
+use cargo_metadata::{BuildScript, Message, Package, PackageId};
+use itertools::Itertools;
+use paths::AbsPathBuf;
+use rustc_hash::FxHashMap;
+use stdx::JodChild;
+
+use crate::{cfg_flag::CfgFlag, CargoConfig};
+
+#[derive(Debug, Clone, Default)]
+pub(crate) struct BuildDataMap {
+    data: FxHashMap<PackageId, BuildData>,
+}
+#[derive(Debug, Clone, Default, PartialEq, Eq)]
+pub struct BuildData {
+    /// List of config flags defined by this package's build script
+    pub cfgs: Vec<CfgFlag>,
+    /// List of cargo-related environment variables with their value
+    ///
+    /// If the package has a build script which defines environment variables,
+    /// they can also be found here.
+    pub envs: Vec<(String, String)>,
+    /// Directory where a build script might place its output
+    pub out_dir: Option<AbsPathBuf>,
+    /// Path to the proc-macro library file if this package exposes proc-macros
+    pub proc_macro_dylib_path: Option<AbsPathBuf>,
+}
+
+impl BuildDataMap {
+    pub(crate) fn new(
+        cargo_toml: &Path,
+        cargo_features: &CargoConfig,
+        packages: &Vec<Package>,
+        progress: &dyn Fn(String),
+    ) -> Result<BuildDataMap> {
+        let mut cmd = Command::new(toolchain::cargo());
+        cmd.args(&["check", "--workspace", "--message-format=json", "--manifest-path"])
+            .arg(cargo_toml);
+
+        // --all-targets includes tests, benches and examples in addition to the
+        // default lib and bins. This is an independent concept from the --targets
+        // flag below.
+        cmd.arg("--all-targets");
+
+        if let Some(target) = &cargo_features.target {
+            cmd.args(&["--target", target]);
+        }
+
+        if cargo_features.all_features {
+            cmd.arg("--all-features");
+        } else {
+            if cargo_features.no_default_features {
+                // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures`
+                // https://github.com/oli-obk/cargo_metadata/issues/79
+                cmd.arg("--no-default-features");
+            }
+            if !cargo_features.features.is_empty() {
+                cmd.arg("--features");
+                cmd.arg(cargo_features.features.join(" "));
+            }
+        }
+
+        cmd.stdout(Stdio::piped()).stderr(Stdio::null()).stdin(Stdio::null());
+
+        let mut child = cmd.spawn().map(JodChild)?;
+        let child_stdout = child.stdout.take().unwrap();
+        let stdout = BufReader::new(child_stdout);
+
+        let mut res = BuildDataMap::default();
+        for message in cargo_metadata::Message::parse_stream(stdout) {
+            if let Ok(message) = message {
+                match message {
+                    Message::BuildScriptExecuted(BuildScript {
+                        package_id,
+                        out_dir,
+                        cfgs,
+                        env,
+                        ..
+                    }) => {
+                        let cfgs = {
+                            let mut acc = Vec::new();
+                            for cfg in cfgs {
+                                match cfg.parse::<CfgFlag>() {
+                                    Ok(it) => acc.push(it),
+                                    Err(err) => {
+                                        anyhow::bail!("invalid cfg from cargo-metadata: {}", err)
+                                    }
+                                };
+                            }
+                            acc
+                        };
+                        let res = res.data.entry(package_id.clone()).or_default();
+                        // cargo_metadata crate returns default (empty) path for
+                        // older cargos, which is not absolute, so work around that.
+                        if out_dir != PathBuf::default() {
+                            let out_dir = AbsPathBuf::assert(out_dir);
+                            res.out_dir = Some(out_dir);
+                            res.cfgs = cfgs;
+                        }
+
+                        res.envs = env;
+                    }
+                    Message::CompilerArtifact(message) => {
+                        progress(format!("metadata {}", message.target.name));
+
+                        if message.target.kind.contains(&"proc-macro".to_string()) {
+                            let package_id = message.package_id;
+                            // Skip rmeta file
+                            if let Some(filename) =
+                                message.filenames.iter().find(|name| is_dylib(name))
+                            {
+                                let filename = AbsPathBuf::assert(filename.clone());
+                                let res = res.data.entry(package_id.clone()).or_default();
+                                res.proc_macro_dylib_path = Some(filename);
+                            }
+                        }
+                    }
+                    Message::CompilerMessage(message) => {
+                        progress(message.target.name.clone());
+                    }
+                    Message::Unknown => (),
+                    Message::BuildFinished(_) => {}
+                    Message::TextLine(_) => {}
+                }
+            }
+        }
+        res.inject_cargo_env(packages);
+        Ok(res)
+    }
+
+    pub(crate) fn with_cargo_env(packages: &Vec<Package>) -> Self {
+        let mut res = Self::default();
+        res.inject_cargo_env(packages);
+        res
+    }
+
+    pub(crate) fn get(&self, id: &PackageId) -> Option<&BuildData> {
+        self.data.get(id)
+    }
+
+    fn inject_cargo_env(&mut self, packages: &Vec<Package>) {
+        for meta_pkg in packages {
+            let resource = self.data.entry(meta_pkg.id.clone()).or_default();
+            inject_cargo_env(meta_pkg, &mut resource.envs);
+
+            if let Some(out_dir) = &resource.out_dir {
+                // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!()
+                if let Some(out_dir) = out_dir.to_str().map(|s| s.to_owned()) {
+                    resource.envs.push(("OUT_DIR".to_string(), out_dir));
+                }
+            }
+        }
+    }
+}
+
+// FIXME: File a better way to know if it is a dylib
+fn is_dylib(path: &Path) -> bool {
+    match path.extension().and_then(OsStr::to_str).map(|it| it.to_string().to_lowercase()) {
+        None => false,
+        Some(ext) => matches!(ext.as_str(), "dll" | "dylib" | "so"),
+    }
+}
+
+/// Recreates the compile-time environment variables that Cargo sets.
+///
+/// Should be synced with <https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates>
+fn inject_cargo_env(package: &cargo_metadata::Package, env: &mut Vec<(String, String)>) {
+    // FIXME: Missing variables:
+    // CARGO, CARGO_PKG_HOMEPAGE, CARGO_CRATE_NAME, CARGO_BIN_NAME, CARGO_BIN_EXE_<name>
+
+    let mut manifest_dir = package.manifest_path.clone();
+    manifest_dir.pop();
+    if let Some(cargo_manifest_dir) = manifest_dir.to_str() {
+        env.push(("CARGO_MANIFEST_DIR".into(), cargo_manifest_dir.into()));
+    }
+
+    env.push(("CARGO_PKG_VERSION".into(), package.version.to_string()));
+    env.push(("CARGO_PKG_VERSION_MAJOR".into(), package.version.major.to_string()));
+    env.push(("CARGO_PKG_VERSION_MINOR".into(), package.version.minor.to_string()));
+    env.push(("CARGO_PKG_VERSION_PATCH".into(), package.version.patch.to_string()));
+
+    let pre = package.version.pre.iter().map(|id| id.to_string()).format(".");
+    env.push(("CARGO_PKG_VERSION_PRE".into(), pre.to_string()));
+
+    let authors = package.authors.join(";");
+    env.push(("CARGO_PKG_AUTHORS".into(), authors));
+
+    env.push(("CARGO_PKG_NAME".into(), package.name.clone()));
+    env.push(("CARGO_PKG_DESCRIPTION".into(), package.description.clone().unwrap_or_default()));
+    //env.push(("CARGO_PKG_HOMEPAGE".into(), package.homepage.clone().unwrap_or_default()));
+    env.push(("CARGO_PKG_REPOSITORY".into(), package.repository.clone().unwrap_or_default()));
+    env.push(("CARGO_PKG_LICENSE".into(), package.license.clone().unwrap_or_default()));
+
+    let license_file =
+        package.license_file.as_ref().map(|buf| buf.display().to_string()).unwrap_or_default();
+    env.push(("CARGO_PKG_LICENSE_FILE".into(), license_file));
+}
diff --git a/crates/project_model/src/cargo_workspace.rs b/crates/project_model/src/cargo_workspace.rs
index a09e1a9ff..c8a5333c4 100644
--- a/crates/project_model/src/cargo_workspace.rs
+++ b/crates/project_model/src/cargo_workspace.rs
@@ -1,24 +1,15 @@
 //! FIXME: write short doc here
 
-use std::{
-    convert::TryInto,
-    ffi::OsStr,
-    io::BufReader,
-    ops,
-    path::{Path, PathBuf},
-    process::{Command, Stdio},
-};
+use std::{convert::TryInto, ops, process::Command};
 
 use anyhow::{Context, Result};
 use base_db::Edition;
-use cargo_metadata::{BuildScript, CargoOpt, Message, MetadataCommand, PackageId};
-use itertools::Itertools;
+use cargo_metadata::{CargoOpt, MetadataCommand};
 use la_arena::{Arena, Idx};
 use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::FxHashMap;
-use stdx::JodChild;
 
-use crate::cfg_flag::CfgFlag;
+use crate::build_data::{BuildData, BuildDataMap};
 use crate::utf8_stdout;
 
 /// `CargoWorkspace` represents the logical structure of, well, a Cargo
@@ -103,17 +94,8 @@ pub struct PackageData {
     pub features: FxHashMap<String, Vec<String>>,
     /// List of features enabled on this package
     pub active_features: Vec<String>,
-    /// List of config flags defined by this package's build script
-    pub cfgs: Vec<CfgFlag>,
-    /// List of cargo-related environment variables with their value
-    ///
-    /// If the package has a build script which defines environment variables,
-    /// they can also be found here.
-    pub envs: Vec<(String, String)>,
-    /// Directory where a build script might place its output
-    pub out_dir: Option<AbsPathBuf>,
-    /// Path to the proc-macro library file if this package exposes proc-macros
-    pub proc_macro_dylib_path: Option<AbsPathBuf>,
+    /// Build script related data for this package
+    pub build_data: BuildData,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -246,17 +228,11 @@ impl CargoWorkspace {
             )
         })?;
 
-        let mut out_dir_by_id = FxHashMap::default();
-        let mut cfgs = FxHashMap::default();
-        let mut envs = FxHashMap::default();
-        let mut proc_macro_dylib_paths = FxHashMap::default();
-        if config.load_out_dirs_from_check {
-            let resources = load_extern_resources(cargo_toml, config, progress)?;
-            out_dir_by_id = resources.out_dirs;
-            cfgs = resources.cfgs;
-            envs = resources.env;
-            proc_macro_dylib_paths = resources.proc_dylib_paths;
-        }
+        let resources = if config.load_out_dirs_from_check {
+            BuildDataMap::new(cargo_toml, config, &meta.packages, progress)?
+        } else {
+            BuildDataMap::with_cargo_env(&meta.packages)
+        };
 
         let mut pkg_by_id = FxHashMap::default();
         let mut packages = Arena::default();
@@ -267,7 +243,7 @@ impl CargoWorkspace {
         meta.packages.sort_by(|a, b| a.id.cmp(&b.id));
         for meta_pkg in meta.packages {
             let id = meta_pkg.id.clone();
-            inject_cargo_env(&meta_pkg, envs.entry(id).or_default());
+            let build_data = resources.get(&id).cloned().unwrap_or_default();
 
             let cargo_metadata::Package { id, edition, name, manifest_path, version, .. } =
                 meta_pkg;
@@ -285,10 +261,7 @@ impl CargoWorkspace {
                 dependencies: Vec::new(),
                 features: meta_pkg.features.into_iter().collect(),
                 active_features: Vec::new(),
-                cfgs: cfgs.get(&id).cloned().unwrap_or_default(),
-                envs: envs.get(&id).cloned().unwrap_or_default(),
-                out_dir: out_dir_by_id.get(&id).cloned(),
-                proc_macro_dylib_path: proc_macro_dylib_paths.get(&id).cloned(),
+                build_data,
             });
             let pkg_data = &mut packages[pkg];
             pkg_by_id.insert(id, pkg);
@@ -365,149 +338,3 @@ impl CargoWorkspace {
         self.packages.iter().filter(|(_, v)| v.name == name).count() == 1
     }
 }
-
-#[derive(Debug, Clone, Default)]
-pub(crate) struct ExternResources {
-    out_dirs: FxHashMap<PackageId, AbsPathBuf>,
-    proc_dylib_paths: FxHashMap<PackageId, AbsPathBuf>,
-    cfgs: FxHashMap<PackageId, Vec<CfgFlag>>,
-    env: FxHashMap<PackageId, Vec<(String, String)>>,
-}
-
-pub(crate) fn load_extern_resources(
-    cargo_toml: &Path,
-    cargo_features: &CargoConfig,
-    progress: &dyn Fn(String),
-) -> Result<ExternResources> {
-    let mut cmd = Command::new(toolchain::cargo());
-    cmd.args(&["check", "--workspace", "--message-format=json", "--manifest-path"]).arg(cargo_toml);
-
-    // --all-targets includes tests, benches and examples in addition to the
-    // default lib and bins. This is an independent concept from the --targets
-    // flag below.
-    cmd.arg("--all-targets");
-
-    if let Some(target) = &cargo_features.target {
-        cmd.args(&["--target", target]);
-    }
-
-    if cargo_features.all_features {
-        cmd.arg("--all-features");
-    } else {
-        if cargo_features.no_default_features {
-            // FIXME: `NoDefaultFeatures` is mutual exclusive with `SomeFeatures`
-            // https://github.com/oli-obk/cargo_metadata/issues/79
-            cmd.arg("--no-default-features");
-        }
-        if !cargo_features.features.is_empty() {
-            cmd.arg("--features");
-            cmd.arg(cargo_features.features.join(" "));
-        }
-    }
-
-    cmd.stdout(Stdio::piped()).stderr(Stdio::null()).stdin(Stdio::null());
-
-    let mut child = cmd.spawn().map(JodChild)?;
-    let child_stdout = child.stdout.take().unwrap();
-    let stdout = BufReader::new(child_stdout);
-
-    let mut res = ExternResources::default();
-    for message in cargo_metadata::Message::parse_stream(stdout) {
-        if let Ok(message) = message {
-            match message {
-                Message::BuildScriptExecuted(BuildScript {
-                    package_id,
-                    out_dir,
-                    cfgs,
-                    env,
-                    ..
-                }) => {
-                    let cfgs = {
-                        let mut acc = Vec::new();
-                        for cfg in cfgs {
-                            match cfg.parse::<CfgFlag>() {
-                                Ok(it) => acc.push(it),
-                                Err(err) => {
-                                    anyhow::bail!("invalid cfg from cargo-metadata: {}", err)
-                                }
-                            };
-                        }
-                        acc
-                    };
-                    // cargo_metadata crate returns default (empty) path for
-                    // older cargos, which is not absolute, so work around that.
-                    if out_dir != PathBuf::default() {
-                        let out_dir = AbsPathBuf::assert(out_dir);
-                        res.out_dirs.insert(package_id.clone(), out_dir);
-                        res.cfgs.insert(package_id.clone(), cfgs);
-                    }
-
-                    res.env.insert(package_id, env);
-                }
-                Message::CompilerArtifact(message) => {
-                    progress(format!("metadata {}", message.target.name));
-
-                    if message.target.kind.contains(&"proc-macro".to_string()) {
-                        let package_id = message.package_id;
-                        // Skip rmeta file
-                        if let Some(filename) = message.filenames.iter().find(|name| is_dylib(name))
-                        {
-                            let filename = AbsPathBuf::assert(filename.clone());
-                            res.proc_dylib_paths.insert(package_id, filename);
-                        }
-                    }
-                }
-                Message::CompilerMessage(message) => {
-                    progress(message.target.name.clone());
-                }
-                Message::Unknown => (),
-                Message::BuildFinished(_) => {}
-                Message::TextLine(_) => {}
-            }
-        }
-    }
-    Ok(res)
-}
-
-// FIXME: File a better way to know if it is a dylib
-fn is_dylib(path: &Path) -> bool {
-    match path.extension().and_then(OsStr::to_str).map(|it| it.to_string().to_lowercase()) {
-        None => false,
-        Some(ext) => matches!(ext.as_str(), "dll" | "dylib" | "so"),
-    }
-}
-
-/// Recreates the compile-time environment variables that Cargo sets.
-///
-/// Should be synced with <https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates>
-fn inject_cargo_env(package: &cargo_metadata::Package, env: &mut Vec<(String, String)>) {
-    // FIXME: Missing variables:
-    // CARGO, CARGO_PKG_HOMEPAGE, CARGO_CRATE_NAME, CARGO_BIN_NAME, CARGO_BIN_EXE_<name>
-
-    let mut manifest_dir = package.manifest_path.clone();
-    manifest_dir.pop();
-    if let Some(cargo_manifest_dir) = manifest_dir.to_str() {
-        env.push(("CARGO_MANIFEST_DIR".into(), cargo_manifest_dir.into()));
-    }
-
-    env.push(("CARGO_PKG_VERSION".into(), package.version.to_string()));
-    env.push(("CARGO_PKG_VERSION_MAJOR".into(), package.version.major.to_string()));
-    env.push(("CARGO_PKG_VERSION_MINOR".into(), package.version.minor.to_string()));
-    env.push(("CARGO_PKG_VERSION_PATCH".into(), package.version.patch.to_string()));
-
-    let pre = package.version.pre.iter().map(|id| id.to_string()).format(".");
-    env.push(("CARGO_PKG_VERSION_PRE".into(), pre.to_string()));
-
-    let authors = package.authors.join(";");
-    env.push(("CARGO_PKG_AUTHORS".into(), authors));
-
-    env.push(("CARGO_PKG_NAME".into(), package.name.clone()));
-    env.push(("CARGO_PKG_DESCRIPTION".into(), package.description.clone().unwrap_or_default()));
-    //env.push(("CARGO_PKG_HOMEPAGE".into(), package.homepage.clone().unwrap_or_default()));
-    env.push(("CARGO_PKG_REPOSITORY".into(), package.repository.clone().unwrap_or_default()));
-    env.push(("CARGO_PKG_LICENSE".into(), package.license.clone().unwrap_or_default()));
-
-    let license_file =
-        package.license_file.as_ref().map(|buf| buf.display().to_string()).unwrap_or_default();
-    env.push(("CARGO_PKG_LICENSE_FILE".into(), license_file));
-}
diff --git a/crates/project_model/src/lib.rs b/crates/project_model/src/lib.rs
index 970a7e140..525c336e6 100644
--- a/crates/project_model/src/lib.rs
+++ b/crates/project_model/src/lib.rs
@@ -6,6 +6,7 @@ mod project_json;
 mod sysroot;
 mod workspace;
 mod rustc_cfg;
+mod build_data;
 
 use std::{
     fs::{read_dir, ReadDir},
diff --git a/crates/project_model/src/workspace.rs b/crates/project_model/src/workspace.rs
index 073c48af7..bc5041e5a 100644
--- a/crates/project_model/src/workspace.rs
+++ b/crates/project_model/src/workspace.rs
@@ -178,7 +178,7 @@ impl ProjectWorkspace {
                     let pkg_root = cargo[pkg].root().to_path_buf();
 
                     let mut include = vec![pkg_root.clone()];
-                    include.extend(cargo[pkg].out_dir.clone());
+                    include.extend(cargo[pkg].build_data.out_dir.clone());
 
                     let mut exclude = vec![pkg_root.join(".git")];
                     if is_member {
@@ -484,23 +484,21 @@ fn add_target_crate_root(
         for feature in pkg.active_features.iter() {
             opts.insert_key_value("feature".into(), feature.into());
         }
-        opts.extend(pkg.cfgs.iter().cloned());
+        opts.extend(pkg.build_data.cfgs.iter().cloned());
         opts
     };
 
     let mut env = Env::default();
-    for (k, v) in &pkg.envs {
+    for (k, v) in &pkg.build_data.envs {
         env.set(k, v.clone());
     }
-    if let Some(out_dir) = &pkg.out_dir {
-        // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!()
-        if let Some(out_dir) = out_dir.to_str().map(|s| s.to_owned()) {
-            env.set("OUT_DIR", out_dir);
-        }
-    }
 
-    let proc_macro =
-        pkg.proc_macro_dylib_path.as_ref().map(|it| proc_macro_loader(&it)).unwrap_or_default();
+    let proc_macro = pkg
+        .build_data
+        .proc_macro_dylib_path
+        .as_ref()
+        .map(|it| proc_macro_loader(&it))
+        .unwrap_or_default();
 
     let display_name = CrateDisplayName::from_canonical_name(pkg.name.clone());
     let crate_id = crate_graph.add_crate_root(