search

ocaml / dune

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

The @all alias does not produce .cmt files which are produced by @check. #3182

Open cristianoc opened 4 years ago

cristianoc commented 4 years ago

Expected Behavior

In example https://github.com/cristianoc/dune-cmt, dune build should produce both a .cmt and .cmti file.

Actual Behavior

dune build produces only a .cmti file. dune build @check produces s both a .cmti and a .cmt.

Specifications

ghost commented 4 years ago

Hmm, it's because @all means "all the targets" but only the final ones, i.e. things like main.exe. So for an executable, @all will only build the various modules in native mode rather than both native and byte code. And since we only produce cmt files when building in bytecode, the cmt files are not produced at all.

For instance, if you add (modes byte exe) to your executable stanza, then @all will build the cmt files.

In Dune, we could either:

  1. explicitly add cmt files to @all
  2. build cmt files when building native code as well

1 is a bit ad-hoc, but it's probably the most viable solution.

cristianoc commented 4 years ago

@diml I'm testing out a global dead code analysis that relies on this. Currently I'm telling every user to use the workaround dune build @check @all. However, if there's a solution in future, I'll recommend to upgrade dune instead.

ghost commented 4 years ago

Ok, I don't have a satisfying answer yet, but I'm making a note to myself to bring the subject up to the rest of the team at the next dev meeting. Hopefully we can find a good compromise.

The problem here is that requesting an alias cannot change the behaviour of the build. i.e. aliases are constructed in a pure manner and have no side effects. However, maybe we could introduce a separate command or setting. Or even change the compiler so that we can produce only the cmt file. That would be nice actually.

ghost commented 4 years ago

@cristianoc we discussed the problem last week. You can find the notes of our discussion here.

Basically, there is no perfect solution, but we decided on the following we should fix this PR: when building in bytecode is not requested by the user, for instance because the user wrote (modes native), then the cmt files should be produced by the rules to compile in native mode.

Would you be happy to write a PR for this change? Happy to give some pointers if yet :)

cristianoc commented 4 years ago

Hey @jeremiedimino thank you for the update. This is the current workaround I am suggesting: https://github.com/cristianoc/reanalyze#full-project-test-infer-native-project

Once a better workaround has shipped, I will update the recommendation.

ghost commented 4 years ago

Fair enough. @rgrinberg I'm assigning this to you, let's get rid of the need for such workarounds!

rgrinberg commented 4 years ago

@cristianoc Is there a reason why you can't enable bytecode targets in your projects?

cristianoc commented 4 years ago

The ides is just to simplify the set-up instructions, so people can use their build with no changes. Not a huge deal, just a convenience.

rgrinberg commented 4 years ago

Thanks, that makes sense. For now I think the correct workaround is to tell people that disabling bytecode targets is a bad idea (for many reasons) On Sep 8, 2020, 11:53 PM -0700, Cristiano Calcagno notifications@github.com, wrote:

The ides is just to simplify the set-up instructions, so people can use their build with no changes. Not a huge deal, just a convenience. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

andreypopp commented 5 months ago

In our projects we rely on dune build ./prog.exe invocations (or aliases set to build .exe). This results in broken merlin/ocamllsp experience as .cmt is missing (for example jump to definition jumps to .mli instead of .ml).

The current workaround we use is to build dune build ./prog.exe @./check so .cmt files are produced.

This is suboptimal for two reasons at least:

I wonder if we can fix these two items.

esope commented 4 days ago

Hi all, is there any news on this issue?

rgrinberg commented 3 days ago

No news. No progress has been made because nobody has yet proposed a solid plan for to fix this issue