Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opam admin check: Set with-test and with-doc to false by default #6335

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

opam admin check: Set with-test and with-doc to false by default #6335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions master_changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ users)
## Admin
* ◈ Add `opam admin compare-versions` to compare package versions for sanity checks [#6197 @mbarbin]
* [BUG] Fix `opam admin check` in the presence of the `with-dev-setup` variable [#6331 @kit-ty-kate - fix #6329]
* ✘ The `-i`/`--ignore-test-doc` argument has been removed from `opam admin check` [#6335 @kit-ty-kate]
* ✘ `opam admin check` now sets `with-test` and `with-doc` to `false` instead of `true` [#6335 @kit-ty-kate]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* ✘ `opam admin check` now sets `with-test` and `with-doc` to `false` instead of `true` [#6335 @kit-ty-kate]
* ✘ `opam admin check` now sets `with-test`, `with-doc`, and `with-dev-setup` to `false` instead of `true` [#6335 @kit-ty-kate]


## Opam installer

Expand Down
21 changes: 8 additions & 13 deletions src/client/opamAdminCheck.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
open OpamTypes
open OpamPackage.Set.Op

let env ~with_test ~with_doc ~with_dev_setup ~dev nv v =
let env nv v =
match OpamVariable.Full.scope v,
OpamVariable.(to_string (Full.variable v))
with
Expand All @@ -22,17 +22,16 @@ let env ~with_test ~with_doc ~with_dev_setup ~dev nv v =
| OpamVariable.Full.Global, "opam-version" ->
Some (S OpamVersion.(to_string current))
| OpamVariable.Full.Global, "with-test" ->
Some (B with_test)
Some (B false)
| OpamVariable.Full.Global, "dev" ->
Some (B dev)
Some (B false)
| OpamVariable.Full.Global, "with-doc" ->
Some (B with_doc)
Some (B false)
| OpamVariable.Full.Global, "with-dev-setup" ->
Some (B with_dev_setup)
Some (B false)
| _ -> None

let get_universe ~with_test ~with_doc ~with_dev_setup ~dev opams =
let env = env ~with_test ~with_doc ~with_dev_setup ~dev in
let get_universe opams =
let packages = OpamPackage.keys opams in
{
u_packages = packages;
Expand Down Expand Up @@ -405,7 +404,7 @@ let get_obsolete univ opams =
if is_obsolete then acc ++ pkgs else acc)
aggregates PkgSet.empty

let check ~quiet ~installability ~cycles ~obsolete ~ignore_test repo_root =
let check ~quiet ~installability ~cycles ~obsolete repo_root =
let pkg_prefixes = OpamRepository.packages_with_prefixes repo_root in
let opams =
OpamPackage.Map.fold (fun nv prefix acc ->
Expand All @@ -419,11 +418,7 @@ let check ~quiet ~installability ~cycles ~obsolete ~ignore_test repo_root =
pkg_prefixes
OpamPackage.Map.empty
in
let univ =
get_universe
~with_test:(not ignore_test) ~with_doc:(not ignore_test) ~with_dev_setup:false ~dev:false
opams
in
let univ = get_universe opams in

(* Installability check *)
let unav_roots, uninstallable =
Expand Down
1 change: 0 additions & 1 deletion src/client/opamAdminCheck.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ val cycle_check: universe -> package_set * formula list list
sets are empty. *)
val check:
quiet:bool -> installability:bool -> cycles:bool -> obsolete:bool ->
ignore_test:bool ->
dirname -> package_set * package_set * package_set * package_set * package_set

(** Returns a subset of "obsolete" packages, i.e. packages for which a strictly
Expand Down
11 changes: 3 additions & 8 deletions src/client/opamAdminCommand.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,11 +761,6 @@ let check_command cli =
are supposed to be available. By default, all checks are run."
]
in
let ignore_test_arg =
OpamArg.mk_flag ~cli OpamArg.cli_original ["ignore-test-doc";"i"]
"By default, $(b,{with-test}) and $(b,{with-doc}) dependencies are \
included. This ignores them, and makes the test more tolerant."
in
let print_short_arg =
OpamArg.mk_flag ~cli OpamArg.cli_original ["s";"short"]
"Only output a list of uninstallable packages"
Expand All @@ -782,7 +777,7 @@ let check_command cli =
OpamArg.mk_flag ~cli OpamArg.cli_original ["obsolete"]
"Analyse for obsolete packages"
in
let cmd global_options ignore_test print_short
let cmd global_options print_short
installability cycles obsolete () =
OpamArg.apply_global_options cli global_options;
let repo_root = checked_repo_root () in
Expand All @@ -793,7 +788,7 @@ let check_command cli =
in
let pkgs, unav_roots, uninstallable, cycle_packages, obsolete =
OpamAdminCheck.check
~quiet:print_short ~installability ~cycles ~obsolete ~ignore_test
~quiet:print_short ~installability ~cycles ~obsolete
repo_root
in
let all_ok =
Expand Down Expand Up @@ -827,7 +822,7 @@ let check_command cli =
OpamStd.Sys.exit_because (if all_ok then `Success else `False)
in
OpamArg.mk_command ~cli OpamArg.cli_original command ~doc ~man
Term.(const cmd $ global_options cli $ ignore_test_arg $ print_short_arg
Term.(const cmd $ global_options cli $ print_short_arg
$ installability_arg $ cycles_arg $ obsolete_arg)

let compare_versions_command_doc = "Compare two package versions"
Expand Down
35 changes: 0 additions & 35 deletions tests/reftests/admin.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,18 +342,6 @@ depends: "lectus"
### # by default, the check does installability & cycle check
### opam admin check
Checking installability of every package. This may take a few minutes...
[ERROR] These packages are not installable (3):
ocaml-system.1.2 risus.1 suspendisse.1
[ERROR] Dependency cycles detected:
* lectus = 1 -> dignissim = 1
* lectus = 1 -> tortor = 1
Summary: out of 25 packages (22 distinct names)
- 3 uninstallable roots
- 3 packages part of dependency cycles

# Return code 1 #
### opam admin check -i
Checking installability of every package. This may take a few minutes...
[ERROR] These packages are not installable (2):
ocaml-system.1.2 risus.1
[ERROR] Dependency cycles detected:
Expand All @@ -365,43 +353,20 @@ Summary: out of 25 packages (22 distinct names)
# Return code 1 #
### opam admin check --installability
Checking installability of every package. This may take a few minutes...
[ERROR] These packages are not installable (3):
ocaml-system.1.2 risus.1 suspendisse.1
Summary: out of 25 packages (22 distinct names)
- 3 uninstallable roots

# Return code 1 #
### opam admin check --installability -i
Checking installability of every package. This may take a few minutes...
[ERROR] These packages are not installable (2):
ocaml-system.1.2 risus.1
Summary: out of 25 packages (22 distinct names)
- 2 uninstallable roots

# Return code 1 #
### opam admin check --obsolete
[ERROR] Obsolete packages detected:
- lorem 2.0
Summary: out of 25 packages (22 distinct names)
- 1 obsolete packages

# Return code 1 #
### opam admin check --obsolete -i
[ERROR] Obsolete packages detected:
- lorem 1.0, 2.0
Summary: out of 25 packages (22 distinct names)
- 2 obsolete packages

# Return code 1 #
### opam admin check --cycles
[ERROR] Dependency cycles detected:
* lectus = 1 -> dignissim = 1
* lectus = 1 -> tortor = 1
Summary: out of 25 packages (22 distinct names)
- 3 packages part of dependency cycles

# Return code 1 #
### opam admin check --cycles -i
[ERROR] Dependency cycles detected:
* lectus = 1 -> tortor = 1
Summary: out of 25 packages (22 distinct names)
Expand Down
Loading