fix recursion issues with crawling paths, embed linker arguments at build time to avoid toolchain issues

Author: makspll

codegen/build.rs

@@ -1,8 +1,48 @@
-fn main() {
+use std::{
+    path::{Path, PathBuf},
+    process::Command,
+};
+
+fn rustc_path(channel: &str) -> PathBuf {
+    let output = Command::new("rustup")
+        .args(["which", "--toolchain", channel, "rustc"])
+        .output()
+        .expect("failed to run rustup which");
+
+    let rustc_path = String::from_utf8(output.stdout).unwrap();
+
+    PathBuf::from(rustc_path.trim())
+}
+
+fn target_libdir(rustc: &Path) -> PathBuf {
+    let output = Command::new(rustc)
+        .args(["--print", "target-libdir"])
+        .output()
+        .expect("failed to run rustc --print target-libdir");
+
+    let libdir = String::from_utf8(output.stdout).unwrap();
+
+    PathBuf::from(libdir.trim())
+}
+
+pub fn main() {
     // Use to set RUSTC_CHANNEL so we can compute target dir from rustc_plugin
     let toolchain_toml = include_str!("rust-toolchain.toml");
     let toolchain_table = toolchain_toml.parse::<toml::Table>().unwrap();
     let toolchain = toolchain_table["toolchain"].as_table().unwrap();
     let channel = toolchain["channel"].as_str().unwrap();
     println!("cargo:rustc-env=RUSTC_CHANNEL={channel}");
+
+    // I believe there was some sort of change in cargo, which motivated this change in the original rustc_plugin:
+    // https://github.com/cognitive-engineering-lab/rustc_plugin/pull/41
+    //
+    // This solves issues with linking to the rustc driver dynamic library, as cargo does not seem to put in the correct LD_LIBRARY_PATH for the driver binary instantiations
+    // Embedding this in the binary solves the problem
+    let rustc_path = rustc_path(channel);
+    let target_libdir = target_libdir(&rustc_path);
+
+    println!(
+        "cargo::rustc-link-arg=-Wl,-rpath,{}",
+        target_libdir.display()
+    );
 }

codegen/src/driver/mod.rs

@@ -19,7 +19,7 @@ pub const CARGO_VERBOSE: &str = "CARGO_VERBOSE";
 pub const WORKSPACE_GRAPH_FILE_ENV: &str = "WORKSPACE_GRAPH_FILE";
 
 pub fn fetch_target_directory(metadata: &Metadata) -> Utf8PathBuf {
-    let plugin_subdir = format!("plugin-{}", env!("RUSTC_CHANNEL"));
+    let plugin_subdir = format!("plugin-{}", crate::CHANNEL);
     metadata.target_directory.join(plugin_subdir)
 }
 
@@ -197,7 +197,7 @@ pub fn driver_main<T: RustcPlugin>(plugin: T) {
         // the driver directly without having to pass --sysroot or anything
         let mut args: Vec<String> = orig_args.clone();
         if !have_sys_root_arg {
-            args.extend(["--sysroot".into(), sys_root]);
+            args.extend(["--sysroot".into(), sys_root.clone()]);
         };
 
         // On a given invocation of rustc, we have to decide whether to act as rustc,
@@ -215,16 +215,15 @@ pub fn driver_main<T: RustcPlugin>(plugin: T) {
 
         let run_plugin = !normal_rustc && is_target_crate;
 
+        // TODO: this is dangerous
+        // ignore all lints that could break the comp in crates that we don't care about
+        // args.extend([String::from("--cap-lints"), String::from("warn")]);
         if run_plugin {
-            // TODO: this is dangerous
-            args.extend([String::from("--cap-lints"), String::from("warn")]);
             log::debug!("Running plugin on crate: {crate_being_built}");
             let plugin_args: T::Args =
                 serde_json::from_str(&env::var(PLUGIN_ARGS).unwrap()).unwrap();
             plugin.run(args, plugin_args);
         } else {
-            // ignore all lints that could break the comp in crates that we don't care about
-            args.extend([String::from("--cap-lints"), String::from("warn")]);
             log::debug!(
                 "Running normal Rust. Relevant variables:\
 normal_rustc={normal_rustc}, \

codegen/src/import_path.rs

@@ -14,6 +14,15 @@ pub(crate) enum ImportPathElement {
     Item(DefId),
 }
 
+impl ImportPathElement {
+    pub fn def_id(&self) -> DefId {
+        match self {
+            Self::Rename(did, _) => *did,
+            Self::Item(did) => *did,
+        }
+    }
+}
+
 impl std::fmt::Debug for ImportPathElement {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
@@ -55,11 +64,12 @@ impl<'tcx> ImportPathFinder<'tcx> {
     fn crawl_module(&mut self, did: DefId, frontier: &[ImportPathElement]) {
         trace!("Crawling module {did:?}");
 
+        // Push current module onto the path frontier
         let mut new_frontier = frontier.to_vec();
         new_frontier.push(ImportPathElement::Item(did));
-        // do not allow modification or weird things happen
         let new_frontier = &new_frontier;
 
+        // Get children of the module
         let children = if did.is_local() {
             self.tcx.module_children_local(did.expect_local())
         } else {
@@ -69,15 +79,16 @@ impl<'tcx> ImportPathFinder<'tcx> {
         for child in children {
             let rename = child.ident.to_string();
 
+            // Skip private items if we don't include them
             if !self.include_private_paths && !child.vis.is_public() {
                 trace!("Skipping private child {rename:?}");
                 continue;
             }
 
-            let did = if let Some(did) = child.res.opt_def_id() {
-                did
-            } else {
-                continue;
+            // Skip if the child has no DefId
+            let did = match child.res.opt_def_id() {
+                Some(did) => did,
+                None => continue,
             };
 
             trace!(
@@ -87,16 +98,24 @@ impl<'tcx> ImportPathFinder<'tcx> {
             );
 
             match self.tcx.def_kind(did) {
-                DefKind::Mod => self.crawl_module(did, new_frontier),
-                DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::Trait => {
-                    // save the rename and the def id
+                DefKind::Mod => {
+                    // Only recurse if this DefId is not already in the current path
+                    if new_frontier.iter().any(|el| el.def_id() == did) {
+                        trace!("Cycle detected for {did:?}, skipping recursion");
+                        continue;
+                    }
+                    self.crawl_module(did, new_frontier)
+                }
 
-                    let mut new_frontier = new_frontier.clone();
-                    new_frontier.push(ImportPathElement::Rename(did, rename));
+                DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::Trait => {
+                    // Save the rename and the DefId
+                    let mut path_for_item = new_frontier.clone();
+                    path_for_item.push(ImportPathElement::Rename(did, rename));
 
-                    trace!("saving import path for {did:?}: {new_frontier:?}");
-                    self.cache.entry(did).or_default().push(new_frontier);
+                    trace!("Saving import path for {did:?}: {path_for_item:?}");
+                    self.cache.entry(did).or_default().push(path_for_item);
                 }
+
                 _ => continue,
             }
         }

codegen/src/lib.rs

@@ -39,3 +39,6 @@ pub use template::{
     Collect, Crate, TEMPLATE_DIR, TemplateKind, configure_tera, extend_context_with_args,
 };
 pub mod driver;
+
+/// The rustc toolchain this crate was built with and needs to work with
+pub const CHANNEL: &str = env!("RUSTC_CHANNEL");

codegen/src/plugin.rs

@@ -50,7 +50,7 @@ impl crate::driver::RustcPlugin for BevyAnalyzer {
 
         // make cargo chatty as well
         if args.verbose.get_log_level_int() >= 3 {
-            cmd.arg("-v");
+            cmd.arg("-vv");
         } else {
             cmd.arg("-q");
         }
@@ -74,7 +74,7 @@ impl crate::driver::RustcPlugin for BevyAnalyzer {
             .map(|a| a.to_string_lossy())
             .collect::<Vec<_>>()
             .join(" ");
-        log::debug!("Running: \n{all_env} {bin_name} {args}",);
+        log::debug!("Running cargo build command: \n{all_env} {bin_name} {args}",);
     }
 }
 

xtask/src/main.rs

@@ -1196,16 +1196,19 @@ impl Xtasks {
         }
 
         let api_gen_dir = Self::codegen_crate_dir(&main_workspace_app_settings)?;
+        let codegen_toolchain = Self::read_rust_toolchain(&api_gen_dir);
         let codegen_app_settings = main_workspace_app_settings
             .clone()
             .with_workspace_dir(api_gen_dir.clone());
+            // .with_toolchain(codegen_toolchain.clone()); // don't think it's needed, the rust toolchain file sorts that out
 
         let bevy_repo_app_settings = main_workspace_app_settings
             .clone()
             .with_workspace_dir(bevy_dir.clone())
-            .with_toolchain(Self::read_rust_toolchain(&api_gen_dir));
+            .with_toolchain(codegen_toolchain.clone());
 
         // run cargo install
+        log::info!("Running bevy_api_gen against toolchain: {codegen_toolchain}");
         Self::run_system_command(
             &codegen_app_settings,
             "cargo",
@@ -2235,9 +2238,13 @@ fn pop_cargo_env() -> Result<()> {
     let env = std::env::vars().collect::<Vec<_>>();
     // RUSTUP TOOLCHAIN exclude is a temporary fix, it might make deving the api codegen crate not work
     let exclude_list = ["CARGO_HOME"];
+    let include_list = []; //"LD_LIBRARY_PATH"
 
     for (key, value) in env.iter() {
-        if key.starts_with("CARGO_") && !exclude_list.contains(&(key.as_str())) {
+        let key_str = &(key.as_str());
+        if (include_list.contains(key_str) || key.starts_with("CARGO_"))
+            && !exclude_list.contains(key_str)
+        {
             let new_key = format!("MAIN_{key}");
             unsafe { std::env::set_var(new_key, value) };
             unsafe { std::env::remove_var(key) };