From 257aa9610c452c0283eabb19a6ecb553d79418a2 Mon Sep 17 00:00:00 2001 From: Ambre Austen Suhamy Date: Mon, 26 Aug 2024 21:11:16 +0200 Subject: [PATCH] Enable dune cache by default (#10710) * Added feature flag to enable dune cache by default --- bin/common.ml | 36 ++++++++++++++++++- src/dune_cache_storage/layout.ml | 8 +++-- src/dune_cache_storage/layout.mli | 7 ++-- src/dune_config_file/dune_config_file.ml | 4 +-- src/dune_engine/action.ml | 2 +- src/dune_engine/action.mli | 3 +- src/dune_engine/build_config.ml | 5 --- src/dune_engine/clflags.ml | 1 + src/dune_engine/clflags.mli | 3 ++ src/dune_rules/dune | 1 + src/dune_rules/simple_rules.ml | 2 +- .../blackbox-tests/test-cases/default-cache.t | 35 ++++++++++++++++++ .../directory-targets/cache-file-and-dir.t | 1 + .../directory-targets/cache-shared-subdir.t | 1 + .../test-cases/directory-targets/cache.t | 1 + .../test-cases/dune-cache/config.t | 1 + .../test-cases/dune-cache/dedup.t | 1 + .../dune-cache/missing-cache-entries.t | 1 + .../test-cases/dune-cache/mode-copy.t | 1 + .../test-cases/dune-cache/mode-hardlink.t | 1 + .../test-cases/dune-cache/readonly-fs.t | 22 ++++++++---- .../test-cases/dune-cache/repro-check.t | 1 + .../test-cases/dune-cache/size.t/run.t | 1 + .../test-cases/dune-cache/symlink.t | 1 + .../test-cases/dune-cache/trim.t | 1 + .../test-cases/pkg/toolchain-installation.t | 4 +-- .../dune_config_file/dune_config_test.ml | 8 ++--- 27 files changed, 123 insertions(+), 30 deletions(-) create mode 100644 test/blackbox-tests/test-cases/default-cache.t diff --git a/bin/common.ml b/bin/common.ml index e0515ed5738..d57b8aaa84d 100644 --- a/bin/common.ml +++ b/bin/common.ml @@ -583,6 +583,7 @@ module Builder = struct ; file_watcher : Dune_engine.Scheduler.Run.file_watcher ; workspace_config : Dune_rules.Workspace.Clflags.t ; cache_debug_flags : Dune_engine.Cache_debug_flags.t + ; cache_rules_default : bool ; report_errors_config : Dune_engine.Report_errors_config.t ; separate_error_messages : bool ; stop_on_first_error : bool @@ -932,6 +933,20 @@ module Builder = struct useful for Dune developers to make Dune tests of the digest cache more \ reproducible.") and+ cache_debug_flags = cache_debug_flags_term + and+ cache_rules_default = + let default = + Dune_lang.Toggle.of_bool !Dune_engine.Clflags.can_go_in_shared_cache_default + in + let doc = + Printf.sprintf + "Enable or disable caching rules (%s). Default is `%s'." + (Arg.doc_alts_enum Config.Toggle.all) + (Config.Toggle.to_string default) + in + Arg.( + value + & opt (enum Config.Toggle.all) default + & info [ "cache-rules" ] ~docs ~env:(Cmd.Env.info ~doc "DUNE_CACHE_RULES") ~doc) and+ report_errors_config = Arg.( value @@ -1008,6 +1023,7 @@ module Builder = struct ; config_from_config_file } ; cache_debug_flags + ; cache_rules_default = Dune_lang.Toggle.enabled cache_rules_default ; report_errors_config ; separate_error_messages ; stop_on_first_error @@ -1163,6 +1179,23 @@ let build (builder : Builder.t) = { builder; root; rpc; stats } ;; +let maybe_init_cache (cache_config : Dune_cache.Config.t) = + match cache_config with + | Disabled -> cache_config + | Enabled _ -> + (match Dune_cache_storage.Layout.create_cache_directories () with + | Ok () -> cache_config + | Error (path, exn) -> + User_warning.emit + ~hints: + [ Pp.textf "Make sure the directory %s can be created" (Path.to_string path) ] + [ Pp.textf + "Cache directories could not be created: %s; disabling cache" + (Unix.error_message exn) + ]; + Disabled) +;; + let init (builder : Builder.t) = let c = build builder in if c.root.dir <> Filename.current_dir_name then Sys.chdir c.root.dir; @@ -1216,7 +1249,7 @@ let init (builder : Builder.t) = Dune_rules.Main.init ~stats:c.stats ~sandboxing_preference:config.sandboxing_preference - ~cache_config + ~cache_config:(maybe_init_cache cache_config) ~cache_debug_flags:c.builder.cache_debug_flags (); Only_packages.Clflags.set c.builder.only_packages; @@ -1241,6 +1274,7 @@ let init (builder : Builder.t) = Dune_rules.Clflags.ignore_lock_dir := c.builder.ignore_lock_dir; Dune_rules.Clflags.on_missing_dune_project_file := if c.builder.require_dune_project_file then Error else Warn; + Dune_engine.Clflags.can_go_in_shared_cache_default := c.builder.cache_rules_default; Log.info [ Pp.textf "Workspace root: %s" diff --git a/src/dune_cache_storage/layout.ml b/src/dune_cache_storage/layout.ml index 4df37baaeaa..7951d5287a5 100644 --- a/src/dune_cache_storage/layout.ml +++ b/src/dune_cache_storage/layout.ml @@ -82,7 +82,9 @@ let value_storage_dir = Versioned.value_storage_dir Version.Value.current let value_path = Versioned.value_path Version.Value.current let create_cache_directories () = - List.iter - [ temp_dir; metadata_storage_dir; file_storage_dir; value_storage_dir ] - ~f:(fun path -> ignore (Fpath.mkdir_p (Path.to_string path) : Fpath.mkdir_p_result)) + [ temp_dir; metadata_storage_dir; file_storage_dir; value_storage_dir ] + |> Result.List.iter ~f:(fun path -> + match Fpath.mkdir_p (Path.to_string path) with + | Already_exists | Created -> Ok () + | exception Unix.Unix_error (e, _, _) -> Error (path, e)) ;; diff --git a/src/dune_cache_storage/layout.mli b/src/dune_cache_storage/layout.mli index 560c7f76795..2ab381e0b23 100644 --- a/src/dune_cache_storage/layout.mli +++ b/src/dune_cache_storage/layout.mli @@ -12,8 +12,11 @@ open Import val root_dir : Path.t (** Create a few subdirectories in [root_dir]. We expose this function because - we don't want to modify the file system when the cache is disabled. *) -val create_cache_directories : unit -> unit + we don't want to modify the file system when the cache is disabled. + + Returns whether creation has succeeded or in the case of error which directory + could not be created. *) +val create_cache_directories : unit -> (unit, Path.t * Unix.error) result (** This directory stores metadata files, one per each historically executed build rule or output-producing action. (While this is a convenient mental diff --git a/src/dune_config_file/dune_config_file.ml b/src/dune_config_file/dune_config_file.ml index 1bf91c61025..ba01baaff28 100644 --- a/src/dune_config_file/dune_config_file.ml +++ b/src/dune_config_file/dune_config_file.ml @@ -289,9 +289,9 @@ module Dune_config = struct ; concurrency = (if Execution_env.inside_dune then Fixed 1 else Auto) ; terminal_persistence = Clear_on_rebuild ; sandboxing_preference = [] - ; cache_enabled = `Disabled + ; cache_enabled = `Enabled ; cache_reproducibility_check = Skip - ; cache_storage_mode = None + ; cache_storage_mode = Some (Dune_cache_storage.Mode.default ()) ; action_stdout_on_success = Print ; action_stderr_on_success = Print ; experimental = [] diff --git a/src/dune_engine/action.ml b/src/dune_engine/action.ml index 2762e900fce..dd06e73575f 100644 --- a/src/dune_engine/action.ml +++ b/src/dune_engine/action.ml @@ -336,7 +336,7 @@ module Full = struct let make ?(env = Env.empty) ?(locks = []) - ?(can_go_in_shared_cache = true) + ?(can_go_in_shared_cache = !Clflags.can_go_in_shared_cache_default) ?(sandbox = Sandbox_config.default) action = diff --git a/src/dune_engine/action.mli b/src/dune_engine/action.mli index 6719d1be3d8..ddd77b02e0c 100644 --- a/src/dune_engine/action.mli +++ b/src/dune_engine/action.mli @@ -152,7 +152,8 @@ module Full : sig val make : ?env:Env.t (** default [Env.empty] *) -> ?locks:Path.t list (** default [[]] *) - -> ?can_go_in_shared_cache:bool (** default [true] *) + -> ?can_go_in_shared_cache:bool + (** default [!Clflags.can_fo_in_shared_cache_default] *) -> ?sandbox:Sandbox_config.t (** default [Sandbox_config.default] *) -> action -> t diff --git a/src/dune_engine/build_config.ml b/src/dune_engine/build_config.ml index 584f7d6df41..742bf8c9fc6 100644 --- a/src/dune_engine/build_config.ml +++ b/src/dune_engine/build_config.ml @@ -106,11 +106,6 @@ let set contexts ~f:(fun ((ctx : Build_context.t), ctx_type) -> ctx.name, (ctx, ctx_type))) in - let () = - match (cache_config : Dune_cache.Config.t) with - | Disabled -> () - | Enabled _ -> Dune_cache_storage.Layout.create_cache_directories () - in Fdecl.set t { contexts diff --git a/src/dune_engine/clflags.ml b/src/dune_engine/clflags.ml index c7358890b74..99119b4b107 100644 --- a/src/dune_engine/clflags.ml +++ b/src/dune_engine/clflags.ml @@ -19,3 +19,4 @@ let promote = ref None let force = ref false let always_show_command_line = ref false let display = ref Display.Quiet +let can_go_in_shared_cache_default = ref false diff --git a/src/dune_engine/clflags.mli b/src/dune_engine/clflags.mli index e5edb0ea22c..15012375ba8 100644 --- a/src/dune_engine/clflags.mli +++ b/src/dune_engine/clflags.mli @@ -34,3 +34,6 @@ val always_show_command_line : bool ref (** The display mode *) val display : Display.t ref + +(** Whether actions are cacheable by default, default [false] *) +val can_go_in_shared_cache_default : bool ref diff --git a/src/dune_rules/dune b/src/dune_rules/dune index 2d75571cb3c..dfcc3d893ea 100644 --- a/src/dune_rules/dune +++ b/src/dune_rules/dune @@ -33,6 +33,7 @@ build_path_prefix_map dune_engine dune_vcs + dune_cache_storage dune_config dune_config_file dune_findlib diff --git a/src/dune_rules/simple_rules.ml b/src/dune_rules/simple_rules.ml index 7823eb60a8f..d5ffa61a30f 100644 --- a/src/dune_rules/simple_rules.ml +++ b/src/dune_rules/simple_rules.ml @@ -63,7 +63,7 @@ let add_user_rule sctx ~dir ~(rule : Rule_conf.t) - ~(action : _ Action_builder.With_targets.t) + ~(action : Action.Full.t Action_builder.With_targets.t) ~expander = let action = diff --git a/test/blackbox-tests/test-cases/default-cache.t b/test/blackbox-tests/test-cases/default-cache.t new file mode 100644 index 00000000000..7404edfc03a --- /dev/null +++ b/test/blackbox-tests/test-cases/default-cache.t @@ -0,0 +1,35 @@ +The dune cache should be enabled by default + + $ echo "(lang dune 3.17)" > dune-project + + $ cat > dune << EOF + > (library + > (name foo)) + > EOF + + $ cat > foo.ml << EOF + > let f x y = x + y + > EOF + +Set up cache directory + + $ export DUNE_CACHE_ROOT=$(pwd)/dune_test_cache + $ mkdir $DUNE_CACHE_ROOT + $ DUNE_CACHE=disabled dune build + $ ls $DUNE_CACHE_ROOT + +We have not written anything to the cache yet. + +Change source files to force a recompilation + + $ cat > foo.ml << EOF + > let f x y = x - y + > EOF + $ dune build + $ ls $DUNE_CACHE_ROOT | sort + files + meta + temp + values + +Cache has been written to! diff --git a/test/blackbox-tests/test-cases/directory-targets/cache-file-and-dir.t b/test/blackbox-tests/test-cases/directory-targets/cache-file-and-dir.t index 5f6f9ae1d7b..580e2513176 100644 --- a/test/blackbox-tests/test-cases/directory-targets/cache-file-and-dir.t +++ b/test/blackbox-tests/test-cases/directory-targets/cache-file-and-dir.t @@ -2,6 +2,7 @@ This checks what happens when a file available in the cache is used in a directo $ export DUNE_CACHE_ROOT=$PWD/.cache $ export DUNE_CACHE=enabled + $ export DUNE_CACHE_RULES=enabled $ . ./helpers.sh $ cat > dune-project << EOF diff --git a/test/blackbox-tests/test-cases/directory-targets/cache-shared-subdir.t b/test/blackbox-tests/test-cases/directory-targets/cache-shared-subdir.t index fc7c7cb3065..8d18166a5d2 100644 --- a/test/blackbox-tests/test-cases/directory-targets/cache-shared-subdir.t +++ b/test/blackbox-tests/test-cases/directory-targets/cache-shared-subdir.t @@ -1,6 +1,7 @@ We create 2 directory targets which share a whole subdirectory. $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ export DUNE_CACHE=enabled $ . ./helpers.sh diff --git a/test/blackbox-tests/test-cases/directory-targets/cache.t b/test/blackbox-tests/test-cases/directory-targets/cache.t index dc84fb3a52c..5dcbcf06252 100644 --- a/test/blackbox-tests/test-cases/directory-targets/cache.t +++ b/test/blackbox-tests/test-cases/directory-targets/cache.t @@ -1,6 +1,7 @@ We test that directory targets can go in the shared cache. See #8067. $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ export DUNE_CACHE=enabled In project a, we create a rule with a directory target. The script that creates diff --git a/test/blackbox-tests/test-cases/dune-cache/config.t b/test/blackbox-tests/test-cases/dune-cache/config.t index b64aae09346..ad9cce8bb43 100644 --- a/test/blackbox-tests/test-cases/dune-cache/config.t +++ b/test/blackbox-tests/test-cases/dune-cache/config.t @@ -26,6 +26,7 @@ Check that old cache configuration format works fine with an old language Test that DUNE_CACHE_ROOT can be used to control the cache location $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled Build succeeds and the 'copy' mode is respected diff --git a/test/blackbox-tests/test-cases/dune-cache/dedup.t b/test/blackbox-tests/test-cases/dune-cache/dedup.t index ba9e13e383a..73b8ef5fdb5 100644 --- a/test/blackbox-tests/test-cases/dune-cache/dedup.t +++ b/test/blackbox-tests/test-cases/dune-cache/dedup.t @@ -2,6 +2,7 @@ Test deduplication of build artifacts when using Dune cache with hard links. $ export DUNE_CACHE=enabled $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ cat > dune-project < (lang dune 2.1) diff --git a/test/blackbox-tests/test-cases/dune-cache/missing-cache-entries.t b/test/blackbox-tests/test-cases/dune-cache/missing-cache-entries.t index 3565ab9d8ea..01b987c804d 100644 --- a/test/blackbox-tests/test-cases/dune-cache/missing-cache-entries.t +++ b/test/blackbox-tests/test-cases/dune-cache/missing-cache-entries.t @@ -1,6 +1,7 @@ Check that Dune cache can cope with missing file/metadata entries. $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ cat > config < (lang dune 2.1) diff --git a/test/blackbox-tests/test-cases/dune-cache/mode-copy.t b/test/blackbox-tests/test-cases/dune-cache/mode-copy.t index a12664fd980..3719e8b775f 100644 --- a/test/blackbox-tests/test-cases/dune-cache/mode-copy.t +++ b/test/blackbox-tests/test-cases/dune-cache/mode-copy.t @@ -5,6 +5,7 @@ variable, and via the [DUNE_CACHE_ROOT] variable. Here we test the former. $ export XDG_RUNTIME_DIR=$PWD/.xdg-runtime $ export XDG_CACHE_HOME=$PWD/.xdg-cache + $ export DUNE_CACHE_RULES=enabled $ cat > config < (lang dune 3.0) diff --git a/test/blackbox-tests/test-cases/dune-cache/mode-hardlink.t b/test/blackbox-tests/test-cases/dune-cache/mode-hardlink.t index ae152ca4860..2058335ff9b 100644 --- a/test/blackbox-tests/test-cases/dune-cache/mode-hardlink.t +++ b/test/blackbox-tests/test-cases/dune-cache/mode-hardlink.t @@ -5,6 +5,7 @@ variable, and via the [DUNE_CACHE_ROOT] variable. Here we test the former. $ export XDG_RUNTIME_DIR=$PWD/.xdg-runtime $ export XDG_CACHE_HOME=$PWD/.xdg-cache + $ export DUNE_CACHE_RULES=enabled $ cat > config < (lang dune 2.1) diff --git a/test/blackbox-tests/test-cases/dune-cache/readonly-fs.t b/test/blackbox-tests/test-cases/dune-cache/readonly-fs.t index c43b3fc2104..e03354f0e19 100644 --- a/test/blackbox-tests/test-cases/dune-cache/readonly-fs.t +++ b/test/blackbox-tests/test-cases/dune-cache/readonly-fs.t @@ -17,16 +17,24 @@ where Dune is supposed to store the cache: $ export DUNE_CACHE_ROOT=$(pwd)/readonly/cache-dir $ dune build - Error: - mkdir($TESTCASE_ROOT/readonly/cache-dir): Permission denied - [1] + Warning: Cache directories could not be created: Permission denied; disabling + cache + Hint: Make sure the directory + $TESTCASE_ROOT/readonly/cache-dir/temp + can be created Likewise, this should also happen if the location is set via XDG variables. $ unset DUNE_CACHE_ROOT $ export XDG_CACHE_HOME=$(pwd)/readonly/xdg-cache-dir + $ export DUNE_CONFIG__SKIP_LINE_BREAK=enabled - $ dune build - Error: - mkdir($TESTCASE_ROOT/readonly/xdg-cache-dir): Permission denied - [1] + $ dune build 2>&1 | sed 's/created: .*;/created: $REASON:/' + Warning: Cache directories could not be created: $REASON: disabling cache + Hint: Make sure the directory $TESTCASE_ROOT/readonly/xdg-cache-dir/dune/db/temp can be created + + $ HOME=/homeless-shelter + $ unset XDG_CACHE_HOME + $ dune build 2>&1 | sed 's/created: .*;/created: $REASON:/' + Warning: Cache directories could not be created: $REASON: disabling cache + Hint: Make sure the directory /homeless-shelter/.cache/dune/db/temp can be created diff --git a/test/blackbox-tests/test-cases/dune-cache/repro-check.t b/test/blackbox-tests/test-cases/dune-cache/repro-check.t index 7be49c92a65..3532343a643 100644 --- a/test/blackbox-tests/test-cases/dune-cache/repro-check.t +++ b/test/blackbox-tests/test-cases/dune-cache/repro-check.t @@ -1,6 +1,7 @@ Test reproducibility check $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ cat > config < (lang dune 3.0) > (cache enabled) diff --git a/test/blackbox-tests/test-cases/dune-cache/size.t/run.t b/test/blackbox-tests/test-cases/dune-cache/size.t/run.t index a0ec710939b..46e0c8416c0 100644 --- a/test/blackbox-tests/test-cases/dune-cache/size.t/run.t +++ b/test/blackbox-tests/test-cases/dune-cache/size.t/run.t @@ -3,6 +3,7 @@ the cache. $ export DUNE_CACHE=enabled $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ cat > config << EOF > (lang dune 3.7) diff --git a/test/blackbox-tests/test-cases/dune-cache/symlink.t b/test/blackbox-tests/test-cases/dune-cache/symlink.t index 485b46a31b7..d7fcfcb520b 100644 --- a/test/blackbox-tests/test-cases/dune-cache/symlink.t +++ b/test/blackbox-tests/test-cases/dune-cache/symlink.t @@ -3,6 +3,7 @@ produced symbolic links work correctly and are appropriately cached. $ export DUNE_CACHE=enabled $ export DUNE_CACHE_ROOT=$PWD/.cache + $ export DUNE_CACHE_RULES=enabled $ cat > dune-project < (lang dune 2.1) diff --git a/test/blackbox-tests/test-cases/dune-cache/trim.t b/test/blackbox-tests/test-cases/dune-cache/trim.t index 3e708ad2a75..383aee75ca7 100644 --- a/test/blackbox-tests/test-cases/dune-cache/trim.t +++ b/test/blackbox-tests/test-cases/dune-cache/trim.t @@ -1,4 +1,5 @@ $ export DUNE_CACHE=enabled + $ export DUNE_CACHE_RULES=enabled $ export XDG_RUNTIME_DIR=$PWD/.xdg-runtime $ export XDG_CACHE_HOME=$PWD/.xdg-cache diff --git a/test/blackbox-tests/test-cases/pkg/toolchain-installation.t b/test/blackbox-tests/test-cases/pkg/toolchain-installation.t index 97d7efcafdf..c6f8158dba9 100644 --- a/test/blackbox-tests/test-cases/pkg/toolchain-installation.t +++ b/test/blackbox-tests/test-cases/pkg/toolchain-installation.t @@ -85,9 +85,7 @@ but the fake compiler will end up installed as a toolchain package. Unrecognized line: "Hello from fake ocamlc!" Enumerate the contents of the fake toolchains directory: - $ find fake-cache | sort | remove_hash - fake-cache - fake-cache/dune + $ find fake-cache/dune/toolchains | sort | remove_hash fake-cache/dune/toolchains fake-cache/dune/toolchains/ocaml-base-compiler.1-HASH fake-cache/dune/toolchains/ocaml-base-compiler.1-HASH/target diff --git a/test/expect-tests/dune_config_file/dune_config_test.ml b/test/expect-tests/dune_config_file/dune_config_test.ml index 28948fdb4e7..57ae3ee8d29 100644 --- a/test/expect-tests/dune_config_file/dune_config_test.ml +++ b/test/expect-tests/dune_config_file/dune_config_test.ml @@ -22,9 +22,9 @@ let%expect_test "cache-check-probability 0.1" = ; concurrency = Fixed 1 ; terminal_persistence = Clear_on_rebuild ; sandboxing_preference = [] - ; cache_enabled = Disabled + ; cache_enabled = Enabled ; cache_reproducibility_check = Check_with_probability 0.1 - ; cache_storage_mode = None + ; cache_storage_mode = Some Hardlink ; action_stdout_on_success = Print ; action_stderr_on_success = Print ; experimental = [] @@ -40,7 +40,7 @@ let%expect_test "cache-storage-mode copy" = ; concurrency = Fixed 1 ; terminal_persistence = Clear_on_rebuild ; sandboxing_preference = [] - ; cache_enabled = Disabled + ; cache_enabled = Enabled ; cache_reproducibility_check = Skip ; cache_storage_mode = Some Copy ; action_stdout_on_success = Print @@ -58,7 +58,7 @@ let%expect_test "cache-storage-mode hardlink" = ; concurrency = Fixed 1 ; terminal_persistence = Clear_on_rebuild ; sandboxing_preference = [] - ; cache_enabled = Disabled + ; cache_enabled = Enabled ; cache_reproducibility_check = Skip ; cache_storage_mode = Some Hardlink ; action_stdout_on_success = Print