fix(package-deps): work around overlapping sandbox inputs

src/dune_rules/pkg_rules.ml

@@ -2622,7 +2622,14 @@ let find_package ctx pkg =
   | false -> Memo.return None
   | true ->
     let+ pkg = resolve_pkg_dep ctx (Loc.none, pkg) in
-    Some (Action_builder.deps (Dep.Set.add (Pkg.package_deps pkg) (Pkg.dep pkg)))
+    (* CR-someday Alizter: This works around an engine bug with overlapping
+       directory and file dependencies in sandboxes (#16547). Depend on whole
+       installation directories once the engine handles these overlaps. *)
+    Some
+      (Action_builder.List.iter (pkg :: Pkg.deps_closure pkg) ~f:(fun { Pkg.paths; _ } ->
+         let open Action_builder.O in
+         let* { Install_cookie.Gen.files; _ } = (Pkg_installed.of_paths paths).cookie in
+         Action_builder.paths (Section.Map.values files |> List.concat)))
 ;;
 
 let package_prefixes ctx =

test/blackbox-tests/test-cases/pkg/install-cookie-load-dedup.t

@@ -1,4 +1,5 @@
-Each install cookie should be deserialised at most once per build.
+Each install cookie should be deserialised at most once per build. With
+per-file package dependencies, this currently happens once per rule instead.
 
   $ make_lockdir
 
@@ -20,10 +21,10 @@ Two rules, both depending on package base:
 
   $ DUNE_TRACE="persistent" dune build out1 out2
 
-Count install-cookie loads in this build. Neither rule needs to load the cookie:
+Count install-cookie loads in this build. Each rule currently loads it separately:
 
   $ dune trace cat | jq -s '
   >   [ .[] | select(.args.module == "INSTALL-COOKIE" and .args.operation == "load") ]
   >   | length
   > '
-  0
+  2

test/blackbox-tests/test-cases/pkg/package-deps-run-binary.t

@@ -27,19 +27,13 @@ correctly under the default sandbox.
   >   (with-stdout-to out (run mybin))))
   > EOF
 
-This should print "from provider", but fails instead:
+The package dependency and the binary dependency can be sandboxed together.
 
-  $ dune build @all 2>&1 | censor
-  Error:
-  symlink(_build/.sandbox/$DIGEST1/_private/default/.pkg/provider.0.0.1-$DIGEST2/target): File exists
-  -> required by _build/default/out
-  -> required by alias all
-  [1]
+  $ dune build @all
   $ cat _build/default/out
-  cat: _build/default/out: No such file or directory
-  [1]
+  from provider
 
-Removing [(deps (package provider))] fixes the problem
+Resolving the binary without a package dependency still works.
 
   $ cat >dune <<'EOF'
   > (rule

test/blackbox-tests/test-cases/pkg/package-library-closure.t

@@ -149,16 +149,16 @@ Check that all the libraries and package data were installed.
   $ test -f "$b_target/share/b-provider/payload"
 
 The requested package and its lock package dependencies are tracked through
-their whole installation directories.
+individual installed files, not their whole installation directories.
 
   $ dune rules --format=json main.exe >rules.json
   $ jq_dune --arg a "$a_target" --arg b "$b_target" \
   > --arg c "$c_target" --arg d "$d_target" '
   >   rulesMatchingTarget("main.exe") | {
-  >     requested_package: ruleHasDepFile($a),
-  >     required_package: ruleHasDepFile($b),
-  >     package_only_dependency: ruleHasDepFile($c),
-  >     sibling_dependency: ruleHasDepFile($d)
+  >     requested_package: ruleHasDepFile($a + "/lib/a/a.cma"),
+  >     required_package: ruleHasDepFile($b + "/lib/b/b.cma"),
+  >     package_only_dependency: ruleHasDepFile($c + "/lib/c/c.cma"),
+  >     sibling_dependency: ruleHasDepFile($d + "/lib/d/d.cma")
   >   }' rules.json
   {
     "requested_package": true,
@@ -209,25 +209,25 @@ does not invoke ocamlfind, so the missing META cannot break the action itself.
   > --arg a "$a_target" --arg b "$b_target" \
   > --arg c "$c_target" --arg d "$d_target" '
   >   rulesMatchingTarget("package-result") | {
-  >     requested_package: ruleHasDepFile($a),
+  >     requested_package: ruleHasDepFile($a + "/lib/a/a.cma"),
   >     requested_metadata: ruleHasDepFile($a + "/lib/a/dune-package"),
-  >     required_package: ruleHasDepFile($b),
-  >     package_only_dependency: ruleHasDepFile($c),
-  >     sibling_dependency: ruleHasDepFile($d)
+  >     required_package: ruleHasDepFile($b + "/lib/b/b.cma"),
+  >     package_only_dependency: ruleHasDepFile($c + "/lib/c/c.cma"),
+  >     sibling_dependency: ruleHasDepFile($d + "/lib/d/d.cma")
   >   }'
   {
     "requested_package": true,
-    "requested_metadata": false,
+    "requested_metadata": true,
     "required_package": true,
     "package_only_dependency": true,
     "sibling_dependency": true
   }
 
 Request workspace package a explicitly. Its library depends on installed
 library b, whose owning lock package is b-provider, not b. The local package is
-materialised and b-provider's installation directory becomes a direct
-dependency, along with the installation directory of its dependency d.
-Package c is not in this closure.
+materialised and b-provider's installed files become direct dependencies,
+along with the installed files of its dependency d. Package c is not in this
+closure.
 
   $ echo '(package (name a))' >>dune-project
   $ mkdir workspace-a
@@ -251,16 +251,16 @@ Package c is not in this closure.
   $ dune rules --format=json main.exe | jq_dune \
   > --arg b "$b_target" --arg c "$c_target" --arg d "$d_target" '
   >   rulesMatchingTarget("main.exe") | {
-  >     required_package: ruleHasDepFile($b),
+  >     required_directory: ruleHasDepFile($b),
   >     required_cookie: ruleHasDepFile($b + "/cookie"),
   >     required_archive: ruleHasDepFile($b + "/lib/b/b.cma"),
-  >     transitive_package: ruleHasDepFile($d),
-  >     unrelated_package: ruleHasDepFile($c)
+  >     transitive_package: ruleHasDepFile($d + "/lib/d/d.cma"),
+  >     unrelated_package: ruleHasDepFile($c + "/lib/c/c.cma")
   >   }'
   {
-    "required_package": true,
-    "required_cookie": false,
-    "required_archive": false,
+    "required_directory": false,
+    "required_cookie": true,
+    "required_archive": true,
     "transitive_package": true,
     "unrelated_package": false
   }