fix: Add or remove dune from package sets to allow resolving packages depending on it (#11103)

* fix: Add `dune` to existing packages to allow resolving

Signed-off-by: Marek Kubica <[email protected]>

* Alternate solution which removes `dune` from formulas

Signed-off-by: Marek Kubica <[email protected]>

* chore: leave a comment

Signed-off-by: Rudi Grinberg <[email protected]>

* fix(pkg): use correct dune version to evaluate

Signed-off-by: Rudi Grinberg <[email protected]>

---------

Signed-off-by: Marek Kubica <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]>
Co-authored-by: Rudi Grinberg <[email protected]>

Author: Leonidas-from-XIV

src/dune_pkg/dune_dep.ml

@@ -1 +1,8 @@
+open Stdune
+
 let name = Package_name.of_string "dune"
+
+let version =
+  let major, minor = Dune_lang.Stanza.latest_version in
+  OpamPackage.Version.of_string @@ sprintf "%d.%d" major minor
+;;

src/dune_pkg/dune_dep.mli

@@ -1 +1,2 @@
 val name : Package_name.t
+val version : OpamPackage.Version.t

src/dune_pkg/lock_dir.ml

@@ -351,6 +351,8 @@ let validate_packages packages =
     Package_name.Map.values packages
     |> List.concat_map ~f:(fun (dependant_package : Pkg.t) ->
       List.filter_map dependant_package.depends ~f:(fun (loc, dependency) ->
+        (* CR-someday rgrinberg: do we need the dune check? aren't
+           we supposed to filter these upfront? *)
         if Package_name.Map.mem packages dependency
            || Package_name.equal dependency Dune_dep.name
         then None

src/dune_pkg/opam_solver.ml

@@ -111,10 +111,6 @@ module Context_for_dune = struct
     ~stats_updater
     ~constraints
     =
-    let dune_version =
-      let major, minor = Dune_lang.Stanza.latest_version in
-      OpamPackage.Version.of_string @@ sprintf "%d.%d" major minor
-    in
     let candidates_cache = Fiber_cache.create (module Package_name) in
     let constraints =
       List.map constraints ~f:(fun (constraint_ : Package_dependency.t) ->
@@ -136,7 +132,7 @@ module Context_for_dune = struct
     ; local_packages
     ; pinned_packages
     ; solver_env
-    ; dune_version
+    ; dune_version = Dune_dep.version
     ; stats_updater
     ; candidates_cache
     ; available_cache
@@ -785,7 +781,7 @@ let reject_unreachable_packages =
     loop roots;
     !seen
   in
-  fun solver_env ~local_packages ~pkgs_by_name ->
+  fun solver_env ~dune_version ~local_packages ~pkgs_by_name ->
     let roots = Package_name.Map.keys local_packages in
     let pkgs_by_version =
       Package_name.Map.merge pkgs_by_name local_packages ~f:(fun name lhs rhs ->
@@ -810,7 +806,7 @@ let reject_unreachable_packages =
             [ "name", Package_name.to_dyn name ]
         | Some (pkg : Lock_dir.Pkg.t), None -> Some (List.map pkg.depends ~f:snd)
         | None, Some (pkg : Local_package.For_solver.t) ->
-          let formula = pkg.dependencies |> Dependency_formula.to_filtered_formula in
+          let formula = Dependency_formula.to_filtered_formula pkg.dependencies in
           (* Use `dev` because at this point we don't have any version *)
           let opam_package =
             OpamPackage.of_string (sprintf "%s.dev" (Package_name.to_string pkg.name))
@@ -820,14 +816,15 @@ let reject_unreachable_packages =
             Resolve_opam_formula.filtered_formula_to_package_names
               env
               ~with_test:true
-              pkgs_by_version
+              (Package_name.Map.set pkgs_by_version Dune_dep.name dune_version)
               formula
           in
           let deps =
             match resolved with
-            | Ok { regular; post = _ } ->
-              (* discard post deps *)
-              regular
+            | Ok { regular; post = _ (* discard post deps *) } ->
+              (* remove Dune from the formula as we remove it from solutions *)
+              List.filter regular ~f:(fun pkg ->
+                not (Package_name.equal Dune_dep.name pkg))
             | Error _ ->
               Code_error.raise
                 "can't find a valid solution for the dependencies"
@@ -948,7 +945,11 @@ let solve_lock_dir
                       (Package_name.to_string dep_name)
                   ]));
         let reachable =
-          reject_unreachable_packages solver_env ~local_packages ~pkgs_by_name
+          reject_unreachable_packages
+            solver_env
+            ~dune_version:(Package_version.of_opam_package_version context.dune_version)
+            ~local_packages
+            ~pkgs_by_name
         in
         let pkgs_by_name =
           Package_name.Map.filteri pkgs_by_name ~f:(fun name _ ->

test/blackbox-tests/test-cases/pkg/implicit-dune-constraint.t

@@ -38,6 +38,6 @@ Create a fake project and ensure `dune` can be used as a dependency:
   >  (allow_empty)
   >  (depends dune))
   > EOF
-  $ dune pkg lock 2>&1 | head -2
-  Internal error, please report upstream including the contents of _build/log.
-  Description:
+  $ dune pkg lock
+  Solution for dune.lock:
+  (no dependencies to lock)