Generate bindings for extra components

There are two ways the bindings are generated.

The first is when `bindings_module` is not set. In that case the header
of the extra component will just be added to the normal esp-idf bindings
generation. All additional bindings will also be in the root of `esp-idf-sys`.

The second: when `bindings_module` is set. For every unique `bindings_module`
bindgen will be invoked to generate the bindings, which will then be appended as a new
module to the `bindings.rs` file. Note though that since this is a new bindgen invocation
duplicate symbols of the normal esp-idf bindings may be generated.

Author: N3xed

Cargo.toml

@@ -36,7 +36,7 @@ embuild = { version = "0.30", features = ["glob", "kconfig"] }
 anyhow = "1"
 regex = "1.5"
 bindgen = "0.60"
-cargo_metadata = { version = "0.15" }
+cargo_metadata = "0.15"
 serde = { version = "1.0", features = ["derive"] }
 strum = { version = "0.24", features = ["derive"] }
 envy = "0.4.2"

build/build.rs

@@ -1,13 +1,16 @@
 #[cfg(not(any(feature = "pio", feature = "native")))]
 compile_error!("One of the features `pio` or `native` must be selected.");
 
+use std::fs;
+use std::io::{BufWriter, Write};
 use std::iter::once;
 
-use ::bindgen::callbacks::{IntKind, ParseCallbacks};
 use anyhow::*;
+use bindgen::callbacks::{IntKind, ParseCallbacks};
 use common::*;
+use embuild::bindgen::BindgenExt;
 use embuild::utils::OsStrExt;
-use embuild::{bindgen, build, cargo, kconfig, path_buf};
+use embuild::{bindgen as bindgen_utils, build, cargo, kconfig, path_buf};
 
 mod common;
 mod config;
