Merge pull request #6125 from rjbou/fix-depsonly-root

Fix regression: deps-only setting direct dependencies as root packages by default

master_changes.md

@@ -31,6 +31,7 @@ users)
 ## Install
   * Fix package name display for no agreement conflicts [#6055 @rjbou - fix #6030]
   * Make fetching an archive from cache add missing symlinks [#6068 @kit-ty-kate - fix #6064]
+  * [BUG] Fix `opam install --deps-only` set direct dependencies as root packages [#6125 @rjbou]
 
 ## Build (package)
   * ◈ Add `--verbose-on` option to enable verbose mode on specified package names [#5682 @desumn @rjbou]
@@ -141,6 +142,7 @@ users)
   * env: Add a test for `build-env` overwrites build env opam environment variables [#5377 @rjbou]
   * clean: Add to check cleaning of sources directories [#5474 @rjbou]
   * Add reftest for `--verbose-on` option [#5682 @rjbou]
+  * Add a test for --deps-only setting direct dependencies as root packages [#6125 @rjbou]
 
 ### Engine
 

src/client/opamClient.ml

@@ -2301,21 +2301,34 @@ let install_t t ?ask ?(ignore_conflicts=false) ?(depext_only=false)
         (OpamSolution.install_depexts ~force_depext:true ~confirm:false t
            (OpamSolver.all_packages solution)), None
       else
-      let add_roots =
-        OpamStd.Option.map (function
-            | true ->
-              OpamPackage.Name.Set.union requested @@
-              OpamPackage.Name.Set.of_list (List.rev_map fst deps_atoms)
-            | false -> OpamPackage.Name.Set.empty)
-          add_to_roots
-      in
-      let t, res =
-        OpamSolution.apply ?ask t
-          ~requested:packages
-          ~print_requested:(print_requested requested formula)
-          ?add_roots ~skip
-          ~download_only ~assume_built solution in
-      t, Some (Success res)
+        let add_roots =
+          match add_to_roots, deps_only with
+          | (Some true | None), true ->
+            let pkgs_solution = OpamSolver.all_packages solution in
+            let requested_deps =
+              OpamPackage.Set.fold (fun nv acc ->
+                  OpamFormula.Or
+                    (OpamPackageVar.all_depends t (OpamSwitchState.opam t nv)
+                       ~depopts:false ~build:true ~post:false,
+                     acc))
+                (OpamPackage.packages_of_names pkgs_solution requested)
+                (* solution contains 'deps-to-xxx' virtual packages
+                   and requested is deps-to-xxx packages *)
+                OpamFormula.Empty
+            in
+            Some (OpamPackage.names_of_packages
+                    (OpamFormula.packages pkgs_solution requested_deps))
+          | Some true, false -> Some requested
+          | Some false, _ -> Some OpamPackage.Name.Set.empty
+          | None, false -> None
+        in
+        let t, res =
+          OpamSolution.apply ?ask t
+            ~requested:packages
+            ~print_requested:(print_requested requested formula)
+            ?add_roots ~skip
+            ~download_only ~assume_built solution in
+        t, Some (Success res)
   in
   OpamStd.Option.iter (OpamSolution.check_solution t) solution;
   t

tests/reftests/deps-only.test

@@ -105,3 +105,123 @@ depends: "foo"
 conflicts: "bar"
 ### opam install d --deps-only
 Nothing to do.
+### :::: check if deps-only installed packages are set root ::::
+### <pkg:roots1.1>
+opam-version: "2.0"
+### <pkg:roots2.1>
+opam-version: "2.0"
+depends: "roots1"
+### <pkg:roots3.1>
+opam-version: "2.0"
+depends: "roots2"
+### <pkg:stoor1.1>
+opam-version: "2.0"
+### <pkg:stoor2.1>
+opam-version: "2.0"
+depends: "stoor1"
+### opam switch create roots --empty
+### : deps of a package
+### opam install roots3 --deps-only
+The following actions will be performed:
+=== install 2 packages
+  - install roots1 1 [required by roots2]
+  - install roots2 1 [required by roots3]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed roots1.1
+-> installed roots2.1
+Done.
+### opam list --roots -s
+roots2
+### opam remove roots1
+The following actions will be performed:
+=== remove 2 packages
+  - remove roots1 1
+  - remove roots2 1 [uses roots1]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> removed   roots2.1
+-> removed   roots1.1
+Done.
+### : deps for 2 package that depend on each other
+### opam install roots3 roots2 --deps-only
+The following actions will be performed:
+=== install 1 package
+  - install roots1 1 [required by roots2]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed roots1.1
+Done.
+### opam list --roots -s
+roots1
+### opam remove roots1
+The following actions will be performed:
+=== remove 1 package
+  - remove roots1 1
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> removed   roots1.1
+Done.
+### : deps for 2 package that depend on each other
+### opam install roots3 stoor2 --deps-only | unordered
+The following actions will be performed:
+=== install 3 packages
+  - install roots1 1 [required by roots2]
+  - install roots2 1 [required by roots3]
+  - install stoor1 1 [required by stoor2]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed roots1.1
+-> installed roots2.1
+-> installed stoor1.1
+Done.
+### opam list --roots -s
+roots2
+stoor1
+### opam remove roots1 stoor1 | unordered
+The following actions will be performed:
+=== remove 3 packages
+  - remove roots1 1
+  - remove roots2 1 [uses roots1]
+  - remove stoor1 1
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> removed   roots2.1
+-> removed   roots1.1
+-> removed   stoor1.1
+Done.
+### : deps with explicite set-root
+### opam install roots3 --deps-only --set-root
+The following actions will be performed:
+=== install 2 packages
+  - install roots1 1 [required by roots2]
+  - install roots2 1 [required by roots3]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed roots1.1
+-> installed roots2.1
+Done.
+### opam list --roots -s
+roots2
+### opam remove roots1
+The following actions will be performed:
+=== remove 2 packages
+  - remove roots1 1
+  - remove roots2 1 [uses roots1]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> removed   roots2.1
+-> removed   roots1.1
+Done.
+### : deps with explicit unset-root
+### opam install roots3 --deps-only --unset-root
+The following actions will be performed:
+=== install 2 packages
+  - install roots1 1 [required by roots2]
+  - install roots2 1 [required by roots3]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed roots1.1
+-> installed roots2.1
+Done.
+### opam list --roots -s