Make --dry-run a real dry-run, and add --show to check the update

hmm on second thought maybe --show is not clear enough. Suggestions ?
Closes ocaml#1142

.ocp-indent

@@ -1 +1,2 @@
-normal
+normal
+strict_else=auto

src/client/opamAction.ml

@@ -26,6 +26,9 @@ open OpamState.Types
    Thus, the update of ~/.opam/<switch/installed MUST not be done
    here.*)
 let install_package t nv =
+  if !OpamGlobals.dryrun then
+      OpamGlobals.msg "Installing %s.\n" (OpamPackage.to_string nv)
+  else
   let build_dir = OpamPath.Switch.build t.root t.switch nv in
   if OpamFilename.exists_dir build_dir then OpamFilename.in_dir build_dir (fun () ->
 
@@ -202,14 +205,27 @@ let install_package t nv =
    * apply the patches
    * substitute the files *)
 let prepare_package_build t nv =
-  let p_build = OpamPath.Switch.build t.root t.switch nv in
   let opam = OpamState.opam t nv in
 
+  (* Substitute the patched files.*)
+  let patches = OpamFile.OPAM.patches opam in
+
+  let iter_patches f =
+    List.iter (fun (base, filter) ->
+        if OpamState.eval_filter t ~opam OpamVariable.Map.empty filter
+        then f base
+      ) patches in
+
+  if !OpamGlobals.dryrun then
+    iter_patches (fun base ->
+        OpamGlobals.msg "Applying %s.\n" (OpamFilename.Base.to_string base))
+  else
+
+  let p_build = OpamPath.Switch.build t.root t.switch nv in
+
   if not (OpamFilename.exists_dir p_build) then
     OpamFilename.mkdir p_build;
 
-  (* Substitute the patched files.*)
-  let patches = OpamFile.OPAM.patches opam in
   OpamFilename.in_dir p_build (fun () ->
     let all = OpamFile.OPAM.substs opam in
     let patches =
@@ -220,13 +236,11 @@ let prepare_package_build t nv =
   );
 
   (* Apply the patches *)
-  List.iter (fun (base, filter) ->
-    let root = OpamPath.Switch.build t.root t.switch nv in
-    let patch = root // OpamFilename.Base.to_string base in
-    if OpamState.eval_filter t ~opam OpamVariable.Map.empty filter then (
+  iter_patches (fun base ->
+      let root = OpamPath.Switch.build t.root t.switch nv in
+      let patch = root // OpamFilename.Base.to_string base in
       OpamGlobals.msg "Applying %s.\n" (OpamFilename.Base.to_string base);
-      OpamFilename.patch patch p_build)
-  ) patches;
+      OpamFilename.patch patch p_build);
 
   (* Substitute the configuration files. We should be in the right
      directory to get the correct absolute path for the
@@ -240,6 +254,8 @@ let prepare_package_build t nv =
 let extract_package t nv =
   log "extract_package: %s" (OpamPackage.to_string nv);
   let build_dir = OpamPath.Switch.build t.root t.switch nv in
+
+  if not !OpamGlobals.dryrun then begin
   OpamFilename.rmdir build_dir;
 
   let extract_and_copy_files nv file =
@@ -259,7 +275,8 @@ let extract_package t nv =
     | None   ->
       let dir = OpamPath.dev_package t.root nv in
       extract_and_copy_files nv (OpamState.download_upstream t nv dir)
-  );
+  )
+  end;
 
   prepare_package_build t nv;
   build_dir
@@ -305,6 +322,7 @@ let update_metadata t ~installed ~installed_roots ~reinstall =
   let reinstall       = mark_pinned_versions reinstall in
   let installed_roots = OpamPackage.Set.inter installed_roots installed in
   let reinstall = OpamPackage.Set.inter installed_roots reinstall in
