Merge pull request #5016 from rjbou/with-dev

Add development tools recommendation in opam file (variable) and cli

doc/pages/Manual.md

@@ -409,9 +409,9 @@ three scopes:
 
 3. Some fields define their own local variables, like `success` in the field
    [`post-messages`](#opamfield-post-messages). Other examples of this include
-   the [`with-test`](#pkgvar-with-test) and [`with-doc`](#pkgvar-with-doc)
-   variables, available in the `depends:`, `depopts:`, `build:`, `install:` and
-   `remove:` fields.
+   the [`with-test`](#pkgvar-with-test), [`with-doc`](#pkgvar-with-doc) and
+   [`with-tools`](#pkgvar-with-tools) variables, available in the `depends:`,
+   `depopts:`, `build:`, `install:` and `remove:` fields.
 
     Within package definition files, the variables `name` and `version`, as
     shortcuts to `_:name` and `_:version`, corresponding to the package being
@@ -520,6 +520,8 @@ Additionally, the following are limited to some package fields (`depends:`,
 - <a id="pkgvar-with-test">`with-test`</a>: only true if tests have been
   enabled for this specific package
 - <a id="pkgvar-with-doc">`with-doc`</a>: similarly for documentation
+- <a id="pkgvar-with-tools">`with-tools`</a>: similarly for developer tools
+
 
 The following are only available in the `depends:` and `depopts:` fields, and
 are used as dependency flags (they don't have a defined `true` or `false` value
@@ -857,8 +859,9 @@ files.
     expected to be produced within the source directory subtree, _i.e._ below
     the command's `$PWD`, during this step.
 
-    The [`with-test`](#pkgvar-with-test) and [`with-doc`](#pkgvar-with-doc)
-    variables are available in the scope of this field: filter testing commands
+    The [`with-test`](#pkgvar-with-test), [`with-doc`](#pkgvar-with-doc), and
+    [`with-tools`](#pkgvar-with-tools) variables are available in the scope of
+    this field: filter testing commands
     with _e.g._ `[make "test"] {with-test}`. The `dev` variable can also be
     useful here to detect that the package is not installed from a release
     tarball, and may need additional preprocessing (_e.g._ `automake`).
@@ -884,9 +887,9 @@ files.
     according to its instructions after calling the commands specified in the
     `install:` field have been run, if any.
 
-    Variables [`with-test`](#pkgvar-with-test) and
-    [`with-doc`](#pkgvar-with-doc) are also available to the filters used in
-    this field, to run specific installation commands when tests or
+    Variables [`with-test`](#pkgvar-with-test), [`with-doc`](#pkgvar-with-doc),
+    and [`with-tools`](#pkgvar-with-tools) are also available to the filters
+    used in this field, to run specific installation commands when tests or
     documentation have been requested.
 
 - <a id="opamfield-build-doc">
@@ -928,8 +931,9 @@ files.
     The filtered package formula can access the global and switch variables, but
     not variables from other packages. Additionally, special boolean variables
     [`build`](#pkgvar-build), [`post`](#pkgvar-post),
-    [`with-test`](#pkgvar-with-test) and [`with-doc`](#pkgvar-with-doc) are
-    defined to allow limiting the scope of the dependency.
+    [`with-test`](#pkgvar-with-test), [`with-doc`](#pkgvar-with-doc), and
+    [`with-tools`](#pkgvar-with-tools) are defined to allow limiting the scope
+    of the dependency.
 
     * `build` dependencies are no longer needed at run-time: they won't trigger
       recompilations of your package.
@@ -942,6 +946,7 @@ files.
       package is explicitly installed with `--with-test`)
     * likewise, `with-doc` dependencies are only required when building the
       package documentation
+    * likewise, `with-tools` dependencies are only required for a developer tool
 
 - <a id="opamfield-depopts">
   `depopts: [ <pkgname> { <filtered-package-formula> } ... ]`</a>:

master_changes.md

@@ -48,6 +48,7 @@ users)
   * ◈ Add `--formula` option to specify a formula to install [#4975 @AltGr]
   * [BUG] Prevent `.changes` files from being updated during dry-run [#5144 @na4zagin3 - fix #5132]
   * Log a summary of recorded `.changes` as a `ACTION` trace log to help debug #4419 [#5144 @na4zagin3]
+  * ◈ Add `--with-tools` option to install recommended development tools from opam file (as `with-test`/`with-doc`), and its environment variable `OPAMWITHTOOLS` [#5016 @rjbou]
 
 ## Remove
   *
@@ -120,6 +121,7 @@ users)
 
 ## Opamfile
   * Fix substring errors in preserved_format [#4941 @rjbou - fix #4936]
+  * Add `with-tools` variable for recommended tools [#5016 @rjbou]
 
 ## External dependencies
   * Set `DEBIAN_FRONTEND=noninteractive` for unsafe-yes confirmation level [#4735 @dra27 - partially fix #4731] [2.1.0~rc2 #4739]
@@ -178,6 +180,7 @@ users)
   * Bump opam-file-format to 2.1.4 [#5117 @kit-ty-kate - fix #5116]
   * Add `sha` dependency [#5042 @kit-ty-kate]
   * Add a 'test' target [#5129 @kit-ty-kate @mehdid - partially fixes #5058]
+  * Add `with-tools` handling in selection process [#5016 @rjbou]
 
 ## Infrastructure
   * Fix caching of Cygwin compiler on AppVeyor [#4988 @dra27]
@@ -347,6 +350,8 @@ users)
   * `OpamClient`: handle formula on several functions, adding a `formula` labelled or optional argument  (`upgrade_t`, `compute_upgrade_t`, `upgrade`, `fixup`, `install_t`, `install`, `remove_t`, and `remove`) [#4975 @AltGr]
   * `OpamSolution`: add `print_requested` to print actions reasons [#4975 @AltGr]
   * `OpamSolution.apply`: take an optional argument `skip`, to avoid filtering solution beforehand [#4975 @AltGr]
+  * `OpamAction`: add `?tools` filtering argument in `build_package`, `install_package` [#5016 @rjbou]
+  * `OpamListCommand`: add `?tools` filtering argument in `dependency_toggles` [#5016 @rjbou]
 
 ## opam-repository
   * `OpamRepositoryConfig`: add in config record `repo_tarring` field and as an argument to config functions, and a new constructor `REPOSITORYTARRING` in `E` environment module and its access function [#5015 @rjbou]
@@ -360,6 +365,9 @@ users)
   * `OpamSwitchState.universe`: `requested` argument moved from `name_package_set` to `package_set`, to precise installed packages with `--best-effort` [#4796 @LasseBlaauwbroek]
   * `OpamSwitchState.universe`: add a chrono for universe loading [#4975 @AltGr]
   * `OpamSwitchState.universe`: set to false unresolved variables used in constraint, and warn [#5141 @rjbou - fix #5139]
+  * `OpamStateConfig`: add with-tools support ; i.e. add `E.withtools`, add `with_tools` in config record [#5016 @rjbou]
+  * `OpamPackageVar`: add `?tools` filtering argument in `filter_depends_formula`, `all_depends` [#5016 @rjbou]
+  * `OpamSwitchState`: add `?tools` filtering argument in `universe` [#5016 @rjbou]
 
 ## opam-solver
   * `OpamCudf`: Change type of `conflict_case.Conflict_cycle` (`string list list` to `Cudf.package action list list`) and `cycle_conflict`, `string_of_explanations`, `conflict_explanations_raw` types accordingly [#4039 @gasche]
@@ -386,6 +394,7 @@ users)
   * `OpamFile.OPAM.to_string_with_preserved_format`: handle substring errors [#4941 @rjbou - fix #4936]
   * `OpamFile.OPAM.effective_part` and `OpamFile.OPAM.effectively_equal` now take an optional `?modulo_state:bool` parameter, that if `true`, eliminates the fields relying on the state of the switch (depends, available, …). This is `false` by default. [#5118 @kit-ty-kate]
   * `OpamTypes`: `request.wish_install` now takes a formula instead of  a conjunction [#4975 @AltGr]
+  * `OpamFilter`: add `?tools` filtering argument in `filter_deps` [#5016 @rjbou]
 
 ## opam-core
   * OpamSystem: avoid calling Unix.environment at top level [#4789 @hannesm]

src/client/opamAction.ml

@@ -870,17 +870,18 @@ let remove_package t ?silent ?changes ?force ?build_dir nv =
   else
     remove_package_aux t ?silent ?changes ?force ?build_dir nv
 
-let local_vars ~test ~doc =
+let local_vars ~test ~doc ~tools =
   OpamVariable.Map.of_list [
     OpamVariable.of_string "with-test", Some (B test);
     OpamVariable.of_string "with-doc", Some (B doc);
+    OpamVariable.of_string "with-tools", Some (B tools);
   ]
 
-let build_package t ?(test=false) ?(doc=false) build_dir nv =
+let build_package t ?(test=false) ?(doc=false) ?(tools=false) build_dir nv =
   let opam = OpamSwitchState.opam t nv in
   let commands =
     OpamFilter.commands
-      (OpamPackageVar.resolve ~opam ~local:(local_vars ~test ~doc) t)
+      (OpamPackageVar.resolve ~opam ~local:(local_vars ~test ~doc ~tools) t)
       (OpamFile.OPAM.build opam) @
     (if test then
        OpamFilter.commands (OpamPackageVar.resolve ~opam t)
@@ -940,12 +941,12 @@ let build_package t ?(test=false) ?(doc=false) build_dir nv =
 
 (* Assumes the package has already been compiled in its build dir.
    Does not register the installation in the metadata! *)
-let install_package t ?(test=false) ?(doc=false) ?build_dir nv =
+let install_package t ?(test=false) ?(doc=false) ?(tools=false) ?build_dir nv =
   let opam = OpamSwitchState.opam t nv in
   let commands =
     OpamFile.OPAM.install opam |>
     OpamFilter.commands
-      (OpamPackageVar.resolve ~opam ~local:(local_vars ~test ~doc) t) |>
+      (OpamPackageVar.resolve ~opam ~local:(local_vars ~test ~doc ~tools) t) |>
     OpamStd.List.filter_map
       (function [] -> None | cmd::args -> Some (cmd, args))
   in

src/client/opamAction.mli

@@ -47,14 +47,14 @@ val prepare_package_build:
     See {!download_package} and {!prepare_package_source} for the previous
     steps. *)
 val build_package:
-  rw switch_state -> ?test:bool -> ?doc:bool -> dirname -> package ->
+  rw switch_state -> ?test:bool -> ?doc:bool -> ?tools:bool -> dirname -> package ->
   exn option OpamProcess.job
 
 (** [install_package t pkg] installs an already built package. Returns
     [None] on success, [Some exn] on error. Do not update opam's
     metadata. See {!build_package} to build the package. *)
 val install_package:
-  rw switch_state -> ?test:bool -> ?doc:bool -> ?build_dir:dirname -> package ->
+  rw switch_state -> ?test:bool -> ?doc:bool -> ?tools:bool -> ?build_dir:dirname -> package ->
   (OpamFile.Dot_config.t option, exn) OpamCompat.Either.t OpamProcess.job
 
 (** Find out if the package source is needed for uninstall *)

src/client/opamArg.ml

@@ -256,6 +256,8 @@ let environment_variables =
        by `opam env --switch=SWITCH --set-switch'.";
       "UNLOCKBASE", cli_original, (fun v -> UNLOCKBASE (env_bool v)),
       "see install option `--unlock-base'.";
+      "WITHTOOLS", cli_from cli2_2, (fun v -> WITHTOOLS (env_bool v)),
+      "see install option `--with-tools'.";
       "WITHDOC", cli_original, (fun v -> WITHDOC (env_bool v)),
       "see install option `--with-doc'.";
       "WITHTEST", cli_original, (fun v -> WITHTEST (env_bool v)),
@@ -580,6 +582,7 @@ type build_options = {
   req_checksums : bool;
   build_test    : bool;
   build_doc     : bool;
+  with_tools      : bool;
   show          : bool;
   dryrun        : bool;
   fake          : bool;
@@ -595,14 +598,14 @@ type build_options = {
 
 let create_build_options
     keep_build_dir reuse_build_dir inplace_build make no_checksums
-    req_checksums build_test build_doc show dryrun skip_update
+    req_checksums build_test build_doc with_tools show dryrun skip_update
     fake jobs ignore_constraints_on unlock_base locked lock_suffix
     assume_depexts no_depexts
     =
   {
     keep_build_dir; reuse_build_dir; inplace_build; make; no_checksums;
-    req_checksums; build_test; build_doc; show; dryrun; skip_update; fake;
-    jobs; ignore_constraints_on; unlock_base; locked; lock_suffix;
+    req_checksums; build_test; build_doc; with_tools; show; dryrun; skip_update;
+    fake; jobs; ignore_constraints_on; unlock_base; locked; lock_suffix;
     assume_depexts; no_depexts;
   }
 
@@ -623,6 +626,7 @@ let apply_build_options cli b =
     (* ?no_base_packages:(flag o.no_base_packages) -- handled globally *)
     ?build_test:(flag b.build_test)
     ?build_doc:(flag b.build_doc)
+    ?with_tools:(flag b.with_tools)
     ?dryrun:(flag b.dryrun)
     ?makecmd:(b.make >>| fun m -> lazy m)
     ?ignore_constraints_on:
@@ -1360,6 +1364,10 @@ let build_options cli =
        the command-line. This is equivalent to setting $(b,\\$OPAMWITHDOC) \
        (or the deprecated $(b,\\$OPAMBUILDDOC)) to \"true\"."
   in
+  let with_tools =
+    mk_flag ~cli (cli_from cli2_2) ["with-tools"] ~section
+      "Include development tools only dependencies."
+  in
   let make =
     mk_opt ~cli (cli_between cli2_0 cli2_1
                    ~replaced:"opam config set[-global] make MAKE")
@@ -1425,9 +1433,10 @@ let build_options cli =
   in
   Term.(const create_build_options
         $keep_build_dir $reuse_build_dir $inplace_build $make
-        $no_checksums $req_checksums $build_test $build_doc $show $dryrun
-        $skip_update $fake $jobs_flag cli cli_original $ignore_constraints_on
-        $unlock_base $locked $lock_suffix $assume_depexts $no_depexts)
+        $no_checksums $req_checksums $build_test $build_doc $with_tools $show
+        $dryrun $skip_update $fake $jobs_flag cli cli_original
+        $ignore_constraints_on $unlock_base $locked $lock_suffix
+        $assume_depexts $no_depexts)
 
 (* Option common to install commands *)
 let assume_built cli =
@@ -1527,6 +1536,10 @@ let package_selection cli =
     mk_flag ~cli cli_original ["t";"test";"with-test"] ~section
       "Include test-only dependencies."
   in
+  let tools =
+    mk_flag ~cli (cli_from cli2_2) ["with-tools"] ~section
+      "Include development only dependencies."
+  in
   let field_match =
     mk_opt_all ~cli cli_original ["field-match"] "FIELD:PATTERN" ~section
       "Filter packages with a match for $(i,PATTERN) on the given $(i,FIELD)"
@@ -1555,12 +1568,12 @@ let package_selection cli =
   in
   let filter
       depends_on required_by conflicts_with coinstallable_with resolve recursive
-      depopts nobuild post dev doc_flag test field_match has_flag has_tag
+      depopts nobuild post dev doc_flag test tools field_match has_flag has_tag
     =
     let dependency_toggles = {
       OpamListCommand.
-      recursive; depopts; build = not nobuild; post; test; doc = doc_flag;
-      dev
+      recursive; depopts; build = not nobuild; post; test; tools;
+      doc = doc_flag; dev
     } in
     List.map (fun flag -> OpamListCommand.Flag flag) has_flag @
     List.map (fun tag -> OpamListCommand.Tag tag) has_tag @
@@ -1589,7 +1602,7 @@ let package_selection cli =
   Term.(const filter $
         depends_on $ required_by $ conflicts_with $ coinstallable_with $
         resolve $ recursive $ depopts $ nobuild $ post $ dev $ doc_flag $
-        test $ field_match $ has_flag $ has_tag)
+        test $ tools $ field_match $ has_flag $ has_tag)
 
 let package_listing_section = "OUTPUT FORMAT OPTIONS"
 

src/client/opamClient.ml

@@ -834,8 +834,9 @@ let check_installed ~build ~post t atoms =
   in
   let test = OpamStateConfig.(!r.build_test) in
   let doc = OpamStateConfig.(!r.build_doc) in
+  let tools = OpamStateConfig.(!r.with_tools) in
   let env p =
-    OpamFilter.deps_var_env ~build ~post ~test ~doc
+    OpamFilter.deps_var_env ~build ~post ~test ~doc ~tools
       ~dev:(OpamSwitchState.is_dev_package t p)
   in
   OpamPackage.Name.Map.fold (fun name versions map ->

src/client/opamClientConfig.mli

@@ -126,6 +126,7 @@ val opam_init:
   ?dl_jobs:int ->
   ?build_test:bool ->
   ?build_doc:bool ->
+  ?with_tools:bool ->
   ?dryrun:bool ->
   ?makecmd:string Lazy.t ->
   ?ignore_constraints_on:OpamPackage.Name.Set.t ->

src/client/opamCommands.ml

@@ -1383,7 +1383,7 @@ let config cli =
               | None -> false)
           |> OpamSolver.dependencies ~depopts:true ~post:true ~build:true
             ~installed:true
-            (OpamSwitchState.universe ~test:true ~doc:true
+            (OpamSwitchState.universe ~test:true ~doc:true ~tools:true
                ~requested:OpamPackage.Set.empty state Query)
           |> OpamPackage.Set.iter process;
           if List.mem "." (OpamStd.Sys.split_path_variable (Sys.getenv "PATH"))

src/client/opamListCommand.ml

@@ -24,6 +24,7 @@ type dependency_toggles = {
   build: bool;
   post: bool;
   test: bool;
+  tools: bool;
   doc: bool;
   dev: bool;
 }
@@ -34,6 +35,7 @@ let default_dependency_toggles = {
   build = true;
   post = false;
   test = false;
+  tools = false;
   doc = false;
   dev = false;
 }
@@ -151,7 +153,7 @@ let package_dependencies st tog nv =
   get_opam st nv |>
   OpamPackageVar.all_depends
     ~build:tog.build ~post:tog.post
-    ~test:tog.test ~doc:tog.doc ~dev:tog.dev
+    ~test:tog.test ~doc:tog.doc ~tools:tog.tools ~dev:tog.dev
     ~depopts:tog.depopts
     st
 
@@ -169,7 +171,7 @@ let get_universe st ?requested tog =
     | None -> st.packages
   in
   OpamSwitchState.universe st
-    ~test:tog.test ~doc:tog.doc ~force_dev_deps:tog.dev
+    ~test:tog.test ~doc:tog.doc ~tools:tog.tools ~force_dev_deps:tog.dev
     ~requested Query
 
 let rec value_strings value =

src/client/opamListCommand.mli

@@ -23,6 +23,7 @@ type dependency_toggles = {
   build: bool;
   post: bool;
   test: bool;
+  tools: bool;
   doc: bool;
   dev: bool;
 }

src/client/opamLockCommand.ml

@@ -157,13 +157,13 @@ let lock_opam ?(only_direct=false) st opam =
                                (OpamPackage.Set.elements set))
   in
   let depends_map = map_of_set `version depends in
-  (* others: dev, test, doc *)
+  (* others: dev, test, doc, with-tools *)
   let open OpamPackage.Set.Op in
-  let select ?(build=false) ?(test=false) ?(doc=false) ?(dev=false)
-      ?(default=false) ?(post=false) () =
+  let select ?(build=false) ?(test=false) ?(doc=false) ?(tools=false)
+      ?(dev=false) ?(default=false) ?(post=false) () =
     OpamFormula.packages st.packages
       (OpamFilter.filter_deps
-         ~build ~test ~doc ~dev ~default ~post
+         ~build ~test ~doc ~dev ~tools ~default ~post
          (OpamFile.OPAM.depends opam))
   in
   let default = select () in
@@ -199,6 +199,7 @@ let lock_opam ?(only_direct=false) st opam =
   in
   let test_depends_map = select_depends "with-test" (select ~test:true ()) in
   let doc_depends_map = select_depends "with-doc" (select ~doc:true ()) in
+  let tools_depends_map = select_depends "with-tools" (select ~tools:true ()) in
   let depends =
     let f a b =
       match a,b with
@@ -213,6 +214,7 @@ let lock_opam ?(only_direct=false) st opam =
       |> union f dev_depends_map
       |> union f test_depends_map
       |> union f doc_depends_map
+      |> union f tools_depends_map
     )
   in
   (* formulas *)
@@ -258,7 +260,7 @@ let lock_opam ?(only_direct=false) st opam =
   let all_depopts =
     OpamFormula.packages st.packages
       (OpamFilter.filter_deps
-         ~build:true ~test:true ~doc:true ~dev:true ~default:true ~post:false
+         ~build:true ~test:true ~doc:true ~tools:true ~default:true ~post:false
          (OpamFile.OPAM.depopts opam))
   in
   let installed_depopts = OpamPackage.Set.inter all_depopts st.installed in