Use stat() to avoid checking content hashes for repository up-to-dateness checks

[lberki]

Description of the feature request:

The way Bazel verifies up-to-dateness of source files is that if some select data from stat() is unchanged, it believes it, and if not, it does a secondary check by checksumming source files to avoid re-running actions after a simple touch.

Repository rules don't work like this: they simply checksum every file that the repository depends on, which makes up-to-dateness much slower than they would otherwise be.

It would be nice to make repository rules do the same thing as actions.

Which category does this issue belong to?

No response

What underlying problem are you trying to solve with this feature?

Make the repository up-to-dateness check more efficient.

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[fmeum]

Just want to note that Bazel doesn't check just mtime, but also ctime and other attributes, before computing a digest: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/vfs/DigestUtils.java;l=42;bpv=1;bpt=1

[lberki]

You're indeed right -- I used "mtime" as a shorthand for "whatever can be gleaned about a file short of looking at its contents".

[SalmaSamy]

@lberki, @fmeum I guess storing only ctime should be enough? as it already tracks changes to the file content and the extra stuff

[lberki]

@SalmaSamy AFAIU the ctime always changes if mtime does, so using that instead of the mtime would cause suboptimal caching in the worst case, which is perfectly fine (if somewhat inconsistent with action inputs, but that's just a minor gripe)

[fmeum]

There is the (probably very theoretical) concern of having the filesystem backing a path change. By chance, the ctime could remain the same but the file contents could differ. Bazel uses more stat properties such as file size and inside for the digest cache key, but I could see that being overkill for repo rules.

[lberki]

Good point! TBH I think the best approach here is to mimic what FileContentsProxy does, which is a tuple of (mtime, ctime, inode number)., It's only very slightly more complicated than relying on ctime alone, and protects against a few odd scenarios like the one you describe above.

[meteorcloudy]

@bazel-io fork 7.1.0

[meisterT]

Note that timestamps on HFS+ have a granularity of one second.

[lberki]

FWIW, that's what TimestampGranularityMonitor is for: it makes Bazel wait long enough at the end of the build so that Bazel can be sure that if something modifies an input after it has returned, its timestamp changes.

[SalmaSamy]

I think just using mtime alone now provides sufficient information for our needs, as it directly reflects changes to the file's content, which is our primary concern. Storing ctime or inode may add unnecessary complexity without offering much additional value as I think the extra information they store would already invalidate the label Ex: file location or name or very rarely changed Ex: file owner.

[Wyverald]

Lowering the priority of this one due to the uncertainty of its correctness (and we're out of time for 7.1.0 anyway).