+  if not !OpamGlobals.dryrun then (
   OpamFile.Installed.write
     (OpamPath.Switch.installed t.root t.switch)
     installed;
@@ -314,6 +332,8 @@ let update_metadata t ~installed ~installed_roots ~reinstall =
   OpamFile.Reinstall.write
     (OpamPath.Switch.reinstall t.root t.switch)
     reinstall
+  );
+  {t with installed; installed_roots; reinstall}
 
 let dev_opam_opt t nv build_dir =
   let opam () = OpamState.opam_opt t nv in
@@ -389,14 +409,18 @@ let remove_package_aux t ~metadata ~rm_build ?(silent=false) nv =
         try
           OpamGlobals.msg "%s\n" (string_of_commands remove);
           let metadata = get_metadata t in
-          OpamFilename.exec ~env ?name exec_dir ~metadata ~keep_going:true remove
+          if not !OpamGlobals.dryrun then
+            OpamFilename.exec ~env ?name exec_dir ~metadata ~keep_going:true
+              remove
         with OpamSystem.Process_error r ->
           if not silent then
             OpamGlobals.warning
               "failure in package uninstall script, some files may remain:\n%s"
               (OpamProcess.string_of_result r)
   end;
 
+  if !OpamGlobals.dryrun then () else begin
+
   (* Remove the libraries *)
   OpamFilename.rmdir (OpamPath.Switch.lib t.root t.switch name);
 
@@ -467,13 +491,15 @@ let remove_package_aux t ~metadata ~rm_build ?(silent=false) nv =
   OpamFilename.rmdir (OpamPath.Switch.dev_package t.root t.switch nv);
 
   (* Update the metadata *)
-  if metadata then (
-    let installed = OpamPackage.Set.remove nv t.installed in
-    let installed_roots = OpamPackage.Set.remove nv t.installed_roots in
-    let reinstall = OpamPackage.Set.remove nv t.reinstall in
-    update_metadata t ~installed ~installed_roots ~reinstall;
-    OpamState.remove_metadata t (OpamPackage.Set.singleton nv);
-  );
+  let t =
+    if metadata then
+      let installed = OpamPackage.Set.remove nv t.installed in
+      let installed_roots = OpamPackage.Set.remove nv t.installed_roots in
+      let reinstall = OpamPackage.Set.remove nv t.reinstall in
+      let t = update_metadata t ~installed ~installed_roots ~reinstall in
+      OpamState.remove_metadata t (OpamPackage.Set.singleton nv);
+      t
+    else t in
 
   (* Remove the dev archive if no switch uses the package anymore *)
   let dev = OpamPath.dev_package t.root nv in
@@ -482,12 +508,13 @@ let remove_package_aux t ~metadata ~rm_build ?(silent=false) nv =
     log "Removing %S" (OpamFilename.Dir.to_string dev);
     OpamFilename.rmdir dev;
   )
+  end
 
 let remove_package t ~metadata ~rm_build ?silent nv =