@@ -94,13 +97,12 @@ fn main() -> anyhow::Result<()> {
 
     cargo::track_file(&header_file);
 
-    let bindings_file = bindgen::run(
-        build_output
-            .bindgen
-            .builder()?
+    // Because we have multiple bindgen invocations and we can't clone a bindgen::Builder,
+    // we have to set the options every time.
+    let configure_bindgen = |bindgen: bindgen::Builder| {
+        Ok(bindgen
             .parse_callbacks(Box::new(BindgenCallbacks))
             .ctypes_prefix("c_types")
-            .header(header_file.try_to_str()?)
             .blocklist_function("strtold")
             .blocklist_function("_strtold_r")
             .blocklist_function("v.*printf")
@@ -118,8 +120,62 @@ fn main() -> anyhow::Result<()> {
                     // We don't really have a similar issue with Xtensa, but we pass it explicitly as well just in case
                     "xtensa"
                 },
-            ]),
-    )?;
+            ]))
+    };
+
+    let bindings_file = bindgen_utils::default_bindings_file()?;
+    let bindgen_err = || {
+        anyhow!(
+            "failed to generate bindings in file '{}'",
+            bindings_file.display()
+        )
+    };
+    let mut headers = vec![header_file];
+
+    #[cfg(feature = "native")]
+    // Add additional headers from extra components.
+    headers.extend(
+        build_output
+            .config
+            .native
+            .combined_bindings_headers()?
+            .into_iter()
+            .inspect(|h| cargo::track_file(h)),
+    );
+
+    configure_bindgen(build_output.bindgen.clone().builder()?)?
+        .headers(headers)?
+        .generate()
+        .with_context(bindgen_err)?
+        .write_to_file(&bindings_file)
+        .with_context(bindgen_err)?;
+
+    // Generate bindings separately for each unique module name.
+    #[cfg(feature = "native")]
+    (|| {
+        let mut output_file =
+            BufWriter::new(fs::File::options().append(true).open(&bindings_file)?);
+
+        for (module_name, headers) in build_output.config.native.module_bindings_headers()? {
+            let bindings = configure_bindgen(build_output.bindgen.clone().builder()?)?
+                .headers(headers.into_iter().inspect(|h| cargo::track_file(h)))?
+                .generate()?;
+
+            writeln!(
+                &mut output_file,
+                "pub mod {} {{\
+                     use crate::c_types;\
+                     {}\
+                 }}",
+                module_name, bindings
+            )?;
+        }
+        Ok(())
+    })()
+    .with_context(bindgen_err)?;
+
+    // Cargo fmt generated bindings.
+    bindgen_utils::cargo_fmt_file(&bindings_file);
 
     let cfg_args = build::CfgArgs {
         args: cfg_args
@@ -130,7 +186,6 @@ fn main() -> anyhow::Result<()> {
             .chain(once(mcu))
             .collect(),
     };
-
     cfg_args.propagate();
     cfg_args.output();
 
@@ -139,13 +194,14 @@ fn main() -> anyhow::Result<()> {
 
     // In case other crates need to have access to the ESP-IDF toolchains
     if let Some(env_path) = build_output.env_path {
-        // TODO: Replace with embuild::build::VAR_ENV_PATH once we have a new embuild release
-        cargo::set_metadata("EMBUILD_ENV_PATH", env_path);
+        cargo::set_metadata(embuild::build::ENV_PATH_VAR, env_path);
     }
 
     // In case other crates need to the ESP-IDF SDK
-    // TODO: Replace with embuild::espidf::XXX paths once we have a new embuild release
-    cargo::set_metadata("EMBUILD_ESP_IDF_PATH", build_output.esp_idf.try_to_str()?);
+    cargo::set_metadata(
+        embuild::build::ESP_IDF_PATH_VAR,
+        build_output.esp_idf.try_to_str()?,
+    );
 
     build_output.cincl_args.propagate();
 

build/common.rs

@@ -11,6 +11,8 @@ use embuild::utils::{OsStrExt, PathExt};
 use embuild::{bindgen, build, kconfig};
 use strum::{Display, EnumDiscriminants, EnumString};
 
+use crate::config::BuildConfig;
+
 #[allow(dead_code)]
 pub const V_4_3_2_PATCHES: &[&str] = &[
     "patches/missing_riscv_atomics_fix.diff",
@@ -28,6 +30,7 @@ pub struct EspIdfBuildOutput {
     pub bindgen: bindgen::Factory,
     pub env_path: Option<String>,
     pub esp_idf: PathBuf,
+    pub config: BuildConfig,
 }
 
 pub struct EspIdfComponents(Vec<String>);

build/native/cargo_driver.rs

@@ -392,6 +392,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
         ),
         env_path: Some(idf.exported_path.try_to_str()?.to_owned()),
         esp_idf: build_info.esp_idf_dir,
+        config,
     };
 
     Ok(build_output)

build/native/cargo_driver/config.rs

@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::path::PathBuf;
 
 use anyhow::{anyhow, bail, Context, Result};
@@ -110,6 +111,63 @@ impl NativeConfig {
             })
     }
 
