Dune should document that files cannot be marked as writeable

[Alizter]

Here is a cram test to demonstrate some non-obvious behaviour:

  $ echo '(lang dune 3.0)' > dune-project

  $ cat >hello.txt<<"EOF"
  > hi
  > EOF

  $ cat >dune<<"EOF"
  > (rule
  >  (alias default)
  >  (action (progn
  >   (copy hello.txt copy.txt)
  >   (run chmod +xw copy.txt)
  >   (run ls -la copy.txt)   
  >   (run chmod -r copy.txt)
  >   (run ls -la copy.txt))))
  > EOF

  $ dune build

  $ ls -la _build/default/

This gives an output of:

+  -rwxrwxr-x 1 ali ali 3 Apr 11 18:55 copy.txt
+  --wx-wx--x 1 ali ali 3 Apr 11 18:55 copy.txt
+  File "dune", line 1, characters 0-173:
+  1 | (rule
+  2 |  (alias default)
+  3 |  (action (progn
+  4 |   (copy hello.txt copy.txt)
+  5 |   (run chmod +xw copy.txt)
+  6 |   (run ls -la copy.txt)   
+  7 |   (run chmod -r copy.txt)
+  8 |   (run ls -la copy.txt))))
+  Error: Error trying to read targets after a rule was run:
+  - copy.txt: Permission denied
+  [1]
 
   $ ls -la _build/default/
+  total 20
+  drwxrwxr-x 3 ali ali 4096 Apr 11 18:55 .
+  drwxrwxr-x 3 ali ali 4096 Apr 11 18:55 ..
+  drwxrwxr-x 2 ali ali 4096 Apr 11 18:55 .dune
+  ---x--x--x 1 ali ali    3 Apr 11 18:55 copy.txt
+  -r--r--r-- 1 ali ali    3 Apr 11 18:55 hello.txt

(Of course the output of ls will need to be scrubbed to make this a proper test).

As you can see reading can be changed, executing can be changed, but writing seems to get overridden by dune at some point. In the rule, dune reports that copy.txt is writeable yet it is actually not after dune has finished.

This is of course perhaps intentional behaviour by dune, allowing users to produce rules with writeable targets is probably a foot gun. However as my example indicates, it is not so obvious that this behaviour is happening.

So perhaps dune should document somewhere that the permissions of files in the _build directory cannot become writeable, at least if the file is a target or dependency. There are of course untracked files that can be.

cc @rgrinberg

[bobot]

As you can see dune seems to fail because the target can't be read anymore, so removing read right is also wrong.

I would not phrase that files in the _build directory cannot become writeable because they can be writeable during the run of the rules that is going to produce them. But indeed dune remove the write permission of all targets at the end of the rules that produce them.

NB: it is documented To reduce the risk of cache corruption, Dune systematically removes write permissions from all build results. but in a section about caching. Perhaps it can be added to the rule stanza.

[bobot]

Possible improvement: add to the documentation about targets that the write permission is removed and that the targets must have the read permission at the end of the rule.

[Alizter]

@bobot Perhaps adding r to this example was a bit distracting, it was to demonstrate that w is the only thing that can't be preserved.

[Alizter]

It would be a nice feature to systematically avoid certain rules from being cached. That way, we could safely (w.r.t cache) allow certain rules to have writable targets. We currently have (universe) which somewhat has this behaviour but that has other undesirable effects.

Of course allowing writable targets pretty much goes against dunes design and is probably a foot gun. The reason this example came up at all was in the context of moving from a makefile based build to a dune one. It was somewhat confusing when the permissions didn't act as expected, so making it clearer in the doc would probably be a good compromise.

[bobot]

Many rules are not cached. But the sanity check of removing the write permission is always done. Lets focus on the doc. I think the modification is at the junior job level.

[ejgallego]

As discussed privately with Ali, this is, in my view, more of a question about what to do with rules that update targets, that is to say, cyclic in nature.

It seems to me that the most natural way is indeed to use a regular rule {foo} --[action]--- {foo.new} and then maybe promote if needed, but it would be interesting if there are more use cases for "update" rules of the form { cache } ---[action]--- {cache}

[bobot]

We can add in the documentation an how-to that describes the promote way for cache. "updates" rules can be quite similar to "fixpoint" rules (e.g. latex rules), which is a big can of worms. A separate RFC that describes the cache use case would be a first step, but I think we should try to have a very specific feature in order to not get an hacked dependency semantic.