Merge pull request ocaml#5687 from rjbou/tree-fx

Tree: fix `--dev` & `--no-switch` options
rjbou · Dec 12, 2023 · 2592c14 · 2592c14
2 parents 173e833 + 2a7fc81
commit 2592c14
Show file tree

Hide file tree

Showing 6 changed files with 164 additions and 29 deletions.
diff --git a/master_changes.md b/master_changes.md
@@ -43,6 +43,8 @@ users)
 ## Update / Upgrade
 
 ## Tree
+  * Fix `--dev` option, force dev dependencies when option is given [#5687 @rjbou - fix #5675]
+  * Fix `--no-switch` option, instead of emptying switch from it installed packages, load a virtual switch at the beginning when `--no-switch` is given [#5687 @rjbou - fix #5675]
 
 ## Exec
 
@@ -98,6 +100,7 @@ users)
 
 ## Reftests
 ### Tests
+  * Add some additional test to tree, for `--dev` && `--no-switch` [#5687 @rjbou]
 
 ### Engine
   * Set `SHELL` to `/bin/sh` in Windows to ensure `opam env` commands are consistent [#5723 @dra27]

diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml
@@ -828,7 +828,12 @@ let tree ?(why=false) cli =
     else
       (apply_global_options cli global_options;
        OpamGlobalState.with_ `Lock_none @@ fun gt ->
-       OpamSwitchState.with_ `Lock_none gt @@ fun st ->
+       OpamRepositoryState.with_ `Lock_none gt @@ fun rt ->
+       (if no_switch then
+          fun k ->
+            k @@ OpamSwitchState.load_virtual gt rt
+        else
+          OpamSwitchState.with_ `Lock_none ~rt gt) @@ fun st ->
        let st, atoms =
          OpamAuxCommands.simulate_autopin
            st ~recurse ?subpath ~quiet:true
@@ -840,7 +845,7 @@ let tree ?(why=false) cli =
            depopts = false;
            build = true;
          } in
-       OpamTreeCommand.run st tog ~no_constraint ~no_switch mode filter atoms;
+       OpamTreeCommand.run st tog ~no_constraint mode filter atoms;
        `Ok ())
   in
   mk_command_ret ~cli (cli_from cli2_2) "tree" ~doc ~man

diff --git a/src/client/opamTreeCommand.ml b/src/client/opamTreeCommand.ml
@@ -379,8 +379,10 @@ let print_solution st new_st missing solution =
 (** Setting states for building *)
 
 let get_universe tog st =
-  let OpamListCommand.{doc; test; dev_setup; _} = tog in
-  OpamSwitchState.universe st ~doc ~test ~dev_setup ~requested:st.installed Query
+  let OpamListCommand.{doc; test; dev_setup; dev; _} = tog in
+  OpamSwitchState.universe st ~doc ~test ~dev_setup ~force_dev_deps:dev
+    ~requested:st.installed
+    Query
 
 let simulate_new_state tog st universe install names =
   match OpamSolver.resolve universe
@@ -401,25 +403,7 @@ let dry_install tog st universe install =
   simulate_new_state tog st universe install
     (OpamPackage.Name.Set.of_list (List.map fst install))
 
-let raw_state tog st install =
-  let OpamListCommand.{doc; test; dev_setup; _} = tog in
-  let names = OpamPackage.Name.Set.of_list (List.map fst install) in
-  let requested =
-    OpamPackage.packages_of_names
-      (Lazy.force st.available_packages)
-      names
-  in
-  let universe =
-    OpamSwitchState.universe st ~doc ~test ~dev_setup ~requested Query
-  in
-  let universe =
-    { universe
-      with u_installed = OpamPackage.Set.empty;
-           u_installed_roots = OpamPackage.Set.empty }
-  in
-  simulate_new_state tog st universe install names
-
-let run st tog ?no_constraint ?(no_switch=false) mode filter atoms =
+let run st tog ?no_constraint mode filter atoms =
   let open OpamPackage.Set.Op in
   let select, missing =
     List.fold_left (fun (select, missing) atom ->
@@ -436,9 +420,6 @@ let run st tog ?no_constraint ?(no_switch=false) mode filter atoms =
     match mode, filter, missing with
     | Deps, _, [] -> st, universe
     | Deps, Roots_from, _::_ ->
-      if no_switch then
-        raw_state tog st missing
-      else
         dry_install tog st universe missing
     | Deps, Leads_to, _::_
     | ReverseDeps, _, _ ->
@@ -462,6 +443,6 @@ let run st tog ?no_constraint ?(no_switch=false) mode filter atoms =
   in
   print ?no_constraint forest;
   if OpamClientConfig.(!r.json_out) <> None then
-    (if not no_switch then
+    (if OpamSwitch.compare st.switch OpamSwitch.unset <> 0 then
        OpamJson.append "switch" (OpamSwitch.to_json st.switch);
      OpamJson.append "tree" (forest_to_json ?no_constraint forest))
diff --git a/src/client/opamTreeCommand.mli b/src/client/opamTreeCommand.mli
@@ -34,6 +34,4 @@ val run :
   OpamListCommand.dependency_toggles ->
   (* output format options *)
   ?no_constraint:bool ->
-  (* do no keep switch consistency *)
-  ?no_switch:bool ->
   mode -> tree_filter -> OpamTypes.atom list -> unit
diff --git a/tests/reftests/json.unix.test b/tests/reftests/json.unix.test
@@ -1301,3 +1301,64 @@ a.1
     }
   ]
 }
+### opam tree c --no-switch --json=out.json
+The following actions are simulated:
+=== install 4 packages
+  - install a       1 [required by c]
+  - install b       1 [required by c]
+  - install c       1
+  - install c-build 1 [required by c]
+
+c.1
+|-- a.1
+|-- b.1
+|   '-- a.1 [*]
+'-- c-build.1 (build)
+### json-cat out.json
+{
+  "opam-version": "currentv",
+  "command-line": [
+    "${OPAM}",
+    "tree",
+    "c",
+    "--no-switch",
+    "--json=out.json"
+  ],
+  "tree": [
+    {
+      "name": "c",
+      "version": "1",
+      "dependencies": [
+        {
+          "name": "a",
+          "version": "1",
+          "satisfies": null,
+          "is_duplicate": false,
+          "dependencies": []
+        },
+        {
+          "name": "b",
+          "version": "1",
+          "satisfies": null,
+          "is_duplicate": false,
+          "dependencies": [
+            {
+              "name": "a",
+              "version": "1",
+              "satisfies": null,
+              "is_duplicate": true,
+              "dependencies": []
+            }
+          ]
+        },
+        {
+          "name": "c-build",
+          "version": "1",
+          "satisfies": "(build)",
+          "is_duplicate": false,
+          "dependencies": []
+        }
+      ]
+    }
+  ]
+}
diff --git a/tests/reftests/tree.test b/tests/reftests/tree.test
@@ -242,6 +242,87 @@ i.1
 
 j.1
 '-- a.1 [*]
+### : dev variable
+### <pkg:l.1>
+opam-version: "2.0"
+depends: [ "a" "l-dev" {dev} ]
+### <pkg:l-dev.1>
+opam-version: "2.0"
+### <pin:l/l.opam>
+opam-version: "2.0"
+depends: [ "a" "l-dev" {dev} ]
+### opam tree l
+The following actions are simulated:
+=== install 1 package
+  - install l 1
+
+l.1
+'-- a.1
+### opam tree l --dev
+The following actions are simulated:
+=== install 2 packages
+  - install l     1
+  - install l-dev 1 [required by l]
+
+l.1
+|-- a.1
+'-- l-dev.1 (dev)
+### opam tree l --no-switch
+The following actions are simulated:
+=== install 2 packages
+  - install a 1 [required by l]
+  - install l 1
+
+l.1
+'-- a.1
+### opam tree l --dev --no-switch
+The following actions are simulated:
+=== install 3 packages
+  - install a     1 [required by l]
+  - install l     1
+  - install l-dev 1 [required by l]
+
+l.1
+|-- a.1
+'-- l-dev.1 (dev)
+### opam pin ./l
+l is now pinned to file://${BASEDIR}/l (version 1)
+
+The following actions will be performed:
+=== install 2 packages
+  - install l     1 (pinned)
+  - install l-dev 1          [required by l]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> retrieved l.1  (file://${BASEDIR}/l)
+-> installed l-dev.1
+-> installed l.1
+Done.
+### opam tree l
+l.1
+|-- a.1
+'-- l-dev.1
+### opam tree l --no-switch
+The following actions are simulated:
+=== install 2 packages
+  - install a 1 [required by l]
+  - install l 1
+
+l.1
+'-- a.1
+### opam tree l --dev --no-switch
+The following actions are simulated:
+=== install 3 packages
+  - install a     1 [required by l]
+  - install l     1
+  - install l-dev 1 [required by l]
+
+l.1
+|-- a.1
+'-- l-dev.1 (dev)
+### ::::::::::::::::
+### : Empty switch :
+### ::::::::::::::::
 ### opam switch create void --empty
 ### opam tree f h
 The following actions are simulated:
@@ -281,6 +362,9 @@ e.1
 [WARNING] Not installed package a, skipping
 [ERROR] No package to display
 # Return code 5 #
+### ::::::::::::::::
+### : --not-switch :
+### ::::::::::::::::
 ### opam tree --no-switch | '…' -> '...' | '`' -> "'"
 opam: --no-switch can't be used without specifying a package or a path
 Usage: opam tree [--recursive] [--subpath=PATH] [OPTION]... [PACKAGES]...
@@ -388,6 +472,9 @@ Done.
     You can temporarily relax the switch invariant with `--update-invariant'
 
 # Return code 20 #
+### ::::::::::::::::::::::::::::::::::::
+### : Arguments as packages, directory :
+### ::::::::::::::::::::::::::::::::::::
 ### <pkg:i.1>
 opam-version: "2.0"
 depends: "a"