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: enforce repository root check for all commands and add upgrade advice #6391

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

opam admin: enforce repository root check for all commands and add upgrade advice #6391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2e684b
admin: enforce repository root check for all commands
rjbou Feb 19, 2025
0f81ab6
reftest: add a test in admin to check repo version and its upgrade ad…
rjbou Feb 19, 2025
799f8b1
admin: add an upgrade advice if repo prior to 2.0
rjbou Feb 19, 2025
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
3 changes: 3 additions & 0 deletions master_changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ users)
* [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]
* Enforce repository root check for all command [#6385 @rjbou]
* Add an upgrade advice is the repository is 1.2 version, for all command except upgrade [#6385 @rjbou]

## Opam installer

Expand Down Expand Up @@ -175,6 +177,7 @@ users)
* Add a test showing the behaviour of `opam upgrade` with packages flagged with `avoid-version`/`deprecated` [#6273 @kit-ty-kate]
* Add a test showing the behaviour when a pin depend is unpinned [#6380 @rjbou]
* Add a test to ensure `opam upgrade <pkg>` will not upgrade unrelated things [#6373 @kit-ty-kate]
* Add a test in admin to test repository version upgrade advice [#6385 @rjbou]

### Engine

Expand Down
36 changes: 29 additions & 7 deletions src/client/opamAdminCommand.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,33 @@ open Cmdliner

type command = unit Cmdliner.Term.t * Cmdliner.Cmd.info

let checked_repo_root () =
let repo_version_lt repo_root v =
let v' =
let open OpamStd.Option.Op in
(OpamFile.Repo.read_opt
(OpamRepositoryPath.repo repo_root)
>>= OpamFile.Repo.opam_version)
+! (OpamVersion.of_string "1.2")
in
if OpamVersion.compare v v' > 0 then Some v' else None

let checked_repo_root ?(check=true)() =
let repo_root = OpamFilename.cwd () in
if not (OpamFilename.exists_dir (OpamRepositoryPath.packages_dir repo_root))
then
OpamConsole.error_and_exit `Bad_arguments
"No repository found in current directory.\n\
Please make sure there is a \"packages%s\" directory" OpamArg.dir_sep;
(if check then
let default_current = "2.0" in
match repo_version_lt repo_root
(OpamVersion.of_string default_current) with
| Some v ->
OpamConsole.warning
Copy link
Member

Choose a reason for hiding this comment

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

I think opam admin should simply refuse to handle 1.2 repositories

Suggested change
OpamConsole.warning
OpamConsole.error_and_exit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It can be done that way too. It need a proper cleaning of the code if there is some that handles 1.2 backward compatibility (expect upgrade ofc).

"The repository is at version %s, \
please consider upgrading to %s with 'opam admin upgrade'"
(OpamVersion.to_string v) default_current
| None -> ());
repo_root

let global_options cli =
Expand Down Expand Up @@ -76,7 +96,7 @@ let index_command cli =
in
let cmd global_options urls_txt () =
OpamArg.apply_global_options cli global_options;
let repo_root = checked_repo_root () in
let repo_root = checked_repo_root () in
let repo_file = OpamRepositoryPath.repo repo_root in
let repo_def =
match OpamFile.Repo.read_opt repo_file with
Expand Down Expand Up @@ -646,18 +666,19 @@ let upgrade_command cli =
in
let cmd global_options clear_cache create_mirror () =
OpamArg.apply_global_options cli global_options;
let repo_root = checked_repo_root ~check:false () in
if clear_cache then OpamAdminRepoUpgrade.clear_cache ()
else match create_mirror with
| None ->
OpamAdminRepoUpgrade.do_upgrade (OpamFilename.cwd ());
OpamAdminRepoUpgrade.do_upgrade repo_root;
if OpamFilename.exists (OpamFilename.of_string "index.tar.gz") ||
OpamFilename.exists (OpamFilename.of_string "urls.txt")
then
OpamConsole.note
"Indexes need updating: you should now run:\n\
\n\
\ opam admin index"
| Some m -> OpamAdminRepoUpgrade.do_upgrade_mirror (OpamFilename.cwd ()) m
| Some m -> OpamAdminRepoUpgrade.do_upgrade_mirror repo_root m
in
OpamArg.mk_command ~cli OpamArg.cli_original command ~doc ~man
Term.(const cmd $ global_options cli $
Expand Down Expand Up @@ -702,7 +723,7 @@ let lint_command cli =
in
let cmd global_options short list incl excl ign warn_error () =
OpamArg.apply_global_options cli global_options;
let repo_root = OpamFilename.cwd () in
let repo_root = checked_repo_root () in
if not (OpamFilename.exists_dir OpamFilename.Op.(repo_root / "packages"))
then
OpamConsole.error_and_exit `Bad_arguments
Expand Down Expand Up @@ -1014,6 +1035,7 @@ let list_command cli =
global_options package_selection disjunction state_selection
package_listing env packages () =
OpamArg.apply_global_options cli global_options;
let repo_root = checked_repo_root () in
let format =
let force_all_versions =
match packages with
Expand All @@ -1040,7 +1062,7 @@ let list_command cli =
List.map (fun x -> Atom x) package_selection);
]
in
let st = get_virtual_switch_state (OpamFilename.cwd ()) env in
let st = get_virtual_switch_state repo_root env in
if not format.OpamListCommand.short && filter <> OpamFormula.Empty then
OpamConsole.msg "# Packages matching: %s\n"
(OpamListCommand.string_of_formula filter);
Expand Down Expand Up @@ -1081,7 +1103,7 @@ let filter_command cli =
global_options package_selection disjunction state_selection env
remove dryrun packages () =
OpamArg.apply_global_options cli global_options;
let repo_root = OpamFilename.cwd () in
let repo_root = checked_repo_root () in
let pattern_selector = OpamListCommand.pattern_selector packages in
let join =
if disjunction then OpamFormula.ors else OpamFormula.ands
Expand Down
8 changes: 3 additions & 5 deletions tests/reftests/admin-cache.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
N0REP0
### mv REPO OPER
### opam repository set-url default OPER/ --set-default
[default] Initialised
### : create repo
### rm -rf REPO
### <repo>
opam-version: "1.2"
opam-version: "2.0"
### <packages/ipsum/ipsum.1/opam>
opam-version: "2.0"
### <packages/dolor/dolor.1/opam>
Expand Down Expand Up @@ -252,7 +250,7 @@ amet : md5 : exists, link to sha512
amet : sha256 : exists, link to sha512
amet : sha512 : exists
### ::::::::::::::::::::::::::::::::::::
### :IV: add a other hashes afterwards :
### :IV: add another hashes afterwards :
### ::::::::::::::::::::::::::::::::::::
### : ipsum : only md5 then adding sha512
### : dolor : md5 & sha256 then adding sha512
Expand Down
20 changes: 20 additions & 0 deletions tests/reftests/admin.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,3 +766,23 @@ no-extra.1
no-extra.2
### opam admin list --short --all-versions --latests-only
no-extra.2
### ::: Test opam repo version warning
### rm -rf packages
### <packages/dummy/dummy.1/opam>
opam-version: "2.0"
### : repo 1.2
### <repo>
opam-version: "1.2"
### opam admin list --short
[WARNING] The repository is at version 1.2, please consider upgrading to 2.0 with 'opam admin upgrade'
dummy
### : missing repo, assuming 1.2
### rm repo
### opam admin list --short
[WARNING] The repository is at version 1.2, please consider upgrading to 2.0 with 'opam admin upgrade'
dummy
### : repo 2.0
### <repo>
opam-version: "2.0"
### opam admin list --short
dummy
Loading