Repository: mozilla/cbindgen (language: rust)

Issue:
Only use metadata for current target
Unlike most other cargo tools, `cargo metadata` counter-intuitively defaults to downloading all dependencies for _all_ targets unless the argument `--filter-platform` is passed. This means that with the current logic, `cbindgen` ends up downloading potentially a very large number of dependencies whenever this is invoked:
https://github.com/eqrion/cbindgen/blob/93eed3aaf3a6ed94ae5bdf59e5df4c508fe05a5b/src/bindgen/cargo/cargo_metadata.rs#L189-L194

Since `cbindgen` doesn't actually need the dependencies themselves (I don't think), I believe `cbindgen` should probably _always_ specify `--filter-platform`, and just specify the host platform. Cargo finds the host platform by running `rustc -Vv` and extracing the `host:` line, which `cbindgen` could probably also do pretty easily:
https://github.com/rust-lang/cargo/blob/fde8ee3ba0cba4eef75f197d42586ef4664f057b/src/cargo/util/rustc.rs#L48-L68

This is also what `rust-analyzer` recently switch to doing: https://github.com/rust-analyzer/rust-analyzer/pull/6912

There's also precedent for relying on `rustc` to determine these kinds of things, such as using it to find the sysroot:
https://github.com/rust-analyzer/rust-analyzer/blob/48802e54d1669c68fd27b007533dd6aa5221d089/crates/project_model/src/sysroot.rs#L126-L127

One way to make this nicer would be if `cargo metadata` supported a "special" value for `--filter-platform` called `host`. I'll look at submitting that as a PR upstream, but the more immediate fix (and also one that would continue to work for older compiler releases) would be to just use `rustc -Vv`.


Discussion / hints:
Note also that even if `--filter-platform` is set
> the dependencies listed in the "packages" array still includes all dependencies. Each package definition is intended to be an unaltered reproduction of the information within `Cargo.toml.

(from `cargo help metadata`)
Seems sensible :)

Fix patch:
diff --git a/src/bindgen/builder.rs b/src/bindgen/builder.rs
index 4ca7e8532..edab0318d 100644
--- a/src/bindgen/builder.rs
+++ b/src/bindgen/builder.rs
@@ -240,6 +240,12 @@ impl Builder {
         self
     }
 
+    #[allow(unused)]
+    pub fn with_only_target_dependencies(mut self, only_target_dependencies: bool) -> Builder {
+        self.config.only_target_dependencies = only_target_dependencies;
+        self
+    }
+
     #[allow(unused)]
     pub fn with_documentation(mut self, documentation: bool) -> Builder {
         self.config.documentation = documentation;
@@ -341,6 +347,7 @@ impl Builder {
                 binding_lib_name.as_deref(),
                 self.config.parse.parse_deps,
                 self.config.parse.clean,
+                self.config.only_target_dependencies,
                 /* existing_metadata = */ None,
             )?;
 
diff --git a/src/bindgen/cargo/cargo.rs b/src/bindgen/cargo/cargo.rs
index 885578c5f..09f0570c2 100644
--- a/src/bindgen/cargo/cargo.rs
+++ b/src/bindgen/cargo/cargo.rs
@@ -40,11 +40,13 @@ impl Cargo {
         binding_crate_name: Option<&str>,
         use_cargo_lock: bool,
         clean: bool,
+        only_target_dependencies: bool,
         existing_metadata_file: Option<&Path>,
     ) -> Result<Cargo, Error> {
         let toml_path = crate_dir.join("Cargo.toml");
-        let metadata = cargo_metadata::metadata(&toml_path, existing_metadata_file)
-            .map_err(|x| Error::CargoMetadata(toml_path.to_str().unwrap().to_owned(), x))?;
+        let metadata =
+            cargo_metadata::metadata(&toml_path, existing_metadata_file, only_target_dependencies)
+                .map_err(|x| Error::CargoMetadata(toml_path.to_str().unwrap().to_owned(), x))?;
         let lock_path = lock_file
             .map(PathBuf::from)
             .unwrap_or_else(|| Path::new(&metadata.workspace_root).join("Cargo.lock"));
diff --git a/src/bindgen/cargo/cargo_metadata.rs b/src/bindgen/cargo/cargo_metadata.rs
index 0b977d4cb..25c9fe40e 100644
--- a/src/bindgen/cargo/cargo_metadata.rs
+++ b/src/bindgen/cargo/cargo_metadata.rs
@@ -180,16 +180,64 @@ impl Eq for Dependency {}
 pub fn metadata(
     manifest_path: &Path,
     existing_metadata_file: Option<&Path>,
+    only_target: bool,
 ) -> Result<Metadata, Error> {
     let output;
     let metadata = match existing_metadata_file {
         Some(path) => Cow::Owned(std::fs::read_to_string(path)?),
         None => {
+            let mut target = None;
+
+            if only_target {
+                target = std::env::var_os("TARGET");
+            }
+
+            if only_target && target.is_none() {
+                // We must be running as a standalone script, not under cargo.
+                // Let's use the host platform instead.
+                // We figure out the host platform through rustc and use that.
+                // We unfortunatelly cannot go through cargo, since cargo rustc _also_ builds.
+                // If `rustc` fails to run, we just fall back to not passing --filter-platforms.
+                //
+                // NOTE: We set the current directory in case of rustup shenanigans.
+                let rustc = env::var("RUSTC").unwrap_or_else(|_| String::from("rustc"));
+                let rustc = Command::new(rustc)
+                    .current_dir(manifest_path.parent().unwrap())
+                    .arg("-vV")
+                    .output();
+                log::debug!("Discovering host platform by {:?}", rustc);
+                if let Ok(out) = rustc {
+                    if let Ok(stdout) = std::str::from_utf8(&out.stdout) {
+                        let field = "host: ";
+                        let value = stdout.lines().find_map(|l| {
+                            // XXX l.strip_prefix(field) re-implemented to preserve MSRV
+                            if l.starts_with(field) {
+                                Some(&l[field.len()..])
+                            } else {
+                                None
+                            }
+                        });
+                        if let Some(value) = value {
+                            target = Some(value.into());
+                        }
+                    }
+                }
+                if target.is_none() {
+                    log::warn!(
+                        "Failed to discover host platform for cargo metadata; \
+                        will fetch dependencies for all platforms."
+                    );
+                }
+            }
+
             let cargo = env::var("CARGO").unwrap_or_else(|_| String::from("cargo"));
             let mut cmd = Command::new(cargo);
             cmd.arg("metadata");
             cmd.arg("--all-features");
             cmd.arg("--format-version").arg("1");
+            if let Some(target) = target {
+                cmd.arg("--filter-platform").arg(target);
+            }
             cmd.arg("--manifest-path");
             cmd.arg(manifest_path);
             output = cmd.output()?;
diff --git a/src/bindgen/config.rs b/src/bindgen/config.rs
index 0aa3cfdd1..ab242321a 100644
--- a/src/bindgen/config.rs
+++ b/src/bindgen/config.rs
@@ -938,6 +938,37 @@ pub struct Config {
     /// Configuration options for pointers
     #[serde(rename = "ptr")]
     pub pointer: PtrConfig,
+    /// Only download sources for dependencies needed for the target platform.
+    ///
+    /// By default, cbindgen will fetch sources for dependencies used on any platform so that if a
+    /// type is defined in terms of a type from a dependency on another target (probably behind a
+    /// `#[cfg]`), cbindgen will be able to generate the appropriate binding as it can see the
+    /// nested type's definition. However, this makes calling cbindgen slower, as it may have to
+    /// download a number of additional dependencies.
+    ///
+    /// As an example, consider this Cargo.toml:
+    ///
+    /// ```toml
+    /// [target.'cfg(windows)'.dependencies]
+    /// windows = "0.7"
+    /// ```
+    ///
+    /// with this declaration in one of the `.rs` files that cbindgen is asked to generate bindings
+    /// for:
+    ///
+    /// ```rust,ignore
+    /// #[cfg(windows)]
+    /// pub struct Error(windows::ErrorCode);
+    /// ```
+    ///
+    /// With the default value (`false`), cbindgen will download the `windows` dependency even when
+    /// not compiling for Windows, and will thus be able to generate the binding for `Error`
+    /// (behind a `#define`).
+    ///
+    /// If this value is instead to `true`, cbindgen will _not_ download the `windows` dependency
+    /// if it's not compiling for Windows, but will also fail to generate a Windows binding for
+    /// `Error` as it does not know the definition for `ErrorCode`.
+    pub only_target_dependencies: bool,
     /// Configuration options specific to Cython.
     pub cython: CythonConfig,
 }
