fix stack overflow and improve logging in bootstrapping, build bootstrap into nearest target

makspll · makspll · commit 9f01ebd7a380 · 2024-09-07T15:18:35.000+01:00
diff --git a/crates/bevy_api_gen/Cargo.bootstrap.toml b/crates/bevy_api_gen/Cargo.bootstrap.toml
@@ -6,6 +6,12 @@ edition = "2021"
 
 [dependencies]
 mlua = { version = "0.9.2", features = ["lua54", "vendored", "send", "macros"] }
-bevy_reflect = { version = "0.14", features = ["bevy", "bevy_math"] }
+bevy_reflect = { version = "0.14", features = [
+    "bevy",
+    "glam",
+    "petgraph",
+    "smallvec",
+    "uuid",
+] }
 
 [workspace]
diff --git a/crates/bevy_api_gen/src/bin/main.rs b/crates/bevy_api_gen/src/bin/main.rs
@@ -2,15 +2,15 @@ use std::{
     collections::HashMap,
     env,
     fs::{create_dir_all, File},
-    io::Write,
-    path::Path,
+    io::{BufRead, Write},
+    path::{Path, PathBuf},
     process::{Command, Stdio},
 };
 
 use bevy_api_gen::*;
 use cargo_metadata::camino::Utf8Path;
 use clap::Parser;
-use log::{debug, info};
+use log::{debug, error, info};
 use strum::VariantNames;
 use tera::Context;
 
@@ -25,19 +25,34 @@ fn main() {
     }
     env_logger::init();
 
+    info!("Computing crate metadata");
     let metadata = cargo_metadata::MetadataCommand::new()
         .no_deps()
         .other_options(["--all-features".to_string(), "--offline".to_string()])
         .exec()
         .unwrap();
+
     let crates = metadata
         .workspace_packages()
         .iter()
         .map(|p| p.name.to_owned())
         .collect::<Vec<_>>();
