Skip to content

Commit bcb2a40

Browse files
committed
fix(pkg): make autolock solver env same as pkg lock
Signed-off-by: Ali Caglayan <[email protected]>
1 parent 308cde2 commit bcb2a40

File tree

3 files changed

+64
-22
lines changed

3 files changed

+64
-22
lines changed

boot/libs.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
open Types
2-
let external_libraries = [ "pp"; "unix"; "csexp"; "threads" ]
2+
let external_libraries = [ "unix"; "threads" ]
33

44
let local_libraries =
55
[ { path = "otherlibs/top-closure"

src/dune_rules/lock_rules.ml

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,11 @@ module Spec = struct
135135
let open Fiber.O in
136136
let* () = Fiber.return () in
137137
let local_packages = Package.Name.Map.map packages ~f:Local_package.for_solver in
138-
(* Whether or not the lock directory we are creating is portable or not
139-
doesn't concern us. We therefore set it as non-portable. *)
140-
let portable_lock_dir = false in
138+
let portable_lock_dir =
139+
match Config.get Compile_time.portable_lock_dir with
140+
| `Enabled -> true
141+
| `Disabled -> false
142+
in
141143
let* solver_env =
142144
(* CR-soon Alizter: This solver environment construction pattern (combining
143145
solver_env_from_current_system with solver_env_from_context, then
@@ -154,15 +156,55 @@ module Spec = struct
154156
Solver_env.unset_multi solver_env unset_solver_vars
155157
in
156158
let* solver_result =
157-
Opam_solver.solve_lock_dir
158-
solver_env
159-
version_preference
160-
repos
161-
~pins
162-
~local_packages
163-
~constraints
164-
~selected_depopts
165-
~portable_lock_dir
159+
if portable_lock_dir
160+
then (
161+
(* CR-someday Alizter: This multi-platform solving logic is duplicated
162+
from bin/pkg/lock.ml:solve_multiple_platforms. The logic for
163+
removing platform-specific variables, solving for multiple platforms
164+
in parallel, merging results, and error handling should be shared
165+
between autolocking and manual locking. Consider extracting this
166+
into a shared function in Dune_pkg.Opam_solver. *)
167+
let portable_solver_env =
168+
Solver_env.unset_multi
169+
solver_env
170+
Dune_lang.Package_variable_name.platform_specific
171+
in
172+
let solve_for_platforms = Solver_env.popular_platform_envs in
173+
let+ results =
174+
Fiber.parallel_map solve_for_platforms ~f:(fun platform_env ->
175+
let solver_env_for_platform =
176+
Solver_env.extend portable_solver_env platform_env
177+
in
178+
Opam_solver.solve_lock_dir
179+
solver_env_for_platform
180+
version_preference
181+
repos
182+
~pins
183+
~local_packages
184+
~constraints
185+
~selected_depopts
186+
~portable_lock_dir)
187+
in
188+
let solver_results, errors =
189+
List.partition_map results ~f:(function
190+
| Ok result -> Left result
191+
| Error e -> Right e)
192+
in
193+
match solver_results, errors with
194+
| [], [] -> Code_error.raise "Solver did not run for any platforms." []
195+
| [], `Manifest_error diagnostic :: _ -> Error (`Manifest_error diagnostic)
196+
| [], `Solve_error diagnostic :: _ -> Error (`Solve_error diagnostic)
197+
| x :: xs, _ -> Ok (List.fold_left xs ~init:x ~f:Opam_solver.Solver_result.merge))
198+
else
199+
Opam_solver.solve_lock_dir
200+
solver_env
201+
version_preference
202+
repos
203+
~pins
204+
~local_packages
205+
~constraints
206+
~selected_depopts
207+
~portable_lock_dir
166208
in
167209
match solver_result with
168210
| Error (`Manifest_error diagnostic) -> raise (User_error.E diagnostic)

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
Test that explicit locking followed by auto-locking produces equivalent results.
1+
Test that explicit locking followed by auto-locking produces equivalent results
2+
and does not trigger unnecessary rebuilds.
23

34
$ . ./helpers.sh
45
$ mkrepo
@@ -54,8 +55,10 @@ Create a project that depends on foo:
5455
> EOF
5556

5657
Lock and build with explicit dune pkg lock:
57-
$ dune_pkg_lock_normalized
58-
Solution for dune.lock:
58+
$ dune pkg lock
59+
Solution for dune.lock
60+
61+
Dependencies common to all supported platforms:
5962
- foo.0.0.1
6063

6164
$ dune exec --display short bar 2>&1 | grep "Building"
@@ -70,12 +73,9 @@ Remove the lock directory:
7073
Enable auto-locking:
7174
$ enable_pkg
7275

73-
Build again - should auto-lock internally. Since the package solution is now
74-
private, it can be specialied to the current platform. This means that the
75-
build plan for the package has changed from the original portable solution
76-
generated by 'dune pkg lock', and so the dependencies must be rebuilt.
77-
$ dune exec --display short bar 2>&1 | grep "Building"
78-
Building foo.0.0.1
76+
Build again - should auto-lock internally and NOT rebuild foo:
77+
$ dune exec --display short bar 2>&1 | grep "Building" || echo "no rebuilds"
78+
no rebuilds
7979

8080
$ dune exec bar
8181
Hello from foo 0.0.1!

0 commit comments

Comments
 (0)