@@ -979,6 +1010,7 @@ impl Default for Config {
             documentation: true,
             documentation_style: DocumentationStyle::Auto,
             pointer: PtrConfig::default(),
+            only_target_dependencies: false,
             cython: CythonConfig::default(),
         }
     }
diff --git a/src/bindgen/parser.rs b/src/bindgen/parser.rs
index ebb393511..c06725a61 100644
--- a/src/bindgen/parser.rs
+++ b/src/bindgen/parser.rs
@@ -154,7 +154,9 @@ impl<'a> Parser<'a> {
                 None => {
                     // This should be an error, but is common enough to just elicit a warning
                     warn!(
-                        "Parsing crate `{}`: can't find lib.rs with `cargo metadata`.",
+                        "Parsing crate `{}`: can't find lib.rs with `cargo metadata`. \
+                        The crate may be available only on a particular platform, \
+                        so consider setting `fetch_all_dependencies` in your cbindgen configuration.",
                         pkg.name
                     );
                 }
diff --git a/src/main.rs b/src/main.rs
index 5c5eebe31..dc5e7cc3c 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -44,6 +44,10 @@ fn apply_config_overrides<'a>(config: &mut Config, matches: &ArgMatches<'a>) {
         config.cpp_compat = true;
     }
 
+    if matches.is_present("only-target-dependencies") {
+        config.only_target_dependencies = true;
+    }
+
     if let Some(style) = matches.value_of("style") {
         config.style = match style {
             "Both" => Style::Both,
@@ -98,6 +102,7 @@ fn load_bindings<'a>(input: &Path, matches: &ArgMatches<'a>) -> Result<Bindings,
         matches.value_of("crate"),
         true,
         matches.is_present("clean"),
+        matches.is_present("only-target-dependencies"),
         matches.value_of("metadata").map(Path::new),
     )?;
 
@@ -159,6 +164,12 @@ fn main() {
                 .long("cpp-compat")
                 .help("Whether to add C++ compatibility to generated C bindings")
         )
+        .arg(
+            Arg::with_name("only-target-dependencies")
+                .long("only-target-dependencies")
+                .help("Only fetch dependencies needed by the target platform. \
+                    The target platform defaults to the host platform; set TARGET to override.")
+        )
         .arg(
             Arg::with_name("style")
                 .short("s")


Test patch: