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

Remove the fmt job when .ocamlformat analysis fails #877

Merged
merged 1 commit into from
Sep 21, 2023
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
13 changes: 3 additions & 10 deletions lib/analyse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -292,16 +292,9 @@ module Analysis = struct
find_opam_repo_commit_for_ocamlformat ~solve
~platforms:(filter_linux_x86_64_platforms platforms)
in
let analysed_ocamlformat =
Analyse_ocamlformat.get_ocamlformat_source job ~opam_files ~root
~find_opam_repo_commit
>>= function
| Error (`Msg msg) ->
Current.Job.log job "%s@." msg;
Lwt_result.return (None, None)
| Ok x -> Lwt_result.return x
in
analysed_ocamlformat >>!= fun (ocamlformat_source, ocamlformat_selection) ->
Analyse_ocamlformat.get_ocamlformat_source job ~opam_files ~root
~find_opam_repo_commit
>>!= fun (ocamlformat_source, ocamlformat_selection) ->
if opam_files = [] then Lwt_result.fail (`Msg "No opam files found!")
else if List.filter Fpath.is_seg opam_files = [] then
Lwt_result.fail (`Msg "No top-level opam files found!")
Expand Down
25 changes: 13 additions & 12 deletions lib/analyse_ocamlformat.ml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
let ( >>!= ) = Lwt_result.Infix.( >>= )

type source =
| Opam of { version : string; opam_repo_commit : string }
| Opam of { version : string; opam_repo_commit : string option }
| Vendored of { path : string }
[@@deriving yojson, eq, ord]

Expand Down Expand Up @@ -74,6 +74,7 @@ let ocamlformat_version_from_file ~root job path =
Lwt.return (Error (`Msg "Multiple 'version=' lines in .ocamlformat"))

let get_ocamlformat_source job ~opam_files ~root ~find_opam_repo_commit =
let open Lwt.Infix in
let ( let* ) = Lwt_result.Infix.( >>= ) in
let proj_is_ocamlformat p =
String.equal (Filename.basename p) "ocamlformat.opam"
Expand All @@ -88,14 +89,14 @@ let get_ocamlformat_source job ~opam_files ~root ~find_opam_repo_commit =
Fpath.(to_string (root / ".ocamlformat"))
in
match version_in_dot_ocamlformat with
| None ->
Lwt_result.return (None, None)
(* This case happens when the ocamlformat file doesn't specify a version.
It will try to get the latest version for the last platform. *)
| Some version ->
let* opam_repo_commit, ocamlformat_selection =
find_opam_repo_commit version
in
Lwt_result.return
( Some (Opam { version; opam_repo_commit }),
Some ocamlformat_selection ))
| None -> Lwt_result.return (None, None)
| Some version -> (
find_opam_repo_commit version >>= function
| Error (`Msg msg) ->
Current.Job.log job "%s@." msg;
Lwt_result.return
(Some (Opam { version; opam_repo_commit = None }), None)
| Ok (x, ocamlformat_selection) ->
Lwt_result.return
( Some (Opam { version; opam_repo_commit = Some x }),
Some ocamlformat_selection )))
2 changes: 1 addition & 1 deletion lib/analyse_ocamlformat.mli
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(** Detect the required version of OCamlFormat used in a source repository. *)

type source =
| Opam of { version : string; opam_repo_commit : string }
| Opam of { version : string; opam_repo_commit : string option }
(** Should install OCamlFormat from opam. *)
| Vendored of { path : string }
(** OCamlFormat is vendored. [path] is relative to the project's root. *)
Expand Down
2 changes: 1 addition & 1 deletion lib/lint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ let commit_from_ocamlformat_source ocamlformat_source =
let open Analyse_ocamlformat in
match ocamlformat_source with
| None | Some (Vendored _) -> None
| Some (Opam { opam_repo_commit; _ }) -> Some opam_repo_commit
| Some (Opam { opam_repo_commit; _ }) -> opam_repo_commit

let fmt_spec ~base ~ocamlformat_source ~selection =
let open Obuilder_spec in
Expand Down
24 changes: 17 additions & 7 deletions lib/spec.ml
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,21 @@ let lint_specs ~analysis selections =
let lint_fmt =
(* Allow fallback if an ocamlformat selection doesn't exist,
as [dune build @fmt] can also format dune files *)
match Analyse.Analysis.ocamlformat_selection analysis with
| None -> lint_doc
| Some spec -> spec
(* match Analyse.Analysis.ocamlformat_selection analysis with *)
let selection =
let ocamlformat_selection =
Analyse.Analysis.ocamlformat_selection analysis
in
let ocamlformat_source = Analyse.Analysis.ocamlformat_source analysis in
match (ocamlformat_selection, ocamlformat_source) with
| _, None -> Some lint_doc
| Some _, Some _ -> ocamlformat_selection
| None, Some _ -> None
in
match selection with
| None -> []
| Some selection ->
[ opam ~label:Variant.fmt_label ~selection ~analysis (`Lint `Fmt) ]
in
let lint_opam =
(* Take the first selection with an OCaml version >= 4.14.0,
Expand All @@ -62,10 +74,8 @@ let lint_specs ~analysis selections =
selection
|> Option.value ~default:[]
in
[
opam ~label:Variant.fmt_label ~selection:lint_fmt ~analysis (`Lint `Fmt);
opam ~label:Variant.doc_label ~selection:lint_doc ~analysis (`Lint `Doc);
]
lint_fmt
@ [ opam ~label:Variant.doc_label ~selection:lint_doc ~analysis (`Lint `Doc) ]
@ optional_spec ~label:Variant.opam_label ~selection:lint_opam ~lint_ty:`Opam

let opam_monorepo builds =
Expand Down