Skip to content

Commit

Permalink
API fix: do not automatically infer name & version from read+lint fun…
Browse files Browse the repository at this point in the history
…ction

it's only suitable for opam files read from repositories, and, as we could have
expected, proves harmful in cases like ocaml-opam/opam-publish#107
  • Loading branch information
AltGr committed Oct 21, 2020
1 parent ffb4692 commit cf08e6a
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/client/opamAdminCommand.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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,_,_) ->
Expand Down
5 changes: 3 additions & 2 deletions src/client/opamCommands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
18 changes: 11 additions & 7 deletions src/state/opamFileTools.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 ->
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 11 additions & 7 deletions src/state/opamFileTools.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down

0 comments on commit cf08e6a

Please sign in to comment.