Skip to content

Conversation

@gilescope
Copy link
Contributor

@gilescope gilescope commented May 23, 2024

AKA 'make docker great again'.

Fixes #12060 (as coded here: master...kornelski:cargo:nanotime )

Let's cut out needless rebuilds. (Thank you @kornelski for suggesting this)

If we are concerned about backward compatibility, we can put this behind a nightly only flag so this slight behaviour change is not the default.

@rustbot
Copy link
Collaborator

rustbot commented May 23, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-rebuild-detection Area: rebuild detection and fingerprinting S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2024
@weihanglo weihanglo added the T-cargo Team: Cargo label May 23, 2024
@weihanglo
Copy link
Member

weihanglo commented May 23, 2024

@rfcbot fcp merge

This helps mitigate the needless rebuild when a filesystem supports only low-precision mtime, with a caveat:

there's only one in a billion chance that the nanosecond part will be exactly 0.

I propose to merge without any nightly flag, as the probability is relatively low.

@rfcbot
Copy link

rfcbot commented May 23, 2024

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels May 23, 2024
@joshtriplett
Copy link
Member

@rfcbot concern one-in-a-billion-chances-happen-at-scale

I'm all for handling timestamp truncation in a reasonable way. But I don't want us making the assumption that one-in-a-billion chances don't happen. If you're running a large fleet of systems doing many many operations per day, one-in-a-billion chances can be a regular occurrence.

Can we guard against this to ensure we only hit this case due to timestamp granularity, and not by random chance?

@ehuss
Copy link
Contributor

ehuss commented May 27, 2024

I don't think 1 billion is an accurate representation of the precision or probability that the sub-second time will be 0. Different systems and filesystems have different precision. For example, NTFS is 100-nanosecond, some virtualization environments or limited hardware might be much less.

I'm not sure why the updated patch here only checks for one file. I would assume if one side or the other is stored on a truncated filesystem, then all files would be truncated, and if not some number from 0 to less than all could possibly be 0.

@gilescope
Copy link
Contributor Author

Doing more experimentation, I've seen drift from 0ns to 14ns on a large codebase.
I'd be inclined to be lenient on the check only if every file is < 25ns (overridable by CARGO_MAX_MTIME_DRIFT_NS)

@bors
Copy link
Contributor

bors commented Oct 8, 2024

☔ The latest upstream changes (presumably #14137) made this pull request unmergeable. Please resolve the merge conflicts.

@kornelski
Copy link
Contributor

I've noticed that the timestamps are wrong only on build_script_build files, so I'm wondering whether there's a different bug in the way build scripts are written or fingerprinted that causes this problem.

@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2024

☔ The latest upstream changes (possibly 081d7ba) made this pull request unmergeable. Please resolve the merge conflicts.

@gilescope
Copy link
Contributor Author

I think this can be closed out in favour of -Z checksum-freshness

@gilescope gilescope closed this Mar 24, 2025
@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Mar 24, 2025
@rfcbot rfcbot removed the disposition-merge FCP with intent to merge label Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-rebuild-detection Area: rebuild detection and fingerprinting S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cargo Team: Cargo

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Spurious rebuilds due to filesystem rounding file modification timestamps

8 participants