Timing errors on some cargo tests on macOS

[ehuss]

Due to the mtime change in #5919, the following tests fail frequently (some are ~50% on my system) on macOS:

• bench::bench_twice_with_build_cmd
• bench::pass_through_command_line
• build_script::flags_go_into_tests
• build_script::fresh_builds_possible_with_link_libs
• build_script::fresh_builds_possible_with_multiple_metadata_overrides
• build_script::optional_build_dep_and_required_normal_dep
• build_script::rebuild_only_on_explicit_paths
• freshness::changing_bin_paths_common_target_features_caches_targets
• freshness::changing_lib_features_caches_targets
• freshness::changing_profiles_caches_targets
• freshness::dont_rebuild_based_on_plugins
• freshness::no_rebuild_transitive_target_deps
• freshness::no_rebuild_when_rename_dir
• freshness::path_dev_dep_registry_updates
• freshness::reuse_workspace_lib
• freshness::same_build_dir_cached_packages
• freshness::unused_optional_dep
• install::do_not_rebuilds_on_local_install
• install::workspace_uses_workspace_target_dir
• local_registry::simple
• metabuild::metabuild_fresh
• patch::add_ignored_patch
• patch::add_patch
• patch::new_major
• patch::nonexistent
• patch::patch_depends_on_another_patch
• patch::patch_git
• patch::patch_in_virtual
• patch::replace
• patch::unused
• patch::unused_git
• path::custom_target_no_rebuild
• path::deep_dependencies_trigger_rebuild
• path::nested_deps_recompile
• path::no_rebuild_dependency
• profile_targets::profile_selection_bench
• profile_targets::profile_selection_build
• profile_targets::profile_selection_build_all_targets
• profile_targets::profile_selection_build_all_targets_release
• profile_targets::profile_selection_build_release
• profile_targets::profile_selection_check_all_targets
• profile_targets::profile_selection_check_all_targets_release
• profile_targets::profile_selection_check_all_targets_test
• profile_targets::profile_selection_test
• profile_targets::profile_selection_test_release
• registry::bump_version_dont_update_registry
• required_features::bench_default_features
• run::run_from_executable_folder
• run::specify_name
• rustc::rustc_fingerprint
• rustflags::cfg_rustflags_normal_source
• rustflags::cfg_rustflags_precedence
• test::bin_does_not_rebuild_tests
• test::pass_through_command_line
• test::selective_testing
• tool_paths::custom_runner
• workspaces::dep_used_with_separate_features

I have a fix for some of them, I'll try to finish the rest tomorrow.

EDIT: Looks like it happens to a lot more than I first anticipated.

[ehuss]

@alexcrichton Do you have any opinion on how to proceed?

Essentially any test that does:

1. Create or modify a file.
2. Build something.
3. Build again.

Where step 2 finishes within 1 second, and step 3 expects the results from 2 to be fresh can fail.

Adding sleeps to all these tests doesn't sound great, and will be a problem that will likely plague future tests. The only real option I can think of is to hash the sources, but I think you've said in the past that you don't want to do that?

We could back out the mtime change, but it does seem disappointing that touching a file immediately after a build would not count as dirty.

[alexcrichton]

Hm so one thing I've always wanted to perfect in the past is just manually changing the mtimes on files. Could that be done to simulate a sleep perhaps?

[ehuss]

That's an interesting idea for some of the tests, but I would be reluctant to try to cover dozens of tests with that kind of logic.

One idea I had would be to add hashing only to files that were modified around the time the build starts (say within 2 seconds or so). That would likely avoid performance issues with hashing larger projects, and make cargo resilient against imprecise mtimes.

[alexcrichton]

It's true yeah that'd work! I'm hesitant though to implement such a large change just to fix a few tests. I can try to test out the changes for moving mtimes into the future when I get a chance, but something like hashing should likely be done as its own standalone feature (with its own discussion and whatnot)

[alexcrichton]

So here's my rough idea of how to solve this -- https://gist.github.com/alexcrichton/2cc9269dbd3061d36aa4e3b66ab60a56

The idea there is that let's try to make cargo's test suite actually consistent across all platforms, and let's stop dealing with filesystem weirdness and spurious failures on CI. To that end the test suite unconditionally moves everything (except the target directory) to a known time just before we ever run Cargo. This means that unless further action is taken, the test suite will assume that all files were created at the start of the build and creating files randomly in tests won't actually work.

We'd then add a method saying "dear project, please update this file" at which point it would store a new mtime for the file (something like the original mtime plus two seconds). When unconditionally setting the mtime of all files, registered and whitelisted files would be allowed future timestamps.

This, I hope, can result in:

• No more sleep is needed
• All the haphazard move_into_the_past can be removed. (I don't think they ever worked anyway)
• Nanosecond resolution shouldn't matter, we're always doing things on the granularity of seconds
• We can't make a mistake by creating a file in the middle of a test

That change definitely breaks tons of tests we'd need to fixup. We'd also need to add probably helper methods for "please update this file to these contents". My guess is that Cargo may also need a slightly tweak here and there as well.

Does that sound reasonable? Would you be willing to help tackle this?

[alexcrichton]

Actually I'm almost done finishing off a patch that does this, I'll post for review soon

[alexcrichton]

Ok, I've posted #5953