Check that the repositories given to "opam repository remove" actually exist#5014
Conversation
d0cde13 to
6a0b06e
Compare
6a0b06e to
935929f
Compare
|
I've put back my original changes together with fixes to what needed fixing together with a couple of new tests. Your original wip commit was 6a0b06e, i've also tried to improve the previous attempt in check-repos-remove-exist-old to no avail. |
ddad6d5 to
dc29f65
Compare
rjbou
left a comment
There was a problem hiding this comment.
You don't want to load repository state to avoid having to load repositories to remove some ones ?
dc29f65 to
3e067c3
Compare
3e067c3 to
bd79d97
Compare
bd79d97 to
fd2a4ea
Compare
|
updated |
fd2a4ea to
1ec90be
Compare
|
rebased |
src/client/opamCommands.ml
Outdated
| let full_wipe = List.mem `All scope in | ||
| let global = global || full_wipe in | ||
| let gt = | ||
| let update_gt () = |
There was a problem hiding this comment.
What do you think of
OpamRepositoryState.with_ `Lock_write gt @@ fun rt ->
if full_wipe then
let repos =
OpamRepositoryName.Map.keys rt.OpamStateTypes.repositories
in
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 begin
let has_known_repos =
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 ()diff
The idea is to factorise some code and make it more step by step, more "readable":
- first loading repostate
- then check availability, display comments, and remove in global case (take advantage of the if)
- then for all, update switch selection (no need to drop as it is in an
with_function - end
There was a problem hiding this comment.
I incorporated this diff except for the factorization of the repo-state loading as it would load it in write mode regardless of need (non full_wipe do not need it to be in write mode)
1ec90be to
0c43661
Compare
|
updated |
0c43661 to
567cd03
Compare
Fixes #5012