Skip to content

Commit

Permalink
opam install --check now checks if all dependencies are installed rec…
Browse files Browse the repository at this point in the history
…ursively

Packages that need reinstalling can happen when the definition of dependency changes after it's been installed
  • Loading branch information
kit-ty-kate authored and rjbou committed Aug 2, 2024
1 parent bb2a11a commit f98490d
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 41 deletions.
1 change: 1 addition & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ users)
* 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]
* [BUG] Fix "opam install --check pkg" when pkg depends on a non-existing package [#6121 @kit-ty-kate]
* Make `opam install --check` check if all dependencies are installed recursively [#6122 @kit-ty-kate - fix #6097]

## Build (package)
* ◈ Add `--verbose-on` option to enable verbose mode on specified package names [#5682 @desumn @rjbou]
Expand Down
97 changes: 57 additions & 40 deletions src/client/opamClient.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1945,10 +1945,6 @@ let init
gt, rt, default_compiler

let check_installed ~build ~post t atoms =
let pkgs =
OpamPackage.to_map
(OpamFormula.packages_of_atoms t.packages atoms)
in
let test = OpamStateConfig.(!r.build_test) in
let doc = OpamStateConfig.(!r.build_doc) in
let dev_setup = OpamStateConfig.(!r.dev_setup) in
Expand All @@ -1961,42 +1957,63 @@ let check_installed ~build ~post t atoms =
| None -> OpamPackageVar.resolve_switch ~package t var
| Some _ -> content
in
OpamPackage.Name.Map.fold (fun name versions map ->
let compliant, missing_opt =
OpamPackage.Version.Set.fold (fun version (found, missing) ->
if found then (found, missing)
else
let pkg = OpamPackage.create name version in
let cnf_formula =
OpamSwitchState.opam t pkg
|> OpamFile.OPAM.depends
|> OpamFilter.filter_formula ~default:false (env pkg)
|> OpamFormula.to_cnf
in
let missing_conj =
List.filter
(List.for_all (fun ((n,_vc) as atom) ->
OpamPackage.Set.for_all
(fun p -> not (OpamFormula.check atom p))
(OpamPackage.packages_of_name t.installed n)))
cnf_formula
in
if missing_conj = [] then true, None
else false, Some (pkg,missing_conj))
versions (false,None)
in
if compliant then map else
match missing_opt with
| None -> assert false (* version set can't be empty *)
| Some (pkg, missing_conj) ->
OpamPackage.Map.add pkg
(List.fold_left (fun names disj ->
List.fold_left (fun names (name, _) ->
OpamPackage.Name.Set.add name names)
names disj)
OpamPackage.Name.Set.empty missing_conj)
map
) pkgs OpamPackage.Map.empty
let rec loop map atoms =
List.fold_left (fun map atom ->
let versions =
OpamPackage.Set.fold (fun pkg versions ->
if OpamFormula.check atom pkg
then OpamPackage.version pkg :: versions
else versions)
t.packages []
in
let _found, missing_opt =
List.fold_left (fun (found, missing) version ->
if found then (found, missing)
else
let pkg = OpamPackage.create (fst atom) version in
let cnf_formula =
OpamSwitchState.opam t pkg
|> OpamFile.OPAM.depends
|> OpamFilter.filter_formula ~default:false (env pkg)
|> OpamFormula.to_cnf
in
let missing_conj, found_conj =
List.partition
(List.for_all (fun ((n,_vc) as atom) ->
OpamPackage.Set.for_all
(fun p -> not (OpamFormula.check atom p))
(OpamPackage.packages_of_name t.installed n)))
cnf_formula
in
(missing_conj = [], Some (pkg, missing_conj, found_conj)))
(false, None) versions
in
match missing_opt with
| None -> assert false (* version set can't be empty *)
| Some (pkg, missing_conj, found_conj) ->
let missing_set =
List.fold_left (fun names disj ->
List.fold_left (fun names (name, _) ->
OpamPackage.Name.Set.add name names)
names disj)
OpamPackage.Name.Set.empty missing_conj
in
let found_atoms =
List.fold_left (fun found disj ->
List.fold_left (fun atoms atom -> atom :: atoms) found disj)
[] found_conj
in
let new_map =
if OpamPackage.Name.Set.is_empty missing_set
then map
else OpamPackage.Map.add pkg missing_set map
in
if found_atoms = []
then new_map
else loop new_map found_atoms
) map atoms
in
loop OpamPackage.Map.empty atoms

let assume_built_restrictions ?available_packages t atoms =
let missing = check_installed ~build:false ~post:false t atoms in
Expand Down
4 changes: 3 additions & 1 deletion tests/reftests/install-check.test
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,6 @@ depends: [
"b" {> "1"}
]
### opam install --check test.3
All dependencies installed
Missing dependencies:
b
# Return code 1 #

0 comments on commit f98490d

Please sign in to comment.