Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings missing on second compilation on OSX #80751

Closed
davidlattimore opened this issue Jan 6, 2021 · 3 comments
Closed

Warnings missing on second compilation on OSX #80751

davidlattimore opened this issue Jan 6, 2021 · 3 comments
Labels
C-bug Category: This is a bug.

Comments

@davidlattimore
Copy link
Contributor

I don't have access to a Mac, but the tests for one of my projects (Evcxr) run on Travis on Mac and I've observed warnings not showing up on the second run of cargo check.

I do the following:

  • Write some code
  • Run cargo check
  • Modify the code so that there should be a warning
  • Run cargo check again

On the second run, the warning that I expect doesn't appear. It's intermittent though. If I run the test 10 times, the warning is typically missing in 7-10 of those runs.

I've tried a range of Rust versions from 1.46 to a recent nightly (2021-01-05).

I've reproduced it for two warnings so far:

  • Variable not used
  • Variable does not need to be mutable

The problem doesn't seem to happen at all on Linux, nor on Windows (which I also run via Travis).

Turning off incremental compilation doesn't seem to make any difference. It also still seems to reproduce if using main.rs instead of lib.rs.

It feels like if this really happened on all Macs, it would have been noticed, so perhaps there's something difference with the setup on Travis.

The following is a test that reproduces the problem. It requires tempfile = "3.1.0" in it's Cargo.toml dependencies.

    #[test]
    fn check_unused_var9() {
        let mut failures = 0;
        for _ in 0..10 {
            let tempdir = tempfile::tempdir().unwrap();
            let src_dir = tempdir.path().join("src");
            std::fs::create_dir_all(&src_dir).unwrap();
            std::fs::write(
                tempdir.path().join("Cargo.toml"),
                r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2018"
"#,
            )
            .unwrap();

            std::fs::write(
                src_dir.join("lib.rs"),
                r#"
pub fn bar() {}
"#,
            )
            .unwrap();

            std::process::Command::new("cargo")
                .current_dir(tempdir.path())
                .arg("check")
                .status()
                .unwrap();

            std::fs::write(
                src_dir.join("lib.rs"),
                r#"
        pub fn bar() {}
        pub fn foo() {
            let mut s = String::new();
        }
"#,
            )
            .unwrap();

            let cargo_output = std::process::Command::new("cargo")
                .current_dir(tempdir.path())
                .arg("check")
                .output()
                .unwrap();

            let stderr = String::from_utf8_lossy(&cargo_output.stderr);
            if !stderr.contains("unused variable") {
                println!("{}", stderr);
                failures += 1;
            }
        }
        assert_eq!(failures, 0);
    }

You can see the log of one recent run here. Full source of the tests in that run is here.

@davidlattimore davidlattimore added the C-bug Category: This is a bug. label Jan 6, 2021
@GroteGnoom
Copy link

GroteGnoom commented Jan 6, 2021

It feels like if this really happened on all Macs, it would have been noticed, so perhaps there's something difference with the setup on Travis.

Yes, on a MacOS 10.15.5 with rust 1.49.0 I can't reproduce the issue after 5 runs of 10 checks.

@ehuss
Copy link
Contributor

ehuss commented Jan 6, 2021

I believe Travis is still using HPFS on macOS, which has an mtime resolution of one second. For various reasons (see rust-lang/cargo#5918 (comment)), Cargo is unable to treat that as a changed file, so it cannot detect the file change. You will need to add a 1 second sleep in-between invocations to allow it to detect the change. Hashing support was started in #75594, but it seems to have stalled.

@davidlattimore
Copy link
Contributor Author

Ah, thanks @ehuss, that makes a lot of sense. Thanks @GroteGnoom for checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants