Skip to content

Commit

Permalink
Auto merge of rust-lang#10411 - ehuss:fix-rustflags-gate, r=alexcrichton
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors authored and ehuss committed Feb 22, 2022
1 parent ea2a21c commit ba955c5
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 26 deletions.
64 changes: 38 additions & 26 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,10 +448,7 @@ impl ser::Serialize for ProfilePackageSpec {
where
S: ser::Serializer,
{
match *self {
ProfilePackageSpec::Spec(ref spec) => spec.serialize(s),
ProfilePackageSpec::All => "*".serialize(s),
}
self.to_string().serialize(s)
}
}

Expand All @@ -471,21 +468,33 @@ impl<'de> de::Deserialize<'de> for ProfilePackageSpec {
}
}

impl fmt::Display for ProfilePackageSpec {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ProfilePackageSpec::Spec(spec) => spec.fmt(f),
ProfilePackageSpec::All => f.write_str("*"),
}
}
}

impl TomlProfile {
pub fn validate(
&self,
name: &str,
features: &Features,
warnings: &mut Vec<String>,
) -> CargoResult<()> {
self.validate_profile(name, features)?;
if let Some(ref profile) = self.build_override {
features.require(Feature::profile_overrides())?;
profile.validate_override("build-override", features)?;
profile.validate_override("build-override")?;
profile.validate_profile(&format!("{name}.build-override"), features)?;
}
if let Some(ref packages) = self.package {
features.require(Feature::profile_overrides())?;
for profile in packages.values() {
profile.validate_override("package", features)?;
for (override_name, profile) in packages {
profile.validate_override("package")?;
profile.validate_profile(&format!("{name}.package.{override_name}"), features)?;
}
}

Expand Down Expand Up @@ -548,21 +557,6 @@ impl TomlProfile {
}
}

if self.rustflags.is_some() {
features.require(Feature::profile_rustflags())?;
}

if let Some(codegen_backend) = &self.codegen_backend {
features.require(Feature::codegen_backend())?;
if codegen_backend.contains(|c: char| !c.is_ascii_alphanumeric() && c != '_') {
bail!(
"`profile.{}.codegen-backend` setting of `{}` is not a valid backend name.",
name,
codegen_backend,
);
}
}

Ok(())
}

Expand Down Expand Up @@ -645,7 +639,28 @@ impl TomlProfile {
Ok(())
}

fn validate_override(&self, which: &str, features: &Features) -> CargoResult<()> {
/// Validates a profile.
///
/// This is a shallow check, which is reused for the profile itself and any overrides.
fn validate_profile(&self, name: &str, features: &Features) -> CargoResult<()> {
if let Some(codegen_backend) = &self.codegen_backend {
features.require(Feature::codegen_backend())?;
if codegen_backend.contains(|c: char| !c.is_ascii_alphanumeric() && c != '_') {
bail!(
"`profile.{}.codegen-backend` setting of `{}` is not a valid backend name.",
name,
codegen_backend,
);
}
}
if self.rustflags.is_some() {
features.require(Feature::profile_rustflags())?;
}
Ok(())
}

/// Validation that is specific to an override.
fn validate_override(&self, which: &str) -> CargoResult<()> {
if self.package.is_some() {
bail!("package-specific profiles cannot be nested");
}
Expand All @@ -661,9 +676,6 @@ impl TomlProfile {
if self.rpath.is_some() {
bail!("`rpath` may not be specified in a `{}` profile", which)
}
if self.codegen_backend.is_some() {
features.require(Feature::codegen_backend())?;
}
Ok(())
}

Expand Down
36 changes: 36 additions & 0 deletions tests/testsuite/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::env;

use cargo_test_support::project;
use cargo_test_support::registry::Package;

#[cargo_test]
fn profile_overrides() {
Expand Down Expand Up @@ -660,6 +661,41 @@ fn rustflags_requires_cargo_feature() {
"\
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
Caused by:
feature `profile-rustflags` is required
The package requires the Cargo feature called `profile-rustflags`, but that feature is \
not stabilized in this version of Cargo (1.[..]).
Consider adding `cargo-features = [\"profile-rustflags\"]` to the top of Cargo.toml \
(above the [package] table) to tell Cargo you are opting in to use this unstable feature.
See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#profile-rustflags-option \
for more information about the status of this feature.
",
)
.run();

Package::new("bar", "1.0.0").publish();
p.change_file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
[dependencies]
bar = "1.0"
[profile.dev.package.bar]
rustflags = ["-C", "link-dead-code=yes"]
"#,
);
p.cargo("check")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `profile-rustflags` is required
Expand Down

0 comments on commit ba955c5

Please sign in to comment.