From dc495fadab80ac6c0c9451078ad95bac16f3c1f1 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 27 Aug 2024 15:21:43 +0200
Subject: [PATCH] feat(pkg): Enable cache on fetch actions

This enables caching of downloaded files as part of the fetch action, if
the file to be retrieved is retrieved via a cryptographic hash (and thus
presumably the exact same as the first time it was downloaded).

Signed-off-by: Marek Kubica <marek@tarides.com>
---
 src/dune_engine/action.ml                     |  1 +
 src/dune_engine/action.mli                    |  3 +
 src/dune_rules/fetch_rules.ml                 |  8 ++-
 .../test-cases/pkg/fetch-cache.t              | 61 +++++++++++++++++++
 4 files changed, 70 insertions(+), 3 deletions(-)
 create mode 100644 test/blackbox-tests/test-cases/pkg/fetch-cache.t

diff --git a/src/dune_engine/action.ml b/src/dune_engine/action.ml
index dd06e73575ff..8f48b8d7fac1 100644
--- a/src/dune_engine/action.ml
+++ b/src/dune_engine/action.ml
@@ -351,5 +351,6 @@ module Full = struct
     { t with can_go_in_shared_cache = t.can_go_in_shared_cache && b }
   ;;
 
+  let allowed_in_shared_cache t = { t with can_go_in_shared_cache = true }
   let add_sandbox s t = { t with sandbox = Sandbox_config.inter t.sandbox s }
 end
diff --git a/src/dune_engine/action.mli b/src/dune_engine/action.mli
index ddd77b02e0ce..5632eb3a54c7 100644
--- a/src/dune_engine/action.mli
+++ b/src/dune_engine/action.mli
@@ -172,5 +172,8 @@ module Full : sig
   val add_sandbox : Sandbox_config.t -> t -> t
   val add_can_go_in_shared_cache : bool -> t -> t
 
+  (** Marks action as safe to be cached *)
+  val allowed_in_shared_cache : t -> t
+
   include Monoid with type t := t
 end
diff --git a/src/dune_rules/fetch_rules.ml b/src/dune_rules/fetch_rules.ml
index c323c09607eb..e6f4b64da3d8 100644
--- a/src/dune_rules/fetch_rules.ml
+++ b/src/dune_rules/fetch_rules.ml
@@ -181,10 +181,10 @@ let find_checksum, find_url =
 ;;
 
 let gen_rules_for_checksum_or_url (loc_url, (url : OpamUrl.t)) checksum =
-  let checksum_or_url =
+  let checksum_or_url, allowed_in_shared_cache =
     match checksum with
-    | Some (_, checksum) -> `Checksum checksum
-    | None -> `Url url
+    | Some (_, checksum) -> `Checksum checksum, Action.Full.allowed_in_shared_cache
+    | None -> `Url url, Fun.id
   in
   let directory_targets =
     let target_dir = make_target ~kind:`Directory checksum_or_url in
@@ -206,6 +206,7 @@ let gen_rules_for_checksum_or_url (loc_url, (url : OpamUrl.t)) checksum =
     let action ~target ~kind =
       action ~url:(loc_url, url) ~checksum ~target ~kind
       |> Action.Full.make
+      |> allowed_in_shared_cache
       |> Action_builder.return
       |> Action_builder.with_no_targets
     in
@@ -327,6 +328,7 @@ let fetch ~target kind (source : Source.t) =
         action ~url:source.url ~checksum:source.checksum ~target ~kind
     in
     Action.Full.make action
+    |> Action.Full.allowed_in_shared_cache
     |> Action_builder.With_targets.return
     |> Action_builder.With_targets.add_directories ~directory_targets:[ target ]
 ;;
diff --git a/test/blackbox-tests/test-cases/pkg/fetch-cache.t b/test/blackbox-tests/test-cases/pkg/fetch-cache.t
new file mode 100644
index 000000000000..db3edf5ae514
--- /dev/null
+++ b/test/blackbox-tests/test-cases/pkg/fetch-cache.t
@@ -0,0 +1,61 @@
+Testing that files are only fetched once.
+
+  $ . ./helpers.sh
+
+No need to set DUNE_CACHE (enabled by default) nor DUNE_CACHE_RULES as the
+fetch rules are always considered safe to cache, but we'll set a custom
+directory for the shared cache.
+
+  $ export DUNE_CACHE_ROOT=$(pwd)/dune-cache
+  $ unset DUNE_CACHE
+  $ unset DUNE_CACHE_RULES
+
+Set up a project that depends on a package that is being downloaded
+
+  $ make_lockdir
+  $ echo "Contents" > tar-contents
+  $ CONTENT_CHECKSUM=$(md5sum tar-contents | cut -f1 -d' ')
+  $ tar cf test.tar tar-contents
+  $ echo test.tar > fake-curls
+  $ SRC_PORT=1
+  $ SRC_CHECKSUM=$(md5sum test.tar | cut -f1 -d' ')
+  $ cat >dune.lock/test.pkg <<EOF
+  > (version 0.0.1)
+  > (source
+  >   (fetch
+  >     (url http://localhost:$SRC_PORT)
+  >     (checksum md5=$SRC_CHECKSUM)))
+  > EOF
+  $ cat > dune-project <<EOF
+  > (lang dune 3.17)
+  > (package (name my) (depends test) (allow_empty))
+  > EOF
+
+The first build should succeed, fetching the source, populating the cache and
+disabling the download of the source a second time.
+
+  $ dune build
+
+Make sure that the file that was fetched is in the cache:
+
+  $ find $DUNE_CACHE_ROOT/files -type f -exec md5sum {} \; | grep --quiet $CONTENT_CHECKSUM
+
+Cleaning the project to force rebuilding. If we attempt to build without the
+cache, it will fail, as the source is 404 now:
+
+  $ dune clean
+  $ export DUNE_CACHE=disabled
+  $ dune build
+  File "dune.lock/test.pkg", line 4, characters 9-27:
+  4 |     (url http://localhost:1)
+               ^^^^^^^^^^^^^^^^^^
+  Error: download failed with code 404
+         
+  [1]
+
+However when enabling the cache again, the file that was fetched in the first
+build should be retrieved from the cache and the build succeed:
+
+  $ dune clean
+  $ export DUNE_CACHE=enabled
+  $ dune build