+    /// Get all bindings C headers of extra components where the bindings will be
+    /// generated combined with the normal `esp-idf` bindings
+    /// (all extra components where [`ExtraComponent::bindings_module`] is [`None`]).
+    ///
+    /// This method will validate that all returned C header files exist.
+    pub fn combined_bindings_headers(&self) -> Result<Vec<PathBuf>> {
+        let mut results = Vec::new();
+        for comp in &self.extra_components {
+            // Skip all extra components with separate bindings.
+            if comp.bindings_module.is_some() {
+                continue;
+            }
+
+            if let Some(header) = &comp.bindings_header {
+                let header_path = header.abspath_relative_to(&comp.manifest_dir);
+
+                if !header_path.exists() {
+                    bail!(
+                        "extra components C header file '{}' specified by crate '{}' does not exists",
+                        header_path.display(), comp.manifest_dir.display()
+                    );
+                }
+                results.push(header_path);
+            }
+        }
+        Ok(results)
+    }
+
+    /// Get all bindings C headers grouped by the [`ExtraComponent::bindings_module`] name.
+    ///
+    /// This method will validate that all returned C header files exist and also that the
+    /// module name only contains ACII alphanumeric and `_` characters.
+    pub fn module_bindings_headers(&self) -> Result<HashMap<&str, Vec<PathBuf>>> {
+        let headers = self.extra_components.iter().filter_map(|comp| {
+            match (&comp.bindings_header, &comp.bindings_module) {
+                (Some(header), Some(module)) => {
+                    Some((header.abspath_relative_to(&comp.manifest_dir), module, comp))
+                }
+                _ => None,
+            }
+        });
+        let mut map = HashMap::<&str, Vec<PathBuf>>::new();
+
+        for (header_path, module_name, comp) in headers {
+            if !header_path.exists() {
+                bail!(
+                    "extra components C header file '{}' specified by crate '{}' does not exists",
+                    header_path.display(),
+                    comp.manifest_dir.display()
+                );
+            }
+            validate_module_name(module_name, comp)?;
+            map.entry(&*module_name).or_default().push(header_path);
+        }
+        Ok(map)
+    }
+
     pub fn with_cargo_metadata(&mut self, root: &Package, metadata: &Metadata) -> Result<()> {
         let EspIdfSys {
             v:
@@ -228,27 +286,28 @@ pub struct ExtraComponent {
     /// Otherwise, if absent, the component bindings will be added to the existing
     /// `esp-idf` bindings (which are available in the crate root).
     ///
-    /// To put the bindings into its own module, a separate bindgen instanace will generate
-    /// the bindings. Note that this will result in duplicate `esp-idf` bindings if the same
-    /// `esp-idf` headers are included by the component(s) that were already processed for
-    /// the `esp-idf` bindings.
+    /// To put the bindings into its own module, a separate bindgen instance will generate
+    /// the bindings. Note that this will result in duplicate `esp-idf` bindings if the
+    /// same `esp-idf` headers that were already processed for the `esp-idf` bindings are
+    /// included by the component(s).
     ///
     /// Optional
     #[serde(default)]
     pub bindings_module: Option<String>,
 
+    /// Internal field; the path of the directory containing the manifest (`Cargo.toml`)
+    /// that defined this [`ExtraComponent`].
     #[serde(skip)]
     pub manifest_dir: PathBuf,
 }
 
 mod parse {
     use std::str::FromStr;
 
-    use embuild::{cmake, git};
-    use serde::{Deserialize, Deserializer};
+    use serde::Deserializer;
     use strum::IntoEnumIterator;
 
-    use super::DEFAULT_CMAKE_GENERATOR;
+    use super::*;
     use crate::config::utils::ValueOrVec;
 
     /// Parse a cmake generator, either `default` or one of [`cmake::Generator`].
@@ -294,3 +353,31 @@ mod parse {
         })
     }
 }
+
+/// A extra component module name can only contain ASCII alphanumeric and `_` characters.
+/// Additionally it must not start with a digit.
+pub fn validate_module_name(module_name: &str, comp: &ExtraComponent) -> Result<()> {
+    if module_name.is_empty() {
+        bail!(
+            "extra component module name '{}' specified by crate '{}' cannot be empty",
+            module_name,
+            comp.manifest_dir.display()
+        );
+    }
+
+    let mut chars = module_name.chars();
+    let first_char = chars.next().unwrap();
+
+    let first_char_valid = first_char.is_ascii_alphabetic() || first_char == '_';
+    let other_valid = chars.all(|c| c.is_ascii_alphanumeric() || c == '_');
+
+    if !first_char_valid || !other_valid {
+        bail!(
+            "extra component module name '{}' specified by crate '{}' can only contain \
+             ASCII alphanumeric or `_` characters and must be a valid Rust module name",
+            module_name,
+            comp.manifest_dir.display()
+        );
+    }
+    Ok(())
+}

build/native/cmake_driver.rs

@@ -58,6 +58,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
             ),
         env_path: None,
         esp_idf: PathBuf::from(env::var(CARGO_CMAKE_BUILD_ESP_IDF_VAR)?),
+        config: Default::default(),
     };
 
     Ok(build_output)

build/pio.rs

@@ -25,10 +25,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
             (pio_scons_vars, None)
         } else {
             let config = BuildConfig::try_from_env().map(|mut config| {
-                config
-                    .with_cargo_metadata()
-                    .context("failed to read configuration from manifest metadata")
-                    .into_warning();
+                config.with_cargo_metadata().into_warning();
                 config
             })?;
             config.print();
@@ -174,6 +171,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
                 }),
         ),
         esp_idf,
+        config: Default::default(),
     };
 
     Ok(build_output)