Disable strip setting in profile and manually strip with llvm the build (#4966)

* disable strip setting in profile and manually strip with llvm the build

* remove rustc_llvm

* disable debug if wasm opt will run

* fix clippy

* don't strip if we are splitting with wasm-split

* keep names when running wasm-split

Author: ealmloff

packages/cli/src/build/request.rs

@@ -842,15 +842,6 @@ impl BuildRequest {
             None => bundle.profile_name(args.release),
         };
 
-        // Warn if the user is trying to build with strip and using manganis
-        Self::warn_manganis_strip(
-            &workspace.krates,
-            &workspace.cargo_toml,
-            main_package,
-            &profile,
-            args.release,
-        );
-
         // Determine if we should codesign
         let should_codesign =
             args.codesign || device.is_some() || args.apple_entitlements.is_some();
@@ -1082,6 +1073,7 @@ impl BuildRequest {
             BuildMode::Base { .. } | BuildMode::Fat => {
                 ctx.status_start_bundle();
 
+                self.strip_binary(&artifacts).await?;
                 self.write_executable(ctx, &artifacts.exe, &mut artifacts.assets)
                     .await
                     .context("Failed to write executable")?;
@@ -3627,32 +3619,19 @@ impl BuildRequest {
         })
     }
 
-    /// Return the platforms that are enabled for the package
-    ///
-    /// Ideally only one platform is enabled but we need to be able to
-    pub(crate) fn warn_manganis_strip(
-        krates: &krates::Krates,
-        cargo_toml: &cargo_toml::Manifest,
-        main_package: &krates::cm::Package,
-        profile: &str,
-        release: bool,
-    ) {
-        let Some(id) = krates.nid_for_kid(&main_package.id.clone().into()) else {
-            return;
-        };
-        let dependencies = krates.direct_dependencies(id);
-        if !dependencies.iter().any(|dep| dep.krate.name == "manganis") {
-            return;
-        }
-
-        let (profile_name, profile) = match (cargo_toml.profile.custom.get(profile), release) {
-            (Some(custom_profile), _) => (profile, Some(custom_profile)),
-            (_, true) => ("release", cargo_toml.profile.release.as_ref()),
-            (_, false) => ("dev", cargo_toml.profile.dev.as_ref()),
+    /// Checks the strip setting for the package, resolving profiles recursively
+    pub(crate) fn get_strip_setting(&self) -> StripSetting {
+        let cargo_toml = &self.workspace.cargo_toml;
+        let profile = &self.profile;
+        let release = self.release;
+        let profile = match (cargo_toml.profile.custom.get(profile), release) {
+            (Some(custom_profile), _) => Some(custom_profile),
+            (_, true) => cargo_toml.profile.release.as_ref(),
+            (_, false) => cargo_toml.profile.dev.as_ref(),
         };
 
         let Some(profile) = profile else {
-            return;
+            return StripSetting::None;
         };
 
         // Get the strip setting from the profile or the profile it inherits from
@@ -3673,15 +3652,11 @@ impl BuildRequest {
         }
 
         let Some(strip) = get_strip(profile, &cargo_toml.profile) else {
-            // If the profile doesn't have a strip option, we don't need to warn
-            return;
+            // If the profile doesn't have a strip option, return None
+            return StripSetting::None;
         };
 
-        if matches!(strip, cargo_toml::StripSetting::Symbols) {
-            tracing::warn!(
-                "The `strip` option is enabled in the `{profile_name}` profile. This may cause manganis assets to be stripped from the final binary.",
-            );
-        }
+        strip
     }
 
     pub(crate) fn renderer_enabled_by_dioxus_dependency(
@@ -3988,6 +3963,31 @@ impl BuildRequest {
         Ok(())
     }
 
+    /// Strip the final binary after extracting all assets with rustc-objcopy
+    async fn strip_binary(&self, artifacts: &BuildArtifacts) -> Result<()> {
+        // Never strip the binary if we are going to bundle split it
+        if self.wasm_split {
+            return Ok(());
+        }
+        let exe = &artifacts.exe;
+        // https://github.com/rust-lang/rust/blob/cb80ff132a0e9aa71529b701427e4e6c243b58df/compiler/rustc_codegen_ssa/src/back/linker.rs#L1433-L1443
+        let strip_arg = match self.get_strip_setting() {
+            StripSetting::Debuginfo => Some("--strip-debug"),
+            StripSetting::Symbols => Some("--strip-all"),
+            StripSetting::None => None,
+        };
+        if let Some(strip_arg) = strip_arg {
+            let rustc_objcopy = self.workspace.rustc_objcopy();
+            let mut command = Command::new(rustc_objcopy);
+            command.arg(strip_arg).arg(exe).arg(exe);
+            let output = command.output().await?;
+            if !output.status.success() {
+                return Err(anyhow::anyhow!("Failed to strip binary"));
+            }
+        }
+        Ok(())
+    }
+
     /// Check if assets should be pre_compressed. This will only be true in release mode if the user
     /// has enabled pre_compress in the web config.
     fn should_pre_compress_web_assets(&self, release: bool) -> bool {
@@ -4036,19 +4036,16 @@ impl BuildRequest {
 
         // Prepare our configuration
         //
-        // we turn off debug symbols in dev mode but leave them on in release mode (weird!) since
-        // wasm-opt and wasm-split need them to do better optimizations.
+        // we turn on debug symbols in dev mode
         //
         // We leave demangling to false since it's faster and these tools seem to prefer the raw symbols.
         // todo(jon): investigate if the chrome extension needs them demangled or demangles them automatically.
-        let will_wasm_opt = self.release || self.wasm_split;
         let keep_debug = self.config.web.wasm_opt.debug
             || self.debug_symbols
             || self.wasm_split
             || !self.release
-            || will_wasm_opt
             || ctx.mode == BuildMode::Fat;
-        let keep_names = will_wasm_opt || ctx.mode == BuildMode::Fat;
+        let keep_names = self.wasm_split || ctx.mode == BuildMode::Fat;
         let demangle = false;
         let wasm_opt_options = WasmOptConfig {
             memory_packing: self.wasm_split,
@@ -4086,10 +4083,6 @@ impl BuildRequest {
         if should_bundle_split {
             ctx.status_splitting_bundle();
 
-            if !will_wasm_opt {
-                bail!("Bundle splitting should automatically enable wasm-opt, but it was not enabled.");
-            }
-
             // Load the contents of these binaries since we need both of them
             // We're going to use the default makeLoad glue from wasm-split
             let original = std::fs::read(exe)?;
@@ -5252,6 +5245,10 @@ __wbg_init({{module_or_path: "/{}/{wasm_path}"}}).then((wasm) => {{
             }
         }
 
+        // Always disable stripping so symbols still exist for the asset system. We will apply strip manually
+        // after assets are built
+        args.push(format!(r#"profile.{profile}.strip=false"#));
+
         // Prepend --config to each argument
         args.into_iter()
             .flat_map(|arg| ["--config".to_string(), arg])

packages/cli/src/workspace.rs

@@ -274,6 +274,17 @@ impl Workspace {
             .join("gcc-ld")
     }
 
+    // wasm-ld: ./rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/wasm-ld
+    // rust-lld: ./rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rust-lld
+    pub fn rustc_objcopy(&self) -> PathBuf {
+        self.sysroot
+            .join("lib")
+            .join("rustlib")
+            .join(Triple::host().to_string())
+            .join("bin")
+            .join("rust-objcopy")
+    }
+
     /// Find the "main" package in the workspace. There might not be one!
     pub fn find_main_package(&self, package: Option<String>) -> Result<NodeId> {
         if let Some(package) = package {