Skip to content

Commit

Permalink
Remove buggy OpamFormula.simplify_version_set and OpamFormula.formula…
Browse files Browse the repository at this point in the history
…_of_version_set
  • Loading branch information
kit-ty-kate committed Dec 19, 2023
1 parent 37db762 commit 57a11d4
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 88 deletions.
13 changes: 7 additions & 6 deletions src/client/opamAdminCheck.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =
Expand Down Expand Up @@ -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

Expand Down
56 changes: 0 additions & 56 deletions src/format/opamFormula.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 0 additions & 11 deletions src/format/opamFormula.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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 *)
Expand Down
16 changes: 1 addition & 15 deletions src/solver/opamCudf.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand All @@ -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
Expand Down

0 comments on commit 57a11d4

Please sign in to comment.