From f8479355c044969b5eb03cbfc1bc787d513b5541 Mon Sep 17 00:00:00 2001 From: Stephen Sherratt Date: Tue, 11 Feb 2025 15:41:38 +1100 Subject: [PATCH] pkg: defer evaluating platform vars in commands Package build and install commands are expressions that generate the command to run by evaluating a series of conditional statements. This allows the commands to be tailored to the current machine where the command is being run. Previously dune would evaluate platform-specific variables (e.g. os, arch) in these expressions at solve-time, producing lockfiles specialized to the machine where they are solved. This change defers evaluation of these variables until build-time. The consequence of deferring evaluation of platform variables to build time is that lockfiles will preserve more of the conditional logic found in the corresponding opam file. Note that some variables are still evaluated, such as `with-test`, as these variables are not based on the platform. The benefit of deferring evaluation of platform variables is that users of computers that differ from the computer where the lockfile was generated will be able to run the build/install commands specialized for their computer, which is more likely to work than attempting to run the build/install commands specialized for the computer where the lockfile was generated. Note that this change alone is not sufficient for making lockfiles portable, as it doesn't address the fact that the dependencies of a package can also vary depending on platform-specific variables. Signed-off-by: Stephen Sherratt --- src/dune_lang/package_variable_name.ml | 4 ++++ src/dune_lang/package_variable_name.mli | 4 ++++ src/dune_pkg/opam_solver.ml | 15 ++++++++++++--- src/dune_pkg/solver_env.mli | 3 +++ .../pkg/solver-vars-in-lockdir-metadata.t | 17 +++++++++-------- 5 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/dune_lang/package_variable_name.ml b/src/dune_lang/package_variable_name.ml index 9c3e06d4953..bc1570e93f6 100644 --- a/src/dune_lang/package_variable_name.ml +++ b/src/dune_lang/package_variable_name.ml @@ -53,6 +53,10 @@ let post = of_string "post" let one_of t xs = List.mem xs ~equal t let dev = of_string "dev" +let platform_specific = + Set.of_list [ arch; os; os_version; os_distribution; os_family; sys_ocaml_version ] +;; + module Project = struct let encode name = Dune_sexp.Encoder.string (":" ^ to_string name) diff --git a/src/dune_lang/package_variable_name.mli b/src/dune_lang/package_variable_name.mli index 02dd9d7e1a7..d0dd1e770a7 100644 --- a/src/dune_lang/package_variable_name.mli +++ b/src/dune_lang/package_variable_name.mli @@ -32,6 +32,10 @@ val build : t val dev : t val one_of : t -> t list -> bool +(** The set of variable names whose values are expected to differ depending on + the current platform. *) +val platform_specific : Set.t + module Project : sig val encode : t Dune_sexp.Encoder.t val decode : t Dune_sexp.Decoder.t diff --git a/src/dune_pkg/opam_solver.ml b/src/dune_pkg/opam_solver.ml index 9d5e3393361..123bf0573d5 100644 --- a/src/dune_pkg/opam_solver.ml +++ b/src/dune_pkg/opam_solver.ml @@ -1736,9 +1736,18 @@ let opam_package_to_lock_file_pkg | [] -> action | env_update -> Action.Withenv (env_update, action) in - let get_solver_var variable_name = - Solver_stats.Updater.expand_variable stats_updater variable_name; - Solver_env.get solver_env variable_name + let get_solver_var = + (* Remove platform-specific variables from the solver env so that the + generated commands are portable rather than specialized to the computer + performing the solve. *) + let portable_solver_env = + Solver_env.unset_multi solver_env Package_variable_name.platform_specific + in + fun variable_name -> + let value = Solver_env.get portable_solver_env variable_name in + if Option.is_some value + then Solver_stats.Updater.expand_variable stats_updater variable_name; + value in let build_command = if Resolved_package.dune_build resolved_package diff --git a/src/dune_pkg/solver_env.mli b/src/dune_pkg/solver_env.mli index d5c7bcc62bb..fb9ecfb5170 100644 --- a/src/dune_pkg/solver_env.mli +++ b/src/dune_pkg/solver_env.mli @@ -21,5 +21,8 @@ val extend : t -> t -> t val with_defaults : t val pp : t -> 'a Pp.t + +(** Remove a set of bindings from the env *) val unset_multi : t -> Package_variable_name.Set.t -> t + val to_env : t -> OpamFilter.env diff --git a/test/blackbox-tests/test-cases/pkg/solver-vars-in-lockdir-metadata.t b/test/blackbox-tests/test-cases/pkg/solver-vars-in-lockdir-metadata.t index 0cd89b3e662..e01a958b42a 100644 --- a/test/blackbox-tests/test-cases/pkg/solver-vars-in-lockdir-metadata.t +++ b/test/blackbox-tests/test-cases/pkg/solver-vars-in-lockdir-metadata.t @@ -140,11 +140,18 @@ stored in the lockdir metadata: (version 0.0.1) (install - (run echo qux)) + (when + (= %{arch} arm) + (run echo qux))) (build (progn - (run echo foo) + (when + (= %{os} linux) + (run echo foo)) + (when + (= %{os} macos) + (run echo bar)) (run echo baz))) $ cat dune.lock/lock.dune (lang package 0.1) @@ -154,9 +161,3 @@ stored in the lockdir metadata: (repositories (complete false) (used)) - - (expanded_solver_variable_bindings - (variable_values - (os linux) - (arch arm)) - (unset_variables x))