[API] Split the deprecated url and descr files implementations to their own submodules

Author: kit-ty-kate

doc/pages/Manual.md

@@ -1211,10 +1211,11 @@ files.
   defines a long description (one or more paragraphs) for the package. This can
   also be defined as the body of an external [`descr`](#descr) file.
 
-- <a id="opamsection-url">`url "{" <url-file> "}"`</a>:
-  defines the URL where the package source can be obtained. This section has
-  contents in the same format as the [`url`](#url) file, and has the same effect
-  as using a separate `url` file.
+- <a id="opamsection-url">`url "{" <url-section> "}"`</a>:
+  defines the URL where the package source can be obtained. Before opam 2.6,
+  this section' content was the same format as the [`url`](#url) file,
+  and had the same effect as using a separate `url` file.
+  As of opam 2.6, the `url` file now only supports the legacy opam 1.2 fields.
 
 - <a id="opamfield-setenv">`setenv: [ <environment-update> ... ]`</a>: defines
   environment variables updates that will be applied upon installing the
@@ -1249,7 +1250,7 @@ files.
   See [`x-env-path-rewrite:`](#opamfield-x-env-path-rewrite)
   for path portability of environment variables on Windows.
 
-- <a id="opamsection-extra-sources">`extra-source <string> "{" <url-file> "}"`</a>:
+- <a id="opamsection-extra-sources">`extra-source <string> "{" <url-section> "}"`</a>:
   allows the definition of extra files that need downloading into the source
   tree before the package can be patched (if necessary) and built. The format is
   similar to that of the `url` section, but here it is expected to point to a

master_changes.md

@@ -65,6 +65,7 @@ users)
 ## Env
 
 ## Opamfile
+  * The `url` file now only supports the legacy opam 1.2 fields [#6827 @kit-ty-kate]
 
 ## External dependencies
 
@@ -160,6 +161,10 @@ users)
 ## opam-solver
 
 ## opam-format
+  * `OpamFile.Descr` was moved to `OpamFile.Descr_legacy` and a simpler `OpamFile.Descr` module was created only containing non-IO functions removing the outdated `descr` file support [#6827 @kit-ty-kate]
+  * `OpamFile.URL` was moved to `OpamFile.URL_legacy` and a simpler `OpamFile.URL` module was created only containing non-IO functions removing the outdated `url` file support [#6827 @kit-ty-kate]
+  * `OpamFile.Descr.of_legacy`: was added [#6827 @kit-ty-kate]
+  * `OpamFile.URL.of_legacy`: was added [#6827 @kit-ty-kate]
 
 ## opam-core
   * `OpamCmdliner` was added. It is accessible through a new `opam-core.cmdliner` sub-library [#6755 @kit-ty-kate]

src/client/opamAdminRepoUpgrade.ml

@@ -219,7 +219,7 @@ let do_upgrade repo_root =
       let descr_file =
         OpamFilename.(opt_file (add_extension (chop_extension comp_file) "descr"))
       in
-      let descr = descr_file >>| fun f -> OpamFile.Descr.read (OpamFile.make f) in
+      let descr = descr_file >>| fun f -> OpamFile.Descr_legacy.read (OpamFile.make f) in
       let nv, ocaml_version, variant =
         match OpamStd.String.cut_at c '+' with
         | None ->

src/format/opamFile.ml

@@ -170,7 +170,7 @@ end
     content. Formerly, (<repo>/packages/.../descr,
     <repo>/compilers/.../<v>.descr) *)
 
-module DescrIO = struct
+module Descr_legacyIO = struct
 
   let internal = "descr"
   let format_version = OpamVersion.of_string "0"
@@ -217,9 +217,34 @@ module DescrIO = struct
   let to_string _ = full
 
 end
+module Descr_legacy = struct
+  include Descr_legacyIO
+  include MakeIO(Descr_legacyIO)
+end
+
 module Descr = struct
-  include DescrIO
-  include MakeIO(DescrIO)
+  type t = string * string
+
+  let empty = "", ""
+
+  let synopsis = fst
+  let body = snd
+
+  let to_string (x,y) =
+    match y with
+    | "" -> x ^ "\n"
+    | y -> String.concat "" [x; "\n\n"; y; "\n"]
+
+  let of_string str =
+    let head, tail =
+      match OpamStd.String.cut_at str '\n' with
+      | None       -> str, ""
+      | Some (h,t) -> h, t in
+    OpamStd.String.strip head, OpamStd.String.strip tail
+
+  let create = of_string
+
+  let of_legacy (x : Descr_legacy.t) : t = x
 end
 
 (* module Comp_descr = Descr *)
@@ -2412,11 +2437,85 @@ end
 (** Package url field in opam file. Formerly, file
     (<repo>/packages/.../url) *)
 
-module URLSyntax = struct
+module URL_legacySyntax = struct
 
   let internal = "url-file"
   let format_version = OpamVersion.of_string "1.2"
 
+  type t = {
+    url     : url;
+    mirrors : url list;
+    checksum: OpamHash.t list;
+    errors  : (string * Pp.bad_format) list;
+  }
+
+  let create ?(mirrors=[]) ?(checksum=[]) url =
+    {
+      url; mirrors; checksum; errors = [];
+    }
+
+  let empty = {
+    url     = OpamUrl.empty;
+    mirrors = [];
+    checksum= [];
+    errors  = [];
+  }
+
+  let url t = t.url
+  let mirrors t = t.mirrors
+  let checksum t = t.checksum
+
+  let with_url url t = { t with url }
+  let with_mirrors mirrors t = { t with mirrors }
+  let with_checksum checksum t = { t with checksum = checksum }
+
+  let fields =
+    let with_url url t =
+      if t.url <> OpamUrl.empty then Pp.bad_format "Too many URLS"
+      else with_url url t
+    in
+    [
+      "src", Pp.ppacc with_url url
+        Pp.V.url;
+      "archive", Pp.ppacc_opt with_url OpamStd.Option.none
+        (Pp.V.url_with_backend `http);
+      "http", Pp.ppacc_opt with_url OpamStd.Option.none
+        (Pp.V.url_with_backend `http);
+      "git",  Pp.ppacc_opt with_url OpamStd.Option.none
+        (Pp.V.url_with_backend `git);
+      "darcs",  Pp.ppacc_opt with_url OpamStd.Option.none
+        (Pp.V.url_with_backend `darcs);
+      "hg",  Pp.ppacc_opt with_url OpamStd.Option.none
+        (Pp.V.url_with_backend `hg);
+      "local",  Pp.ppacc_opt with_url OpamStd.Option.none
+        (Pp.V.url_with_backend `rsync);
+      "checksum", Pp.ppacc with_checksum checksum
+        (Pp.V.map_list ~depth:1
+           (Pp.V.string -| Pp.of_module "checksum" (module OpamHash)));
+      "mirrors", Pp.ppacc with_mirrors mirrors
+        (Pp.V.map_list ~depth:1 Pp.V.url);
+    ]
+
+  let pp_contents =
+    let name = internal in
+    Pp.I.fields ~name ~empty fields -|
+    Pp.I.on_errors ~name (fun t e -> {t with errors = e::t.errors}) -|
+    Pp.pp ~name
+      (fun ~pos t ->
+         if t.url = OpamUrl.empty then OpamPp.bad_format ~pos "missing URL"
+         else t)
+      (fun x -> x)
+
+  let pp = Pp.I.map_file pp_contents
+
+end
+module URL_legacy = struct
+  include URL_legacySyntax
+  include SyntaxFile(URL_legacySyntax)
+end
+
+module URL = struct
+
   type t = {
     url     : url;
     mirrors : url list;
@@ -2485,7 +2584,7 @@ module URLSyntax = struct
     ]
 
   let pp_contents =
-    let name = internal in
+    let name = "url-file" in
     Pp.I.fields ~name ~empty fields -|
     Pp.I.on_errors ~name (fun t e -> {t with errors = e::t.errors}) -|
     Pp.pp ~name
@@ -2513,12 +2612,8 @@ module URLSyntax = struct
              swhid = None;
              mirrors = OpamSWHID.to_url swhid :: t.mirrors })
 
-  let pp = Pp.I.map_file pp_contents
-
-end
-module URL = struct
-  include URLSyntax
-  include SyntaxFile(URLSyntax)
+  let of_legacy {URL_legacy.url; mirrors; checksum; errors} =
+    {empty with mirrors; checksum; errors; url}
 end
 
 
@@ -3102,7 +3197,7 @@ module OPAMSyntax = struct
         Pp.V.os_constraint;
       "descr", no_cleanup Pp.ppacc_opt with_descr OpamStd.Option.none
         (Pp.V.string_tr -|
-         Pp.of_pair "descr" Descr.(of_string (), to_string ()));
+         Pp.of_pair "descr" Descr.(of_string, to_string));
       "extra-sources", no_cleanup Pp.ppacc_opt
         with_extra_sources OpamStd.Option.none
         (Pp.V.map_list ~depth:2 @@
@@ -4199,7 +4294,7 @@ module CompSyntax = struct
       env = comp.env;
       flags = [Pkgflag_Compiler];
       url;
-      descr = descr_opt;
+      descr = Option.map Descr.of_legacy descr_opt;
     }
 
 end

src/format/opamFile.mli

@@ -297,7 +297,7 @@ module InitConfig: sig
 end
 
 (** Package descriptions: [$opam/descr/] *)
-module Descr: sig
+module Descr_legacy: sig
 
   include IO_FILE
 
@@ -317,11 +317,53 @@ module Descr: sig
 
 end
 
+module Descr: sig
+
+  type t
+
+  val empty: t
+  val create: string -> t
+
+  (** Return the first line *)
+  val synopsis: t -> string
+
+  (** Return the body *)
+  val body: t -> string
+
+  val of_legacy: Descr_legacy.t -> t
+
+end
+
 (** {2 Urls for OPAM repositories} *)
-module URL: sig
+module URL_legacy: sig
 
   include IO_FILE
 
+  val create:
+    ?mirrors:url list -> ?checksum:OpamHash.t list ->
+    url -> t
+
+  (** URL address *)
+  val url: t -> url
+
+  val mirrors: t -> url list
+
+  (** Archive checksum *)
+  val checksum: t -> OpamHash.t list
+
+  (** Constructor *)
+  val with_url: url -> t -> t
+  val with_checksum: OpamHash.t list -> t -> t
+  val with_mirrors: OpamUrl.t list -> t -> t
+
+end
+
+module URL: sig
+
+  type t
+
+  val empty: t
+
   val create:
     ?mirrors:url list -> ?checksum:OpamHash.t list ->
     ?swhid:OpamSWHID.t -> ?subpath:subpath ->
@@ -347,6 +389,7 @@ module URL: sig
 
   val subpath: t -> subpath option
 
+  val of_legacy: URL_legacy.t -> t
 end
 
 (** OPAM files *)
@@ -872,7 +915,7 @@ module Comp: sig
       created with "VARIANT" the part of the compiler name on the right of the
       "+". In both case, the version corresponds to the OCaml version and is
       [version comp]. *)
-  val to_package: ?package:package -> t -> Descr.t option -> OPAM.t
+  val to_package: ?package:package -> t -> Descr_legacy.t option -> OPAM.t
 
 end
 

src/state/opamFileTools.ml

@@ -1321,20 +1321,21 @@ let add_aux_files ?dir ?(files_subdir_hashes=false) opam =
   match dir with
   | None -> opam
   | Some dir ->
-    let (url_file: OpamFile.URL.t OpamFile.t) =
+    let (url_file: OpamFile.URL_legacy.t OpamFile.t) =
       OpamFile.make (dir // "url")
     in
-    let (descr_file: OpamFile.Descr.t OpamFile.t)  =
+    let (descr_file: OpamFile.Descr_legacy.t OpamFile.t)  =
       OpamFile.make (dir // "descr")
     in
     let files_dir =
       OpamFilename.Op.(dir / "files")
     in
     let opam =
-      match OpamFile.OPAM.url opam, try_read OpamFile.URL.read_opt url_file with
-      | None, (Some url, None) -> OpamFile.OPAM.with_url url opam
+      match OpamFile.OPAM.url opam, try_read OpamFile.URL_legacy.read_opt url_file with
+      | None, (Some url, None) ->
+        OpamFile.OPAM.with_url (OpamFile.URL.of_legacy url) opam
       | Some opam_url, (Some url, errs) ->
-        if url = opam_url && errs = None then
+        if (OpamFile.URL.of_legacy url) = opam_url && errs = None then
           log "Duplicate definition of url in '%s' and opam file"
             (OpamFile.to_string url_file)
         else
@@ -1349,8 +1350,9 @@ let add_aux_files ?dir ?(files_subdir_hashes=false) opam =
     in
     let opam =
       match OpamFile.OPAM.descr opam,
-            try_read OpamFile.Descr.read_opt descr_file with
-      | None, (Some descr, None) -> OpamFile.OPAM.with_descr descr opam
+            try_read OpamFile.Descr_legacy.read_opt descr_file with
+      | None, (Some descr, None) ->
+        OpamFile.OPAM.with_descr (OpamFile.Descr.of_legacy descr) opam
       | Some _, (Some _, _) ->
         log "Duplicate descr in '%s' and opam file"
           (OpamFile.to_string descr_file);

src/state/opamFormatUpgrade.ml

@@ -561,9 +561,9 @@ let from_1_3_dev2_to_1_3_dev5 ~on_the_fly:_ root conf =
           in
           let descr =
             OpamStd.Option.default
-              (OpamFile.Descr.create
+              (OpamFile.Descr_legacy.create
                  "Switch relying on a system-wide installation of OCaml")
-              (OpamFile.Descr.read_opt descr_f)
+              (OpamFile.Descr_legacy.read_opt descr_f)
           in
           let comp_opam =
             OpamFile.Comp.to_package comp (Some descr)

src/state/opamFormatUpgrade.mli

@@ -79,7 +79,7 @@ val opam_file:
 (** Convert the comp file to an opam one, using {!OpamFile.Comp.to_package} and
     applying filter rewriting *)
 val comp_file:
-  ?package:package -> ?descr:OpamFile.Descr.t -> OpamFile.Comp.t ->
+  ?package:package -> ?descr:OpamFile.Descr_legacy.t -> OpamFile.Comp.t ->
   OpamFile.OPAM.t
 
 (** Runs the opam file format from the file's format to current, and adds data