Skip to content

Commit

Permalink
Auto merge of #9657 - ehuss:clearer-nightly-requirements, r=alexcrichton
Browse files Browse the repository at this point in the history
Update nightly failure notification.

This makes several changes to try to clarify errors with nightly requirements.

- Don't tell the user to edit `Cargo.toml` if it is not a local package. Things like registry packages are not under their control.
- Include the version number in the error message.
- Try to make better suggestions on what to do.
- Remove the redirects for stabilized features in unstable.md, and instead include a small stub that tells the user when it was stabilized and where to find more information. This should help with people using older releases which provide links to this page, to help them know which version they will need.

Closes #9610
  • Loading branch information
bors committed Jul 12, 2021
2 parents 2517af4 + 2c99f65 commit 01ddc25
Show file tree
Hide file tree
Showing 13 changed files with 339 additions and 105 deletions.
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
If this looks unexpected, it may be a bug in Cargo. Please file a bug report at\n\
https://github.com/rust-lang/cargo/issues/ with as much information as you\n\
can provide.\n\
{} running on `{}` target `{}`\n\
cargo {} running on `{}` target `{}`\n\
First unit: {:?}\n\
Second unit: {:?}",
describe_collision(unit, other_unit, path),
Expand Down
145 changes: 94 additions & 51 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
//!
//! 1. Update the feature to be stable, based on the kind of feature:
//! 1. `cargo-features`: Change the feature to `stable` in the `features!`
//! macro below.
//! macro below, and include the version and a URL for the documentation.
//! 2. `-Z unstable-options`: Find the call to `fail_if_stable_opt` and
//! remove it. Be sure to update the man pages if necessary.
//! 3. `-Z` flag: Change the parsing code in [`CliUnstable::add`][CliUnstable]
Expand All @@ -87,13 +87,13 @@
//! necessary.
//! 2. Remove `masquerade_as_nightly_cargo` from any tests, and remove
//! `cargo-features` from `Cargo.toml` test files if any.
//! 3. Remove the docs from unstable.md and update the redirect at the bottom
//! of that page. Update the rest of the documentation to add the new
//! feature.
//! 3. Update the docs in unstable.md to move the section to the bottom
//! and summarize it similar to the other entries. Update the rest of the
//! documentation to add the new feature.
use std::collections::BTreeSet;
use std::env;
use std::fmt;
use std::fmt::{self, Write};
use std::str::FromStr;

