In praise of ocaml

Community
43

Same, I don’t use dune’s default, I just change the default set of flags
to remove most warn-errors: moonpool/dune at main · c-cube/moonpool · GitHub

1 Like
44

While it makes me feel better to see that I’m not the only one who doesn’t like some of the more pedantic warnings as errors during the early stages of development, it also makes me even more worried that this is a serious problem:

  • Experienced ocaml devs (overwhelmingly?) disable warnings-as-errors
  • Meanwhile, newcomers have a bad experience out of the box.
2 Likes
45

It’s worth keeping in mind that the people happy with (or at least unbothered by) the current configuration, don’t have much reason to complain, so we are not making noise on this topic.

I’m not saying the default shouldn’t be changed, and I have no idea if experienced ocaml devs “overwhelmingly” disable warnings-as-erros, but I can report that I don’t disable this in my projects, and AFAICT, plenty of other projects also don’t seem to have this disabled, including this random selection (based on a quick search of the sources of some projects on github):

Again, for those invested in trying to get this changed, the issue that is best tracking this request in dune is the (too narrowly titled) warning 9 enabled by default in dev mode · Issue #8645 · ocaml/dune · GitHub .

3 Likes
46

There might be a whole bunch of people who disable warnings during development (either by using the release profile or in one of the many other ways mentioned in this thread) but don’t check in whatever logic they use to disable the checks. I fall into that category.

1 Like
47

I think that there are two important things to distinguish here:

  1. I believe that the reason why Dune made the choice to turn warnings into errors is primarily technical: it is not easy to collect warnings and replay them on rebuild (this would require extra implementation work, and it is not obvious which behavior would be expected and natural), whereas errors have no such issue.

  2. Then there is the question of which warnings are enabled or not at all. On this aspect Dune arguably made opinionated choices that differ from the upstream compiler. But I think that in fact this is also related to point (1): in general having warnings on unused variables is a reasonable thing (it’s good that warnings help us eliminate dead calls), but then suppose you are the maintainer of a build system that only support warning-as-errors, do you silence the warning completely (bad choice) or turn it into an error (bad choice)? So the lack of support for warnings-that-are-just-warnings has painted Dune into a corner and forced its caretakers to make value judgments on the compiler warnings.

My impression is that the correct long-term approach would be to bite the bullet and support warnings-that-are-not-errors, but one has to be mindful of the fact that Dune has limited development resources right now.

4 Likes
48

I think this again entirely misses the point.

The main point is not about warnings as errors, the point is about enabling warnings that should not be enabled when you are developing and which are, in fact, not enabled upstream.

Even though I personally don’t practice this, I think that warnings as errors during development is a perfectly valid strategy if you are thoughtful about which warnings are enabled at given point of the development cycle (and if you don’t start to ask people not to deprecate stuff because that breaks their dev build which is the kind of absurd discussion we started to have upstream…)

And for that the unused-var[-strict] warning is actually kind of interesting and, in my eyes, walks a thin line: it can find you real bugs more often than you’d expect. But for other unused-* warnings while some can theoretically also do so, I expect that to be statistically much less prevalent and importantly they can be treated without refactoring. So I think you should not bother developers with these during development time but rather defer these to lint builds which can happen at various other points of your development cycle, for exemple CI, the build that tests your release tarball, etc.

I’m kind of baffled that no nuanced discussion can be had about this and I absolutely don’t think that supporting “warnings-that-are-not-errors” is a pre-requisite to get a more usable dune. This can perfectly be solved by letting dune users having more thoughtfully designed build profiles than the ones that currently exist.

P.S. Regarding 1) it honestly makes me chuckle, it may be a hard problem for `dune` but it's a trivial problem if you have a good build model. I (and `odig` users) live with a system that does that in the utmost trivial way since 2017 that likely received a ridiculous amount of engineering time compared to what went into `dune`. Maybe if people were not busy reinventing yet another package system that could be a solved problem by now?
1 Like
49

