Merge pull request #3018 from OCamlPro/drop-features

This discards the unused `features:` field

doc/pages/Manual.md

@@ -209,9 +209,12 @@ and _options_:
   It corresponds to a case of the `<list>` non-terminal and is a list of
   `<list-contents>` repeated any number of times. The square brackets can be
   omitted when `<list-contents>` occurs just once.
+- `<element> { <opt> ... }` means `<element> "{" { <opt> }* "}"`, and is a
+  shortcut for the `<option>` non-terminal.
 - `<element> { <opt> }` means
   `<element> [ "{" <opt> "}" ]`.
-  It corresponds to a specific case of the `<option>` non-terminal.
+  It corresponds to a specific case of the `<option>` non-terminal where there
+  is exactly one element within the braces..
 
 ### General syntax
 
@@ -927,17 +930,13 @@ files.
       [`depexts:`](#opamfield-depexts) field.
 
 - <a id="opamfield-features">
-  `features: [ <ident> <string> { <filter> } ... ]`</a> (EXPERIMENTAL):
-  this field is currently experimental and shouldn't be used on the main package
-  repository. It allows to define custom variables that better document what
-  _features_ are available in a given package build. Each feature is defined as
-  an identifier, a documentation string, and a filter expression. The filter
-  expression can evaluate to either a boolean or a string, and the defined
-  identifier can be used as a variable in any filter (but recursive features are
-  not allowed and will be _undefined_).
-
-    This is typically useful to pass appropriate flags to `./configure` scripts,
-    depending on what is installed.
+  `features: [ <ident> { <pkgname> { <filtered-package-formula> } ... } { <string> } ... ]`
+  </a> (EXPERIMENTAL):
+  This field binds given idents to dependency formulas, and a documentation
+  text. The intent is to allow, in the future, packages to depend on additional
+  characteristics of their dependencies. This can be understood as a way to
+  further document the consequences of the presence of absence of optional
+  dependencies.
 
 - <a id="opamfield-synopsis">`synopsis: <string>`</a>:
   defines a short description (one line) for the package. This can also be

src/client/opamAdminRepoUpgrade.ml

@@ -72,6 +72,10 @@ let wrapper_conf_script =
   \    (Sys.file_exists (Filename.concat libdir \"dynlink.cmxa\"));\n\
   \  p \"  stubsdir: %%S\"\n\
   \    stubsdir;\n\
+  \  p \"  preinstalled: %{ocaml-system:installed}%\";\n\
+  \  p \"  compiler: \\\"%{ocaml-system:installed?system:}%\
+                       %{ocaml-base-compiler:version}%\
+                       %{ocaml-variants:version}%\\\"\";\n\
   \  p \"}\";\n\
   \  close_out oc\n\
   "
@@ -351,22 +355,7 @@ let do_upgrade repo_root =
               ocaml_system_pkgname,
               Atom (Constraint (`Eq, FString str_version))
             )
-          ]) |>
-      O.with_features [
-        OpamVariable.of_string "preinstalled",
-        "The concrete compiler was installed outside of opam",
-        FIdent ([Some ocaml_system_pkgname], OpamVariable.of_string "installed",
-                None);
-        OpamVariable.of_string "compiler",
-        "Detailed OCaml or variant compiler name",
-        FString
-          (Printf.sprintf "%%{%s:installed?system:}%%\
-                           %%{%s:installed?%s:}%%\
-                           %%{%s:version}%%"
-             (OpamPackage.Name.to_string ocaml_system_pkgname)
-             (OpamPackage.Name.to_string ocaml_official_pkgname) str_version
-             (OpamPackage.Name.to_string ocaml_variants_pkgname));
-      ]
+          ])
     in
     write_opam ~add_files:[conf_script_name^".in", wrapper_conf_script]
       wrapper_opam

src/client/opamConfigCommand.ml

@@ -95,21 +95,14 @@ let list gt ns =
             with Failure _ -> None)
           OpamPackageVar.package_variable_names
       in
-      let feature_vars =
-        List.map (fun (v, desc, filt) ->
-            let v = OpamVariable.Full.create name v in
-            v, OpamFilter.eval_to_string ~default:"#undefined" env filt, desc
-          )
-          (OpamFile.OPAM.features opam)
-      in
       let conf_vars =
         List.map (fun (v,c) ->
             OpamVariable.Full.create name v,
             OpamVariable.string_of_variable_contents c,
             "")
           (OpamFile.Dot_config.bindings conf)
       in
-      pkg_vars @ feature_vars @ conf_vars
+      pkg_vars @ conf_vars
     with Not_found -> []
   in
   let vars = List.flatten (List.map list_vars ns) in

src/format/opamFile.ml

@@ -1896,7 +1896,7 @@ module OPAMSyntax = struct
     substs     : basename list;
     patches    : (basename * filter option) list;
     build_env  : env_update list;
-    features   : (OpamVariable.t * string * filter) list;
+    features   : (OpamVariable.t * filtered_formula * string) list;
     extra_sources: (basename * URL.t) list;
 
     (* User-facing data used by opam *)
@@ -2370,7 +2370,11 @@ module OPAMSyntax = struct
       "build-env", no_cleanup Pp.ppacc with_build_env build_env
         (Pp.V.map_list ~depth:2 Pp.V.env_binding);
       "features", no_cleanup Pp.ppacc with_features features
-        Pp.V.features;
+        (Pp.V.map_list ~depth:2 @@
+         Pp.V.map_options_2
+           (Pp.V.ident -| Pp.of_module "variable" (module OpamVariable))
+           (Pp.V.package_formula_items `Conj Pp.V.(filtered_constraints ext_version))
+           (Pp.singleton -| Pp.V.string));
 
       "messages", no_cleanup Pp.ppacc with_messages messages
         (Pp.V.map_list ~depth:1 @@

src/format/opamFile.mli

@@ -275,7 +275,7 @@ module OPAM: sig
     substs     : basename list;
     patches    : (basename * filter option) list;
     build_env  : env_update list;
-    features   : (OpamVariable.t * string * filter) list;
+    features   : (OpamVariable.t * filtered_formula * string) list;
     extra_sources: (basename * URL.t) list;
 
     (* User-facing data used by opam *)
@@ -403,7 +403,7 @@ module OPAM: sig
   val conflict_class: t -> name list
 
   (** Contents of the 'features' field *)
-  val features: t -> (OpamVariable.t * string * filter) list
+  val features: t -> (OpamVariable.t * filtered_formula * string) list
 
   (** List of exported libraries *)
   val libraries: t -> (string * filter option) list
@@ -506,7 +506,7 @@ module OPAM: sig
 
   val with_conflict_class: name list -> t -> t
 
-  val with_features: (OpamVariable.t * string * filter) list -> t -> t
+  val with_features: (OpamVariable.t * filtered_formula * string) list -> t -> t
 
   (** Construct as [build] *)
   val with_build: command list -> t -> t

src/format/opamFormat.ml

@@ -458,7 +458,7 @@ module V = struct
   let package_atom constraints =
     map_option pkgname constraints
 
-  let package_formula kind constraints =
+  let package_formula_items kind constraints =
     let split, join = match kind with
       | `Conj -> OpamFormula.(ands_to_list, ands)
       | `Disj -> OpamFormula.(ors_to_list, ors)
@@ -494,7 +494,10 @@ module V = struct
       in
       List.map (aux ~in_and:false) (split f)
     in
-    list -| pp ~name:"pkg-formula" parse_formula print_formula
+    pp ~name:"pkg-formula" parse_formula print_formula
+
+  let package_formula kind constraints =
+    list -| package_formula_items kind constraints
 
   let env_binding =
     let parse ~pos:_ = function
@@ -507,24 +510,6 @@ module V = struct
     in
     list -| singleton -| pp ~name:"env-binding" parse print
 
-  let features =
-    let var = ident -| of_module "variable" (module OpamVariable) in
-    let doc_filt = map_option string filter in
-    let rec parse_features ~pos = function
-      | [] -> []
-      | [_] -> unexpected ()
-      | id :: opt :: r ->
-        let doc, filt = parse doc_filt ~pos:(value_pos opt) opt in
-        (parse var ~pos id, doc, filt) ::
-        parse_features ~pos:(OpamStd.Option.default pos (values_pos r)) r
-    in
-    let print ft =
-      List.fold_right (fun (id, doc, filt) acc ->
-          print var id :: print doc_filt (doc, filt) :: acc)
-        ft []
-    in
-    list -| pp ~name:"(variable \"doc\" {filter})*" parse_features print
-
   (* Only used by the deprecated "os" field *)
   let os_constraint =
     let rec parse_osc ~pos:_ l =

src/format/opamFormat.mli

@@ -165,12 +165,15 @@ module V : sig
     (value list, 'a) t ->
     (value, (name * 'a) OpamFormula.formula) t
 
+  (** Like [package_formula], but takes the list items directly *)
+  val package_formula_items :
+    [< `Conj | `Disj ] ->
+    (value list, 'a) t ->
+    (value list, (name * 'a) OpamFormula.formula) t
+
   (** Environment variable updates syntax *)
   val env_binding : (value, env_update) t
 
-  (** For the "features" field, e.g. a list of [(variable "doc" {filter})] *)
-  val features : (value, (variable * string * filter) list) t
-
   val os_constraint : (value, (bool * string) OpamFormula.formula) t
 end
 

src/state/opamFileTools.ml

@@ -45,8 +45,10 @@ let all_filters ?(exclude_post=false) t =
           | Constraint (_,f) -> f :: acc
           | Filter f -> f :: acc)
         acc f)
-    [] (OpamFormula.ands [t.depends; t.depopts]) @
-  List.map (fun (_,_,f) -> f) t.features
+    [] (OpamFormula.ands
+          (t.depends ::
+           t.depopts ::
+           List.map (fun (_,f,_) -> f) t.features))
 
 let all_variables ?exclude_post t =
   OpamFilter.commands_variables (all_commands t) @
@@ -93,12 +95,8 @@ let map_all_variables f t =
       )
   in
   let map_features =
-    List.map (fun (var, doc, filter) ->
-        let var = f (OpamVariable.Full.self var) in
-        assert OpamVariable.Full.(scope var = Self);
-        OpamVariable.Full.variable var,
-        doc,
-        OpamFilter.map_variables f filter)
+    List.map (fun (var, fformula, doc) ->
+        var, map_filtered_formula fformula, doc)
   in
   t |>
   with_patches (List.map map_optfld t.patches) |>
@@ -377,7 +375,7 @@ let lint t =
          OpamPackage.Name.Set.empty (all_variables ~exclude_post:true t)
      in
      cond 41 `Warning
-       "Some packages are mentioned in package scripts of features, but \
+       "Some packages are mentioned in package scripts or features, but \
         there is no dependency or depopt toward them"
        ~detail:OpamPackage.Name.
                  (List.map to_string (Set.elements undep_pkgs))

src/state/opamPackageVar.ml

@@ -148,7 +148,7 @@ let all_depends ?build ?test ?doc ?dev ?(filter_default=false)
     ~env:(resolve_switch ~package:(OpamFile.OPAM.package opam) st) deps
 
 (* filter handling *)
-let rec resolve st ?opam:opam_arg ?(local=OpamVariable.Map.empty) v =
+let resolve st ?opam:opam_arg ?(local=OpamVariable.Map.empty) v =
   let dirname dir = string (OpamFilename.Dir.to_string dir) in
   let pkgname = OpamStd.Option.map OpamFile.OPAM.name opam_arg in
   let read_package_var v =
@@ -178,25 +178,6 @@ let rec resolve st ?opam:opam_arg ?(local=OpamVariable.Map.empty) v =
         | some -> some
       with Not_found -> None
   in
-  let get_features_var opam v =
-    let to_str opam =
-      OpamPackage.to_string @@
-      OpamPackage.create (OpamFile.OPAM.name opam) (OpamFile.OPAM.version opam)
-    in
-    let features = OpamFile.OPAM.features opam in
-    try
-      let v, _descr, filt = List.find (fun (id,_,_) -> id = v) features in
-      let local = (* Avoid recursion *)
-        OpamVariable.Map.add v None local in
-      try Some (OpamFilter.eval (resolve st ~opam ~local) filt)
-      with Failure _ ->
-        OpamConsole.warning "Feature %s of %s didn't resolve%s"
-          (OpamVariable.to_string v) (to_str opam)
-          (match opam_arg with None -> "" | Some o ->
-            Printf.sprintf " (referred to from %s)" (to_str o));
-        None
-    with Not_found -> None
-  in
   let get_package_var v =
     if OpamVariable.Full.is_global v then None else
     let var_str = OpamVariable.to_string (OpamVariable.Full.variable v) in
@@ -216,10 +197,6 @@ let rec resolve st ?opam:opam_arg ?(local=OpamVariable.Map.empty) v =
           Some (OpamPackage.Map.find nv st.opams)
         with Not_found -> None
     in
-    let feat = match opam with
-      | Some o -> get_features_var o (OpamVariable.Full.variable v)
-      | None -> None in
-    if feat <> None then feat else
     let get_nv opam = OpamPackage.create name (OpamFile.OPAM.version opam) in
     let root = st.switch_global.root in
     match var_str, opam with