Compare commits

...

7 Commits

Author SHA1 Message Date
Jubilee ccfc47232d
Merge 71ff28a31d into eee4ea2f5a 2024-04-27 03:28:17 +02:00
bors eee4ea2f5a Auto merge of #13812 - Muscraft:dont-always-inherit-workspace-lints, r=epage
fix(cargo-lints): Don't always inherit workspace lints

When working on changes for #13805, I noticed that we always passed the contents of `[workspace.lints.cargo]` into the currently implemented lints,  even if `[lints]` was not specified or did not contain `workspace = true`. This PR makes it so we only pass in the workspace cargo lints if `[lints]` contains `workspace = true`.

You can verify this change by looking at the first commit, where I added a test showing the current behavior, and looking at the second commit and seeing the test output no longer shows a warning about specifying `im-a-teapot`.
2024-04-27 00:56:12 +00:00
bors c4e19cc890 Auto merge of #13811 - ehuss:remove-sleep-test, r=weihanglo
Update SleepTraker returns_in_order unit test

This updates the `returns_in_order` SleepTracker unit test so that it is not so sensitive to how fast the system is running. Previously it assumed that the function calls would take less than a millisecond to finish, but that is not a valid assumption for a slow-running system.

I have changed it to simplify the test, with the assumption that it takes less than 30 seconds for it to run, which should have a safety margin of a few orders of magnitude.
2024-04-26 23:30:40 +00:00
Eric Huss 06fb65e753 Update SleepTraker returns_in_order unit test 2024-04-26 16:02:09 -07:00
Scott Schafer cf197fc499
fix(cargo-lints): Don't always inherit workspace lints 2024-04-26 16:37:41 -06:00
Scott Schafer c3b104e11e
test(cargo-lints): Show workspace lints always inherited 2024-04-26 16:26:36 -06:00
Jubilee Young 71ff28a31d Document variance in strip
Two variances in strip will remain, even after MSVC shenanigans settle:
- Its default based on profile.
- That it can sometimes make backtraces nonsensical.

In my experience, people will complain to me if backtraces are fucked up,
but sometimes it was their own decision to make the backtrace really bad.
Inform them of this. Hopefully this also encourages people to report if
their backtraces regress in some platform-dependent way without enabling
`strip = "symbols"`, which we would want to know about.
2024-03-31 19:42:49 -07:00
5 changed files with 95 additions and 36 deletions

View File

@ -1147,11 +1147,26 @@ 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()
.map(|(k, v)| (k.replace('-', "_"), v))
.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)?
self.emit_lints(pkg, &path, &ws_cargo_lints)?
}
}
let warnings = match maybe_pkg {
@ -1179,22 +1194,12 @@ impl<'gctx> Workspace<'gctx> {
Ok(())
}
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> 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()
.map(|(k, v)| (k.replace('-', "_"), v))
.collect();
pub fn emit_lints(
&self,
pkg: &Package,
path: &Path,
ws_cargo_lints: &manifest::TomlToolLints,
) -> CargoResult<()> {
let mut error_count = 0;
let toml_lints = pkg
.manifest()
@ -1212,11 +1217,21 @@ impl<'gctx> Workspace<'gctx> {
.map(|(name, lint)| (name.replace('-', "_"), lint))
.collect();
// 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);
check_im_a_teapot(
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
@ -1224,7 +1239,7 @@ impl<'gctx> Workspace<'gctx> {
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;
@ -1232,7 +1247,7 @@ impl<'gctx> Workspace<'gctx> {
pkg,
&path,
&normalized_lints,
&ws_cargo_lints,
ws_cargo_lints,
&mut error_count,
self.gctx,
)?;

View File

@ -88,7 +88,7 @@ impl Lint {
pub fn level(
&self,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
edition: Edition,
) -> (LintLevel, LintLevelReason) {
self.groups
@ -188,7 +188,7 @@ fn level_priority(
default_level: LintLevel,
edition_lint_opts: Option<(Edition, LintLevel)>,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
edition: Edition,
) -> (LintLevel, LintLevelReason, i8) {
let (unspecified_level, reason) = if let Some(level) = edition_lint_opts
@ -211,7 +211,7 @@ fn level_priority(
LintLevelReason::Package,
defined_level.priority(),
)
} else if let Some(defined_level) = ws_lints.get(name) {
} else if let Some(defined_level) = ws_lints.and_then(|l| l.get(name)) {
(
defined_level.level().into(),
LintLevelReason::Workspace,
@ -234,7 +234,7 @@ pub fn check_im_a_teapot(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
@ -306,7 +306,7 @@ pub fn check_implicit_features(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
@ -390,7 +390,7 @@ pub fn unused_dependencies(
pkg: &Package,
path: &Path,
pkg_lints: &TomlToolLints,
ws_lints: &TomlToolLints,
ws_lints: Option<&TomlToolLints>,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {

View File

@ -90,13 +90,15 @@ impl<T> SleepTracker<T> {
#[test]
fn returns_in_order() {
let mut s = SleepTracker::new();
s.push(3, 3);
s.push(30_000, 30_000);
s.push(1, 1);
s.push(6, 6);
s.push(5, 5);
s.push(2, 2);
s.push(10000, 10000);
assert_eq!(s.len(), 6);
std::thread::sleep(Duration::from_millis(100));
assert_eq!(s.to_retry(), &[1, 2, 3, 5, 6]);
assert_eq!(s.len(), 2);
std::thread::sleep(Duration::from_millis(2));
assert_eq!(s.to_retry(), &[1]);
assert!(s.to_retry().is_empty());
let next = s.time_to_next().expect("should be next");
assert!(
next < Duration::from_millis(30_000),
"{next:?} should be less than 30s"
);
}

View File

@ -117,7 +117,9 @@ strip = "debuginfo"
```
Possible string values of `strip` are `"none"`, `"debuginfo"`, and `"symbols"`.
The default is `"none"`.
The default is `"none"` by default, and `"debuginfo"` in release profile.
Be aware that stripping `"symbols"` may make backtraces incomprehensible,
depending on the platform.
You can also configure this option with the boolean values `true` or `false`.
`strip = true` is equivalent to `strip = "symbols"`. `strip = false` is
@ -293,7 +295,7 @@ The default settings for the `release` profile are:
opt-level = 3
debug = false
split-debuginfo = '...' # Platform-specific.
strip = "none"
strip = "debuginfo"
debug-assertions = false
overflow-checks = false
lto = false

View File

@ -982,3 +982,43 @@ error: `im_a_teapot` is specified
)
.run();
}
#[cargo_test]
fn dont_always_inherit_workspace_lints() {
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["foo"]
[workspace.lints.cargo]
im-a-teapot = "warn"
"#,
)
.file(
"foo/Cargo.toml",
r#"
cargo-features = ["test-dummy-unstable"]
[package]
name = "foo"
version = "0.0.1"
edition = "2015"
authors = []
im-a-teapot = true
"#,
)
.file("foo/src/lib.rs", "")
.build();
p.cargo("check -Zcargo-lints")
.masquerade_as_nightly_cargo(&["cargo-lints"])
.with_stderr(
"\
[CHECKING] foo v0.0.1 ([CWD]/foo)
[FINISHED] [..]
",
)
.run();
}