Fix package validation when portable lockdirs is enabled

The `dune pkg validate-lockdir` command checks that the lockdir contains
the transitive dependency closure of all local packages. When run with
portable lockdirs enabled, some packages were omitted from the
dependency closure because the wrong solver environment was being
treated as the current platform. This change fixes the issue by using
the correct solver environment to represent the current platform, and
does a minor refactor so the validation logic isn't juggling two
different solver environments (the second environment is the one stored
in the lockdir capturing the opam solver variables used to generate the
solution).

Signed-off-by: Stephen Sherratt <[email protected]>

Author: gridbugs

src/dune_pkg/package_universe.ml

@@ -5,7 +5,6 @@ type t =
   ; lock_dir : Lock_dir.t
   ; platform : Solver_env.t
   ; version_by_package_name : Package_version.t Package_name.Map.t
-  ; solver_env : Solver_env.t
   }
 
 let lockdir_regenerate_hints =
@@ -54,7 +53,10 @@ let concrete_dependencies_of_local_package t local_package_name ~with_test =
     |> Dependency_formula.to_filtered_formula
     |> Resolve_opam_formula.filtered_formula_to_package_names
          ~with_test
-         ~env:(Solver_env.to_env t.solver_env)
+         ~env:
+           (Solver_stats.Expanded_variable_bindings.to_solver_env
+              t.lock_dir.expanded_solver_variable_bindings
+            |> Solver_env.to_env)
          ~packages:t.version_by_package_name
   with
   | Ok { regular; post = _ } -> regular
@@ -89,7 +91,6 @@ let all_non_local_dependencies_of_local_packages t =
 let check_for_unnecessary_packges_in_lock_dir
       ~platform
       (lock_dir : Lock_dir.t)
-      solver_env
       all_non_local_dependencies_of_local_packages
   =
   let packages = Lock_dir.Packages.pkgs_on_platform_by_name lock_dir.packages ~platform in
@@ -98,7 +99,7 @@ let check_for_unnecessary_packges_in_lock_dir
       match
         Lock_dir.transitive_dependency_closure
           lock_dir
-          ~platform:solver_env
+          ~platform
           all_non_local_dependencies_of_local_packages
       with
       | Ok x -> x
@@ -221,25 +222,21 @@ let validate_dependency_hash local_packages ~saved_dependency_hash =
         ]
 ;;
 
-let validate ~platform t =
+let validate t =
   validate_dependency_hash
     t.local_packages
     ~saved_dependency_hash:t.lock_dir.dependency_hash;
   all_non_local_dependencies_of_local_packages t
-  |> check_for_unnecessary_packges_in_lock_dir ~platform t.lock_dir t.solver_env
+  |> check_for_unnecessary_packges_in_lock_dir ~platform:t.platform t.lock_dir
 ;;
 
 let create ~platform local_packages lock_dir =
   try
     let version_by_package_name =
       version_by_package_name ~platform local_packages lock_dir
     in
-    let solver_env =
-      Solver_stats.Expanded_variable_bindings.to_solver_env
-        lock_dir.expanded_solver_variable_bindings
-    in
-    let t = { local_packages; lock_dir; platform; version_by_package_name; solver_env } in
-    validate ~platform t;
+    let t = { local_packages; lock_dir; platform; version_by_package_name } in
+    validate t;
     Ok t
   with
   | User_error.E e -> Error e
@@ -281,7 +278,7 @@ let transitive_dependency_closure_without_test t start =
     match
       Lock_dir.transitive_dependency_closure
         t.lock_dir
-        ~platform:t.solver_env
+        ~platform:t.platform
         Package_name.Set.(
           union
             non_local_immediate_dependencies_of_local_transitive_dependency_closure

test/blackbox-tests/test-cases/pkg/portable-lockdirs/portable-lockdirs-validation.t

@@ -0,0 +1,65 @@
+Exercise the `dune pkg validate-lockdir` command on portable lockdirs.
+
+  $ . ../helpers.sh
+  $ mkrepo
+  $ add_mock_repo_if_needed
+
+Define some interdependent opam packages:
+  $ mkpkg a 0.0.1 <<EOF
+  > depends: [ "c" "d" ]
+  > EOF
+  $ mkpkg b 0.0.1 <<EOF
+  > EOF
+  $ mkpkg b 0.0.2 <<EOF
+  > EOF
+  $ mkpkg c <<EOF
+  > depends: [ "e" ]
+  > EOF
+  $ mkpkg d <<EOF
+  > EOF
+  $ mkpkg e <<EOF
+  > EOF
+
+Define some local packages.
+  $ cat >dune-project <<EOF
+  > (lang dune 3.20)
+  > (package (name foo) (depends a (b (>= 0.0.2))))
+  > (package (name bar) (depends foo c))
+  > EOF
+
+Solve dependencies:
+  $ DUNE_CONFIG__PORTABLE_LOCK_DIR=enabled dune pkg lock
+  Solution for dune.lock
+  
+  This solution supports the following platforms:
+  - arch = x86_64; os = linux
+  - arch = arm64; os = linux
+  - arch = x86_64; os = macos
+  - arch = arm64; os = macos
+  - arch = x86_64; os = win32
+  
+  Dependencies on all supported platforms:
+  - a.0.0.1
+  - b.0.0.2
+  - c.0.0.1
+  - d.0.0.1
+  - e.0.0.1
+
+Validate the lockdir. This will succeed because dune generates valid lockdirs.
+  $ dune pkg validate-lockdir
+
+Remove a package from the lockdir.
+  $ rm ${source_lock_dir}/a.0.0.1.pkg
+
+Validate the lockdir. This time dune detects that a required lockfile is missing.
+  $ dune pkg validate-lockdir
+  Lockdir dune.lock does not contain a solution for local packages:
+  File "dune-project", line 2, characters 0-47:
+  Error: The dependencies of local package "foo" could not be satisfied from
+  the lockdir:
+  Package "a" is missing
+  Hint: The lockdir no longer contains a solution for the local packages in
+  this project. Regenerate the lockdir by running: 'dune pkg lock'
+  Error: Some lockdirs do not contain solutions for local packages:
+  - dune.lock
+  [1]