Skip to content

Commit

Permalink
Auto merge of #13618 - ehuss:always-rebuild-unit-graph, r=epage
Browse files Browse the repository at this point in the history
Fix debuginfo strip when using `--target`

This fixes an issue where automatic `strip` of debuginfo added in #13257 wasn't working when the `--target` flag was used.

The problem is that the adjustment code was only running in the optimization pass that is done when `--target` is *not* specified.

The solution is to just always run the unit graph rebuild. I believe it should be safe to do so, since the adjustments it makes should be conditional on just the scenarios that matter when `--target` is not specified. The downside is that this might be a small performance hit when `--target` is used. Trying to avoid that I think would be quite challenging.

Fixes #13617
  • Loading branch information
bors committed Mar 21, 2024
2 parents b8b7fa5 + e0e000e commit 8bcecfe
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
38 changes: 20 additions & 18 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,23 +427,16 @@ pub fn create_bcx<'a, 'gctx>(
.requested_kinds
.iter()
.any(CompileKind::is_host);
let should_share_deps = host_kind_requested
|| gctx.cli_unstable().bindeps
&& unit_graph
.iter()
.any(|(unit, _)| unit.artifact_target_for_features.is_some());
if should_share_deps {
// Rebuild the unit graph, replacing the explicit host targets with
// CompileKind::Host, removing `artifact_target_for_features` and merging any dependencies
// shared with build and artifact dependencies.
(units, scrape_units, unit_graph) = rebuild_unit_graph_shared(
interner,
unit_graph,
&units,
&scrape_units,
host_kind_requested.then_some(explicit_host_kind),
);
}
// Rebuild the unit graph, replacing the explicit host targets with
// CompileKind::Host, removing `artifact_target_for_features` and merging any dependencies
// shared with build and artifact dependencies.
(units, scrape_units, unit_graph) = rebuild_unit_graph_shared(
interner,
unit_graph,
&units,
&scrape_units,
host_kind_requested.then_some(explicit_host_kind),
);

let mut extra_compiler_args = HashMap::new();
if let Some(args) = extra_args {
Expand Down Expand Up @@ -545,7 +538,8 @@ where `<compatible-ver>` is the latest version supporting rustc {rustc_version}"
Ok(bcx)
}

/// This is used to rebuild the unit graph, sharing host dependencies if possible.
/// This is used to rebuild the unit graph, sharing host dependencies if possible,
/// and applying other unit adjustments based on the whole graph.
///
/// This will translate any unit's `CompileKind::Target(host)` to
/// `CompileKind::Host` if `to_host` is not `None` and the kind is equal to `to_host`.
Expand All @@ -567,6 +561,14 @@ where `<compatible-ver>` is the latest version supporting rustc {rustc_version}"
/// to the `Unit`, this allows the `CompileKind` to be changed back to `Host`
/// and `artifact_target_for_features` to be removed without fear of an unwanted
/// collision for build or artifact dependencies.
///
/// This is also responsible for adjusting the `strip` profile option to
/// opportunistically strip if debug is 0 for all dependencies. This helps
/// remove debuginfo added by the standard library.
///
/// This is also responsible for adjusting the `debug` setting for host
/// dependencies, turning off debug if the user has not explicitly enabled it,
/// and the unit is not shared with a target unit.
fn rebuild_unit_graph_shared(
interner: &UnitInterner,
unit_graph: UnitGraph,
Expand Down
6 changes: 5 additions & 1 deletion tests/testsuite/profiles.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Tests for profiles.

use cargo_test_support::project;
use cargo_test_support::registry::Package;
use cargo_test_support::{project, rustc_host};
use std::env;

#[cargo_test]
Expand Down Expand Up @@ -650,6 +650,10 @@ fn strip_debuginfo_in_release() {
p.cargo("build --release -v")
.with_stderr_contains("[RUNNING] `rustc [..] -C strip=debuginfo[..]`")
.run();
p.cargo("build --release -v --target")
.arg(rustc_host())
.with_stderr_contains("[RUNNING] `rustc [..] -C strip=debuginfo[..]`")
.run();
}

#[cargo_test]
Expand Down

0 comments on commit 8bcecfe

Please sign in to comment.