Better error message when trying to upgrade a non-existing repository…

… / a non-pinned package.

Fix #507

Also improve the behavior of update of pinned packages: in this case it is neither necessary to run a full repository update and to rebuild the global cache so don't do it.

src/client/opamClient.ml

@@ -114,28 +114,37 @@ let update_repositories t ~show_compilers repositories =
   if show_compilers then
     print_updated_compilers ~old_compilers ~new_compilers
 
+(* Check for updates in pinned packages *)
+let update_pinned_packages t packages =
+  log "update-pinned-packages packages=%s" (OpamPackage.Name.Set.to_string packages);
+  let pinned =
+    OpamPackage.Name.Map.filter
+      (fun n _ -> OpamPackage.Name.Set.mem n packages)
+      t.pinned in
+  let pinned = OpamPackage.Name.Map.bindings pinned in
+  (* Check if a pinned packages has been updated. *)
+  let aux = function
+    | n, (Local p | Git p | Darcs p) ->
+      if OpamState.mem_installed_package_by_name t n then
+        let nv = OpamState.find_installed_package_by_name t n in
+        match OpamState.update_pinned_package t n with
+        | Up_to_date _  -> None
+        | Result _      -> Some nv
+        | Not_available ->
+          OpamGlobals.error "%s is not available" (OpamFilename.Dir.to_string p);
+          None
+      else
+        None
+    | _ -> None in
+  let updates = OpamMisc.filter_map aux pinned in
+  OpamPackage.Set.of_list updates
+
 (* Update the package contents, display the new packages and update
    reinstall *)
-let update_packages t ~show_packages repositories =
+let update_packages t ~show_packages repositories pinned_packages =
   log "update_packages %s" (OpamState.string_of_repositories repositories);
-  (* Update the pinned packages *)
-  let pinned_updated =
-    OpamPackage.Set.of_list (
-      OpamMisc.filter_map
-        (function
-         | n, (Local p | Git p | Darcs p) ->
-           if OpamState.mem_installed_package_by_name t n then
-             let nv = OpamState.find_installed_package_by_name t n in
-             match OpamState.update_pinned_package t n with
-             | Up_to_date _  -> None
-             | Result _      -> Some nv
-             | Not_available ->
-               OpamGlobals.error "%s is not available" (OpamFilename.Dir.to_string p);
-               None
-           else
-             None
-         | _ -> None)
-        (OpamPackage.Name.Map.bindings t.pinned)) in
+
+  let pinned_updated = update_pinned_packages t pinned_packages in
 
   (* then update $opam/repo/index *)
   OpamRepositoryCommand.update_index t;
@@ -179,6 +188,7 @@ let update_packages t ~show_packages repositories =
         ) accu repo_s
       )
     ) t.repo_index OpamPackage.Set.empty in
+
   if show_packages then
     print_updated_packages t updated pinned_updated;
 
@@ -514,36 +524,105 @@ module API = struct
       else
         let aux r _ = List.mem r repos in
         OpamRepositoryName.Map.filter aux t.repositories in
-    let repositories_need_update = not (OpamRepositoryName.Map.is_empty repositories) in
+    let repositories_need_update =
+      not (OpamRepositoryName.Map.is_empty repositories) in
 
+    let pinned_packages =
+      if repos = [] then
+        OpamPackage.Name.Set.of_list (OpamPackage.Name.Map.keys t.pinned)
+      else
+        let names =
+          List.map (OpamRepositoryName.to_string |> OpamPackage.Name.of_string) repos in
+        OpamPackage.Name.Set.of_list (List.filter (OpamState.is_pinned t) names) in
     let pinned_packages_need_update =
