Skip to content

Commit d6cdde5

Browse files
committed
Auto merge of #10411 - ehuss:fix-rustflags-gate, r=alexcrichton
Add common profile validation. This fixes an oversight where `rustflags` is not nightly-gated in a profile override (like `[profile.dev.package.foo]`). The problem is that the validation was only being done for the top-level profile. The solution here is to consolidate common profile validation that should be done for both the top level and the overrides. In the process I also fixed validation of `codegen-backend` which also is shared. This will hopefully help prevent other oversights in the future.
2 parents dcb2888 + 99bfbbc commit d6cdde5

File tree

2 files changed

+74
-26
lines changed

2 files changed

+74
-26
lines changed

src/cargo/util/toml/mod.rs

+38-26
Original file line numberDiff line numberDiff line change
@@ -458,10 +458,7 @@ impl ser::Serialize for ProfilePackageSpec {
458458
where
459459
S: ser::Serializer,
460460
{
461-
match *self {
462-
ProfilePackageSpec::Spec(ref spec) => spec.serialize(s),
463-
ProfilePackageSpec::All => "*".serialize(s),
464-
}
461+
self.to_string().serialize(s)
465462
}
466463
}
467464

@@ -481,21 +478,33 @@ impl<'de> de::Deserialize<'de> for ProfilePackageSpec {
481478
}
482479
}
483480

481+
impl fmt::Display for ProfilePackageSpec {
482+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
483+
match self {
484+
ProfilePackageSpec::Spec(spec) => spec.fmt(f),
485+
ProfilePackageSpec::All => f.write_str("*"),
486+
}
487+
}
488+
}
489+
484490
impl TomlProfile {
485491
pub fn validate(
486492
&self,
487493
name: &str,
488494
features: &Features,
489495
warnings: &mut Vec<String>,
490496
) -> CargoResult<()> {
497+
self.validate_profile(name, features)?;
491498
if let Some(ref profile) = self.build_override {
492499
features.require(Feature::profile_overrides())?;
493-
profile.validate_override("build-override", features)?;
500+
profile.validate_override("build-override")?;
501+
profile.validate_profile(&format!("{name}.build-override"), features)?;
494502
}
495503
if let Some(ref packages) = self.package {
496504
features.require(Feature::profile_overrides())?;
497-
for profile in packages.values() {
498-
profile.validate_override("package", features)?;
505+
for (override_name, profile) in packages {
506+
profile.validate_override("package")?;
507+
profile.validate_profile(&format!("{name}.package.{override_name}"), features)?;
499508
}
500509
}
501510

@@ -558,21 +567,6 @@ impl TomlProfile {
558567
}
559568
}
560569

561-
if self.rustflags.is_some() {
562-
features.require(Feature::profile_rustflags())?;
563-
}
564-
565-
if let Some(codegen_backend) = &self.codegen_backend {
566-
features.require(Feature::codegen_backend())?;
567-
if codegen_backend.contains(|c: char| !c.is_ascii_alphanumeric() && c != '_') {
568-
bail!(
569-
"`profile.{}.codegen-backend` setting of `{}` is not a valid backend name.",
570-
name,
571-
codegen_backend,
572-
);
573-
}
574-
}
575-
576570
Ok(())
577571
}
578572

@@ -655,7 +649,28 @@ impl TomlProfile {
655649
Ok(())
656650
}
657651

658-
fn validate_override(&self, which: &str, features: &Features) -> CargoResult<()> {
652+
/// Validates a profile.
653+
///
654+
/// This is a shallow check, which is reused for the profile itself and any overrides.
655+
fn validate_profile(&self, name: &str, features: &Features) -> CargoResult<()> {
656+
if let Some(codegen_backend) = &self.codegen_backend {
657+
features.require(Feature::codegen_backend())?;
658+
if codegen_backend.contains(|c: char| !c.is_ascii_alphanumeric() && c != '_') {
659+
bail!(
660+
"`profile.{}.codegen-backend` setting of `{}` is not a valid backend name.",
661+
name,
662+
codegen_backend,
663+
);
664+
}
665+
}
666+
if self.rustflags.is_some() {
667+
features.require(Feature::profile_rustflags())?;
668+
}
669+
Ok(())
670+
}
671+
672+
/// Validation that is specific to an override.
673+
fn validate_override(&self, which: &str) -> CargoResult<()> {
659674
if self.package.is_some() {
660675
bail!("package-specific profiles cannot be nested");
661676
}
@@ -671,9 +686,6 @@ impl TomlProfile {
671686
if self.rpath.is_some() {
672687
bail!("`rpath` may not be specified in a `{}` profile", which)
673688
}
674-
if self.codegen_backend.is_some() {
675-
features.require(Feature::codegen_backend())?;
676-
}
677689
Ok(())
678690
}
679691

tests/testsuite/profiles.rs

+36
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use std::env;
44

55
use cargo_test_support::project;
6+
use cargo_test_support::registry::Package;
67

78
#[cargo_test]
89
fn profile_overrides() {
@@ -663,6 +664,41 @@ fn rustflags_requires_cargo_feature() {
663664
"\
664665
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
665666
667+
Caused by:
668+
feature `profile-rustflags` is required
669+
670+
The package requires the Cargo feature called `profile-rustflags`, but that feature is \
671+
not stabilized in this version of Cargo (1.[..]).
672+
Consider adding `cargo-features = [\"profile-rustflags\"]` to the top of Cargo.toml \
673+
(above the [package] table) to tell Cargo you are opting in to use this unstable feature.
674+
See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#profile-rustflags-option \
675+
for more information about the status of this feature.
676+
",
677+
)
678+
.run();
679+
680+
Package::new("bar", "1.0.0").publish();
681+
p.change_file(
682+
"Cargo.toml",
683+
r#"
684+
[package]
685+
name = "foo"
686+
version = "0.0.1"
687+
688+
[dependencies]
689+
bar = "1.0"
690+
691+
[profile.dev.package.bar]
692+
rustflags = ["-C", "link-dead-code=yes"]
693+
"#,
694+
);
695+
p.cargo("check")
696+
.masquerade_as_nightly_cargo()
697+
.with_status(101)
698+
.with_stderr(
699+
"\
700+
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
701+
666702
Caused by:
667703
feature `profile-rustflags` is required
668704

0 commit comments

Comments
 (0)