+
+    info!("Computing active features");
     let include_crates = match (&args.workspace_root, args.cmd.is_generate()) {
         (Some(root), true) => {
             let feature_graph = FeatureGraph::from_metadata(&metadata, root);
+            info!(
+                "Using workspace root: {}, found {} crates",
+                feature_graph.workspace_root,
+                feature_graph.crates.len()
+            );
+
+            info!(
+                "Computing all transitive dependencies for enabled top-level features: {}",
+                args.features.join(",")
+            );
+
             let dependencies = feature_graph
                 .dependencies_for_features(args.features.as_ref(), !args.no_default_features)
                 .into_iter()
@@ -52,6 +67,8 @@ fn main() {
     let plugin_subdir = format!("plugin-{}", env!("RUSTC_CHANNEL"));
     let plugin_target_dir = metadata.target_directory.join(plugin_subdir);
 
+    info!("Computing wokrspace metadata");
+
     // inform the deps about the workspace crates, this is going to be useful when working with meta files as we will be able to
     // know when to panic if a crate is not found
     // it's also useful to pass around the output directory for our Args default values to be able to compute them
@@ -132,14 +149,13 @@ fn main() {
         _ => {}
     }
 
-    let temp_dir = tempdir::TempDir::new("bevy_api_gen_bootstrap")
-        .expect("Error occured when trying to acquire temp file");
+    let temp_dir = find_bootstrap_dir();
 
-    debug!("Temporary directory: {}", &temp_dir.path().display());
+    debug!("Bootstrap directory: {}", &temp_dir.as_path().display());
 
-    write_bootstrap_files(temp_dir.path());
+    write_bootstrap_files(temp_dir.as_path());
 
-    let bootstrap_rlibs = build_bootstrap(temp_dir.path(), &plugin_target_dir.join("bootstrap"));
+    let bootstrap_rlibs = build_bootstrap(temp_dir.as_path(), &plugin_target_dir.join("bootstrap"));
 
     if bootstrap_rlibs.len() == BOOTSTRAP_DEPS.len() {
         let extern_args = bootstrap_rlibs
@@ -195,20 +211,50 @@ fn build_bootstrap(
     let mut cmd = Command::new("cargo")
         .current_dir(temp_dir)
         .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
         .args(["build", "--message-format=json"])
         .spawn()
         .unwrap();
 
+    info!(
+        "cd {} && cargo build --message-format=json",
+        temp_dir.display()
+    );
+
     let reader = std::io::BufReader::new(cmd.stdout.take().unwrap());
+    let err_reader = std::io::BufReader::new(cmd.stderr.take().unwrap());
 
     std::fs::create_dir_all(cache_dir).unwrap();
 
     let mut bootstrap_rlibs = HashMap::with_capacity(BOOTSTRAP_DEPS.len());
     for msg in cargo_metadata::Message::parse_stream(reader) {
-        if let cargo_metadata::Message::CompilerArtifact(artifact) = msg.unwrap() {
+        let msg = msg.unwrap();
+        if let cargo_metadata::Message::CompilerArtifact(artifact) = msg {
             for artifact in artifact.filenames.into_iter() {
                 process_artifact(artifact, &mut bootstrap_rlibs);
             }
+        } else {
+            match msg {
+                cargo_metadata::Message::BuildFinished(finished) => {
+                    if !finished.success {
+                        error!("Bootstrapping crate failed to build artifact");
+                    }
+                }
+                cargo_metadata::Message::TextLine(t) => {
+                    info!("{t}");
+                }
+                cargo_metadata::Message::CompilerMessage(msg) => {
+                    info!("{msg}");
+                }
+                _ => {}
+            }
+        }
+    }
+    for msg in err_reader.lines() {
+        if let Ok(line) = msg {
+            info!("{line}");
+        } else {
+            panic!("Failed to read cargo stderr");
         }
     }
 
@@ -224,10 +270,17 @@ fn build_bootstrap(
             std::fs::copy(path, dest).unwrap();
         }
     }
+    match cmd.wait() {
+        Ok(status) => {
+            if !status.success() {
+                panic!("Building bootstrap crate returned a failure status code");
+            }
+        }
+        Err(e) => {
+            panic!("Failed to wait on cargo build process: {}", e);
+        }
+    }
 
-    if !cmd.wait().unwrap().success() {
-        panic!("Building bootstrap crate returned a failure status code");
-    };
     bootstrap_rlibs
 }
 
@@ -248,6 +301,26 @@ fn process_artifact(
     }
 }
 
+/// finds best location for bootstrapping crate
+/// this will be the nearest target/bevy_api_gen_bootstrap directory
+fn find_bootstrap_dir() -> PathBuf {
+    let mut path = env::current_dir().unwrap();
+    loop {
+        if path.join("target").exists() {
+            break;
+        } else {
+            path = path.parent().unwrap().to_owned();
+        }
+    }
+
+    path.push("target");
+    path.push("bevy_api_gen_bootstrap");
+
+    // create all the directories
+    create_dir_all(&path).unwrap();
+    path
+}
+
 /// Generate bootstrapping crate files
 fn write_bootstrap_files(path: &Path) {
     // write manifest file 'Cargo.toml'
diff --git a/crates/bevy_api_gen/src/feature_graph.rs b/crates/bevy_api_gen/src/feature_graph.rs
@@ -1,12 +1,14 @@
 use std::collections::{HashMap, HashSet};
 
-use cargo_metadata::{Metadata, Package};
+use cargo_metadata::{DependencyKind, Metadata, Package};
 use itertools::{Either, Itertools};
+use log::{debug, info};
 
 #[derive(Clone, Debug)]
 pub enum FeatureEffect {
     /// A feature which enables another feature
     /// in a dependency
+    /// if enable_optional is true, the dependency may not itself be enabled in which case the feature is not enabled
     EnableDepFeature {
         feature: String,
         dependency: String,
@@ -53,8 +55,8 @@ pub enum Depdenency {
 
 #[derive(Debug)]
 pub struct FeatureGraph {
-    workspace_root: String,
-    crates: Vec<Crate>,
+    pub workspace_root: String,
+    pub crates: Vec<Crate>,
 }
 
 impl FeatureGraph {
@@ -72,7 +74,7 @@ impl FeatureGraph {
             .find(|c| c.name == self.workspace_root)
             .unwrap();
         let mut buffer = Default::default();
-        self.dependencies_for_features_on_crate(root, features, include_default, &mut buffer);
+        self.dependencies_for_features_on_crate(root, features, include_default, &mut buffer, 0);
 
         buffer.iter().map(|c| c.name.as_str()).collect()
     }
@@ -83,7 +85,18 @@ impl FeatureGraph {
         features: &[String],
         include_default: bool,
         buffer: &mut HashSet<&'a Crate>,
+        depth: usize,
     ) {
+        let log_prefix = "|".to_owned() + &"-".repeat(depth);
+        debug!(
+            "{log_prefix}Processing dependencies for crate: `{}` with features: {}",
+            crate_.name,
+            features.join(", ")
+        );
+        if depth > 30 {
+            panic!("Recursion depth exceeded");
+        }
+
         let active_features = features
             .iter()
             .map(|f| {
@@ -106,6 +119,7 @@ impl FeatureGraph {
                     dependency,
                     enable_optional,
                 } => {
+                    // we ignore optional dependencies's features, is this what we want to do?
                     if *enable_optional {
                         deps.entry(self.crates.iter().find(|c| c.name == *dependency).unwrap())
                             .or_default()
@@ -129,10 +143,26 @@ impl FeatureGraph {
         }));
 
         // repeat for all dependencies recursively
+        // we also ignore optional dependencies here, again is this what we want to do? I can't remember
         for (dep, features) in deps.iter() {
+            debug!(
+                "{log_prefix}Adding dependency: {} with features {:?}",
+                dep.name, features
+            );
             buffer.insert(dep);
-            self.dependencies_for_features_on_crate(dep, features, include_default, buffer);
+            self.dependencies_for_features_on_crate(
+                dep,
+                features,
+                include_default,
+                buffer,
+                depth + 1,
+            );
         }
+
+        debug!(
+            "{log_prefix}Finished processing dependencies for crate: `{}`",
+            crate_.name
+        );
     }
 
     /// "flattens" feature effects to a list of effects based on the selected active features, features which enable other features are expanded until
@@ -182,6 +212,7 @@ impl FeatureGraph {
         let (optional_dependencies, other_dependencies) = meta
             .dependencies
             .iter()
+            .filter(|d| d.kind == DependencyKind::Normal) // dev dependencies can introduce weird cycles, and we don't care about them anyway
             .map(|f| (f.name.clone(), f.optional))
             .partition_map(|(name, opt)| {
                 if opt {
diff --git a/makefile b/makefile
@@ -60,7 +60,7 @@ clean_bevy:
 	cd ${BEVY_PATH} && cargo clean
 
 generate_bevy:
-	cd ${BEVY_PATH} && cargo +${NIGHTLY_VERSION} bevy-api-gen generate --output ${OUTPUT_PATH} --template-args '{ "self_is_bevy_script_api": true}' --features ${GEN_BEVY_FEATURES}
+	cd ${BEVY_PATH} && cargo +${NIGHTLY_VERSION} bevy-api-gen generate --output ${OUTPUT_PATH} --template-args '{ "self_is_bevy_script_api": true}' --features ${GEN_BEVY_FEATURES} -vv
 
 collect_bevy:
 	cd ${BEVY_PATH} && cargo +${NIGHTLY_VERSION} bevy-api-gen collect --output ${OUTPUT_PATH} --template-args '{ "self_is_bevy_script_api": true}'
