Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove [loc] from rule digest #9052

Merged
merged 1 commit into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/changes/9052.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Do not re-run rules when their location changes (#9052, @rgrinberg)
5 changes: 2 additions & 3 deletions src/dune_engine/build_system.ml
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ end = struct

(* The current version of the rule digest scheme. We should increment it when
making any changes to the scheme, to avoid collisions. *)
let rule_digest_version = 18
let rule_digest_version = 19

let compute_rule_digest
(rule : Rule.t)
Expand Down Expand Up @@ -716,7 +716,7 @@ end = struct
let digest =
let { Rule.Anonymous_action.action =
{ action; env; locks; can_go_in_shared_cache; sandbox }
; loc
; loc = _
; dir
; alias
}
Expand All @@ -743,7 +743,6 @@ end = struct
, Dep.Set.digest deps
, Action.for_shell action
, List.map locks ~f:Path.to_string
, loc
, dir
, alias
, capture_stdout
Expand Down
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/dune-cache/mode-copy.t
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ never built [target1] before.
$ dune build --config-file=config target1 --debug-cache=shared,workspace-local \
> 2>&1 | grep '_build/default/source\|_build/default/target'
Workspace-local cache miss: _build/default/source: never seen this target before
Shared cache miss [43284c58c2079faf9e5421c4d82a28c2] (_build/default/source): not found in cache
Shared cache miss [4c8aba9580c271d7ac111bf2d72a147a] (_build/default/source): not found in cache
Workspace-local cache miss: _build/default/target1: never seen this target before
Shared cache miss [d92cafbd0f0c19a3a6c99407b959dc72] (_build/default/target1): not found in cache
Shared cache miss [68e477811b0e612a0cc0bb83c205420a] (_build/default/target1): not found in cache

$ dune_cmd stat hardlinks _build/default/source
1
Expand Down
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/dune-cache/mode-hardlink.t
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ never built [target1] before.
$ dune build --config-file=config target1 --debug-cache=shared,workspace-local \
> 2>&1 | grep '_build/default/source\|_build/default/target'
Workspace-local cache miss: _build/default/source: never seen this target before
Shared cache miss [afae5cd4afe24e40e88a52ffd372da14] (_build/default/source): not found in cache
Shared cache miss [4a1c82562ca4c3348fe36436814a9842] (_build/default/source): not found in cache
Workspace-local cache miss: _build/default/target1: never seen this target before
Shared cache miss [8dd10495c1458bf12c3c55807797879c] (_build/default/target1): not found in cache
Shared cache miss [3ffbc9519f4ca5e44997bf8ba62de0bb] (_build/default/target1): not found in cache

$ dune_cmd stat hardlinks _build/default/source
3
Expand Down
6 changes: 3 additions & 3 deletions test/blackbox-tests/test-cases/dune-cache/repro-check.t
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Set 'cache-check-probability' to 1.0, which should trigger the check
> EOF
$ rm -rf _build
$ dune build --config-file config reproducible non-reproducible
Warning: cache store error [f533fad74b151f0d6cbf8c3f5a91fa30]: ((in_cache
Warning: cache store error [761869532e88535d64e09b60102c4416]: ((in_cache
((non-reproducible 1c8fc4744d4cef1bd2b8f5e915b36be9))) (computed
((non-reproducible 6cfaa7a90747882bcf4ffe7252c1cf89)))) after executing
(echo 'build non-reproducible';cp dep non-reproducible)
Expand Down Expand Up @@ -119,7 +119,7 @@ Test that the environment variable and the command line flag work too

$ rm -rf _build
$ DUNE_CACHE_CHECK_PROBABILITY=1.0 dune build --cache=enabled reproducible non-reproducible
Warning: cache store error [f533fad74b151f0d6cbf8c3f5a91fa30]: ((in_cache
Warning: cache store error [761869532e88535d64e09b60102c4416]: ((in_cache
((non-reproducible 1c8fc4744d4cef1bd2b8f5e915b36be9))) (computed
((non-reproducible 6cfaa7a90747882bcf4ffe7252c1cf89)))) after executing
(echo 'build non-reproducible';cp dep non-reproducible)
Expand All @@ -131,7 +131,7 @@ Test that the environment variable and the command line flag work too

$ rm -rf _build
$ dune build --cache=enabled --cache-check-probability=1.0 reproducible non-reproducible
Warning: cache store error [f533fad74b151f0d6cbf8c3f5a91fa30]: ((in_cache
Warning: cache store error [761869532e88535d64e09b60102c4416]: ((in_cache
((non-reproducible 1c8fc4744d4cef1bd2b8f5e915b36be9))) (computed
((non-reproducible 6cfaa7a90747882bcf4ffe7252c1cf89)))) after executing
(echo 'build non-reproducible';cp dep non-reproducible)
Expand Down
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/dune-cache/trim.t
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ entries uniformly.

$ (cd "$PWD/.xdg-cache/dune/db/meta/v5"; grep -rws . -e 'metadata' | sort ) > out
$ cat out
./8c/8c4aba72abad331531e3af2fb9f9cfaa:((8:metadata)(5:files(8:target_b32:8a53bfae3829b48866079fa7f2d97781)))
./df/df307ded076980fa7dc2f5dbd563dbc1:((8:metadata)(5:files(8:target_a32:5637dd9730e430c7477f52d46de3909c)))
./50/50673e014878b6b71ee39f1f32ca4726:((8:metadata)(5:files(8:target_a32:5637dd9730e430c7477f52d46de3909c)))
./8f/8f658cc1fc1f083f42e98bbcd5a6ce2f:((8:metadata)(5:files(8:target_b32:8a53bfae3829b48866079fa7f2d97781)))

$ digest="$(awk -F: '/target_b/ { digest=$1 } END { print digest }' < out)"

Expand Down
2 changes: 1 addition & 1 deletion test/blackbox-tests/test-cases/patch-back-source-tree.t
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ produced in the sandbox and copied back:
This is the internal stamp file:

$ ls _build/.actions/default/blah*
_build/.actions/default/blah-182327d6e04fe09497790fcdbab8ca83
_build/.actions/default/blah-e7a0efae1209023d2186a50341cd25fa

And we check that it isn't copied in the source tree:

Expand Down
42 changes: 30 additions & 12 deletions test/blackbox-tests/test-cases/rule/digest.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,41 @@ Now the same but with an alias.
running...
digest: $2

Let's add a comment to the dune file. One might think that it doesn't affect
the rule digest, but it does because all the locations gets shifted.
Let's add a comment to the dune file. Dune does not re-run the rule because it
has the same digest:

$ changelocation() {
> sed -i.bak '1s/^/;; spurious location change\n/' dune
> }

$ changelocation

Now we make sure that failed rules re-run when the location changes:

$ cat >dune <<EOF
> ; hello
> (rule
> (alias default)
> (deps (sandbox always) pwd.ml)
> (action (run ocaml pwd.ml)))
> (alias foo)
> (action (system "echo failing; exit 1")))
> EOF

... but it does! It would be nice to encode the locations in a way that would
make them more resilient to non semantic changes.
$ dune build @foo
File "dune", line 1, characters 0-61:
1 | (rule
2 | (alias foo)
3 | (action (system "echo failing; exit 1")))
failing
[1]

$ changelocation

# CR-someday amokhov: Remove actual digests from this test so that we don't
# need to update it when rule digest version changes.
This should re-run the action

$ dune build @foo
File "dune", line 2, characters 0-61:
2 | (rule
3 | (alias foo)
4 | (action (system "echo failing; exit 1")))
failing
[1]

$ dune build @default
running...
digest: $3
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/rule/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(cram
(applies_to digest)
(deps %{bin:sed} %{bin:mktemp}))