ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.61k stars 400 forks source link

Dune should warn about dependencies on unused libraries #650

Open dra27 opened 6 years ago

dra27 commented 6 years ago

It's still quite commonplace to end up with (libraries (... bytes ...)) even though using Dune implies OCaml >= 4.02.3.

It would be better if Dune dropped the dependency and issued a warning (especially now that we don't depend on findlib, requiring a dependency on base-bytes in opam-repsository).

EDIT: It would be better if we had such a warning for all libraries in fact. This should be possible via ocamlobjinfo.

dra27 commented 6 years ago

@diml - do we, or are we, ever intending to support OCaml < 4.02.3 in build contexts?

ghost commented 6 years ago

Well, currently supporting an OCaml version in build contexts means supporting building Dune itself with this version, which is a bit painful. There might also be some assumptions that don't hold with older OCaml versions, such as module aliases. So far nobody asked for such support, so I don't think it's worth it.

rgrinberg commented 6 years ago

I'm also -1 for supporting older OCaml's. It's a feature that gets more and more irrelevant by the day. I'd rather we spend the time supporting new OCaml features ;)

pbiggar commented 6 years ago

I had a recent issue where we depended on a lot of libraries we didn't need, and it led to output-size and compilation-time problems. It would certainly be wonderful for dune to have a warning for unused dependencies!

rgrinberg commented 6 years ago

I'm surprised that it made a difference to output sizes, as ocaml should have not linked in any unused libraries (unless you passed in -linkall).

As for compilation speed, we're working on it #430

pbiggar commented 6 years ago

I definitely observed the change in the output binaries in native mode. However, the major change was js_of_ocaml output size. Here's the discussion where I discovered it, including the jbuild files: https://discuss.ocaml.org/t/reducing-the-size-of-js-of-ocaml-output/2538. (Let me know if I should move this to a different issue; didnt intend to hijack).

rgrinberg commented 6 years ago

Which unused libraries did you remove that decreased the output size? From what I can tell you simply switches to smaller libs and got rid of code generators that whose generated code you weren't actually using.

pbiggar commented 6 years ago

While I did remove some derivers whose output I wasn't using, the big change I got was when I was told that ppx_deriving_yojson should not go in the library section. Apparently, it was compiling it, and also all it's dependencies. I continued to use ppx_deriving_yojson generated code, but I stopped compiling it into the binary. For my jsoo output, it reduced it by about 1.7MB, with no change in functionality.

So the warning I would hope for is to say "you are naming ppx_deriving_yojson in your library stanza, but you are not using it`.

rgrinberg commented 6 years ago

Ah, that's because ppx_deriving_yojson probably forces -linkall. Anyway, dune doesn't have a way of detecting such cases unfortunately. We might gain this ability once we improve the description of libraries. So that we'd be able to tell that we're using a ppx_rewriter in libraries position.

ghost commented 6 years ago

I suppose we could use ocamlobjinfo to determine what files the compiler actually read. That would tell us what dependencies where not used.