Merge pull request #6540 from hannesm/switch-create-dry-run

[2.5] When doing opam sw create ... --dry-run, avoid creating of the directories

Author: kit-ty-kate

master_changes.md

@@ -31,6 +31,8 @@ users)
 ## UI
 
 ## Switch
+  * [BUG] Stop `opam switch create --dry-run` from creating a directory: no longer create build dir and save switch-state cache. [#6540 @hannesm - fix #5918]
+  * No longer clean build directory [#6540 @hannesm]
 
 ## Config
 
@@ -112,6 +114,7 @@ users)
 ## Reftests
 ### Tests
   *  Add test cases to `update.test` for version-equivalent renames [#6774 @arozovyk fix #6754]
+  * Add a test case for `opam switch create --dry-run` [#6540 @kit-ty-kate]
 
 ### Engine
 

src/client/opamSolution.ml

@@ -648,7 +648,8 @@ let parallel_apply t
         try true, OpamPackage.Map.find nv inplace
         with Not_found ->
           let dir = OpamPath.Switch.build t.switch_global.root t.switch nv in
-          if not OpamClientConfig.(!r.reuse_build_dir) then
+          if not OpamClientConfig.(!r.reuse_build_dir) &&
+             not OpamStateConfig.(!r.dryrun) then
             OpamFilename.rmdir dir;
           false, dir
       in
@@ -659,10 +660,11 @@ let parallel_apply t
         OpamStateConfig.(!r.dev_setup) && found
       in
       let source_dir = source_dir nv in
-      (if OpamFilename.exists_dir source_dir
-       then (if not is_inplace then
-               OpamFilename.copy_dir ~src:source_dir ~dst:build_dir)
-       else OpamFilename.mkdir build_dir;
+      (if not OpamStateConfig.(!r.dryrun) then
+         if OpamFilename.exists_dir source_dir
+         then (if not is_inplace then
+                 OpamFilename.copy_dir ~src:source_dir ~dst:build_dir)
+         else OpamFilename.mkdir build_dir;
        OpamAction.prepare_package_source t nv build_dir @@+ function
        | Some exn -> store_time (); Done (`Exception exn)
        | None ->
@@ -823,31 +825,32 @@ let parallel_apply t
   (* 2/ Display errors and finalize *)
 
   let save_installed_cache failed =
-    OpamSwitchState.Installed_cache.save
-      (OpamPath.Switch.installed_opams_cache t.switch_global.root t.switch)
-      (OpamPackage.Set.fold (fun nv opams ->
-           (* NOTE: We need to know whether an action was successful
-              or not to know which version of the opam file to store
-              in the case: the previous one if it failed, or the new
-              one if it succeeded. *)
-           let pkg_failed =
-             List.exists (function
-                 | `Fetch ps -> List.for_all (OpamPackage.equal nv) ps
-                 | `Build p | `Change (_, _, p) | `Install p
-                 | `Reinstall p | `Remove p -> OpamPackage.equal nv p)
-               failed
-           in
-           let add_to_opams opam =
-             let opam = OpamFile.OPAM.with_metadata_dir None opam in
-             OpamPackage.Map.add nv opam opams
-           in
-           if pkg_failed then
-             match OpamPackage.Map.find_opt nv t.installed_opams with
-             | None -> opams
-             | Some opam -> add_to_opams opam
-           else
-             add_to_opams (OpamSwitchState.opam t nv))
-          t.installed OpamPackage.Map.empty);
+    if not OpamStateConfig.(!r.dryrun) then
+      OpamSwitchState.Installed_cache.save
+        (OpamPath.Switch.installed_opams_cache t.switch_global.root t.switch)
+        (OpamPackage.Set.fold (fun nv opams ->
+             (* NOTE: We need to know whether an action was successful
+                or not to know which version of the opam file to store
+                in the case: the previous one if it failed, or the new
+                one if it succeeded. *)
+             let pkg_failed =
+               List.exists (function
+                   | `Fetch ps -> List.for_all (OpamPackage.equal nv) ps
+                   | `Build p | `Change (_, _, p) | `Install p
+                   | `Reinstall p | `Remove p -> OpamPackage.equal nv p)
+                 failed
+             in
+             let add_to_opams opam =
+               let opam = OpamFile.OPAM.with_metadata_dir None opam in
+               OpamPackage.Map.add nv opam opams
+             in
+             if pkg_failed then
+               match OpamPackage.Map.find_opt nv t.installed_opams with
+               | None -> opams
+               | Some opam -> add_to_opams opam
+             else
+               add_to_opams (OpamSwitchState.opam t nv))
+            t.installed OpamPackage.Map.empty);
   in
   begin match action_results with
   | `Exception _ | `Error Aborted -> ()

tests/reftests/dot-install.test

@@ -128,7 +128,7 @@ ACTION                          prepare_package_source: dot.~dev at ${BASEDIR}/O
 ACTION                          Removing dot.~dev
 -> removed   dot.~dev
 Installing dot.~dev.
-TRACK                           after install: 0 elements, 0 added, scanned in 0.000s
+TRACK                           after install: 3 elements, 0 added, scanned in 0.000s
 -> installed dot.~dev
 ACTION                          Cleaning up artefacts of dot.~dev
 Done.
@@ -196,7 +196,7 @@ The following actions will be simulated:
 ACTION                          download_package: dot.~dev
 ACTION                          prepare_package_source: dot.~dev at ${BASEDIR}/OPAM/inst/.opam-switch/build/dot.~dev
 Installing dot.~dev.
-TRACK                           after install: 0 elements, 0 added, scanned in 0.000s
+TRACK                           after install: 1 elements, 0 added, scanned in 0.000s
 -> installed dot.~dev
 Done.
 ### ocaml cat.ml inst dot

tests/reftests/dry-run.test

@@ -0,0 +1,34 @@
+N0REP0
+### <pkg:comp.1>
+opam-version: "2.0"
+flags: compiler
+### ::: opam switch create --dry-run local
+### opam switch create --dry-run --empty ./local
+### test -d ./local
+# Return code 1 #
+### opam switch create --dry-run ./local comp
+
+<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><>
+Switch invariant: ["comp"]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+Installing comp.1.
+-> installed comp.1
+Done.
+### test -d ./local
+# Return code 1 #
+### ::: opam switch create --dry-run global
+### opam switch create --dry-run --empty global
+### test -d OPAM/global
+# Return code 1 #
+### opam switch create --dry-run global comp
+
+<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><>
+Switch invariant: ["comp"]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+Installing comp.1.
+-> installed comp.1
+Done.
+### test -d OPAM/global
+# Return code 1 #

tests/reftests/dune.inc

@@ -566,6 +566,27 @@
    %{targets}
    (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:download.test} %{read-lines:testing-env}))))
 
+(rule
+ (alias reftest-dry-run)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (action
+  (diff dry-run.test dry-run.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (deps (alias reftest-dry-run)))
+
+(rule
+ (targets dry-run.out)
+ (deps root-N0REP0)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:dry-run.test} %{read-lines:testing-env}))))
+
 (rule
  (alias reftest-effectively-equal)
  (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))