From bbaf53d723365ba27af98e361cf4051e1c40ed56 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 16 Nov 2024 21:24:33 -0500 Subject: [PATCH 1/4] feat: simply traverse the filesystem instead of intercepting the linker --- packages/cli/src/build/request.rs | 60 +++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/build/request.rs b/packages/cli/src/build/request.rs index c2cd4bb9c3..84ae466dba 100644 --- a/packages/cli/src/build/request.rs +++ b/packages/cli/src/build/request.rs @@ -6,7 +6,12 @@ use crate::{link::LinkAction, BuildArgs}; use crate::{AppBundle, Platform}; use anyhow::Context; use serde::Deserialize; -use std::{path::PathBuf, process::Stdio, time::Instant}; +use std::{ + ffi::OsStr, + path::{Path, PathBuf}, + process::Stdio, + time::Instant, +}; use tokio::{io::AsyncBufReadExt, process::Command}; #[derive(Clone, Debug)] @@ -60,7 +65,7 @@ impl BuildRequest { let start = Instant::now(); self.prepare_build_dir()?; let exe = self.build_cargo().await?; - let assets = self.collect_assets().await?; + let assets = self.collect_assets(&exe).await?; Ok(BuildArtifacts { exe, @@ -102,9 +107,6 @@ impl BuildRequest { .args(self.build_arguments()) .envs(self.env_vars()?); - // todo(jon): save the temps into a file that we use for asset extraction instead of the weird double compile. - // .args(["--", "-Csave-temps=y"]); - if let Some(target_dir) = self.custom_target_dir.as_ref() { cmd.env("CARGO_TARGET_DIR", target_dir); } @@ -203,19 +205,45 @@ impl BuildRequest { Ok(out_location) } - /// Run the linker intercept and then fill in our AssetManifest from the incremental artifacts + /// Traverse the target directory and collect all assets from the incremental cache /// - /// This will execute `dx` with an env var set to force `dx` to operate as a linker, and then - /// traverse the .o and .rlib files rustc passes that new `dx` instance, collecting the link - /// tables marked by manganis and parsing them as a ResourceAsset. - pub(crate) async fn collect_assets(&self) -> Result { + /// This uses "known paths" that have stayed relatively stable during cargo's lifetime. + /// One day this system might break and we might need to go back to using the linker approach. + pub(crate) async fn collect_assets(&self, exe: &Path) -> Result { tracing::debug!("Collecting assets ..."); if self.build.skip_assets { return Ok(AssetManifest::default()); } - self.deep_linker_asset_extract().await + // walk every file in the incremental cache dir, reading and inserting items into the manifest. + let mut manifest = AssetManifest::default(); + + // Add from the deps folder where the rlibs are stored for dependencies + let deps_folder = exe.parent().unwrap().join("deps"); + tracing::trace!("Adding assets from deps folder: {deps_folder:?}"); + for entry in deps_folder.read_dir()?.flatten() { + if entry.path().extension() == Some(OsStr::new("rlib")) { + _ = manifest.add_from_object_path(entry.path()); + } + } + + // Add from the incremental cache folder by recursively walking the folder + // it seems that this sticks around no matter what - and cargo doesn't clean it up since the .os are cached anyway + fn recusive_add(manifest: &mut AssetManifest, path: &Path) -> Result<()> { + if path.extension() == Some(OsStr::new("o")) { + _ = manifest.add_from_object_path(path.to_path_buf()); + } else if let Ok(dir) = path.read_dir() { + for entry in dir.flatten() { + recusive_add(manifest, &entry.path())?; + } + } + + Ok(()) + } + recusive_add(&mut manifest, &exe.parent().unwrap().join("incremental"))?; + + Ok(manifest) } /// Create a list of arguments for cargo builds @@ -408,6 +436,7 @@ impl BuildRequest { /// /// There's a chance that's not actually true, so this function is kept around in case we do /// need to revert to "deep extraction". + #[allow(unused)] async fn deep_linker_asset_extract(&self) -> Result { // Create a temp file to put the output of the args // We need to do this since rustc won't actually print the link args to stdout, so we need to @@ -619,9 +648,7 @@ impl BuildRequest { /// a final build output somewhere. Some platforms have basically no build command, and can simply /// be ran by executing the exe directly. pub(crate) fn root_dir(&self) -> PathBuf { - let platform_dir = self - .krate - .build_dir(self.build.platform(), self.build.release); + let platform_dir = self.platform_dir(); match self.build.platform() { Platform::Web => platform_dir.join("public"), @@ -639,6 +666,11 @@ impl BuildRequest { } } + pub(crate) fn platform_dir(&self) -> PathBuf { + self.krate + .build_dir(self.build.platform(), self.build.release) + } + pub fn asset_dir(&self) -> PathBuf { match self.build.platform() { Platform::MacOS => self From 9b99e4663d66456ae0b4e087b349075db682dd66 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 16 Nov 2024 21:28:07 -0500 Subject: [PATCH 2/4] handle LTO approach too --- packages/cli/src/assets.rs | 4 ++-- packages/cli/src/build/request.rs | 7 +++++-- packages/cli/src/cli/link.rs | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/assets.rs b/packages/cli/src/assets.rs index 659aa5fec1..3384058343 100644 --- a/packages/cli/src/assets.rs +++ b/packages/cli/src/assets.rs @@ -24,8 +24,8 @@ impl AssetManifest { } /// Fill this manifest with a file object/rlib files, typically extracted from the linker intercepted - pub(crate) fn add_from_object_path(&mut self, path: PathBuf) -> anyhow::Result<()> { - let data = std::fs::read(path.clone())?; + pub(crate) fn add_from_object_path(&mut self, path: &Path) -> anyhow::Result<()> { + let data = std::fs::read(path)?; match path.extension().and_then(|ext| ext.to_str()) { // Parse an rlib as a collection of objects diff --git a/packages/cli/src/build/request.rs b/packages/cli/src/build/request.rs index 84ae466dba..2a2d40e67d 100644 --- a/packages/cli/src/build/request.rs +++ b/packages/cli/src/build/request.rs @@ -224,7 +224,7 @@ impl BuildRequest { tracing::trace!("Adding assets from deps folder: {deps_folder:?}"); for entry in deps_folder.read_dir()?.flatten() { if entry.path().extension() == Some(OsStr::new("rlib")) { - _ = manifest.add_from_object_path(entry.path()); + _ = manifest.add_from_object_path(&entry.path()); } } @@ -232,7 +232,7 @@ impl BuildRequest { // it seems that this sticks around no matter what - and cargo doesn't clean it up since the .os are cached anyway fn recusive_add(manifest: &mut AssetManifest, path: &Path) -> Result<()> { if path.extension() == Some(OsStr::new("o")) { - _ = manifest.add_from_object_path(path.to_path_buf()); + _ = manifest.add_from_object_path(path); } else if let Ok(dir) = path.read_dir() { for entry in dir.flatten() { recusive_add(manifest, &entry.path())?; @@ -243,6 +243,9 @@ impl BuildRequest { } recusive_add(&mut manifest, &exe.parent().unwrap().join("incremental"))?; + // And then add from the exe directly, just in case it's LTO compiled and has no incremental cache + _ = manifest.add_from_object_path(exe); + Ok(manifest) } diff --git a/packages/cli/src/cli/link.rs b/packages/cli/src/cli/link.rs index 09f1f11a7a..88576cb107 100644 --- a/packages/cli/src/cli/link.rs +++ b/packages/cli/src/cli/link.rs @@ -92,7 +92,7 @@ impl LinkAction { if item.ends_with(".o") || item.ends_with(".rlib") { let path_to_item = PathBuf::from(item); if let Ok(path) = path_to_item.canonicalize() { - _ = manifest.add_from_object_path(path); + _ = manifest.add_from_object_path(&path); } } } From c40f32ba2f7b8639696f6689144e1ef780b13551 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 16 Nov 2024 21:29:59 -0500 Subject: [PATCH 3/4] typo... --- packages/cli/src/build/request.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/build/request.rs b/packages/cli/src/build/request.rs index 2a2d40e67d..a72a562f9f 100644 --- a/packages/cli/src/build/request.rs +++ b/packages/cli/src/build/request.rs @@ -230,18 +230,18 @@ impl BuildRequest { // Add from the incremental cache folder by recursively walking the folder // it seems that this sticks around no matter what - and cargo doesn't clean it up since the .os are cached anyway - fn recusive_add(manifest: &mut AssetManifest, path: &Path) -> Result<()> { + fn recursive_add(manifest: &mut AssetManifest, path: &Path) -> Result<()> { if path.extension() == Some(OsStr::new("o")) { _ = manifest.add_from_object_path(path); } else if let Ok(dir) = path.read_dir() { for entry in dir.flatten() { - recusive_add(manifest, &entry.path())?; + recursive_add(manifest, &entry.path())?; } } Ok(()) } - recusive_add(&mut manifest, &exe.parent().unwrap().join("incremental"))?; + recursive_add(&mut manifest, &exe.parent().unwrap().join("incremental"))?; // And then add from the exe directly, just in case it's LTO compiled and has no incremental cache _ = manifest.add_from_object_path(exe); From bbbcd5a29d340ac935270fa9c1efc63df13f9d27 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sat, 16 Nov 2024 22:14:50 -0500 Subject: [PATCH 4/4] add deeplinker just in case --- packages/cli/src/build/request.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/cli/src/build/request.rs b/packages/cli/src/build/request.rs index a72a562f9f..b8633da457 100644 --- a/packages/cli/src/build/request.rs +++ b/packages/cli/src/build/request.rs @@ -216,6 +216,12 @@ impl BuildRequest { return Ok(AssetManifest::default()); } + // Experimental feature for testing - if the env var is set, we'll use the deeplinker + if std::env::var("DEEPLINK").is_ok() { + tracing::debug!("Using deeplinker instead of incremental cache"); + return self.deep_linker_asset_extract().await; + } + // walk every file in the incremental cache dir, reading and inserting items into the manifest. let mut manifest = AssetManifest::default();