Merge pull request #5919 from kit-ty-kate/fix-wrong-var-assign-tree

Fix opam tree --test --doc assigning the with-{test,doc} variables to unrequested packages

master_changes.md

@@ -42,6 +42,8 @@ users)
 ## Update / Upgrade
 
 ## Tree
+  * [BUG] Fix `opam tree --with-*` assigning the `with-*` variables to unrequested packages [#5919 @kit-ty-kate @rjbou - fix #5755]
+  * [BUG] Fix combinations of `opam tree --with-*` and `--no-switch` [#5919 @kit-ty-kate @rjbou - fix #5920]
 
 ## Exec
 
@@ -102,6 +104,8 @@ users)
 
 ## Reftests
 ### Tests
+  * tree: add a test for packages that have variables in their transitive dependencies [#5919 @rjbou]
+  * tree: add test for `opam tree pkg --with-test --no-switch` [#5919 @rjbou]
 
 ### Engine
 

src/client/opamTreeCommand.ml

@@ -378,19 +378,22 @@ let print_solution st new_st missing solution =
 
 (** Setting states for building *)
 
-let get_universe tog st =
+let get_universe tog requested st =
   let OpamListCommand.{doc; test; dev_setup; dev; _} = tog in
   OpamSwitchState.universe st ~doc ~test ~dev_setup ~force_dev_deps:dev
-    ~requested:st.installed
+    ~requested
     Query
 
-let simulate_new_state tog st universe install names =
+let dry_install tog st universe install =
   match OpamSolver.resolve universe
           (OpamSolver.request ~install ()) with
   | Success solution ->
     let new_st = OpamSolution.dry_run st solution in
-    print_solution st new_st names solution;
-    new_st, get_universe tog new_st
+    print_solution st new_st
+      (OpamPackage.Name.Set.of_list (List.map fst install))
+      solution;
+    let requested = OpamFormula.packages_of_atoms new_st.installed install in
+    new_st, get_universe tog requested new_st
   | Conflicts cs ->
     OpamConsole.error
       "Could not simulate installing the specified package(s) to this switch:";
@@ -399,10 +402,6 @@ let simulate_new_state tog st universe install names =
          (OpamSwitchState.unavailable_reason st) cs);
     OpamStd.Sys.exit_because `No_solution
 
-let dry_install tog st universe install =
-  simulate_new_state tog st universe install
-    (OpamPackage.Name.Set.of_list (List.map fst install))
-
 let run st tog ?no_constraint mode filter atoms =
   let open OpamPackage.Set.Op in
   let select, missing =
@@ -412,11 +411,18 @@ let run st tog ?no_constraint mode filter atoms =
         in
         if OpamPackage.Set.is_empty installed then
           (select, atom :: missing)
-        else (installed ++ select, missing))
+        else
+          (installed ++ select, missing))
       (OpamPackage.Set.empty, []) atoms
   in
   let st, universe =
-    let universe = get_universe tog st in
+    let universe =
+      let requested =
+        OpamFormula.packages_of_atoms
+          (if missing = [] then st.installed else st.packages) atoms
+      in
+      get_universe tog requested st
+    in
     match mode, filter, missing with
     | Deps, _, [] -> st, universe
     | Deps, Roots_from, _::_ ->

tests/reftests/tree.test

@@ -320,6 +320,30 @@ The following actions are simulated:
 l.1
 |-- a.1
 '-- l-dev.1 (dev)
+### : dependency have variables in their dependencies
+### <pkg:m.1>
+opam-version: "2.0"
+depends: [ "j" "d" {with-doc} ]
+# j depends on k with-test
+### opam install m k --with-doc
+The following actions will be performed:
+=== install 2 packages
+  - install k 1
+  - install m 1
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed k.1
+-> installed m.1
+Done.
+### opam tree m
+m.1
+'-- j.1
+    '-- a.1
+### opam tree m --with-test --with-doc
+m.1
+|-- d.1 (with-doc)
+'-- j.1
+    '-- a.1
 ### ::::::::::::::::
 ### : Empty switch :
 ### ::::::::::::::::
@@ -343,12 +367,14 @@ h.1
     '-- b.1 (>= 1 & < 3) [*]
 ### opam tree j --with-test
 The following actions are simulated:
-=== install 2 packages
+=== install 3 packages
   - install a 1 [required by j]
   - install j 1
+  - install k 1 [required by j]
 
 j.1
-'-- a.1
+|-- a.1
+'-- k.1 (with-test)
 ### opam tree e
 The following actions are simulated:
 === install 3 packages
@@ -363,7 +389,7 @@ e.1
 [ERROR] No package to display
 # Return code 5 #
 ### ::::::::::::::::
-### : --not-switch :
+### : --no-switch :
 ### ::::::::::::::::
 ### opam tree --no-switch | '…' -> '...' | '`' -> "'"
 opam: --no-switch can't be used without specifying a package or a path
@@ -452,6 +478,28 @@ The following actions are simulated:
   - install j 1
 
 j.1
+### # e depend on d with-test
+### opam tree e --no-switch
+The following actions are simulated:
+=== install 3 packages
+  - install a 1 [required by e]
+  - install b 1
+  - install e 1
+
+e.1
+'-- a.1
+### opam tree e --with-test --no-switch
+The following actions are simulated:
+=== install 4 packages
+  - install a 1 [required by e]
+  - install b 1
+  - install d 1 [required by e]
+  - install e 1
+
+e.1
+|-- a.1
+'-- d.1 (< 2 & with-test)
+### : invariant
 ### <pkg:h.1>
 opam-version: "2.0"
 flags: compiler