-  if not (!OpamGlobals.fake || !OpamGlobals.dryrun) then (
+  if !OpamGlobals.fake || !OpamGlobals.show then
+    OpamGlobals.msg "Would remove: %s.\n" (OpamPackage.to_string nv)
+  else
     remove_package_aux t ~metadata ~rm_build ?silent nv
-  ) else
-    OpamGlobals.msg "(simulation) Removing %s.\n" (OpamPackage.to_string nv)
 
 (* Remove all the packages appearing in a solution (and which need to
    be removed, eg. because of a direct uninstall action or because of
@@ -513,8 +540,9 @@ let remove_all_packages t ~metadata sol =
     let installed = OpamPackage.Set.diff t.installed deleted in
     let installed_roots = OpamPackage.Set.diff t.installed_roots deleted in
     let reinstall = OpamPackage.Set.diff t.reinstall deleted in
-    update_metadata t ~installed ~installed_roots ~reinstall;
-    OpamState.remove_metadata t deleted;
+    let t = update_metadata t ~installed ~installed_roots ~reinstall in
+    if not !OpamGlobals.dryrun then
+      OpamState.remove_metadata t deleted
   );
   deleted
 
@@ -543,6 +571,7 @@ let build_and_install_package_aux t ~metadata nv =
         | []       -> ()
         | commands ->
           OpamGlobals.msg "%s:\n%s\n" name (string_of_commands commands);
+          if !OpamGlobals.dryrun then () else
           let name = OpamPackage.Name.to_string (OpamPackage.name nv) in
           let metadata = get_metadata t in
           OpamFilename.exec ~env ~name ~metadata p_build commands in
@@ -576,7 +605,7 @@ let build_and_install_package_aux t ~metadata nv =
         let installed = OpamPackage.Set.add nv t.installed in
         let installed_roots = OpamPackage.Set.add nv t.installed_roots in
         let reinstall = OpamPackage.Set.remove nv t.reinstall in
-        update_metadata t ~installed ~installed_roots ~reinstall;
+        let t = update_metadata t ~installed ~installed_roots ~reinstall in
         OpamState.install_metadata t nv;
       )
 

src/client/opamAction.mli

@@ -36,4 +36,4 @@ val update_metadata:
   installed:package_set ->
   installed_roots:package_set ->
   reinstall:package_set ->
-  unit
+  t

src/client/opamArg.ml

