[WIP] Cudf request preprocessing

.ocamlinit

@@ -1,4 +1,5 @@
 #use "topfind";;
+#use "down.top";;
 #require "opam-client";;
 
 OpamClientConfig.opam_init ();;

Makefile

@@ -107,7 +107,7 @@ endif
 endif
 
 ifneq ($(LIBINSTALL_DIR),)
-    OPAMINSTALLER_FLAGS += --libdir "$(LIBINSTALL_DIR)"
+    OPAMINSTALLER_FLAGS += --libdir "$(LIBINSTALL_DIR)" --docdir "$(LIBINSTALL_DIR)/../doc"
 endif
 
 opam-devel.install: $(DUNE_DEP)

master_changes.md

@@ -19,7 +19,8 @@ New option/command/subcommand are prefixed with ◈.
   * The stdout of `pre-` and `post-session` hooks is now propagated to the user [#4382 @AltGr - fix #4359]
 
 ## Remove
-  *
+  * Fix `opam remove --autoremove <PKG>` to not autoremove unrelated packages [#4369 @AltGr - fix #4250 #4332]
+  * Fix cases where `opam remove -a` could trigger conflicts in the presence of orphan packages [#4369 @AltGr - fix #4250 #4332]
 
 ## Switch
   * Fix `--update-invariant` when removing or changing package name [#4360 @AltGr - fix #4353]

src/client/opamClient.ml

@@ -63,7 +63,6 @@ let orphans ?changes ?(transitive=false) t =
     | Some ch ->
       if OpamPackage.Set.is_empty orphans then orphans else
       let recompile_cone =
-        OpamPackage.Set.of_list @@
         OpamSolver.reverse_dependencies
           ~depopts:true ~installed:true ~unavailable:true
           ~build:true ~post:false
@@ -93,7 +92,6 @@ let orphans ?changes ?(transitive=false) t =
     let rec add_trans full_orphans orphan_versions =
       (* fixpoint to check all packages with no available version *)
       let new_orphans =
-        OpamPackage.Set.of_list @@
         OpamSolver.reverse_dependencies
           ~depopts:false ~installed:false ~unavailable:true
           ~build:true ~post:false
@@ -293,8 +291,9 @@ let upgrade_t
       OpamConsole.warning
         "Upgrade is not possible because of conflicts or packages that \
          are no longer available:";
-      OpamConsole.errmsg "%s"
-        (OpamStd.Format.itemize (OpamCudf.string_of_conflict ~indent:4) reasons);
+      OpamConsole.errmsg "  %s"
+        (OpamStd.Format.itemize (OpamCudf.string_of_conflict ~start_column:2)
+           reasons);
       OpamConsole.errmsg
         "\nYou may run \"opam upgrade --fixup\" to let opam fix the \
          current state.\n"
@@ -1057,15 +1056,16 @@ let assume_built_restrictions ?available_packages t atoms =
     (OpamPackage.Map.values missing
      |> List.fold_left OpamPackage.Name.Set.union OpamPackage.Name.Set.empty
      |> OpamPackage.packages_of_names available_packages)
-    -- OpamPackage.Set.of_list installed_dependencies
+    -- installed_dependencies
   in
   let available_packages = lazy (
     (available_packages -- uninstalled_dependencies) ++ t.installed ++ pinned
   ) in
   let fixed_atoms =
     List.map (fun nv ->
         (OpamPackage.name nv , Some (`Eq, OpamPackage.version nv)))
-      ((OpamPackage.Set.elements pinned) @ installed_dependencies)
+      (OpamPackage.Set.elements pinned @
+       OpamPackage.Set.elements installed_dependencies)
   in
   { t with available_packages }, fixed_atoms
 
@@ -1261,10 +1261,10 @@ let remove_t ?ask ~autoremove ~force atoms t =
     (slog OpamFormula.string_of_atoms) atoms;
 
   let t, full_orphans, orphan_versions =
-    let changes =
-      if autoremove then None
-      else Some (OpamSwitchState.packages_of_atoms t atoms) in
-    orphans ?changes t
+    if atoms = [] then t, OpamPackage.Set.empty, OpamPackage.Set.empty
+    else
+    let changes = OpamSwitchState.packages_of_atoms t atoms in
+    orphans ~changes t
   in
 
   let nothing_to_do = ref true in
@@ -1300,32 +1300,30 @@ let remove_t ?ask ~autoremove ~force atoms t =
         Remove
     in
     let to_remove =
-      OpamPackage.Set.of_list
-        (OpamSolver.reverse_dependencies ~build:true ~post:true
-           ~depopts:false ~installed:true universe packages) in
+      OpamSolver.reverse_dependencies ~build:true ~post:true
+        ~depopts:false ~installed:true universe packages
+    in
     let to_keep =
       (if autoremove then t.installed_roots %% t.installed else t.installed)
       ++ universe.u_base
-      -- to_remove -- full_orphans -- orphan_versions
+      -- to_remove
     in
     let to_keep =
-      OpamPackage.Set.of_list
-        (OpamSolver.dependencies ~build:true ~post:true
-           ~depopts:true ~installed:true universe to_keep) in
+      OpamSolver.dependencies ~build:true ~post:true
+        ~depopts:true ~installed:true universe to_keep in
     (* to_keep includes the depopts, because we don't want to autoremove
        them. But that may re-include packages that we wanted removed, so we
        need to remove them again *)
     let to_keep = to_keep -- to_remove in
     let requested = OpamPackage.names_of_packages packages in
     let to_remove =
       if autoremove then
-        let to_remove = t.installed -- to_keep in
-        if atoms = [] then to_remove
+        let to_remove1 = t.installed -- to_keep in
+        if atoms = [] then to_remove1
         else (* restrict to the dependency cone of removed pkgs *)
-          to_remove %%
-          (OpamPackage.Set.of_list
-             (OpamSolver.dependencies ~build:true ~post:true
-                ~depopts:true ~installed:true universe to_remove))
+          to_remove1 %%
+          (OpamSolver.dependencies ~build:true ~post:true
+             ~depopts:true ~installed:true universe to_remove)
       else to_remove in
     let t, solution =
       OpamSolution.resolve_and_apply ?ask t Remove

src/client/opamListCommand.ml

@@ -232,7 +232,6 @@ let apply_selector ~base st = function
       ~installed:false ~unavailable:true
       (get_universe st tog)
       (packages_of_atoms st atoms)
-    |> OpamPackage.Set.of_list
   | Required_by (tog, atoms) ->
     atom_dependencies st tog atoms |>
     OpamFormula.packages base
@@ -664,14 +663,8 @@ let display st format packages =
           ~requested:(OpamPackage.names_of_packages packages)
           Query
       in
-      let deps_packages =
-        OpamSolver.dependencies
-          ~depopts:true ~installed:false ~unavailable:true
-          ~build:true ~post:false
-          universe packages
-      in
-      List.filter (fun nv -> OpamPackage.Set.mem nv packages) deps_packages |>
-      List.rev
+      OpamSolver.dependency_sort ~depopts:true ~build:true ~post:false
+        universe packages
     else match format.order with
       | `Custom o -> List.sort o (OpamPackage.Set.elements packages)
       | _ -> OpamPackage.Set.elements packages

src/client/opamLockCommand.ml

@@ -113,7 +113,7 @@ let lock_opam ?(only_direct=false) st opam =
     OpamSolver.dependencies
       ~depopts:true ~build:true ~post:true ~installed:true
       univ (OpamPackage.Set.singleton nv) |>
-    List.filter (fun nv1 -> nv1 <> nv)
+    OpamPackage.Set.remove nv
   in
   let depends =
     if only_direct then
@@ -123,14 +123,14 @@ let lock_opam ?(only_direct=false) st opam =
         OpamFormula.fold_left (fun acc (n,_) -> OpamPackage.Name.Set.add n acc)
           OpamPackage.Name.Set.empty
       in
-      List.filter (fun nv -> OpamPackage.Name.Set.mem nv.name names) all_depends
+      OpamPackage.packages_of_names all_depends names
     else all_depends
   in
   let map_of_set x set =
     OpamPackage.Map.of_list (List.map (fun nv -> nv, x)
                                (OpamPackage.Set.elements set))
   in
-  let depends_map = map_of_set `version (OpamPackage.Set.of_list depends) in
+  let depends_map = map_of_set `version depends in
   (* others: dev, test, doc *)
   let open OpamPackage.Set.Op in
   let select ?(build=false) ?(test=false) ?(doc=false) ?(dev=false)
@@ -153,11 +153,10 @@ let lock_opam ?(only_direct=false) st opam =
       let depends_map = map_of_set `other installed in
       if only_direct then depends_map
       else
-        ((OpamPackage.Set.of_list
-            (OpamSolver.dependencies
-               ~depopts:false ~build:true ~post:true ~installed:true
-               univ installed))
-         -- (OpamPackage.Set.of_list all_depends))
+        (OpamSolver.dependencies
+           ~depopts:false ~build:true ~post:true ~installed:true
+           univ installed
+         -- all_depends)
         |> map_of_set (`other_dep typ)
         |> OpamPackage.Map.union (fun _v _o -> `other_dep typ) depends_map
     else
@@ -245,29 +244,30 @@ let lock_opam ?(only_direct=false) st opam =
          (OpamPackage.Name.Set.elements uninstalled_depopts))
   in
   let pin_depends =
-    OpamStd.List.filter_map (fun nv ->
-        if not (OpamSwitchState.is_pinned st nv.name) then None else
+    OpamPackage.Set.fold (fun nv acc ->
+        if not (OpamSwitchState.is_pinned st nv.name) then acc else
         match OpamSwitchState.primary_url st nv with
-        | None -> None
+        | None -> acc
         | Some u ->
           match OpamUrl.local_dir u with
           | Some d ->
             let local_warn () =
               OpamConsole.warning "Dependency %s is pinned to local target %s"
                 (OpamPackage.to_string nv) (OpamUrl.to_string u);
-              None
+              acc
             in
             (match u.OpamUrl.backend with
              | #OpamUrl.version_control ->
                (match get_git_url u nv d with
                 | Some resolved_u ->
                   OpamConsole.note "Local pin %s resolved to %s"
                     (OpamUrl.to_string u) (OpamUrl.to_string resolved_u);
-                  Some (nv, resolved_u)
+                  (nv, resolved_u) :: acc
                 | None -> local_warn ())
              | _ -> local_warn ())
-          | None -> Some (nv, u))
-      all_depends
+          | None -> (nv, u) :: acc)
+      all_depends []
+    |> List.rev
   in
   opam |>
   OpamFile.OPAM.with_depopts OpamFormula.Empty |>