use anyhow::{bail, Error};
Expand Down Expand Up @@ -130,13 +130,22 @@ pub enum Edition {
// - Gate on that new feature in TomlManifest::to_real_manifest.
// - Update the shell completion files.
// - Update any failing tests (hopefully there are very few).
// - Update unstable.md to add a new section for this new edition (see
// https://github.com/rust-lang/cargo/blob/3ebb5f15a940810f250b68821149387af583a79e/src/doc/src/reference/unstable.md?plain=1#L1238-L1264
// as an example).
//
// Stabilization instructions:
// - Set LATEST_UNSTABLE to None.
// - Set LATEST_STABLE to the new version.
// - Update `is_stable` to `true`.
// - Set the editionNNNN feature to stable in the features macro below.
// - Update the man page for the --edition flag.
// - Update unstable.md to move the edition section to the bottom.
// - Update the documentation:
// - Update any features impacted by the edition.
// - Update manifest.md#the-edition-field.
// - Update the --edition flag (options-new.md).
// - Rebuild man pages.
impl Edition {
/// The latest edition that is unstable.
///
Expand Down Expand Up @@ -279,6 +288,7 @@ macro_rules! features {
$($feature: bool,)*
activated: Vec<String>,
nightly_features_allowed: bool,
is_local: bool,
}

impl Feature {
Expand Down Expand Up @@ -362,7 +372,7 @@ features! {
(stable, rename_dependency, "1.31", "reference/specifying-dependencies.html#renaming-dependencies-in-cargotoml"),

// Whether a lock file is published with this crate
(removed, publish_lockfile, "", PUBLISH_LOCKFILE_REMOVED),
(removed, publish_lockfile, "1.37", "reference/unstable.html#publish-lockfile"),

// Overriding profiles for dependencies.
(stable, profile_overrides, "1.41", "reference/profiles.html#overrides"),
Expand Down Expand Up @@ -395,14 +405,6 @@ features! {
(unstable, per_package_target, "", "reference/unstable.html#per-package-target"),
}

const PUBLISH_LOCKFILE_REMOVED: &str = "The publish-lockfile key in Cargo.toml \
has been removed. The Cargo.lock file is always included when a package is \
published if the package contains a binary target. `cargo install` requires \
the `--locked` flag to use the Cargo.lock file.\n\
See https://doc.rust-lang.org/cargo/commands/cargo-package.html and \
https://doc.rust-lang.org/cargo/commands/cargo-install.html for more \
information.";

pub struct Feature {
name: &'static str,
stability: Status,
Expand All @@ -416,9 +418,11 @@ impl Features {
features: &[String],
config: &Config,
warnings: &mut Vec<String>,
is_local: bool,
) -> CargoResult<Features> {
let mut ret = Features::default();
ret.nightly_features_allowed = config.nightly_features_allowed;
ret.is_local = is_local;
for feature in features {
ret.add(feature, config, warnings)?;
ret.activated.push(feature.to_string());
Expand All @@ -433,6 +437,7 @@ impl Features {
warnings: &mut Vec<String>,
) -> CargoResult<()> {
let nightly_features_allowed = self.nightly_features_allowed;
let is_local = self.is_local;
let (slot, feature) = match self.status(feature_name) {
Some(p) => p,
None => bail!("unknown cargo feature `{}`", feature_name),
Expand Down Expand Up @@ -460,15 +465,19 @@ impl Features {

match feature.stability {
Status::Stable => {
let warning = format!(
"the cargo feature `{}` has been stabilized in the {} \
release and is no longer necessary to be listed in the \
manifest\n {}",
feature_name,
feature.version,
see_docs()
);
warnings.push(warning);
// The user can't do anything about non-local packages.
// Warnings are usually suppressed, but just being cautious here.
if is_local {
let warning = format!(
"the cargo feature `{}` has been stabilized in the {} \
release and is no longer necessary to be listed in the \
manifest\n {}",
feature_name,
feature.version,
see_docs()
);
warnings.push(warning);
}
}
Status::Unstable if !nightly_features_allowed => bail!(
"the cargo feature `{}` requires a nightly version of \
Expand All @@ -490,13 +499,27 @@ impl Features {
}
}
}
Status::Removed => bail!(
"the cargo feature `{}` has been removed\n\
Remove the feature from Cargo.toml to remove this error.\n\
{}",
feature_name,
feature.docs
),
Status::Removed => {
let mut msg = format!(
"the cargo feature `{}` has been removed in the {} release\n\n",
feature_name, feature.version
);
if self.is_local {
drop(writeln!(
msg,
"Remove the feature from Cargo.toml to remove this error."
));
} else {
drop(writeln!(
msg,
"This package cannot be used with this version of Cargo, \
as the unstable feature `{}` is no longer supported.",
feature_name
));
}
drop(writeln!(msg, "{}", see_docs()));
bail!(msg);
}
}

*slot = true;
Expand All @@ -510,30 +533,50 @@ impl Features {

pub fn require(&self, feature: &Feature) -> CargoResult<()> {
if feature.is_enabled(self) {
Ok(())
} else {
let feature = feature.name.replace("_", "-");
let mut msg = format!("feature `{}` is required", feature);

if self.nightly_features_allowed {
let s = format!(
"\n\nconsider adding `cargo-features = [\"{0}\"]` \
to the manifest",
feature
);
msg.push_str(&s);
return Ok(());
}
let feature_name = feature.name.replace("_", "-");
let mut msg = format!(
"feature `{}` is required\n\
\n\
The package requires the Cargo feature called `{}`, but \
that feature is not stabilized in this version of Cargo ({}).\n\
",
feature_name,
feature_name,
crate::version(),
);

if self.nightly_features_allowed {
if self.is_local {
drop(writeln!(
msg,
"Consider adding `cargo-features = [\"{}\"]` \
to the top of Cargo.toml (above the [package] table) \
to tell Cargo you are opting in to use this unstable feature.",
feature_name
));
} else {
let s = format!(
"\n\n\
this Cargo does not support nightly features, but if you\n\
switch to nightly channel you can add\n\
`cargo-features = [\"{}\"]` to enable this feature",
feature
);
msg.push_str(&s);
drop(writeln!(
msg,
"Consider trying a more recent nightly release."
));
}
bail!("{}", msg);
} else {
drop(writeln!(
msg,
"Consider trying a newer version of Cargo \
(this may require the nightly release)."
));
}
drop(writeln!(
msg,
"See https://doc.rust-lang.org/nightly/cargo/{} for more information \
about the status of this feature.",
feature.docs
));

bail!("{}", msg);
}

pub fn is_enabled(&self, feature: &Feature) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct VersionInfo {

impl fmt::Display for VersionInfo {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "cargo {}.{}.{}", self.major, self.minor, self.patch)?;
write!(f, "{}.{}.{}", self.major, self.minor, self.patch)?;
if let Some(channel) = self.cfg_info.as_ref().map(|ci| &ci.release_channel) {
if channel != "stable" {
write!(f, "-{}", channel)?;
Expand Down Expand Up @@ -100,7 +100,7 @@ pub fn display_error(err: &Error, shell: &mut Shell) {
"we would appreciate a bug report: https://github.com/rust-lang/cargo/issues/",
),
);
drop(shell.note(format!("{}", version())));
drop(shell.note(format!("cargo {}", version())));
// Once backtraces are stabilized, this should print out a backtrace
// if it is available.
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult<
if let Some(user_agent) = &http.user_agent {
handle.useragent(user_agent)?;
} else {
handle.useragent(&version().to_string())?;
handle.useragent(&format!("cargo {}", version()))?;
}

fn to_ssl_version(s: &str) -> CargoResult<SslVersion> {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ impl TomlManifest {
// Parse features first so they will be available when parsing other parts of the TOML.
let empty = Vec::new();
let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty);
let features = Features::new(cargo_features, config, &mut warnings)?;
let features = Features::new(cargo_features, config, &mut warnings, source_id.is_path())?;

let project = me.project.as_ref().or_else(|| me.package.as_ref());
let project = project.ok_or_else(|| anyhow!("no `package` section found"))?;
Expand Down Expand Up @@ -1451,7 +1451,7 @@ impl TomlManifest {
let mut deps = Vec::new();
let empty = Vec::new();
let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty);
let features = Features::new(cargo_features, config, &mut warnings)?;
let features = Features::new(cargo_features, config, &mut warnings, source_id.is_path())?;

let (replace, patch) = {
let mut cx = Context {
Expand Down
Loading

0 comments on commit 01ddc25

Please sign in to comment.