From 6222549d15675c58f663904894ffb5492586460c Mon Sep 17 00:00:00 2001 From: Kate Date: Thu, 20 Jan 2022 14:22:55 +0000 Subject: [PATCH] Check that the repositories given to "opam repository remove" actually exist --- master_changes.md | 1 + src/client/opamCommands.ml | 37 ++++++++++++++++++++++++++-------- tests/reftests/repository.test | 3 ++- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/master_changes.md b/master_changes.md index 64d2b77cc2e..1894e5fba48 100644 --- a/master_changes.md +++ b/master_changes.md @@ -63,6 +63,7 @@ users) ## Repository * Accurately tag `curl` download command when loaded from global config file [#6270 @rjbou] + * Check that the repositories given to `opam repository remove` actually exist [#5014 @kit-ty-kate - fixes #5012] ## Lock diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index 1d1266926fe..f8ce0964e8c 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -2345,13 +2345,18 @@ let repository cli = OpamStd.List.insert_at rank new_repo (List.filter (( <> ) new_repo ) repos) in - let check_for_repos rt names err = + let check_for_repos' rt names err = match List.filter (fun n -> not (OpamRepositoryName.Map.mem n rt.repositories)) names - with [] -> () | l -> - err (OpamStd.List.concat_map " " OpamRepositoryName.to_string l) + with [] -> true | l -> + err (OpamStd.List.concat_map " " OpamRepositoryName.to_string l); + List.compare_lengths l names <> 0 + in + let check_for_repos rt names err = + let _has_known_repos : bool = check_for_repos' rt names err in + () in OpamGlobalState.with_ `Lock_none @@ fun gt -> let all_switches = OpamFile.Config.installed_switches gt.config in @@ -2435,11 +2440,27 @@ let repository cli = "No configured repositories by these names found: %s"); OpamRepositoryState.drop @@ List.fold_left OpamRepositoryCommand.remove rt names - else if scope = [`Current_switch] then - OpamConsole.msg - "Repositories removed from the selections of switch %s. \ - Use '--all' to forget about them altogether.\n" - (OpamSwitch.to_string (OpamStateConfig.get_switch ())); + else begin + let has_known_repos = + List.fold_left (fun has_known_repos switch -> + let rt = + OpamSwitchState.with_ `Lock_none ~switch gt @@ fun st -> + st.OpamStateTypes.switch_repos + in + check_for_repos' rt names + (OpamConsole.warning + "No configured repositories by these names found in \ + the selection of switch '%s': %s" + (OpamSwitch.to_string switch)) + || has_known_repos) + false switches + in + if scope = [`Current_switch] && has_known_repos then + OpamConsole.msg + "Repositories removed from the selections of switch %s. \ + Use '--all' to forget about them altogether.\n" + (OpamSwitch.to_string (OpamStateConfig.get_switch ())); + end; `Ok () | Some `add, [name] -> let name = OpamRepositoryName.of_string name in diff --git a/tests/reftests/repository.test b/tests/reftests/repository.test index 10d4ab662a0..cb22dac99e1 100644 --- a/tests/reftests/repository.test +++ b/tests/reftests/repository.test @@ -894,10 +894,11 @@ to-many-commits oper3 oper ### opam repository remove does-not-exist -Repositories removed from the selections of switch rm-unknown. Use '--all' to forget about them altogether. +[WARNING] No configured repositories by these names found in the selection of switch 'rm-unknown': does-not-exist ### opam repository remove does-not-exist --all [WARNING] No configured repositories by these names found: does-not-exist ### opam repository remove does-not-exist oper +[WARNING] No configured repositories by these names found in the selection of switch 'rm-unknown': does-not-exist Repositories removed from the selections of switch rm-unknown. Use '--all' to forget about them altogether. ### opam repository remove does-not-exist repo2 --all [WARNING] No configured repositories by these names found: does-not-exist