Compare commits

...

7 Commits

Author SHA1 Message Date
heisen 812323e8c1
Merge 3134866c5b into cf7b3c4cf3 2024-05-03 03:23:48 +02:00
bors cf7b3c4cf3 Auto merge of #13852 - Muscraft:revert-seperating-lints, r=epage
fix(lints): Prevent inheritance from bring exposed for published packages

#13843 demonstrated a regression caused by #13801, where we started to keep `[lints]` and `[workspace.lints]` separate, and not truly resolve `[lints]`. This was a nice thing to have and made it easier to tell when a lint came from a workspace. The downside of doing so is the lints table would not get resolved when vendoring or publishing.

To fix this issue, I reverted the change for keeping `[lints]` and `[workspace.lints]` separate and modified how cargo's linting system figures out where a lint is coming from. Due to this change, we no longer specify that a lint was set by `[workspace.lints]`, only `[lints]`. It is true that a lint level is set by `[lints]` always, as it would've had to specify the lint outright or specify that it was inheriting it, seeing that, I do not think this is a regression in diagnostic quality. I still manage to keep the ability to render a lint's location in the workspace's manifest when running ` analyze_cargo_lints_table`, which I am pleased about.
2024-05-03 00:49:36 +00:00
Scott Schafer 6c2334613c
fix(lints): Prevent inheritance from bring exposed for published packages 2024-05-02 15:37:53 -06:00
bors 97181c67e1 Auto merge of #13851 - weihanglo:macos, r=epage
refactor: remove unnecessary branch for link binary on macOS
2024-05-02 20:11:50 +00:00
Weihang Lo f8aead9338
refactor: remove unnecessary branch for link binary on macOS
The other workaround branch should have covered that.
2024-05-02 16:04:26 -04:00
heisen-li 3134866c5b [doc]:Fix link errors 2024-04-29 10:22:50 +08:00
heisen-li 5655e3e0d1 [doc]:fix project to package 2024-04-28 20:03:56 +08:00
14 changed files with 141 additions and 244 deletions

View File

