Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint: add check_upstream option #3758

Merged
merged 1 commit into from
Feb 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/client/opamCommands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2795,7 +2795,12 @@ let lint =
an opam file directly."
Arg.(some package) None
in
let lint global_options files package normalise short warnings_sel =
let check_upstream =
mk_flag ["check-upstream"]
"Check upstream, archive availability and checksum(s)"
in
let lint global_options files package normalise short warnings_sel
check_upstream =
apply_global_options global_options;
let opam_files_in_dir d =
match OpamPinned.files_in_source d with
Expand Down Expand Up @@ -2845,9 +2850,9 @@ let lint =
try
let warnings,opam =
match opam_f with
| Some f -> OpamFileTools.lint_file f
| Some f -> OpamFileTools.lint_file ~check_upstream f
| None ->
OpamFileTools.lint_channel
OpamFileTools.lint_channel ~check_upstream
(OpamFile.make (OpamFilename.of_string "-")) stdin
in
let enabled =
Expand Down Expand Up @@ -2898,7 +2903,8 @@ let lint =
in
if err then OpamStd.Sys.exit_because `False
in
Term.(const lint $global_options $files $package $normalise $short $warnings),
Term.(const lint $global_options $files $package $normalise $short $warnings
$check_upstream),
term_info "lint" ~doc ~man

(* CLEAN *)
Expand Down
91 changes: 65 additions & 26 deletions src/state/opamFileTools.ml
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ let template nv =
|> with_bug_reports [""]
|> with_synopsis ""

let lint ?check_extra_files t =
let lint ?check_extra_files ?(check_upstream=false) t =
let format_errors =
List.map (fun (field, (pos, msg)) ->
3, `Error,
Expand Down Expand Up @@ -617,22 +617,62 @@ let lint ?check_extra_files t =
cond 57 `Error
"Synopsis and description must not be both empty"
(t.descr = None || t.descr = Some OpamFile.Descr.empty);
let vars = all_variables ~exclude_post:false t in
let exists svar =
List.exists (fun v -> v = OpamVariable.Full.of_string svar) vars
in
let rem_test = exists "test" in
let rem_doc = exists "doc" in
cond 58 `Warning
(let var, s_, nvar =
match rem_test, rem_doc with
| true, true -> "`test` and `doc`", "s", "s are `with-test` and `with-doc`"
| true, false -> "`test`", "", " is `with-test`"
| false, true -> "`doc`", "", " is `with-doc`"
| _ -> "","",""
in
Printf.sprintf "Found %s variable%s, predefined one%s" var s_ nvar)
(rem_test || rem_doc);
(let vars = all_variables ~exclude_post:false t in
let exists svar =
List.exists (fun v -> v = OpamVariable.Full.of_string svar) vars
in
let rem_test = exists "test" in
let rem_doc = exists "doc" in
cond 58 `Warning
(let var, s_, nvar =
match rem_test, rem_doc with
| true, true -> "`test` and `doc`", "s", "s are `with-test` and `with-doc`"
| true, false -> "`test`", "", " is `with-test`"
| false, true -> "`doc`", "", " is `with-doc`"
| _ -> "","",""
in
Printf.sprintf "Found %s variable%s, predefined one%s" var s_ nvar)
(rem_test || rem_doc));
cond 59 `Warning "url don't contain a checksum"
(check_upstream &&
OpamStd.Option.map OpamFile.URL.checksum t.url = Some []);
(let upstream_error =
if not check_upstream then None
else
match t.url with
| None -> Some "No url defined"
| Some url ->
let open OpamProcess.Job.Op in
OpamProcess.Job.run @@
OpamFilename.with_tmp_dir_job @@ fun dir ->
OpamProcess.Job.catch (function
Failure msg -> Done (Some msg)
| OpamDownload.Download_fail (s,l) ->
Done (Some (OpamStd.Option.default l s))
| e -> Done (Some (Printexc.to_string e)))
@@ fun () ->
OpamDownload.download ~overwrite:false (OpamFile.URL.url url) dir
@@| fun f ->
(match OpamFile.URL.checksum url with
| [] -> None
| chks ->
let not_corresponding =
List.filter (fun chk ->
not (OpamHash.check_file (OpamFilename.to_string f) chk))
chks
in
if not_corresponding = [] then None
else
let msg =
Printf.sprintf "Cheksum%s %s don't verify archive"
(if List.length chks = 1 then "" else "s")
(OpamStd.List.to_string OpamHash.to_string not_corresponding)
in
Some msg)
in
cond 60 `Error "Upstream check failed"
~detail:[OpamStd.Option.default "" upstream_error]
(upstream_error <> None));
]
in
format_errors @
Expand All @@ -649,7 +689,7 @@ let extra_files_default filename =
OpamHash.check_file (OpamFilename.to_string f))
(OpamFilename.rec_files dir)

let lint_gen ?check_extra_files reader filename =
let lint_gen ?check_extra_files ?check_upstream reader filename =
let warnings, t =
let warn_of_bad_format (pos, msg) =
2, `Error, Printf.sprintf "File format error%s: %s"
Expand Down Expand Up @@ -717,11 +757,10 @@ let lint_gen ?check_extra_files reader filename =
| None -> extra_files_default filename
| Some f -> f
in
warnings @ (match t with Some t -> lint ~check_extra_files t | None -> []),
warnings @ (match t with Some t -> lint ~check_extra_files ?check_upstream t | None -> []),
t


let lint_file ?check_extra_files filename =
let lint_file ?check_extra_files ?check_upstream filename =
let reader filename =
try
let ic = OpamFilename.open_in (OpamFile.filename filename) in
Expand All @@ -733,15 +772,15 @@ let lint_file ?check_extra_files filename =
OpamConsole.error_and_exit `Bad_arguments "File %s not found"
(OpamFile.to_string filename)
in
lint_gen ?check_extra_files reader filename
lint_gen ?check_extra_files ?check_upstream reader filename

let lint_channel ?check_extra_files filename ic =
let lint_channel ?check_extra_files ?check_upstream filename ic =
let reader filename = OpamFile.Syntax.of_channel filename ic in
lint_gen ?check_extra_files reader filename
lint_gen ?check_extra_files ?check_upstream reader filename

let lint_string ?check_extra_files filename string =
let lint_string ?check_extra_files ?check_upstream filename string =
let reader filename = OpamFile.Syntax.of_string filename string in
lint_gen ?check_extra_files reader filename
lint_gen ?check_extra_files ?check_upstream reader filename

let warns_to_string ws =
OpamStd.List.concat_map "\n"
Expand Down
4 changes: 4 additions & 0 deletions src/state/opamFileTools.mli
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ val template: package -> OpamFile.OPAM.t
checked. *)
val lint:
?check_extra_files:(basename * (OpamHash.t -> bool)) list ->
?check_upstream: bool ->
OpamFile.OPAM.t -> (int * [`Warning|`Error] * string) list

(** Same as [lint], but operates on a file, which allows catching parse errors
Expand All @@ -31,6 +32,7 @@ val lint:
[filename] *)
val lint_file:
?check_extra_files:(basename * (OpamHash.t -> bool)) list ->
?check_upstream: bool ->
OpamFile.OPAM.t OpamFile.typed_file ->
(int * [`Warning|`Error] * string) list * OpamFile.OPAM.t option

Expand All @@ -39,6 +41,7 @@ val lint_file:
[filename] *)
val lint_channel:
?check_extra_files:(basename * (OpamHash.t -> bool)) list ->
?check_upstream: bool ->
OpamFile.OPAM.t OpamFile.typed_file -> in_channel ->
(int * [`Warning|`Error] * string) list * OpamFile.OPAM.t option

Expand All @@ -47,6 +50,7 @@ val lint_channel:
directory besides [filename] *)
val lint_string:
?check_extra_files:(basename * (OpamHash.t -> bool)) list ->
?check_upstream: bool ->
OpamFile.OPAM.t OpamFile.typed_file -> string ->
(int * [`Warning|`Error] * string) list * OpamFile.OPAM.t option

Expand Down