Merge pull request #12719 from Alizter/push-mnmqpqzysnkr

fix(pkg): make autolock solver env same as pkg lock

Author: Alizter

boot/libs.ml

@@ -1,5 +1,5 @@
 open Types
-let external_libraries = [ "pp"; "unix"; "csexp"; "threads" ]
+let external_libraries = [ "unix"; "threads" ]
 
 let local_libraries =
   [ { path = "otherlibs/top-closure"

src/dune_rules/lock_rules.ml

@@ -135,9 +135,11 @@ module Spec = struct
     let open Fiber.O in
     let* () = Fiber.return () in
     let local_packages = Package.Name.Map.map packages ~f:Local_package.for_solver in
-    (* Whether or not the lock directory we are creating is portable or not
-       doesn't concern us. We therefore set it as non-portable. *)
-    let portable_lock_dir = false in
+    let portable_lock_dir =
+      match Config.get Compile_time.portable_lock_dir with
+      | `Enabled -> true
+      | `Disabled -> false
+    in
     let* solver_env =
       (* CR-soon Alizter: This solver environment construction pattern (combining
        solver_env_from_current_system with solver_env_from_context, then
@@ -154,15 +156,55 @@ module Spec = struct
       Solver_env.unset_multi solver_env unset_solver_vars
     in
     let* solver_result =
-      Opam_solver.solve_lock_dir
-        solver_env
-        version_preference
-        repos
-        ~pins
-        ~local_packages
-        ~constraints
-        ~selected_depopts
-        ~portable_lock_dir
+      if portable_lock_dir
+      then (
+        (* CR-someday Alizter: This multi-platform solving logic is duplicated
+           from bin/pkg/lock.ml:solve_multiple_platforms. The logic for
+           removing platform-specific variables, solving for multiple platforms
+           in parallel, merging results, and error handling should be shared
+           between autolocking and manual locking. Consider extracting this
+           into a shared function in Dune_pkg.Opam_solver. *)
+        let portable_solver_env =
+          Solver_env.unset_multi
+            solver_env
+            Dune_lang.Package_variable_name.platform_specific
+        in
+        let solve_for_platforms = Solver_env.popular_platform_envs in
+        let+ results =
+          Fiber.parallel_map solve_for_platforms ~f:(fun platform_env ->
+            let solver_env_for_platform =
+              Solver_env.extend portable_solver_env platform_env
+            in
+            Opam_solver.solve_lock_dir
+              solver_env_for_platform
+              version_preference
+              repos
+              ~pins
+              ~local_packages
+              ~constraints
+              ~selected_depopts
+              ~portable_lock_dir)
+        in
+        let solver_results, errors =
+          List.partition_map results ~f:(function
+            | Ok result -> Left result
+            | Error e -> Right e)
+        in
+        match solver_results, errors with
+        | [], [] -> Code_error.raise "Solver did not run for any platforms." []
+        | [], `Manifest_error diagnostic :: _ -> Error (`Manifest_error diagnostic)
+        | [], `Solve_error diagnostic :: _ -> Error (`Solve_error diagnostic)
+        | x :: xs, _ -> Ok (List.fold_left xs ~init:x ~f:Opam_solver.Solver_result.merge))
+      else
+        Opam_solver.solve_lock_dir
+          solver_env
+          version_preference
+          repos
+          ~pins
+          ~local_packages
+          ~constraints
+          ~selected_depopts
+          ~portable_lock_dir
     in
     match solver_result with
     | Error (`Manifest_error diagnostic) -> raise (User_error.E diagnostic)

test/blackbox-tests/test-cases/pkg/pkg-lock-then-autolock.t

@@ -1,4 +1,5 @@
-Test that explicit locking followed by auto-locking produces equivalent results.
+Test that explicit locking followed by auto-locking produces equivalent results
+and does not trigger unnecessary rebuilds.
 
   $ . ./helpers.sh
   $ mkrepo
@@ -54,8 +55,10 @@ Create a project that depends on foo:
   > EOF
 
 Lock and build with explicit dune pkg lock:
-  $ dune_pkg_lock_normalized
-  Solution for dune.lock:
+  $ dune pkg lock
+  Solution for dune.lock
+  
+  Dependencies common to all supported platforms:
   - foo.0.0.1
 
   $ dune exec --display short bar 2>&1 | grep "Building"
@@ -70,12 +73,9 @@ Remove the lock directory:
 Enable auto-locking:
   $ enable_pkg
 
-Build again - should auto-lock internally. Since the package solution is now
-private, it can be specialied to the current platform. This means that the
-build plan for the package has changed from the original portable solution
-generated by 'dune pkg lock', and so the dependencies must be rebuilt.
-  $ dune exec --display short bar 2>&1 | grep "Building"
-      Building foo.0.0.1
+Build again - should auto-lock internally and NOT rebuild foo:
+  $ dune exec --display short bar 2>&1 | grep "Building" || echo "no rebuilds"
+  no rebuilds
 
   $ dune exec bar
   Hello from foo 0.0.1!