From 9ecde1b31da7724128fd35c16bf373a3a8d76fa3 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 30 Jul 2024 09:00:04 +0200 Subject: [PATCH 01/12] Add an admin command to compare 2 package versions This is meant for quick sanity checks for example. - Added some basic tests to cover the command. The tests I added do not duplicate the tests for the actual comparison function, rather that meant to cover the basic cases encountered by the command. Signed-off-by: Mathieu Barbin --- doc/man/opam-admin-topics.inc | 11 +++++++ master_changes.md | 2 ++ src/client/opamAdminCommand.ml | 34 ++++++++++++++++++++ tests/reftests/compare-package-versions.test | 7 ++++ tests/reftests/dune.inc | 18 +++++++++++ 5 files changed, 72 insertions(+) create mode 100644 tests/reftests/compare-package-versions.test diff --git a/doc/man/opam-admin-topics.inc b/doc/man/opam-admin-topics.inc index c2618ecb6cf..1e5cdc95fb0 100644 --- a/doc/man/opam-admin-topics.inc +++ b/doc/man/opam-admin-topics.inc @@ -60,6 +60,16 @@ (diff opam-admin-list.err %{dep:opam-admin-list.0}))) (package opam)) +(rule + (with-stdout-to opam-admin-compare-package-versions.0 (echo ""))) +(rule + (targets opam-admin-compare-package-versions.1 opam-admin-compare-package-versions.err) + (deps using-built-opam) + (action (progn (with-stderr-to opam-admin-compare-package-versions.err + (with-stdout-to opam-admin-compare-package-versions.1 (run %{bin:opam} admin compare-package-versions --help=groff))) + (diff opam-admin-compare-package-versions.err %{dep:opam-admin-compare-package-versions.0}))) + (package opam)) + (rule (with-stdout-to opam-admin-check.0 (echo ""))) (rule @@ -130,6 +140,7 @@ opam-admin-add-constraint.1 opam-admin-filter.1 opam-admin-list.1 + opam-admin-compare-package-versions.1 opam-admin-check.1 opam-admin-lint.1 opam-admin-upgrade.1 diff --git a/master_changes.md b/master_changes.md index 47973be912d..ba5b6cafe11 100644 --- a/master_changes.md +++ b/master_changes.md @@ -16,6 +16,8 @@ users) ## Global CLI * Add cli version 2.3 [#6045 #6151 @rjbou] + * ◈ Add `opam admin compare-package-versions` to compare package versions for sanity checks [#6124 @mbarbin] + * Add cli version 2.3 [#6045 @rjbou] ## Plugins diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index 38954d1c6b1..c2ce78b3e4b 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -830,6 +830,39 @@ let check_command cli = Term.(const cmd $ global_options cli $ ignore_test_arg $ print_short_arg $ installability_arg $ cycles_arg $ obsolete_arg) +let compare_package_versions_command_doc = "Compare 2 package versions" +let compare_package_versions_command cli = + let version_arg n = + let doc = + Arg.info + ~docv:"VERSION" + ~doc:"Package version to compare" [] + in + Arg.(required & pos n (Arg.some' OpamArg.package_version) None & doc) + in + let command = "compare-package-versions" in + let doc = compare_package_versions_command_doc in + let man = [ + `S Manpage.s_description; + `P "This command compares 2 package versions for quick sanity checks, and prints the result of the comparison to the console. For example:"; + `I ("For example:", "opam admin compare-package-versions 0.0.9 0.0.10"); + `I ("outputs:", "-1 (0.0.9 < 0.0.10)"); + `S Manpage.s_arguments; + `S Manpage.s_options; + ] + in + let cmd global_options v1 v2 () = + OpamArg.apply_global_options cli global_options; + let result = OpamPackage.Version.compare v1 v2 in + OpamConsole.formatted_msg "%d (%s %s %s)\n" + result + (OpamPackage.Version.to_string v1) + (if result < 0 then "<" else if result = 0 then "=" else ">") + (OpamPackage.Version.to_string v2); + in + OpamArg.mk_command ~cli OpamArg.cli_original command ~doc ~man + Term.(const cmd $ global_options cli $ version_arg 0 $ version_arg 1) + let pattern_list_arg = OpamArg.arg_list "PATTERNS" "Package patterns with globs. matching against $(b,NAME) or \ @@ -1227,6 +1260,7 @@ let admin_subcommands cli = upgrade_command cli; lint_command cli; check_command cli; + compare_package_versions_command cli; list_command cli; filter_command cli; add_constraint_command cli; diff --git a/tests/reftests/compare-package-versions.test b/tests/reftests/compare-package-versions.test new file mode 100644 index 00000000000..22233ca08f7 --- /dev/null +++ b/tests/reftests/compare-package-versions.test @@ -0,0 +1,7 @@ +N0REP0 +### opam admin compare-package-versions 0.0.9 0.0.10 +-1 (0.0.9 < 0.0.10) +### opam admin compare-package-versions 1.2.3 1.2.3~preview +1 (1.2.3 > 1.2.3~preview) +### opam admin compare-package-versions 0.1.0 0.01.0 +0 (0.1.0 = 0.01.0) diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index 9496f04fec6..9d3604a8650 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -188,6 +188,24 @@ %{targets} (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:cli-versioning.test} %{read-lines:testing-env})))) +(rule + (alias reftest-compare-package-versions) + (action + (diff compare-package-versions.test compare-package-versions.out))) + +(alias + (name reftest) + (deps (alias reftest-compare-package-versions))) + +(rule + (targets compare-package-versions.out) + (deps root-N0REP0) + (package opam) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:compare-package-versions.test} %{read-lines:testing-env})))) + (rule (alias reftest-config) (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) From 3b5cb567eea71370be5f6d6fbac4ee62cb2c8695 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 30 Jul 2024 13:48:15 +0200 Subject: [PATCH 02/12] Remove internal int from the output of [compare-package-versions] - Update test trace Signed-off-by: Mathieu Barbin --- src/client/opamAdminCommand.ml | 3 +-- tests/reftests/compare-package-versions.test | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index c2ce78b3e4b..c0fcd4d31fb 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -854,8 +854,7 @@ let compare_package_versions_command cli = let cmd global_options v1 v2 () = OpamArg.apply_global_options cli global_options; let result = OpamPackage.Version.compare v1 v2 in - OpamConsole.formatted_msg "%d (%s %s %s)\n" - result + OpamConsole.formatted_msg "%s %s %s\n" (OpamPackage.Version.to_string v1) (if result < 0 then "<" else if result = 0 then "=" else ">") (OpamPackage.Version.to_string v2); diff --git a/tests/reftests/compare-package-versions.test b/tests/reftests/compare-package-versions.test index 22233ca08f7..b0236474c86 100644 --- a/tests/reftests/compare-package-versions.test +++ b/tests/reftests/compare-package-versions.test @@ -1,7 +1,7 @@ N0REP0 ### opam admin compare-package-versions 0.0.9 0.0.10 --1 (0.0.9 < 0.0.10) +0.0.9 < 0.0.10 ### opam admin compare-package-versions 1.2.3 1.2.3~preview -1 (1.2.3 > 1.2.3~preview) +1.2.3 > 1.2.3~preview ### opam admin compare-package-versions 0.1.0 0.01.0 -0 (0.1.0 = 0.01.0) +0.1.0 = 0.01.0 From a6271d22640f5b12792f4466e615e55f69c2e678 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 30 Jul 2024 14:02:43 +0200 Subject: [PATCH 03/12] Fix man page according to latest change to [compare-package-versions] Signed-off-by: Mathieu Barbin --- src/client/opamAdminCommand.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index c0fcd4d31fb..2026a21a180 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -846,7 +846,7 @@ let compare_package_versions_command cli = `S Manpage.s_description; `P "This command compares 2 package versions for quick sanity checks, and prints the result of the comparison to the console. For example:"; `I ("For example:", "opam admin compare-package-versions 0.0.9 0.0.10"); - `I ("outputs:", "-1 (0.0.9 < 0.0.10)"); + `I ("outputs:", "0.0.9 < 0.0.10"); `S Manpage.s_arguments; `S Manpage.s_options; ] From 45802d9479c06ec34ebc86b7882860ad4fade613 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 30 Jul 2024 15:40:16 +0200 Subject: [PATCH 04/12] Rename command to [compare-versions] Signed-off-by: Mathieu Barbin --- master_changes.md | 1 + src/client/opamAdminCommand.ml | 12 ++++++------ tests/reftests/compare-package-versions.test | 7 ------- tests/reftests/compare-versions.test | 7 +++++++ tests/reftests/dune.inc | 10 +++++----- 5 files changed, 19 insertions(+), 18 deletions(-) delete mode 100644 tests/reftests/compare-package-versions.test create mode 100644 tests/reftests/compare-versions.test diff --git a/master_changes.md b/master_changes.md index ba5b6cafe11..086e735e277 100644 --- a/master_changes.md +++ b/master_changes.md @@ -17,6 +17,7 @@ users) ## Global CLI * Add cli version 2.3 [#6045 #6151 @rjbou] * ◈ Add `opam admin compare-package-versions` to compare package versions for sanity checks [#6124 @mbarbin] + * ◈ Add `opam admin compare-versions` to compare package versions for sanity checks [#6124 @mbarbin] * Add cli version 2.3 [#6045 @rjbou] ## Plugins diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index 2026a21a180..edeac918c0d 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -830,8 +830,8 @@ let check_command cli = Term.(const cmd $ global_options cli $ ignore_test_arg $ print_short_arg $ installability_arg $ cycles_arg $ obsolete_arg) -let compare_package_versions_command_doc = "Compare 2 package versions" -let compare_package_versions_command cli = +let compare_versions_command_doc = "Compare 2 package versions" +let compare_versions_command cli = let version_arg n = let doc = Arg.info @@ -840,12 +840,12 @@ let compare_package_versions_command cli = in Arg.(required & pos n (Arg.some' OpamArg.package_version) None & doc) in - let command = "compare-package-versions" in - let doc = compare_package_versions_command_doc in + let command = "compare-versions" in + let doc = compare_versions_command_doc in let man = [ `S Manpage.s_description; `P "This command compares 2 package versions for quick sanity checks, and prints the result of the comparison to the console. For example:"; - `I ("For example:", "opam admin compare-package-versions 0.0.9 0.0.10"); + `I ("For example:", "opam admin compare-versions 0.0.9 0.0.10"); `I ("outputs:", "0.0.9 < 0.0.10"); `S Manpage.s_arguments; `S Manpage.s_options; @@ -1259,7 +1259,7 @@ let admin_subcommands cli = upgrade_command cli; lint_command cli; check_command cli; - compare_package_versions_command cli; + compare_versions_command cli; list_command cli; filter_command cli; add_constraint_command cli; diff --git a/tests/reftests/compare-package-versions.test b/tests/reftests/compare-package-versions.test deleted file mode 100644 index b0236474c86..00000000000 --- a/tests/reftests/compare-package-versions.test +++ /dev/null @@ -1,7 +0,0 @@ -N0REP0 -### opam admin compare-package-versions 0.0.9 0.0.10 -0.0.9 < 0.0.10 -### opam admin compare-package-versions 1.2.3 1.2.3~preview -1.2.3 > 1.2.3~preview -### opam admin compare-package-versions 0.1.0 0.01.0 -0.1.0 = 0.01.0 diff --git a/tests/reftests/compare-versions.test b/tests/reftests/compare-versions.test new file mode 100644 index 00000000000..19eee39cf43 --- /dev/null +++ b/tests/reftests/compare-versions.test @@ -0,0 +1,7 @@ +N0REP0 +### opam admin compare-versions 0.0.9 0.0.10 +0.0.9 < 0.0.10 +### opam admin compare-versions 1.2.3 1.2.3~preview +1.2.3 > 1.2.3~preview +### opam admin compare-versions 0.1.0 0.01.0 +0.1.0 = 0.01.0 diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index 9d3604a8650..cc4f3ccf8af 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -189,22 +189,22 @@ (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:cli-versioning.test} %{read-lines:testing-env})))) (rule - (alias reftest-compare-package-versions) + (alias reftest-compare-versions) (action - (diff compare-package-versions.test compare-package-versions.out))) + (diff compare-versions.test compare-versions.out))) (alias (name reftest) - (deps (alias reftest-compare-package-versions))) + (deps (alias reftest-compare-versions))) (rule - (targets compare-package-versions.out) + (targets compare-versions.out) (deps root-N0REP0) (package opam) (action (with-stdout-to %{targets} - (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:compare-package-versions.test} %{read-lines:testing-env})))) + (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:compare-versions.test} %{read-lines:testing-env})))) (rule (alias reftest-config) From b40ad5c82fdbe37086198235dc91536e7aa3180f Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 30 Jul 2024 18:03:33 +0200 Subject: [PATCH 05/12] Add optional parameter to control the exit code if needed Signed-off-by: Mathieu Barbin --- doc/man/opam-admin-topics.inc | 12 ++--- src/client/opamAdminCommand.ml | 81 ++++++++++++++++++++++++---- tests/reftests/compare-versions.test | 8 +++ 3 files changed, 86 insertions(+), 15 deletions(-) diff --git a/doc/man/opam-admin-topics.inc b/doc/man/opam-admin-topics.inc index 1e5cdc95fb0..ffbb4aed863 100644 --- a/doc/man/opam-admin-topics.inc +++ b/doc/man/opam-admin-topics.inc @@ -61,13 +61,13 @@ (package opam)) (rule - (with-stdout-to opam-admin-compare-package-versions.0 (echo ""))) + (with-stdout-to opam-admin-compare-versions.0 (echo ""))) (rule - (targets opam-admin-compare-package-versions.1 opam-admin-compare-package-versions.err) + (targets opam-admin-compare-versions.1 opam-admin-compare-versions.err) (deps using-built-opam) - (action (progn (with-stderr-to opam-admin-compare-package-versions.err - (with-stdout-to opam-admin-compare-package-versions.1 (run %{bin:opam} admin compare-package-versions --help=groff))) - (diff opam-admin-compare-package-versions.err %{dep:opam-admin-compare-package-versions.0}))) + (action (progn (with-stderr-to opam-admin-compare-versions.err + (with-stdout-to opam-admin-compare-versions.1 (run %{bin:opam} admin compare-versions --help=groff))) + (diff opam-admin-compare-versions.err %{dep:opam-admin-compare-versions.0}))) (package opam)) (rule @@ -140,7 +140,7 @@ opam-admin-add-constraint.1 opam-admin-filter.1 opam-admin-list.1 - opam-admin-compare-package-versions.1 + opam-admin-compare-versions.1 opam-admin-check.1 opam-admin-lint.1 opam-admin-upgrade.1 diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index edeac918c0d..23c9c9e50ac 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -830,8 +830,54 @@ let check_command cli = Term.(const cmd $ global_options cli $ ignore_test_arg $ print_short_arg $ installability_arg $ cycles_arg $ obsolete_arg) +module Compare_version_operator = struct + type t = + [ `Lt + | `Let + | `Eq + | `Gt + | `Get + ] + + let all = [ `Lt ; `Let ; `Eq ; `Gt ; `Get ] + + let eval : t -> int -> int -> bool = function + | `Lt -> ( < ) + | `Let -> ( <= ) + | `Eq -> ( = ) + | `Gt -> ( > ) + | `Get -> ( >= ) + + let assert_ t result = eval t result 0 + + let to_string : t -> string = function + | `Lt -> "<" + | `Let -> "<=" + | `Eq -> "=" + | `Gt -> ">" + | `Get -> ">=" +end + let compare_versions_command_doc = "Compare 2 package versions" let compare_versions_command cli = + let operators = + List.map (fun op -> Compare_version_operator.to_string op, op) + Compare_version_operator.all + in + let assert_result = + let doc = + Arg.info + ~docv:"OP" + ~doc:(Printf.sprintf + "When supplied, the output is suppressed and the result of the \ + comparison is checked againts the provided operator. The command exits 0 \ + if the comparison holds, and 1 otherwise. \ + $(docv) must be %s.\n" (Arg.doc_alts_enum ~quoted:true operators)) + [ "assert" ] + in + let op_conv = Arg.enum operators in + Arg.(value & opt (Arg.some' op_conv) None doc) + in let version_arg n = let doc = Arg.info @@ -844,23 +890,40 @@ let compare_versions_command cli = let doc = compare_versions_command_doc in let man = [ `S Manpage.s_description; - `P "This command compares 2 package versions for quick sanity checks, and prints the result of the comparison to the console. For example:"; - `I ("For example:", "opam admin compare-versions 0.0.9 0.0.10"); - `I ("outputs:", "0.0.9 < 0.0.10"); + `P "This command compares 2 package versions for quick sanity checks, and by default prints the result of the comparison to the console. You may optionally control the exit-code with '--assert=OP'. For example:"; + `Pre "\n\ + \\$ opam admin compare-versions 0.0.9 0.0.10\n\ + 0.0.9 < 0.0.10\n\ + \n\ + \\$ opam admin compare-versions 0.0.9 0.0.10 --assert '<'\n\ + [0]\n\ + \n\ + \\$ opam admin compare-versions 0.0.9 0.0.10 --assert '>='\n\ + [1]"; `S Manpage.s_arguments; `S Manpage.s_options; ] in - let cmd global_options v1 v2 () = + let cmd global_options v1 v2 assert_result () = OpamArg.apply_global_options cli global_options; let result = OpamPackage.Version.compare v1 v2 in - OpamConsole.formatted_msg "%s %s %s\n" - (OpamPackage.Version.to_string v1) - (if result < 0 then "<" else if result = 0 then "=" else ">") - (OpamPackage.Version.to_string v2); + if Option.is_none assert_result then + OpamConsole.formatted_msg "%s %s %s\n" + (OpamPackage.Version.to_string v1) + (if result < 0 then "<" else if result = 0 then "=" else ">") + (OpamPackage.Version.to_string v2); + let exit_reason = + match assert_result with + | None -> `Success + | Some op -> + if Compare_version_operator.assert_ op result + then `Success + else `False + in + OpamStd.Sys.exit_because exit_reason in OpamArg.mk_command ~cli OpamArg.cli_original command ~doc ~man - Term.(const cmd $ global_options cli $ version_arg 0 $ version_arg 1) + Term.(const cmd $ global_options cli $ version_arg 0 $ version_arg 1 $ assert_result) let pattern_list_arg = OpamArg.arg_list "PATTERNS" diff --git a/tests/reftests/compare-versions.test b/tests/reftests/compare-versions.test index 19eee39cf43..12ee1e0c03b 100644 --- a/tests/reftests/compare-versions.test +++ b/tests/reftests/compare-versions.test @@ -5,3 +5,11 @@ N0REP0 1.2.3 > 1.2.3~preview ### opam admin compare-versions 0.1.0 0.01.0 0.1.0 = 0.01.0 +### opam admin compare-versions 0.0.9 0.0.10 --assert '<' +### opam admin compare-versions 0.0.9 0.0.10 --assert '<=' +### opam admin compare-versions 0.0.9 0.0.10 --assert '=' +# Return code 1 # +### opam admin compare-versions 0.0.9 0.0.10 --assert '>' +# Return code 1 # +### opam admin compare-versions 0.0.9 0.0.10 --assert '>=' +# Return code 1 # From 8a546cd84994872b1260f6dda5588de36c1f47de Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 5 Aug 2024 14:55:10 +0200 Subject: [PATCH 06/12] Tweaks to compare-versions Signed-off-by: Mathieu Barbin --- src/client/opamAdminCommand.ml | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index 23c9c9e50ac..67670631b7b 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -881,7 +881,7 @@ let compare_versions_command cli = let version_arg n = let doc = Arg.info - ~docv:"VERSION" + ~docv:(Printf.sprintf "VERSION%d" (n+1)) ~doc:"Package version to compare" [] in Arg.(required & pos n (Arg.some' OpamArg.package_version) None & doc) @@ -890,15 +890,18 @@ let compare_versions_command cli = let doc = compare_versions_command_doc in let man = [ `S Manpage.s_description; - `P "This command compares 2 package versions for quick sanity checks, and by default prints the result of the comparison to the console. You may optionally control the exit-code with '--assert=OP'. For example:"; + `P "This command compares 2 package versions for quick sanity checks, \ + and by default prints the result of the comparison to the console. \ + You may optionally control the exit-code with '--assert=OP'. \ + For example:"; `Pre "\n\ \\$ opam admin compare-versions 0.0.9 0.0.10\n\ 0.0.9 < 0.0.10\n\ \n\ - \\$ opam admin compare-versions 0.0.9 0.0.10 --assert '<'\n\ + \\$ opam admin compare-versions 0.0.9 0.0.10 --assert='<'\n\ [0]\n\ \n\ - \\$ opam admin compare-versions 0.0.9 0.0.10 --assert '>='\n\ + \\$ opam admin compare-versions 0.0.9 0.0.10 --assert='>='\n\ [1]"; `S Manpage.s_arguments; `S Manpage.s_options; @@ -907,20 +910,17 @@ let compare_versions_command cli = let cmd global_options v1 v2 assert_result () = OpamArg.apply_global_options cli global_options; let result = OpamPackage.Version.compare v1 v2 in - if Option.is_none assert_result then + match assert_result with + | None -> OpamConsole.formatted_msg "%s %s %s\n" (OpamPackage.Version.to_string v1) (if result < 0 then "<" else if result = 0 then "=" else ">") - (OpamPackage.Version.to_string v2); - let exit_reason = - match assert_result with - | None -> `Success - | Some op -> - if Compare_version_operator.assert_ op result - then `Success - else `False - in - OpamStd.Sys.exit_because exit_reason + (OpamPackage.Version.to_string v2) + | Some op -> + OpamStd.Sys.exit_because + (if Compare_version_operator.assert_ op result + then `Success + else `False) in OpamArg.mk_command ~cli OpamArg.cli_original command ~doc ~man Term.(const cmd $ global_options cli $ version_arg 0 $ version_arg 1 $ assert_result) From 74b5c895d7e3cb1e21d265f8c43ba8b3938b0e0a Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 5 Aug 2024 15:03:54 +0200 Subject: [PATCH 07/12] Add tests Signed-off-by: Mathieu Barbin --- tests/reftests/compare-versions.test | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/reftests/compare-versions.test b/tests/reftests/compare-versions.test index 12ee1e0c03b..1170ebc94c4 100644 --- a/tests/reftests/compare-versions.test +++ b/tests/reftests/compare-versions.test @@ -5,10 +5,17 @@ N0REP0 1.2.3 > 1.2.3~preview ### opam admin compare-versions 0.1.0 0.01.0 0.1.0 = 0.01.0 +### opam admin compare-versions 0.1.0 0.01.0 --assert '=' ### opam admin compare-versions 0.0.9 0.0.10 --assert '<' ### opam admin compare-versions 0.0.9 0.0.10 --assert '<=' +### opam admin compare-versions 1.2.3 1.2.3~preview --assert '>' +### opam admin compare-versions 1.2.3 1.2.3~preview --assert '>=' ### opam admin compare-versions 0.0.9 0.0.10 --assert '=' # Return code 1 # +### opam admin compare-versions 1.2.3 1.2.3~preview --assert '<' +# Return code 1 # +### opam admin compare-versions 1.2.3-option 1.2.3 --assert '<=' +# Return code 1 # ### opam admin compare-versions 0.0.9 0.0.10 --assert '>' # Return code 1 # ### opam admin compare-versions 0.0.9 0.0.10 --assert '>=' From 2204f02bbc9fd439b4085f1a70f09d0cdbfda0eb Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 12 Aug 2024 15:21:11 +0200 Subject: [PATCH 08/12] Update master_changes.md --- master_changes.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/master_changes.md b/master_changes.md index 086e735e277..886b124de04 100644 --- a/master_changes.md +++ b/master_changes.md @@ -15,10 +15,8 @@ users) * Bump opam-root-version to 2.2 [#5980 @kit-ty-kate] ## Global CLI - * Add cli version 2.3 [#6045 #6151 @rjbou] - * ◈ Add `opam admin compare-package-versions` to compare package versions for sanity checks [#6124 @mbarbin] * ◈ Add `opam admin compare-versions` to compare package versions for sanity checks [#6124 @mbarbin] - * Add cli version 2.3 [#6045 @rjbou] + * Add cli version 2.3 [#6045 #6151 @rjbou] ## Plugins From 49b3d1a988aeac48984c183e93a6c8961cc28705 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 12 Aug 2024 15:25:37 +0200 Subject: [PATCH 09/12] Formatting (ocp-indent) --- src/client/opamAdminCommand.ml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index 67670631b7b..c1f245e1da7 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -869,10 +869,10 @@ let compare_versions_command cli = Arg.info ~docv:"OP" ~doc:(Printf.sprintf - "When supplied, the output is suppressed and the result of the \ - comparison is checked againts the provided operator. The command exits 0 \ - if the comparison holds, and 1 otherwise. \ - $(docv) must be %s.\n" (Arg.doc_alts_enum ~quoted:true operators)) + "When supplied, the output is suppressed and the result of \ + the comparison is checked againts the provided operator. \ + The command exits 0 if the comparison holds, and 1 otherwise. \ + $(docv) must be %s.\n" (Arg.doc_alts_enum ~quoted:true operators)) [ "assert" ] in let op_conv = Arg.enum operators in @@ -895,14 +895,14 @@ let compare_versions_command cli = You may optionally control the exit-code with '--assert=OP'. \ For example:"; `Pre "\n\ - \\$ opam admin compare-versions 0.0.9 0.0.10\n\ - 0.0.9 < 0.0.10\n\ - \n\ - \\$ opam admin compare-versions 0.0.9 0.0.10 --assert='<'\n\ - [0]\n\ - \n\ - \\$ opam admin compare-versions 0.0.9 0.0.10 --assert='>='\n\ - [1]"; + \\$ opam admin compare-versions 0.0.9 0.0.10\n\ + 0.0.9 < 0.0.10\n\ + \n\ + \\$ opam admin compare-versions 0.0.9 0.0.10 --assert='<'\n\ + [0]\n\ + \n\ + \\$ opam admin compare-versions 0.0.9 0.0.10 --assert='>='\n\ + [1]"; `S Manpage.s_arguments; `S Manpage.s_options; ] @@ -923,7 +923,7 @@ let compare_versions_command cli = else `False) in OpamArg.mk_command ~cli OpamArg.cli_original command ~doc ~man - Term.(const cmd $ global_options cli $ version_arg 0 $ version_arg 1 $ assert_result) + Term.(const cmd $ global_options cli $ version_arg 0 $ version_arg 1 $ assert_result) let pattern_list_arg = OpamArg.arg_list "PATTERNS" From 70e95be8c6ff6b3f381d4091ee182806801167de Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Mon, 12 Aug 2024 15:30:46 +0200 Subject: [PATCH 10/12] Simplify Cmdliner.Arg expression --- src/client/opamAdminCommand.ml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index c1f245e1da7..6d2edfcc638 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -875,8 +875,7 @@ let compare_versions_command cli = $(docv) must be %s.\n" (Arg.doc_alts_enum ~quoted:true operators)) [ "assert" ] in - let op_conv = Arg.enum operators in - Arg.(value & opt (Arg.some' op_conv) None doc) + Arg.(value & opt (some (enum operators)) None doc) in let version_arg n = let doc = @@ -884,7 +883,7 @@ let compare_versions_command cli = ~docv:(Printf.sprintf "VERSION%d" (n+1)) ~doc:"Package version to compare" [] in - Arg.(required & pos n (Arg.some' OpamArg.package_version) None & doc) + Arg.(required & pos n (some OpamArg.package_version) None & doc) in let command = "compare-versions" in let doc = compare_versions_command_doc in From 7c6ae4134141124a1eb3c17606e11a23cd3b227e Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 13 Aug 2024 09:25:24 +0200 Subject: [PATCH 11/12] Update src/client/opamAdminCommand.ml Co-authored-by: Kate --- src/client/opamAdminCommand.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index 6d2edfcc638..f177e2c183b 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -889,8 +889,8 @@ let compare_versions_command cli = let doc = compare_versions_command_doc in let man = [ `S Manpage.s_description; - `P "This command compares 2 package versions for quick sanity checks, \ - and by default prints the result of the comparison to the console. \ + `P "This command compares 2 package versions for quick sanity checks. \ + By default it prints the result of the comparison to the console. \ You may optionally control the exit-code with '--assert=OP'. \ For example:"; `Pre "\n\ From 857c1c303bf3f5c681041ce4b9b3ea51c0213cb3 Mon Sep 17 00:00:00 2001 From: Mathieu Barbin Date: Tue, 13 Aug 2024 09:24:01 +0200 Subject: [PATCH 12/12] Reuse OpamFormula rel operators --- src/client/opamAdminCommand.ml | 36 ++++------------------------------ src/format/opamFormula.ml | 2 ++ src/format/opamFormula.mli | 7 +++++++ 3 files changed, 13 insertions(+), 32 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index f177e2c183b..1433c798e59 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -830,39 +830,11 @@ let check_command cli = Term.(const cmd $ global_options cli $ ignore_test_arg $ print_short_arg $ installability_arg $ cycles_arg $ obsolete_arg) -module Compare_version_operator = struct - type t = - [ `Lt - | `Let - | `Eq - | `Gt - | `Get - ] - - let all = [ `Lt ; `Let ; `Eq ; `Gt ; `Get ] - - let eval : t -> int -> int -> bool = function - | `Lt -> ( < ) - | `Let -> ( <= ) - | `Eq -> ( = ) - | `Gt -> ( > ) - | `Get -> ( >= ) - - let assert_ t result = eval t result 0 - - let to_string : t -> string = function - | `Lt -> "<" - | `Let -> "<=" - | `Eq -> "=" - | `Gt -> ">" - | `Get -> ">=" -end - let compare_versions_command_doc = "Compare 2 package versions" let compare_versions_command cli = let operators = - List.map (fun op -> Compare_version_operator.to_string op, op) - Compare_version_operator.all + List.map (fun op -> OpamFormula.string_of_relop op, op) + OpamFormula.all_relop in let assert_result = let doc = @@ -908,16 +880,16 @@ let compare_versions_command cli = in let cmd global_options v1 v2 assert_result () = OpamArg.apply_global_options cli global_options; - let result = OpamPackage.Version.compare v1 v2 in match assert_result with | None -> + let result = OpamPackage.Version.compare v1 v2 in OpamConsole.formatted_msg "%s %s %s\n" (OpamPackage.Version.to_string v1) (if result < 0 then "<" else if result = 0 then "=" else ">") (OpamPackage.Version.to_string v2) | Some op -> OpamStd.Sys.exit_because - (if Compare_version_operator.assert_ op result + (if OpamFormula.eval_relop op v1 v2 then `Success else `False) in diff --git a/src/format/opamFormula.ml b/src/format/opamFormula.ml index 96e651175e7..84e6857b090 100644 --- a/src/format/opamFormula.ml +++ b/src/format/opamFormula.ml @@ -21,6 +21,8 @@ let neg_relop = function let string_of_relop = OpamPrinter.FullPos.relop_kind +let all_relop = [ `Eq ; `Neq ; `Geq ; `Gt ; `Leq ; `Lt ] + type version_constraint = relop * OpamPackage.Version.t type atom = OpamPackage.Name.t * version_constraint option diff --git a/src/format/opamFormula.mli b/src/format/opamFormula.mli index 6b5539eb093..65d1dba4879 100644 --- a/src/format/opamFormula.mli +++ b/src/format/opamFormula.mli @@ -17,6 +17,13 @@ type relop = OpamParserTypes.FullPos.relop_kind (* = [ `Eq | `Neq | `Geq | `Gt | val compare_relop : relop -> relop -> int +(** A list containing each available operator once. *) +val all_relop : relop list + +(** Returns a string representing the operator in infix syntax, as + used in opam files (">", "=", etc.) *) +val string_of_relop : relop -> string + (** Version constraints for OPAM *) type version_constraint = relop * OpamPackage.Version.t