testsuite: explictly update the mtime when needed

[ChrisDenton]

Currently some tests use Project::change_file with an empty content string just to update the modified time of the file e.g.:

cargo/tests/testsuite/freshness.rs

Line 558 in bdef274

p.change_file("src/lib.rs", "");

However, updating a zero length file to be zero length isn't guaranteed to change the modified time (it might be a no-op). Cargo's tests should explicitly set the modified time using the new File::set_times API.

There are at least a couple of ways this could be done. An explicit method could be added and used in place of calling change_file with an empty string. Or else change_file (or the underlying FileBuilder) could be updated to always set the modified time.

[ehuss]

updating a zero length file to be zero length isn't guaranteed to change the modified time

I'm not aware of any tests that do this. Can you point to some? They should always be modifying a file that already has content, which absolutely should change the mtime. (That is what the linked rebuild_tests_if_lib_changes does.)

I would be reluctant to have tests manually changing mtimes. They already do that a lot, but I wouldn't want them to do that more. I like having tests try to behave as close to the way real users would be doing things as possible.

If the change_file("src/lib.rs", "") isn't clear enough, a suggestion would be to something like change_file("src/lib.rs", "// modified") to make it clearer what its intent is.

[ChrisDenton]

Tests affected include: build_script::rebuild_only_on_explicit_paths, build_script_env::rerun_if_env_or_file_changes and freshness::bust_patched_dep.

If the change_file("src/lib.rs", "") isn't clear enough, a suggestion would be to something like change_file("src/lib.rs", "// modified") to make it clearer what its intent is.

Using "// modified" would work. The issue is not just that change_file("src/lib.rs", "") is unclear, but that it might be a no-op if the file exists and is already empty. Once you peel back all the abstractions, it's doing fs::write(&dst, contents), which is documented as a convenience wrapper for File::create and write_all. Neither of these functions promise to update the mtime if the file is already empty (e.g. File::create can just open a zero length file without modifying it and File::write on an empty string doesn't have to do anything).