@@ -71,6 +71,7 @@ type build_options = {
   no_checksums  : bool;
   build_test    : bool;
   build_doc     : bool;
+  show          : bool;
   dryrun        : bool;
   cudf_file     : string option;
   fake          : bool;
@@ -81,10 +82,10 @@ type build_options = {
 
 let create_build_options
     keep_build_dir make no_checksums build_test
-    build_doc dryrun external_tags cudf_file fake
+    build_doc show dryrun external_tags cudf_file fake
     jobs json = {
   keep_build_dir; make; no_checksums;
-  build_test; build_doc; dryrun; external_tags;
+  build_test; build_doc; show; dryrun; external_tags;
   cudf_file; fake; jobs; json
 }
 
@@ -99,6 +100,7 @@ let apply_build_options b =
   OpamGlobals.no_checksums   := !OpamGlobals.no_checksums || b.no_checksums;
   OpamGlobals.build_test     := !OpamGlobals.build_test || b.build_test;
   OpamGlobals.build_doc      := !OpamGlobals.build_doc || b.build_doc;
+  OpamGlobals.show           := !OpamGlobals.show || b.show;
   OpamGlobals.dryrun         := !OpamGlobals.dryrun || b.dryrun;
   OpamGlobals.external_tags  := b.external_tags;
   OpamGlobals.cudf_file      := b.cudf_file;
@@ -410,9 +412,12 @@ let build_options =
     mk_opt ["m";"make"] "MAKE"
       "Use $(docv) as the default 'make' command."
       Arg.(some string) None in
+  let show =
+    mk_flag ["show"]
+      "Call the solver and display the actions. Don't perform any changes." in
   let dryrun =
     mk_flag ["dry-run"]
-      "Simply call the solver without actually performing any build/install operations." in
+      "Simulate the command, but don't actually perform any changes." in
   let external_tags =
     mk_opt ["e";"external"] "TAGS"
       "Display the external packages associated to the given tags."
@@ -423,13 +428,13 @@ let build_options =
       Arg.(some string) None in
   let fake =
     mk_flag ["fake"]
-      "WARNING: This option is for testing purposes only! Using this option without    \
-       care is the best way to corrupt your current compiler environment. When using \
-       this option OPAM will run a dry-run of the solver and then fake the build and  \
-       install commands." in
+      "This option registers the actions into the OPAM database, without \
+       actually performing them. \
+       WARNING: This option is dangerous and likely to break your OPAM \
+       environment. You probably want `--dry-run'. You've been warned." in
   Term.(pure create_build_options
     $keep_build_dir $make $no_checksums $build_test
-    $build_doc $dryrun $external_tags $cudf_file $fake
+    $build_doc $show $dryrun $external_tags $cudf_file $fake
     $jobs_flag $json_flag)
 
 let init_dot_profile shell dot_profile =

src/client/opamSolution.ml

@@ -226,6 +226,7 @@ let print_variable_warnings t =
 
 (* Is a recovery possible ? *)
 let can_try_to_recover_from_error l =
+  not !OpamGlobals.dryrun &&
   List.exists (function (n,_) ->
     match n with
     | To_change(Some _,_) -> true
@@ -315,11 +316,13 @@ let parallel_apply t action solution =
     s_installed_roots = t.installed_roots;
     s_reinstall       = t.reinstall;
   } in
+  let t_ref = ref t in
   let update_state () =
     let installed       = state.s_installed in
     let installed_roots = state.s_installed_roots in
     let reinstall       = state.s_reinstall in
-    OpamAction.update_metadata t ~installed ~installed_roots ~reinstall in
+    t_ref :=
+      OpamAction.update_metadata t ~installed ~installed_roots ~reinstall in
 
   let root_installs =
     let names =
@@ -346,7 +349,8 @@ let parallel_apply t action solution =
     if OpamPackage.Name.Set.mem (OpamPackage.name nv) root_installs then
       state.s_installed_roots <- OpamPackage.Set.add nv state.s_installed_roots;
     update_state ();
-    OpamState.install_metadata t nv in
+    if not !OpamGlobals.dryrun then
+      OpamState.install_metadata t nv in
 
   let remove_from_install deleted =
     state.s_installed       <- OpamPackage.Set.diff state.s_installed deleted;
@@ -355,15 +359,14 @@ let parallel_apply t action solution =
 
   (* Installation and recompilation are done by child the processes *)
   let child n =
-    (* We are guaranteed to load the state when all the dependencies
+    (* We are guaranteed to get the state when all the dependencies
        have been correctly updated. Thus [t.installed] should be
-       up-to-date.
-       XXX: do we really need to load the state again here ? *)
-    let t = OpamState.load_state "child" in
+       up-to-date. *)
+    let t = !t_ref in
     match n with
-    | To_change (_, nv)
-    | To_recompile nv   -> OpamAction.build_and_install_package ~metadata:false t nv
-    | To_delete _       -> assert false in
+    | To_change (_, nv) | To_recompile nv ->
+      OpamAction.build_and_install_package ~metadata:false t nv
+    | To_delete _ -> assert false in
 
   (* Not pre-condition (yet ?) *)
   let pre _ = () in
@@ -384,8 +387,6 @@ let parallel_apply t action solution =
     (* 2/ We install the new packages *)
     PackageActionGraph.Parallel.iter
       (OpamState.jobs t) solution.to_process ~pre ~child ~post;
-    if !OpamGlobals.fake then
-      OpamGlobals.msg "Simulation complete.\n";
 
     (* XXX: we might want to output the sucessful actions as well. *)
     output_json_actions [];
@@ -480,7 +481,7 @@ let apply ?(force = false) t action solution =
     );
 
     let continue =
-      if !OpamGlobals.dryrun then false
+      if !OpamGlobals.show then false
       else if !OpamGlobals.external_tags <> [] then (
         let packages = OpamSolver.new_packages solution in
         let external_tags = OpamMisc.StringSet.of_list !OpamGlobals.external_tags in

src/core/opamGlobals.ml

@@ -41,6 +41,7 @@ let yes              = check "YES"
 let strict           = check "STRICT"
 let build_test       = check "BUILDTEST"
 let build_doc        = check "BUILDDOC"
+let show             = check "SHOW"
 let dryrun           = check "DRYRUN"
 let fake             = check "FAKE"
 let print_stats      = check "STATS"