-      let pinned_packages =
-        if repos = [] then
-          OpamPackage.Name.Map.keys t.pinned
-        else
-          let names =
-            List.map (OpamRepositoryName.to_string |> OpamPackage.Name.of_string) repos in
-          List.filter (OpamState.is_pinned t) names in
-      pinned_packages <> [] in
+      not (OpamPackage.Name.Set.is_empty pinned_packages) in
+
+    let valid_repositories =
+      OpamMisc.StringSet.of_list
+        (List.map OpamRepositoryName.to_string
+           (OpamRepositoryName.Map.keys repositories)) in
+    let valid_pinned_packages =
+      OpamMisc.StringSet.of_list
+        (List.map OpamPackage.Name.to_string
+           (OpamPackage.Name.Map.keys t.pinned)) in
+    let unknown_names, not_pinned =
+      if repos = [] then
+        [], []
+      else
+        let all =
+          OpamMisc.StringSet.of_list (List.map OpamRepositoryName.to_string repos) in
+        let valid_names =
+          OpamMisc.StringSet.of_list
+            (List.map
+               (OpamPackage.name |> OpamPackage.Name.to_string)
+               (OpamPackage.Set.elements t.packages)) in
+        let (--) = OpamMisc.StringSet.diff in
+        let unknown_names = all -- valid_repositories -- valid_names in
+        let not_pinned = (OpamMisc.StringSet.inter all valid_names) -- valid_pinned_packages in
+        OpamMisc.StringSet.elements unknown_names,
+        OpamMisc.StringSet.elements not_pinned in
+
+    begin
+      let valid_repositories =
+        match OpamMisc.StringSet.elements valid_repositories with
+        | []  -> ""
+        | [s] -> Printf.sprintf " Valid repository is %s." s
+        | l   ->
+          Printf.sprintf
+            " Valid repositories are %s."
+            (OpamMisc.pretty_list l) in
+      match unknown_names with
+      | []  -> ()
+      | [s] ->
+        OpamGlobals.msg
+          "Cannot update the repository %s.%s\n"
+          s valid_repositories
+      | _   ->
+        OpamGlobals.msg
+          "Cannot update the repositories %s.%s\n"
+          (OpamMisc.pretty_list unknown_names) valid_repositories
+    end;
+    begin
+      let valid_pinned_packages =
+        match OpamMisc.StringSet.elements valid_pinned_packages with
+        | []  -> ""
+        | [s] -> Printf.sprintf " Only %s is currently pinned." s
+        | l   ->
+          Printf.sprintf
+            " The currently pinned packages are %s."
+            (OpamMisc.pretty_list l) in
+      match not_pinned with
+      | []  -> ()
+      | [s] ->
+        OpamGlobals.msg
+          "Cannot update the package %s because it is not pinned.%s\n"
+          s valid_pinned_packages
+      | _   ->
+        OpamGlobals.msg
+          "Cannot update %s because none of them is pinned.%s\n"
+          (OpamMisc.pretty_list not_pinned) valid_pinned_packages
+    end;
 
     if repositories_need_update then
       update_repositories t ~show_compilers:true repositories;
 
     if repositories_need_update
-    || pinned_packages_need_update then
-      update_packages t ~show_packages:true repositories;
+    || pinned_packages_need_update then (
+
+      update_packages t ~show_packages:true repositories pinned_packages;
 
-    if save_cache then
-      OpamState.rebuild_state_cache ();
+      if repositories_need_update then
+        OpamState.rebuild_state_cache ();
 
-    match dry_upgrade () with
-    | None   -> OpamGlobals.msg "Everything is up-to-date.\n"
-    | Some stats ->
-      if OpamSolution.sum stats > 0 then (
-        OpamGlobals.msg "%s\n" (OpamSolver.string_of_stats stats);
-        OpamGlobals.msg "You can now run 'opam upgrade' to upgrade your system.\n"
-      ) else
-        OpamGlobals.msg "Everything is up-to-date.\n"
+      match dry_upgrade () with
+      | None   -> OpamGlobals.msg "Everything is up-to-date.\n"
+      | Some stats ->
+        if OpamSolution.sum stats > 0 then (
+          OpamGlobals.msg "%s\n" (OpamSolver.string_of_stats stats);
+          OpamGlobals.msg "You can now run 'opam upgrade' to upgrade your system.\n"
+        ) else
+          OpamGlobals.msg "Everything is up-to-date.\n"
+    )
 
   let () =
     OpamState.update_hook := update_aux
@@ -639,7 +718,7 @@ module API = struct
       let switch = OpamSwitch.of_string (OpamCompiler.to_string compiler) in
       let quiet = (compiler = OpamCompiler.system) in
       OpamState.install_compiler t ~quiet switch compiler;
-      update_packages t ~show_packages:false t.repositories;
+      update_packages t ~show_packages:false t.repositories OpamPackage.Name.Set.empty;
 
       (* Finally, load the complete state and install the compiler packages *)
       log "installing compiler packages";