From 567cd03aa24f9dfa266a85d86861d83e74ee3310 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 | 2 + src/client/opamCommands.ml | 57 ++++++++++++++++++---------- src/client/opamRepositoryCommand.mli | 3 ++ tests/reftests/repository.test | 5 ++- 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/master_changes.md b/master_changes.md index 4ff8801c530..45b56deb343 100644 --- a/master_changes.md +++ b/master_changes.md @@ -70,6 +70,7 @@ users) * [BUG] Fix SWH liveness check [#6036 @rjbou - fix #5721] * Update SWH API request [#6036 @rjbou] * Rework SWH fallback to have a more correct archive retrieval and more fine grained error handling [#6036 @rjbou - fix #5721] + * Check that the repositories given to `opam repository remove` actually exist [#5014 @kit-ty-kate - fixes #5012] ## Lock @@ -185,6 +186,7 @@ users) ## opam-client * `OpamArg.InvalidCLI`: export exception [#6150 @rjbou] * `OpamArg`: export `require_checksums` and `no_checksums`, that are shared with `build_options` [#5563 @rjbou] + * `OpamRepositoryCommand.switch_repos`: expose the function [#5014 @kit-ty-kate] ## opam-repository * `OpamDownload.get_output`: fix `wget` option for `POST` requests [#6036 @rjbou] diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index 942cb578b0a..e76dff1734b 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -2345,13 +2345,14 @@ 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 repos names err = match List.filter (fun n -> - not (OpamRepositoryName.Map.mem n rt.repositories)) + not (List.exists (OpamRepositoryName.equal n) repos)) 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 OpamGlobalState.with_ `Lock_none @@ fun gt -> let all_switches = OpamFile.Config.installed_switches gt.config in @@ -2421,33 +2422,51 @@ let repository cli = `Ok () | Some `remove, names -> let names = List.map OpamRepositoryName.of_string names in - let rm = - List.filter (fun n -> - not (List.exists (OpamRepositoryName.equal n) names)) - in let full_wipe = List.mem `All scope in let global = global || full_wipe in - let gt = - OpamRepositoryCommand.update_selection gt - ~global ~switches:switches rm - in if full_wipe then OpamRepositoryState.with_ `Lock_write gt @@ fun rt -> - check_for_repos rt names + let repos = + OpamRepositoryName.Map.keys rt.OpamStateTypes.repositories + in + ignore @@ check_for_repos repos names (OpamConsole.warning "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 = + OpamRepositoryState.with_ `Lock_none gt @@ fun rt -> + List.fold_left (fun has_known_repos switch -> + let repos = OpamRepositoryCommand.switch_repos rt switch in + check_for_repos repos 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; + let rm = + List.filter (fun n -> + not (List.exists (OpamRepositoryName.equal n) names)) + in + ignore @@ OpamRepositoryCommand.update_selection gt + ~global ~switches:switches rm; `Ok () | Some `add, [name] -> let name = OpamRepositoryName.of_string name in OpamRepositoryState.with_ `Lock_none gt (fun rt -> - check_for_repos rt [name] + let repos = + OpamRepositoryName.Map.keys rt.OpamStateTypes.repositories + in + ignore @@ check_for_repos repos [name] (OpamConsole.error_and_exit `Not_found "No configured repository '%s' found, you must specify an URL")); OpamGlobalState.drop @@ diff --git a/src/client/opamRepositoryCommand.mli b/src/client/opamRepositoryCommand.mli index c0d14f51d54..110d3f6b299 100644 --- a/src/client/opamRepositoryCommand.mli +++ b/src/client/opamRepositoryCommand.mli @@ -14,6 +14,9 @@ open OpamTypes open OpamStateTypes +(** Returns the repository names registered in the given switch *) +val switch_repos : 'a repos_state -> OpamSwitch.t -> OpamRepositoryName.t list + (** List the selected repositories in the global default and/or selected switches. *) val list: diff --git a/tests/reftests/repository.test b/tests/reftests/repository.test index 0b2cb992a2a..fa07335b318 100644 --- a/tests/reftests/repository.test +++ b/tests/reftests/repository.test @@ -894,13 +894,14 @@ 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 -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 repo2 ### opam repository remove does-not-exist repo2 --all [WARNING] No configured repositories by these names found: does-not-exist ### opam repository --all --short