I don’t know about others but personally I don’t see how a more nuanced discussion is feasible here because how would a build system talk about, let alone enforce, different workflows at different stages of the development lifecycle? People today complain about not remembering dune init project my-project and what the lib and bin directories are for. How would we convince them to have separate build settings for development, review, CI, deployment? How would we handle the diversity of different needs in every project, or the absence of some of these stages in some projects? What about projects that don’t have a CI or don’t publish to opam?

I don’t see how this is viable and looking at other language ecosystems, I’ve never seen this done (except through generic ‘profiles’, which dune already has). Do you know of a build tool for any language that detects when it is being run in CI and enables strict linting? I’ve never heard of it…

50

Well, this was a thing I had in my OMake library before I decided that omake had a bleak future in the OCaml community, and I stopped fighting the tide. My library was broken by three separate toolchain releases with changes made at the behest of the dune team, and the last time it happened to me, I gave up.

It’s one of the features I still miss. And it’s one of the features I was hoping to get back by adopting Bazel with rules that are community maintained. That doesn’t seem to be happening either.

51

I do think it is feasible. Who says we need to solve all this within the confines of dune?

  • We can add support for dune lint [proposal]

  • We can integrate dune lint into our community-owned CI actions, running it at the end of a workflow by default

  • We can update our tutorials and documentation to promote dune lint

1 Like
52

We would have to flip the meaning of dune’s dev profile (remember, the dev profile is the default if a profile is not specified) to essentially be what the release profile is today. And if we did that, we might as well flip the meaning of the release profile to what dev is today. And then make the dune lint command be just an alias for dune build --profile=release. Then the question might arise, if we did that, do we actually need a new toplevel dune subcommand lint? Why wouldn’t dune build --profile=release be good enough? It’s worth considering.

So the basic question here is, can, or should we flip the meanings of the two built-in profiles, including the default one dev? People complain about dune docs today, I am trying to imagine how confusing they would find it to read that after a certain version of dune the meanings of these two profiles just flipped completely. Somehow that doesn’t seem like it would lessen people’s overall confusion with dune.

You might argue that we shouldn’t do specifically this; maybe we should add a lint profile and just leave the release profile as is–exactly the same as the dev profile. Somehow I don’t see this being less confusing either. Any change in dune’s default behaviour is likely to lead to a lot of UX confusion down the line. Is it really worth it for a few unused value warnings?

53

I think that this is what you are getting wrong: the build system is not the workflow.

The build system is part of your workflows. There’s nothing wrong in providing a diversity of settings suited for different parts of them. Thinking that dune can provide a one size fits-all experience is the biggest error it is making. How you operate on projects is contextual and differs from one organisation to another. Other example: testing on the opam-repository CI is currently broken because it seems there is a lack of nuance about which test you can specify on an dune run test, see here.

As for the “I have never seen any other language eco-system do that” line, then the news at 11:00 is that OCaml tooling is allowed to be creative and better than what other languages do rather to sheeplessly copy what others are doing and is being sold to be the best in town.

I’m not exactly sure what ends up being confusing here. Here an extremely simple plan based on profiles that almost disrupts nothing to the way dune operates now:

  1. dev: default set of warnings and warn-error.
  2. release default set of warnings without warn-errors.
  3. lint warn-error with more stringent set of warnings enabled.

The worst that can happen is that people will ship (or keep) dead code. But on the other hand I will no longer have to apologize to people that I entice to try to OCaml when they come back to me telling me that the way this dune system operates is crazy, confusing and annoying.

54

Right after the comment you linked to in the issue, Rudi said:

There’s no more discussion about changing the default set of warnings, I think we’ve already reached agreement that dune should stop changing the default warning set.

So yeah, I don’t see how your (1) is feasible. Which is why the only option left is to disable warn-error in the dev profile, aka make it the same as the release profile. Unless you want to convince the dune maintainers that they actually do need to change the default warning set.

55

See Stop changing the default warning set of the compiler by nojb · Pull Request #12766 · ocaml/dune · GitHub for an attempt in this direction.

Cheers,
Nicolas

3 Likes