cache: some permissions can't be represented, causing cache misses

[emillon]

In a nutshell: in some cases, dune will restore some artifacts in a way that causes subsequent runs to digest them differently.

Reproduction case

This can be reproduced in the following cram test:

  $ export DUNE_CACHE_ROOT=$PWD/.cache
  $ export DUNE_CACHE=enabled

  $ cat > dune-project <<EOF
  > (lang dune 3.0)
  > (using directory-targets 0.1)
  > EOF

  $ cat > dune <<EOF
  > (rule
  >  (targets (dir d))
  >  (action
  >   (progn
  >    (bash "echo building d")
  >    (run mkdir d)
  >    (run chmod 755 d)
  >    (run touch d/x))))
  > 
  > (rule
  >  (deps d)
  >  (action
  >   (progn
  >    (echo building other)
  >    (write-file other data))))
  > EOF

  $ dune build other
  building d
  building other
  $ dune_cmd stat permissions _build/default/d
  755
  $ dune clean
  $ dune build other
  building other
  $ dune_cmd stat permissions _build/default/d
  775
  $ dune clean
  $ dune build other

When the action is executed, the directory is created with mode 755 (in particular, g-w), but when it it restored, it gets mode 775 (in particular g+w).

This causes a cache miss because the stored result for the rule that produces other has a different digest for its dependency d.

The third run can restore cleanly because it has cached the 775 version.

(note that this behavior does not happen if chmod 775 is in the dune file; and that the issue also happens when a subdirectory of the directory target has such a permission)

Expected behavior

The expected behavior would be that the second run can build cleanly (with only cache hits).

Context

This issue manifests itself in the package management rules. In particular, ocamlfind.1.9.6+dune will create ocamlfind/target/bin/ with mode 755, so cleaning and rebuilding will restore a different version of ocamlfind/target/bin/ (with mode 775) and trigger rebuilds of all the packages that depend on ocamlfind. And if such dependent packages have similar directory permissions, they will be rebuilt on subsequent builds.

Suggested solution

Storing just an executable bit instead of the full permissions seems like a good tradeoff - this is what git does for example.

To fix this issue, digesting could be changed to only take into account what is going to be restored. For example, by clearing or normalizing the group bits in Digest.Stats_for_digest.of_unix_stats, or maybe by replacing that part by just an executable boolean.

FTR, git determines that a file is executable by checking if any of the +x bits are set.

@mefyl also mentioned that something similar might be happening with the +w bits - when hardlinking we reset these bits so they might alter the digest.

[emillon]

This depends on the currently set umask, so this prompts the question of whether the restore function and the install -d in the install instruction don't use the same value.

[Leonidas-from-XIV]

Storing just an executable bit instead of the full permissions seems like a good tradeoff - this is what git does for example.

Out of curiosity, why don't we chmod the file to the recorded permissions in that case, ignoring the inherited umask?

[emillon]

Yes that would work too, but there's no room for this metadata at the moment.
(the umask issue is a bit different, it only corresponds to the "default" permissions of created files)

[Leonidas-from-XIV]

Yes that would work too, but there's no room for this metadata at the moment.

Ah you mean that the cache only differentiates between executable and non-executable? Yes, in that case I totally see your point (and #11541 is then the only right way to deal with it), but maybe it would be worth considering whether we want to record the exact permissions in the cache instead.

I honestly don't know, thus I am curious why git does it this way. There's probably good reasons for it.

[mefyl]

The git approach makes sense to me, and I think the same thought process should apply to dune cache:

• Regarding the "other" and "group" permissions, I don't think it makes sense for git or to store them, as it's really dependent on the local setup of the cloner. Most people may have a very usual setup with umask = 022 because they are the sole user of their machine, but if some other have more complex groups or ACLs locally it should probably not interfere. In other word, because you decide to leave "other" permissions open doesn't mean I should too, if my directory structure is shared and a bit sensitive.

• Regarding u+r, the use removing your own read permission usefulness is very debatable, and I'm not sure cloning a repository (so, forging my own copies of the files) should stripe me of the permission to read their contents. Sounds more like a headache that will make many script fail to me.

• Regarding u+w, I could see the use. You may want to commit generated files that should not be manually edited, and removing write permission can help prevent accidental edition. However, in hardlink mode, dune cache will (or used to, at the very least) strip write permissions on files pulled from the cache, and editing them would be catastrophic as you'd be editing many other build trees and break all reproducibility assumptions. So I fear we have no choice but to also consider this permission bit local to each build tree and not store it.

• Only u+x remains, which I think we all agree should be stored and restored.

[maiste]

Since #11541 solved this, should we close this issue?