From 26bf38754527956cda82e200f7ac6db79f88b42b Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Sat, 20 Apr 2024 02:25:19 +0100 Subject: [PATCH] stop recording deps in Action_builder.contents PR #9552 changed the semantics of Action_builder.contents so that it records a dependency. The Coq rules were relying on the previous behaviour of not recording a dependency in order to generate a build plan. This caused issue #10149 to appear. This PR fixes #10149 by removing the dependency being recorded in Action_builder.contents matching the previous semantics. Signed-off-by: Ali Caglayan --- doc/changes/10446.md | 2 ++ src/dune_rules/action_builder.ml | 7 +------ .../test-cases/coq/no-rebuild-on-dep-change.t | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-) create mode 100644 doc/changes/10446.md diff --git a/doc/changes/10446.md b/doc/changes/10446.md new file mode 100644 index 00000000000..c38b382f368 --- /dev/null +++ b/doc/changes/10446.md @@ -0,0 +1,2 @@ +- Fix a bug where Coq projects were being rebuilt from scratch each time the + dependency graph changed. (#10446, fixes #10149, @alizter) diff --git a/src/dune_rules/action_builder.ml b/src/dune_rules/action_builder.ml index 5192e2ec9a1..870987b0d79 100644 --- a/src/dune_rules/action_builder.ml +++ b/src/dune_rules/action_builder.ml @@ -37,12 +37,7 @@ let paths ps = deps (Dep.Set.of_files ps) let path_set ps = deps (Dep.Set.of_files_set ps) let dyn_paths paths = dyn_deps (paths >>| fun (x, paths) -> x, Dep.Set.of_files paths) let dyn_paths_unit paths = dyn_deps (paths >>| fun paths -> (), Dep.Set.of_files paths) - -let contents p = - let* () = Dep.file p |> Dep.Set.singleton |> Build_system.record_deps in - of_memo (Build_system.read_file p) -;; - +let contents p = of_memo (Build_system.read_file p) let lines_of p = contents p >>| String.split_lines let read_sexp p = diff --git a/test/blackbox-tests/test-cases/coq/no-rebuild-on-dep-change.t b/test/blackbox-tests/test-cases/coq/no-rebuild-on-dep-change.t index 8b13f365d08..9df18a23153 100644 --- a/test/blackbox-tests/test-cases/coq/no-rebuild-on-dep-change.t +++ b/test/blackbox-tests/test-cases/coq/no-rebuild-on-dep-change.t @@ -11,8 +11,7 @@ This test makes sure that a full rebuild is not triggered when the output of coqdep is changed. -Currently this is the case and a bug: +This is as expected: $ dune build --display=short coqdep .bug.theory.d - coqc root.{glob,vo} coqc leaf.{glob,vo}