From cf08e6a5d756310314949bfd9c422a0c15310fa2 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Tue, 20 Oct 2020 11:40:22 +0200 Subject: [PATCH] API fix: do not automatically infer name & version from read+lint function it's only suitable for opam files read from repositories, and, as we could have expected, proves harmful in cases like https://github.com/ocaml-opam/opam-publish/issues/107 --- src/client/opamAdminCommand.ml | 2 +- src/client/opamCommands.ml | 5 +++-- src/state/opamFileTools.ml | 18 +++++++++++------- src/state/opamFileTools.mli | 18 +++++++++++------- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/client/opamAdminCommand.ml b/src/client/opamAdminCommand.ml index b760455dd2b..f4e5caa3155 100644 --- a/src/client/opamAdminCommand.ml +++ b/src/client/opamAdminCommand.ml @@ -602,7 +602,7 @@ let lint_command cli = let ret = OpamPackage.Map.fold (fun nv prefix ret -> let opam_file = OpamRepositoryPath.opam repo_root prefix nv in - let w, _ = OpamFileTools.lint_file opam_file in + let w, _ = OpamFileTools.lint_file ~handle_dirname:true opam_file in if List.exists (fun (n,_,_) -> List.mem n ign) w then ret else let w = List.filter (fun (n,_,_) -> diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index e5d62ad3c5a..1a46a268667 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -3391,9 +3391,10 @@ let lint cli = try let warnings,opam = match opam_f with - | Some f -> OpamFileTools.lint_file ~check_upstream f + | Some f -> + OpamFileTools.lint_file ~check_upstream ~handle_dirname:true f | None -> - OpamFileTools.lint_channel ~check_upstream + OpamFileTools.lint_channel ~check_upstream ~handle_dirname:false (OpamFile.make (OpamFilename.of_string "-")) stdin in let enabled = diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml index f028e9bb9ba..f7e478caf29 100644 --- a/src/state/opamFileTools.ml +++ b/src/state/opamFileTools.ml @@ -777,7 +777,8 @@ let extra_files_default filename = OpamHash.check_file (OpamFilename.to_string f)) (OpamFilename.rec_files dir) -let lint_gen ?check_extra_files ?check_upstream reader filename = +let lint_gen ?check_extra_files ?check_upstream ?(handle_dirname=false) + reader filename = let warnings, t = let warn_of_bad_format (pos, msg) = 2, `Error, Printf.sprintf "File format error%s: %s" @@ -795,6 +796,7 @@ let lint_gen ?check_extra_files ?check_upstream reader filename = (OpamFormat.I.map_file OpamFile.OPAM.pp_raw_fields) f in let t, warnings = + if handle_dirname = false then t, [] else match OpamPackage.of_filename (OpamFile.filename filename) with | None -> t, [] | Some nv -> @@ -851,7 +853,7 @@ let lint_gen ?check_extra_files ?check_upstream reader filename = warnings @ (match t with Some t -> lint ~check_extra_files ?check_upstream t | None -> []), t -let lint_file ?check_extra_files ?check_upstream filename = +let lint_file ?check_extra_files ?check_upstream ?handle_dirname filename = let reader filename = try let ic = OpamFilename.open_in (OpamFile.filename filename) in @@ -863,15 +865,17 @@ let lint_file ?check_extra_files ?check_upstream filename = OpamConsole.error_and_exit `Bad_arguments "File %s not found" (OpamFile.to_string filename) in - lint_gen ?check_extra_files ?check_upstream reader filename + lint_gen ?check_extra_files ?check_upstream ?handle_dirname reader filename -let lint_channel ?check_extra_files ?check_upstream filename ic = +let lint_channel ?check_extra_files ?check_upstream ?handle_dirname + filename ic = let reader filename = OpamFile.Syntax.of_channel filename ic in - lint_gen ?check_extra_files ?check_upstream reader filename + lint_gen ?check_extra_files ?check_upstream ?handle_dirname reader filename -let lint_string ?check_extra_files ?check_upstream filename string = +let lint_string ?check_extra_files ?check_upstream ?handle_dirname + filename string = let reader filename = OpamFile.Syntax.of_string filename string in - lint_gen ?check_extra_files ?check_upstream reader filename + lint_gen ?check_extra_files ?check_upstream ?handle_dirname reader filename let all_lint_warnings () = t_lint ~all:true OpamFile.OPAM.empty diff --git a/src/state/opamFileTools.mli b/src/state/opamFileTools.mli index 7d4fc9cecd2..06280367292 100644 --- a/src/state/opamFileTools.mli +++ b/src/state/opamFileTools.mli @@ -19,20 +19,22 @@ val template: package -> OpamFile.OPAM.t (** Runs several sanity checks on the opam file; returns a list of warnings. [`Error] level should be considered unfit for publication, while [`Warning] are advisory but may be accepted. The int is an identifier for this specific - warning/error. If [check_extra_files] is unspecified, warning 53 won't be - checked. *) + warning/error. If [check_extra_files] is unspecified or false, warning 53 + won't be checked. *) val lint: ?check_extra_files:(basename * (OpamHash.t -> bool)) list -> - ?check_upstream: bool -> + ?check_upstream:bool -> OpamFile.OPAM.t -> (int * [`Warning|`Error] * string) list (** Same as [lint], but operates on a file, which allows catching parse errors - too. You can specify an expected name and version. [check_extra_files] - defaults to a function that will look for a [files/] directory besides - [filename] *) + too. [check_extra_files] defaults to a function that will look for a [files/] + directory besides [filename]. [handle_dirname] is used for warning 4, and + should be set when reading packages from a repository, so that package name + and version are inferred from the filename. *) val lint_file: ?check_extra_files:(basename * (OpamHash.t -> bool)) list -> - ?check_upstream: bool -> + ?check_upstream:bool -> + ?handle_dirname:bool -> OpamFile.OPAM.t OpamFile.typed_file -> (int * [`Warning|`Error] * string) list * OpamFile.OPAM.t option @@ -42,6 +44,7 @@ val lint_file: val lint_channel: ?check_extra_files:(basename * (OpamHash.t -> bool)) list -> ?check_upstream: bool -> + ?handle_dirname:bool -> OpamFile.OPAM.t OpamFile.typed_file -> in_channel -> (int * [`Warning|`Error] * string) list * OpamFile.OPAM.t option @@ -51,6 +54,7 @@ val lint_channel: val lint_string: ?check_extra_files:(basename * (OpamHash.t -> bool)) list -> ?check_upstream: bool -> + ?handle_dirname:bool -> OpamFile.OPAM.t OpamFile.typed_file -> string -> (int * [`Warning|`Error] * string) list * OpamFile.OPAM.t option