Compare commits

...

9 Commits

Author SHA1 Message Date
LuuuXXX ebbe0e164a
Merge c166c14e0b into eee4ea2f5a 2024-04-27 03:27:35 +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
LuuuX c166c14e0b Move detection to be in the fingerprint code 2024-03-18 09:20:13 +08:00
LuuuX 02a0e0918e Add detect while using environment variable 2024-03-16 19:29:08 +08:00
LuuuX e7ca1af047 Add test to ensure fix it 2024-03-16 19:21:13 +08:00
7 changed files with 146 additions and 36 deletions

View File

@ -29,6 +29,7 @@ pub enum DirtyReason {
MetadataChanged,
ConfigSettingsChanged,
CompileKindChanged,
EnvConfigChanged,
LocalLengthsChanged,
PrecalculatedComponentsChanged {
old: String,
@ -172,6 +173,9 @@ impl DirtyReason {
DirtyReason::CompileKindChanged => {
s.dirty_because(unit, "the rustc compile kind changed")
}
DirtyReason::EnvConfigChanged => {
s.dirty_because(unit, "the environment variable changed")
}
DirtyReason::LocalLengthsChanged => {
s.dirty_because(unit, "the local lengths changed")?;
s.note(

View File

@ -420,8 +420,14 @@ pub fn prepare_target(
let mtime_on_use = build_runner.bcx.gctx.cli_unstable().mtime_on_use;
let dirty_reason = compare_old_fingerprint(unit, &loc, &*fingerprint, mtime_on_use, force);
let Some(dirty_reason) = dirty_reason else {
return Ok(Job::new_fresh());
let dirty_reason = match dirty_reason {
Some(dr) => dr,
None => {
let Some(dr) = env_config_modified(bcx.gctx) else {
return Ok(Job::new_fresh());
};
dr
}
};
// We're going to rebuild, so ensure the source of the crate passes all
@ -2234,3 +2240,17 @@ pub fn parse_rustc_dep_info(rustc_dep_info: &Path) -> CargoResult<RustcDepInfo>
Ok(ret)
}
}
/// Detects if environment variables from config `[env]` is newly seted.
fn env_config_modified(gctx: &crate::GlobalContext) -> Option<DirtyReason> {
for (key, value) in gctx.env_config().unwrap().iter() {
if !gctx.env().any(|(k, _)| k == key) {
continue;
}
if !value.is_force() && gctx.env().find(|(k, _)| k == key).is_some() {
return Some(DirtyReason::EnvConfigChanged);
}
}
None
}

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

@ -242,3 +242,32 @@ fn env_applied_to_target_info_discovery_rustc() {
.with_stderr_contains("MAIN ENV_TEST:from-env")
.run();
}
#[cargo_test]
fn env_reset() {
let p = project()
.file("Cargo.toml", &basic_bin_manifest("foo"))
.file(
"src/main.rs",
r#"
use std::env;
fn main() {
println!( "{}", env!("ENV_TEST") );
}
"#,
)
.file(
".cargo/config.toml",
r#"
[env]
ENV_TEST = "from-config"
"#,
)
.build();
p.cargo("run").with_stdout_contains("from-config").run();
p.cargo("run")
.env("ENV_TEST", "from-env")
.with_stdout_contains("from-env")
.run();
}

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();
}