@ -1286,10 +1286,6 @@ pub trait TestEnv: Sized {
.env_remove("USER") // not set on some rust-lang docker images
.env_remove("XDG_CONFIG_HOME") // see #2345
.env_remove("OUT_DIR"); // see #13204
if cfg!(target_os = "macos") {
// Work-around a bug in macOS 10.15, see `link_or_copy` for details.
self = self.env("__CARGO_COPY_DONT_LINK_DO_NOT_USE_THIS", "1");
}
if cfg!(windows) {
self = self.env("USERPROFILE", paths::home());
}

View File

@ -565,26 +565,18 @@ fn _link_or_copy(src: &Path, dst: &Path) -> Result<()> {
src
};
symlink(src, dst)
} else if env::var_os("__CARGO_COPY_DONT_LINK_DO_NOT_USE_THIS").is_some() {
// This is a work-around for a bug in macOS 10.15. When running on
// APFS, there seems to be a strange race condition with
// Gatekeeper where it will forcefully kill a process launched via
// `cargo run` with SIGKILL. Copying seems to avoid the problem.
// This shouldn't affect anyone except Cargo's test suite because
// it is very rare, and only seems to happen under heavy load and
// rapidly creating lots of executables and running them.
// See https://github.com/rust-lang/cargo/issues/7821 for the
// gory details.
fs::copy(src, dst).map(|_| ())
} else {
if cfg!(target_os = "macos") {
// This is a work-around for a bug on macos. There seems to be a race condition
// with APFS when hard-linking binaries. Gatekeeper does not have signing or
// hash information stored in kernel when running the process. Therefore killing it.
// This problem does not appear when copying files as kernel has time to process it.
// Note that: fs::copy on macos is using CopyOnWrite (syscall fclonefileat) which should be
// as fast as hardlinking.
// See https://github.com/rust-lang/cargo/issues/10060 for the details
// There seems to be a race condition with APFS when hard-linking
// binaries. Gatekeeper does not have signing or hash information
// stored in kernel when running the process. Therefore killing it.
// This problem does not appear when copying files as kernel has
// time to process it. Note that: fs::copy on macos is using
// CopyOnWrite (syscall fclonefileat) which should be as fast as
// hardlinking. See these issues for the details:
//
// * https://github.com/rust-lang/cargo/issues/7821
// * https://github.com/rust-lang/cargo/issues/10060
fs::copy(src, dst).map_or_else(
|e| {
if e.raw_os_error()

View File

@ -1149,25 +1149,11 @@ impl<'gctx> Workspace<'gctx> {
}
pub fn emit_warnings(&self) -> CargoResult<()> {
let ws_lints = self
.root_maybe()
.workspace_config()
.inheritable()
.and_then(|i| i.lints().ok())
.unwrap_or_default();
let ws_cargo_lints = ws_lints
.get("cargo")
.cloned()
.unwrap_or_default()
.into_iter()
.collect();
for (path, maybe_pkg) in &self.packages.packages {
let path = path.join("Cargo.toml");
if let MaybePackage::Package(pkg) = maybe_pkg {
if self.gctx.cli_unstable().cargo_lints {
self.emit_lints(pkg, &path, &ws_cargo_lints)?
self.emit_lints(pkg, &path)?
}
}
let warnings = match maybe_pkg {
@ -1195,12 +1181,7 @@ impl<'gctx> Workspace<'gctx> {
Ok(())
}
pub fn emit_lints(
&self,
pkg: &Package,
path: &Path,
ws_cargo_lints: &manifest::TomlToolLints,
) -> CargoResult<()> {
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
let mut error_count = 0;
let toml_lints = pkg
.manifest()
@ -1214,16 +1195,6 @@ impl<'gctx> Workspace<'gctx> {
.cloned()
.unwrap_or(manifest::TomlToolLints::default());
// We should only be using workspace lints if the `[lints]` table is
// present in the manifest, and `workspace` is set to `true`
let ws_cargo_lints = pkg
.manifest()
.resolved_toml()
.lints
.as_ref()
.is_some_and(|l| l.workspace)
.then(|| ws_cargo_lints);
let ws_contents = match self.root_maybe() {
MaybePackage::Package(pkg) => pkg.manifest().contents(),
MaybePackage::Virtual(v) => v.contents(),
@ -1238,36 +1209,14 @@ impl<'gctx> Workspace<'gctx> {
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
ws_contents,
ws_document,
self.root_manifest(),
self.gctx,
)?;
check_im_a_teapot(
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_implicit_features(
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
unused_dependencies(
pkg,
&path,
&cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
check_implicit_features(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
unused_dependencies(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
if error_count > 0 {
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
"encountered {error_count} errors(s) while running lints"

View File

@ -19,7 +19,6 @@ pub fn analyze_cargo_lints_table(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
ws_contents: &str,
ws_document: &ImDocument<String>,
ws_path: &Path,
@ -30,20 +29,11 @@ pub fn analyze_cargo_lints_table(
let manifest_path = rel_cwd_manifest_path(path, gctx);
let ws_path = rel_cwd_manifest_path(ws_path, gctx);
let mut unknown_lints = Vec::new();
for (lint_name, specified_in) in pkg_lints
.keys()
.map(|name| (name, SpecifiedIn::Package))
.chain(
ws_lints
.map(|l| l.keys())
.unwrap_or_default()
.map(|name| (name, SpecifiedIn::Workspace)),
)
{
for lint_name in pkg_lints.keys().map(|name| name) {
let Some((name, default_level, edition_lint_opts, feature_gate)) =
find_lint_or_group(lint_name)
else {
unknown_lints.push((lint_name, specified_in));
unknown_lints.push(lint_name);
continue;
};
@ -52,7 +42,6 @@ pub fn analyze_cargo_lints_table(
*default_level,
*edition_lint_opts,
pkg_lints,
ws_lints,
manifest.edition(),
);
@ -66,7 +55,6 @@ pub fn analyze_cargo_lints_table(
verify_feature_enabled(
name,
feature_gate,
reason,
manifest,
&manifest_path,
ws_contents,
@ -83,7 +71,6 @@ pub fn analyze_cargo_lints_table(
manifest,
&manifest_path,
pkg_lints,
ws_lints,
ws_contents,
ws_document,
&ws_path,
@ -130,7 +117,6 @@ fn find_lint_or_group<'a>(
fn verify_feature_enabled(
lint_name: &str,
feature_gate: &Feature,
reason: LintLevelReason,
manifest: &Manifest,
manifest_path: &str,
ws_contents: &str,
@ -151,55 +137,55 @@ fn verify_feature_enabled(
"consider adding `cargo-features = [\"{}\"]` to the top of the manifest",
dash_feature_name
);
let message = match reason {
LintLevelReason::Package => {
let span =
get_span(manifest.document(), &["lints", "cargo", lint_name], false).unwrap();
Level::Error
.title(&title)
.snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(Level::Error.span(span).label(&label))
.fold(true),
)
.footer(Level::Help.title(&help))
}
LintLevelReason::Workspace => {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
let message = if let Some(span) =
get_span(manifest.document(), &["lints", "cargo", lint_name], false)
{
Level::Error
.title(&title)
.snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(Level::Error.span(span).label(&label))
.fold(true),
)
.unwrap();
let inherit_span_key =
get_span(manifest.document(), &["lints", "workspace"], false).unwrap();
let inherit_span_value =
get_span(manifest.document(), &["lints", "workspace"], true).unwrap();
.footer(Level::Help.title(&help))
} else {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
)
.expect(&format!(
"could not find `cargo::{lint_name}` in `[lints]`, or `[workspace.lints]` "
));
Level::Error
.title(&title)
.snippet(
Snippet::source(ws_contents)
.origin(&ws_path)
.annotation(Level::Error.span(lint_span).label(&label))
.fold(true),
)
.footer(
Level::Note.title(&second_title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(
Level::Note
.span(inherit_span_key.start..inherit_span_value.end),
)
.fold(true),
),
)
.footer(Level::Help.title(&help))
}
_ => unreachable!("LintLevelReason should be one that is user specified"),
let inherited_note = if let (Some(inherit_span_key), Some(inherit_span_value)) = (
get_span(manifest.document(), &["lints", "workspace"], false),
get_span(manifest.document(), &["lints", "workspace"], true),
) {
Level::Note.title(&second_title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(
Level::Note.span(inherit_span_key.start..inherit_span_value.end),
)
.fold(true),
)
} else {
Level::Note.title(&second_title)
};
Level::Error
.title(&title)
.snippet(
Snippet::source(ws_contents)
.origin(&ws_path)
.annotation(Level::Error.span(lint_span).label(&label))
.fold(true),
)
.footer(inherited_note)
.footer(Level::Help.title(&help))
};
*error_count += 1;
@ -287,7 +273,6 @@ impl Lint {
pub fn level(
&self,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
edition: Edition,
unstable_features: &Features,
) -> (LintLevel, LintLevelReason) {
@ -310,7 +295,6 @@ impl Lint {
g.default_level,
g.edition_lint_opts,
pkg_lints,
ws_lints,
edition,
),
)
@ -322,7 +306,6 @@ impl Lint {
self.default_level,
self.edition_lint_opts,
pkg_lints,
ws_lints,
edition,
),
)))
@ -378,7 +361,6 @@ pub enum LintLevelReason {
Default,
Edition(Edition),
Package,
Workspace,
}
impl Display for LintLevelReason {
@ -387,7 +369,6 @@ impl Display for LintLevelReason {
LintLevelReason::Default => write!(f, "by default"),
LintLevelReason::Edition(edition) => write!(f, "in edition {}", edition),
LintLevelReason::Package => write!(f, "in `[lints]`"),
LintLevelReason::Workspace => write!(f, "in `[workspace.lints]`"),
}
}
}
@ -398,22 +379,15 @@ impl LintLevelReason {
LintLevelReason::Default => false,
LintLevelReason::Edition(_) => false,
LintLevelReason::Package => true,
LintLevelReason::Workspace => true,
}
}
}
enum SpecifiedIn {
Package,
Workspace,
}
fn level_priority(
name: &str,
default_level: LintLevel,
edition_lint_opts: Option<(Edition, LintLevel)>,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
edition: Edition,
) -> (LintLevel, LintLevelReason, i8) {
let (unspecified_level, reason) = if let Some(level) = edition_lint_opts
@ -436,12 +410,6 @@ fn level_priority(
LintLevelReason::Package,
defined_level.priority(),
)
} else if let Some(defined_level) = ws_lints.and_then(|l| l.get(name)) {
(
defined_level.level().into(),
LintLevelReason::Workspace,
defined_level.priority(),
)
} else {
(unspecified_level, reason, 0)
}
@ -460,17 +428,12 @@ pub fn check_im_a_teapot(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let manifest = pkg.manifest();
let (lint_level, reason) = IM_A_TEAPOT.level(
pkg_lints,
ws_lints,
manifest.edition(),
manifest.unstable_features(),
);
let (lint_level, reason) =
IM_A_TEAPOT.level(pkg_lints, manifest.edition(), manifest.unstable_features());
if lint_level == LintLevel::Allow {
return Ok(());
@ -534,7 +497,6 @@ pub fn check_implicit_features(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
@ -547,7 +509,7 @@ pub fn check_implicit_features(
}
let (lint_level, reason) =
IMPLICIT_FEATURES.level(pkg_lints, ws_lints, edition, manifest.unstable_features());
IMPLICIT_FEATURES.level(pkg_lints, edition, manifest.unstable_features());
if lint_level == LintLevel::Allow {
return Ok(());
}
@ -611,30 +573,25 @@ const UNKNOWN_LINTS: Lint = Lint {
};
fn output_unknown_lints(
unknown_lints: Vec<(&String, SpecifiedIn)>,
unknown_lints: Vec<&String>,
manifest: &Manifest,
manifest_path: &str,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
ws_contents: &str,
ws_document: &ImDocument<String>,
ws_path: &str,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let (lint_level, reason) = UNKNOWN_LINTS.level(
pkg_lints,
ws_lints,
manifest.edition(),
manifest.unstable_features(),
);
let (lint_level, reason) =
UNKNOWN_LINTS.level(pkg_lints, manifest.edition(), manifest.unstable_features());
if lint_level == LintLevel::Allow {
return Ok(());
}
let level = lint_level.to_diagnostic_level();
let mut emitted_source = None;
for (lint_name, specified_in) in unknown_lints {
for lint_name in unknown_lints {
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
*error_count += 1;
}
@ -651,50 +608,50 @@ fn output_unknown_lints(
let help =
matching.map(|(name, kind)| format!("there is a {kind} with a similar name: `{name}`"));
let mut message = match specified_in {
SpecifiedIn::Package => {
let span =
get_span(manifest.document(), &["lints", "cargo", lint_name], false).unwrap();
let mut message = if let Some(span) =
get_span(manifest.document(), &["lints", "cargo", lint_name], false)
{
level.title(&title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(Level::Error.span(span))
.fold(true),
)
} else {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
)
.expect(&format!(
"could not find `cargo::{lint_name}` in `[lints]`, or `[workspace.lints]` "
));
level.title(&title).snippet(
let inherited_note = if let (Some(inherit_span_key), Some(inherit_span_value)) = (
get_span(manifest.document(), &["lints", "workspace"], false),
get_span(manifest.document(), &["lints", "workspace"], true),
) {
Level::Note.title(&second_title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(Level::Error.span(span))
.annotation(
Level::Note.span(inherit_span_key.start..inherit_span_value.end),
)
.fold(true),
)
}
SpecifiedIn::Workspace => {
let lint_span = get_span(
ws_document,
&["workspace", "lints", "cargo", lint_name],
false,
)
.unwrap();
let inherit_span_key =
get_span(manifest.document(), &["lints", "workspace"], false).unwrap();
let inherit_span_value =
get_span(manifest.document(), &["lints", "workspace"], true).unwrap();
} else {
Level::Note.title(&second_title)
};
level
.title(&title)
.snippet(
Snippet::source(ws_contents)
.origin(&ws_path)
.annotation(Level::Error.span(lint_span))
.fold(true),
)
.footer(
Level::Note.title(&second_title).snippet(
Snippet::source(manifest.contents())
.origin(&manifest_path)
.annotation(
Level::Note
.span(inherit_span_key.start..inherit_span_value.end),
)
.fold(true),
),
)
}
level
.title(&title)
.snippet(
Snippet::source(ws_contents)
.origin(&ws_path)
.annotation(Level::Error.span(lint_span))
.fold(true),
)
.footer(inherited_note)
};
if emitted_source.is_none() {
@ -728,7 +685,6 @@ pub fn unused_dependencies(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
@ -739,12 +695,8 @@ pub fn unused_dependencies(
return Ok(());
}
let (lint_level, reason) = UNUSED_OPTIONAL_DEPENDENCY.level(
pkg_lints,
ws_lints,
edition,
manifest.unstable_features(),
);
let (lint_level, reason) =
UNUSED_OPTIONAL_DEPENDENCY.level(pkg_lints, edition, manifest.unstable_features());
if lint_level == LintLevel::Allow {
return Ok(());
}

View File

@ -492,7 +492,15 @@ fn resolve_toml(
}
resolved_toml.target = (!resolved_target.is_empty()).then_some(resolved_target);
resolved_toml.lints = original_toml.lints.clone();
let resolved_lints = original_toml
.lints
.clone()
.map(|value| lints_inherit_with(value, || inherit()?.lints()))
.transpose()?;
resolved_toml.lints = resolved_lints.map(|lints| manifest::InheritableLints {
workspace: false,
lints,
});
resolved_toml.badges = original_toml.badges.clone();
} else {
@ -1336,18 +1344,18 @@ fn to_real_manifest(
}
}
let resolved_lints = resolved_toml
.lints
.clone()
.map(|value| {
lints_inherit_with(value, || {
load_inheritable_fields(gctx, manifest_file, &workspace_config)?.lints()
})
})
.transpose()?;
verify_lints(resolved_lints.as_ref(), gctx, warnings)?;
let rustflags = lints_to_rustflags(&resolved_lints.unwrap_or_default());
verify_lints(
resolved_toml.resolved_lints().expect("previously resolved"),
gctx,
warnings,
)?;
let default = manifest::TomlLints::default();
let rustflags = lints_to_rustflags(
resolved_toml
.resolved_lints()
.expect("previously resolved")
.unwrap_or(&default),
);
let metadata = ManifestMetadata {
description: resolved_package

View File

@ -8,10 +8,10 @@
* [Cargo Guide](guide/index.md)
* [Why Cargo Exists](guide/why-cargo-exists.md)
* [Creating a New Package](guide/creating-a-new-project.md)
* [Working on an Existing Package](guide/working-on-an-existing-project.md)
* [Creating a New Package](guide/creating-a-new-package.md)
* [Working on an Existing Package](guide/working-on-an-existing-package.md)
* [Dependencies](guide/dependencies.md)
* [Package Layout](guide/project-layout.md)
* [Package Layout](guide/package-layout.md)
* [Cargo.toml vs Cargo.lock](guide/cargo-toml-vs-cargo-lock.md)
* [Tests](guide/tests.md)
* [Continuous Integration](guide/continuous-integration.md)

View File

@ -254,7 +254,7 @@ manifest is located. (Compare with [*package root*](#package).)
[cargo-unstable]: ../reference/unstable.md
[config option]: ../reference/config.md
[crates.io]: https://crates.io/
[directory layout]: ../guide/project-layout.md
[directory layout]: ../guide/package-layout.md
[edition guide]: ../../edition-guide/index.html
[edition-field]: ../reference/manifest.md#the-edition-field
[environment variable]: ../reference/environment-variables.md

View File

@ -4,10 +4,10 @@ This guide will give you all that you need to know about how to use Cargo to
develop Rust packages.
* [Why Cargo Exists](why-cargo-exists.md)
* [Creating a New Package](creating-a-new-project.md)
* [Working on an Existing Cargo Package](working-on-an-existing-project.md)
* [Creating a New Package](creating-a-new-package.md)
* [Working on an Existing Cargo Package](working-on-an-existing-package.md)
* [Dependencies](dependencies.md)
* [Package Layout](project-layout.md)
* [Package Layout](package-layout.md)
* [Cargo.toml vs Cargo.lock](cargo-toml-vs-cargo-lock.md)
* [Tests](tests.md)
* [Continuous Integration](continuous-integration.md)

View File

@ -382,7 +382,7 @@ autobins = false
[documentation examples]: ../../rustdoc/documentation-tests.html
[features]: features.md
[nightly channel]: ../../book/appendix-07-nightly-rust.html
[package layout]: ../guide/project-layout.md
[package layout]: ../guide/package-layout.md
[package-edition]: manifest.md#the-edition-field
[proc-macro-reference]: ../../reference/procedural-macros.html
[procedural macro]: ../../book/ch19-06-macros.html

View File

@ -654,7 +654,7 @@ more detail.
<script>
(function() {
var fragments = {
"#the-project-layout": "../guide/project-layout.html",
"#the-package-layout": "../guide/package-layout.html",
"#examples": "cargo-targets.html#examples",
"#tests": "cargo-targets.html#tests",
"#integration-tests": "cargo-targets.html#integration-tests",

View File

@ -975,7 +975,7 @@ error: `im_a_teapot` is specified
13 | im-a-teapot = true
| ^^^^^^^^^^^^^^^^^^
|
= note: `cargo::im_a_teapot` is set to `forbid` in `[workspace.lints]`
= note: `cargo::im_a_teapot` is set to `forbid` in `[lints]`
",
)
.run();