From 57a11d4f93a98b14b0e5e4bb13ddcec558772d63 Mon Sep 17 00:00:00 2001 From: Kate Date: Tue, 19 Dec 2023 20:19:59 +0000 Subject: [PATCH] Remove buggy OpamFormula.simplify_version_set and OpamFormula.formula_of_version_set --- src/client/opamAdminCheck.ml | 13 +++++---- src/format/opamFormula.ml | 56 ------------------------------------ src/format/opamFormula.mli | 11 ------- src/solver/opamCudf.ml | 16 +---------- 4 files changed, 8 insertions(+), 88 deletions(-) diff --git a/src/client/opamAdminCheck.ml b/src/client/opamAdminCheck.ml index ce2b6fcb25e..6ce6facce32 100644 --- a/src/client/opamAdminCheck.ml +++ b/src/client/opamAdminCheck.ml @@ -83,7 +83,7 @@ let installability_check univ = let unav_roots = filter_roots graph uninstallable in unav_roots, uninstallable -let formula_of_pkglist packages = function +let formula_of_pkglist = function | [] -> OpamFormula.Empty | [p] -> let nv = OpamCudf.cudf2opam p in @@ -93,10 +93,11 @@ let formula_of_pkglist packages = function let nvs = List.map OpamCudf.cudf2opam (p::ps) in Atom (name, - OpamFormula.formula_of_version_set - (OpamPackage.versions_of_name packages name) - (OpamPackage.versions_of_packages - (OpamPackage.Set.of_list nvs))) + OpamFormula.ors + (OpamPackage.Set.fold + (fun pkg acc -> OpamFormula.Atom (`Eq, OpamPackage.version pkg) :: acc) + (OpamPackage.Set.of_list nvs) + [])) let cycle_check univ = let cudf_univ = @@ -221,7 +222,7 @@ let cycle_check univ = let cycle_formulas = cy |> List.map @@ List.map @@ fun p -> - formula_of_pkglist univ.u_packages (OpamCudf.Map.find p node_map) + formula_of_pkglist (OpamCudf.Map.find p node_map) in cycle_packages, cycle_formulas diff --git a/src/format/opamFormula.ml b/src/format/opamFormula.ml index 6931055e492..a0c16fd8317 100644 --- a/src/format/opamFormula.ml +++ b/src/format/opamFormula.ml @@ -598,59 +598,3 @@ let simplify_ineq_formula vcomp f = let simplify_version_formula f = simplify_ineq_formula OpamPackage.Version.compare f - -(** Takes an ordered list of atoms and a predicate, returns a formula describing - the subset of matching atoms *) -let gen_formula l f = - let l = List.map (fun x -> f x, x) l in - let rec aux (t, x as bound) l = match t, l with - | true, (false, y) :: (true, _) :: r - | false, (true, y) :: (false, _) :: r -> - let a = (if t then `Neq else `Eq), y in - (match aux bound r with - | b :: r -> b :: a :: r - | r -> a :: r) - | true, (true, _) :: r - | false, (false, _) :: r -> - aux bound r - | true, (false, _ as bound') :: r - | false, (true, _ as bound') :: r -> - ((if t then `Geq else `Lt), x) :: aux bound' r - | _, [] -> [(if t then `Geq else `Lt), x] - in - let rec aux2 = function - | (`Geq|`Neq), _ as a :: r -> - let rec find_upper acc = function - | `Lt, _ as a :: r -> - ands (List.rev_append acc [Atom a]) :: aux2 r - | `Neq, _ as a :: r -> - find_upper (Atom a :: acc) r - | r -> ands (List.rev acc) :: aux2 r - in - find_upper [Atom a] r - | a :: r -> Atom a :: aux2 r - | [] -> [Empty] - in - match l with - | [] -> Some Empty - | (t, x) :: r -> - match aux (t, x) r with - | [] -> assert false - | [`Geq, _] -> Some Empty - | [`Lt, _] -> None - | _ :: r -> Some (ors (aux2 r)) - -let formula_of_version_set set subset = - let module S = OpamPackage.Version.Set in - match - gen_formula (S.elements set) (fun x -> S.mem x subset) - with - | Some f -> f - | None -> invalid_arg "Empty subset" - -let simplify_version_set set f = - let module S = OpamPackage.Version.Set in - if S.is_empty set then Empty else - let set = fold_left (fun set (_relop, v) -> S.add v set) set f in - gen_formula (S.elements set) (check_version_formula f) |> - OpamStd.Option.default f diff --git a/src/format/opamFormula.mli b/src/format/opamFormula.mli index 07bf7216b06..a106b2c5b18 100644 --- a/src/format/opamFormula.mli +++ b/src/format/opamFormula.mli @@ -199,17 +199,6 @@ val simplify_ineq_formula: (** Like [simplify_ineq_formula], but specialised on version formulas *) val simplify_version_formula: version_formula -> version_formula option -(** A more aggressive version of [simplify_version_formula] that attempts to - find a shorter formula describing the same subset of versions within a given - set. The empty formula is returned for an empty set, and the original - formula is otherwise returned as is if no versions match. *) -val simplify_version_set: OpamPackage.Version.Set.t -> version_formula -> version_formula - -(** [formula_of_version_set set subset] generates a formula that is enough to - describe all packages of [subset] and exclude packages otherwise in [set] *) -val formula_of_version_set: - OpamPackage.Version.Set.t -> OpamPackage.Version.Set.t -> version_formula - (** {2 Atoms} *) (** Return all the atoms *) diff --git a/src/solver/opamCudf.ml b/src/solver/opamCudf.ml index da07b36ebdc..81bf817b0e9 100644 --- a/src/solver/opamCudf.ml +++ b/src/solver/opamCudf.ml @@ -841,19 +841,6 @@ let extract_explanations cudfnv2opam reasons : explanation list = let module CS = ChainSet in (* Definitions and printers *) log ~level:3 "Reasons: %a" (Pp_explanation.pp_reasonlist cudfnv2opam) reasons; - let all_opam = - let add p set = - if is_artefact p then set - else OpamPackage.Set.add (cudf2opam p) set - in - List.fold_left (fun acc -> function - | Conflict (l, r, _) -> add l @@ add r @@ acc - | Dependency (l, _, rs) -> - List.fold_left (fun acc p -> add p acc) (add l acc) rs - | Missing (p, _) -> add p acc) - OpamPackage.Set.empty - reasons - in let print_set pkgs = if Set.exists is_artefact pkgs then if Set.exists is_opam_invariant pkgs then "(invariant)" @@ -866,9 +853,8 @@ let extract_explanations cudfnv2opam reasons : explanation list = in let strs = OpamPackage.Name.Map.mapi (fun name versions -> - let all_versions = OpamPackage.versions_of_name all_opam name in let formula = - OpamFormula.formula_of_version_set all_versions versions + OpamFormula.ors (OpamPackage.Version.Set.fold (fun v acc -> OpamFormula.Atom (`Eq, v) :: acc) versions []) in OpamFormula.to_string (Atom (name, formula))) nvs