cargo doesn't rebuild either main or test binaries in a mixed language project

[yshui]

Demo repo: https://github.com/yshui/cargo-bug
This project uses a build script to compile a C file.

Steps to reproduce

1. Clone the repo. Run cargo build, and cargo test. Both commands should do some compiling, which is expected.

2. Change the C file, c/test.c. You can simply touch it.

3. Run cargo {build, test} again, in any order.

Expected behaviour

Both cargo build and cargo test rebuilds

Actual behaviour

Only the first run command rebuilds, the second one doesn't. Leaving either the test binary or the main binary out-of-date

[stale]

As there hasn't been any activity here in over 6 months I've marked this as stale and if no further activity happens for 7 days I will close it.

I'm a bot so this may be in error! If this issue should remain open, could someone (the author, a team member, or any interested party) please comment to that effect?

The team would be especially grateful if such a comment included details such as:

• Is this still relevant?
• If so, what is blocking it?
• Is it known what could be done to help move this forward?

Thank you for contributing!

(The cargo team is currently evaluating the use of Stale bot, and using #6035 as the tracking issue to gather feedback.)

If you're reading this comment from the distant future, fear not if this was closed automatically. If you believe it's still an issue please leave a comment and a team member can reopen this issue. Opening a new issue is also acceptable!

[briansmith]

I have noticed this too in ring. It seems lately when I modify only a C source file, the build script gets skipped. This used to work fine until sometime late last year. I suspect something changed where Cargo doesn't run the build.rs script as often as previously.

[ehuss]

@alexcrichton It looks like the fingerprints don't track build script results at all (src), which is a bit surprising to me. I think ideally the fingerprint would track the mtime of all of the libraries being linked against, but Cargo does not necessarily know the exact paths. Perhaps it could include the mtime of the time when the build script was run? Thoughts?

Here's a test that demonstrates this problem:

#[test]
fn dirty_both_lib_and_test() {
    // This tests that all artifacts that depend on the results of a build
    // script will get rebuilt when the build script reruns. It does the
    // following:
    //
    // 1. Project "foo" has a build script which will compile a small
    //    staticlib to link against.
    // 2. Build the library.
    // 3. Build the unit test. The staticlib intentionally has a bad value.
    // 4. Rewrite the staticlib with the correct value.
    // 5. Build the library again.
    // 6. Build the unit test.  <-- ERROR HERE. This does not get rebuilt!

    let slib = |n| {
        format!(
            r#"
            #include <stdint.h>

            int32_t doit() {{
                return {};
            }}
            "#,
            n
        )
    };

    let p = project()
        .file("Cargo.toml", r#"
            [project]
            name = "foo"
            version = "0.1.0"

            [build-dependencies]
            cc = { git = "https://github.com/alexcrichton/cc-rs.git" }
        "#)
        .file(
            "src/lib.rs",
            r#"
            extern "C" {
                fn doit() -> i32;
            }

            #[test]
            fn t1() {
                assert_eq!(unsafe { doit() }, 1, "doit assert failure");
            }
            "#,
        )
        .file(
            "build.rs",
            r#"
            use std::env;
            use std::path::PathBuf;

            fn main() {
                let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
                cc::Build::new()
                    .file("slib.c")
                    .out_dir(&out_dir)
                    .compile("slib");
                println!("cargo:rustc-link-lib=static=slib");
                println!("cargo:rustc-link-search={}", out_dir.display());
            }
            "#,
        )
        .file("slib.c", &slib(2))
        .build();

    p.cargo("build").run();

    p.cargo("test --lib")
        .with_status(101)
        .with_stdout_contains("[..]doit assert failure[..]")
        .run();

    p.change_file("slib.c", &slib(1));

    p.cargo("build").run();
    p.cargo("test --lib").run();  // ERROR: This fails when it should pass!
}

Note that this test won't be usable as a real Cargo test due to the cc dependency. I tried to get rustc to produce a linkable staticlib, but that seems to be impossible? With a custom panic handler, I get multiple definition of rust_begin_unwind on linux.

[alexcrichton]

Whelp that'd do it! @ehuss that exact test is why we started including transitive dependencies in the fingerprint calculations, and I think your example perfectly shows why we also need to do it for build scripts and why that comment is wrong.

I don't really remember why that comment was there and what breaks (if anything) if it's removed, but it definitely looks like we need to remove it. I think the bit of information we can track is that the fingerprint of a build script execution is the SystemTime that it finishes at. I don't think we need to track the actual outputs themselves (we're not trying to track things like system libraries), but rather if the build script re-executed we just assume that everything else needs to be rebuilt as well.

As for writing a test, in theory a staticlib should work... If you want to game out a fix though using cc I can work on a test afterwards that uses a staticlib (or I can help debug a staticlib test as well)

[ehuss]

Here's the issue I had with static linking. Create the following two files:

slib.rs:

#![no_std]
#[no_mangle]
pub extern "C" fn doit() -> i32 {
    return 2;
}

use core::panic::PanicInfo;
#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
    loop {}
}

foo.rs:

extern "C" {
    fn doit() -> i32;
}

fn main() {
    assert_eq!(unsafe { doit() }, 1, "doit assert failure");
}

Building with:

rustc --crate-type=staticlib -C panic=abort slib.rs
rustc foo.rs -l slib -L .

Here's the error that I get.

note: /home/eric/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-e39317eb74365d3c.rlib(std-e39317eb74365d3c.std.6yaz5a4r-cgu.0.rcgu.o): In function `rust_begin_unwind':
  /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858//src/libstd/panicking.rs:311: multiple definition of `rust_begin_unwind'
  ./libslib.a(slib.slib.3a1fbbbh-cgu.0.rcgu.o):slib.3a1fbbbh-cgu.0:(.text.rust_begin_unwind+0x0): first defined here
  collect2: error: ld returned 1 exit status

rust_begin_unwind visibility was changed in rust-lang/rust#52993. Strangely, this works on macos, but fails on linux. Any help is appreciated!

[alexcrichton]

Well that's a bummer. For reasons totally unrelated to Cargo that doesn't work, and it definitely shows a weak point of Rust in that it's actually surprisingly difficult to link two Rust libraries into the same binary if they're built independently (not in tandem through Cargo).

Believe it or not though if you remove #![no_std] and #[panic_handler], it should work (works for me on Linux at least)

[Twey]

What does cargo do here? Deduplicate the symbols?