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

rerun-if-changed doesn't handle past timestamps #10791

Open
ehuss opened this issue Jun 25, 2022 · 0 comments
Open

rerun-if-changed doesn't handle past timestamps #10791

ehuss opened this issue Jun 25, 2022 · 0 comments
Labels
A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@ehuss
Copy link
Contributor

ehuss commented Jun 25, 2022

Problem

Using rerun-if-changed to a file that changes, but changes with old timestamps, Cargo won't detect that it needs to run. For example:

  1. Extract a tarball with file foo with timestamp 1.
  2. cargo build with rerun-if-changed of foo. This marks the build script as timestamp 5.
  3. Extract a different tarball with file foo with timestamp 2.
  4. Run cargo build again, it doesn't pick up the change from timestamp 1 to 2 because both values are less than 5.

I would expect any change to the file to cause it to re-run the build script.

The real-world use case is in the rust LLVM build system. It extracts LLVM from a downloaded tarball. The rustc_llvm build script has a rerun-if-changed=/path/to/llvm-config. When rustbuild determines a new tarball needs to be downloaded, the rustc_llvm build script doesn't get the memo and doesn't get rerun. This can cause it to link against the wrong file.

Steps

The following is a test using Cargo's testsuite:

#[cargo_test]
fn past_rerun_if_changed() {
    // Tests that a file that changes with an old timestamp triggers a build
    // script to rerun. This can happen when extracting files from a tarball
    // which preserves the timestamps.
    let p = project()
        .file("src/lib.rs", "")
        .file(
            "build.rs",
            r#"
                fn main() {
                    println!("cargo:rerun-if-changed=somefile");
                }
            "#,
        )
        .file("somefile", "")
        .build();

    // Simulate extracting some tarball.
    let now = std::time::SystemTime::now();
    let time_a = FileTime::from_system_time(now - Duration::new(60 * 10, 0));
    let time_b = FileTime::from_system_time(now - Duration::new(60 * 5, 0));
    let somefile = p.root().join("somefile");
    filetime::set_file_times(&somefile, time_a, time_a).unwrap();
    p.cargo("check").run();
    // Simulate extracting a different tarball, but whose timestamp is still
    // in the past.
    p.change_file("somefile", "123");
    filetime::set_file_times(&somefile, time_b, time_b).unwrap();
    // FIXME: This should trigger a rebuild, but it currently does not.
    p.cargo("check -v")
        .with_stderr(
            "\
[COMPILING] foo [..]
[RUNNING] `[ROOT]/foo/target/debug/build/foo-[..]/build-script-build`
[RUNNING] `rustc --crate-name foo [..]
[FINISHED] [..]
",
        )
        .run();
}

Possible Solution(s)

A few possible options:

  • Hash the timestamp and file size of all rerun-if-changed files, and include that in the final hash (instead of just comparing the timestamps). This could potentially trigger rebuilds more often, though offhand I can't think of how it could happen.
  • Hash the contents of the files (such as (Option to) Fingerprint by file contents instead of mtime #6529). This might be very expensive so I'm not sure if it would be a good idea to do this (possibly make this optional?).

Notes

No response

Version

cargo 1.63.0-nightly (03a849043 2022-06-19)
@ehuss ehuss added C-bug Category: bug A-rebuild-detection Area: rebuild detection and fingerprinting A-build-scripts Area: build.rs scripts labels Jun 25, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 26, 2022
…-Simulacrum

Fix LLVM rebuild with download-ci-llvm.

This fixes an issue where updating a local checkout that includes a change in `src/version` causes a linking failure.

The cause is that the `rustc_llvm` build script uses `rerun-if-changed` of `llvm-config` to know if it needs to rerun. Cargo only compares the timestamp of the last time the build script to the file. However, extracting the tar files retains the timestamps in the tarball which may be some time in the past. Since `src/version` is included in the LLVM `.so` filename, `rustc` attempts to load the wrong shared library since the `rustc_llvm` build script doesn't rerun.

rust-lang/cargo#10791 contains a more detailed explanation.

The solution here is a hack which updates the timestamp of `llvm-config` to the current time when it is extracted.

This is a bit of a hack, but seems to be the best solution I can think of until rust-lang/cargo#10791 is fixed. There are likely several other situations where this is a problem (such as using system LLVM), and this isn't really a complete fix.

Note that apple platforms are not directly affected by this problem because they don't have a version in the dylib filename.

How to test this:

1. On a linux host, enable download-ci-llvm
2. Check out 7036449 (the commit just before the last version bump)
3. `./x.py build library/std`
4. Check out 5f015a2 (the commit that bumped the version)
5. `./x.py build library/std`

Fixes rust-lang#98495
@epage epage added the S-triage Status: This issue is waiting on initial triage. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

2 participants