Should we skip linting when ocamlformat_source is None?

[favonia]

I have a question about this part of the code:

ocaml-ci/lib/lint.ml

Lines 22 to 26 in a94d6da

	@@ (match ocamlformat_source with
	| Some src -> install_ocamlformat src
	| None -> empty)
	@@ copy ~chown:"opam" ~src:["./"] ~dst:"./" ()
	@@ run "opam exec -- dune build @fmt || (echo \"dune build @fmt failed\"; exit 2)"

It seems when ocamlformat_source is None, it is impossible to have a working ocamlformat around. If so, should we skip dune build @fmt completely?

[talex5]

I asked @craigfe about this recently, and he said it was to do with wanting to lint dune files even in projects without ocamlformat. The dune docs say:

By default, formatting will be enabled for all languages and dialects present in the project that dune knows about.

Which seems a bit odd, as new ones might get added without you doing anything. I guess people should use (formatting disabled) if they don't want formatting, or enabled_for and list the formatters they want:

https://dune.readthedocs.io/en/latest/dune-files.html#formatting

[favonia]

Yeah, I added (formatting disabled) after studying why my package (originally) did not pass CI. How about an error message like this? I can imagine that a beginner would be confused (after ocaml-ci goes more public).

dune build @fmt failed.

It is likely that dune could not find the binary executable ocamlformat in the current environment.
Possible solutions: specify the version of OCamlFormat in .ocamlformat so that ocaml-ci will install it,
or disable formatting in dune-project. See https://dune.readthedocs.io/en/latest/dune-files.html#formatting.

Perhaps some analysis of the actual errors (before printing out the above message) would also help.

[avsm]

I think this is a bug in dune -- a newer version of the dune client should not break older workflows without a versioning step involved. We shouldn't be working around this with complex logic in ocaml-ci, as other CIs will eventually hit it then. I've filed ocaml/dune#3642 for downstream discussion.

[novemberkilo]

Visiting this as part of a general cleanup -

@emillon can I please check in on the status of the linked dune issue.

Possibly related: At the moment a missing version in .ocamlformat causes the analysis step to fail when it should probably handle it gracefully and continue to build the project while failing the linting step.

Anything that you'd like to add to help decide what action we could take on this issue please @avsm @talex5 @favonia? Thanks //cc @tmcgilchrist

[tmcgilchrist]

Possibly related: At the moment a missing version in .ocamlformat causes the analysis step to fail when it should probably handle it gracefully and continue to build the project while failing the linting step.

We should handle that gracefully, also reported via #834. Pinged the linked issue on dune which has been inactive since 2020.

[emillon]

I replied over there to clarify the scope of the issue.