Over approximate ppx in .merlin #1947
Conversation
|
The ppx binary should be added as a dependency to the .merlin rule. |
|
Yh, it was fine until now as it was the same binary as for compiling the .ml files, so it was always built. |
|
What about the plan of warnings users about inaccurate .merlin files? |
|
Is that a dune or merlin plan? |
|
Dune plan. We make |
|
Yh, especially if merlin files will disappear eventually |
rgrinberg
force-pushed
the
inaccurate-dot-merlin
branch
from
8a559df to
e9ba5d1
Compare
|
I've added such a warning. Now, we just need a project setting to disable it. |
So actually, it's indeed a problem. The issue is that the .merlin itself is implicitly a dependency of library targets (via the prefix mechanism). This is done for convenience so that the .merlin is built whenever a library is built. It's too complicated to fix this, so I suggest we just get rid of the over approximation altogether and just have the warning. The over approximation is imperfect anyway and we should suggest users to split their stuff into separate dirs. |
|
It seems to me that we are starting to have a substantial amount of code for something we hope will disappear entirely. I suggest that we simply do nothing for now and continue living with the current non-optimal solution until merlin files disappear entirely. |
|
But how soon will be soon? I know multiple people who spent months with broken merlin because they didn't know about this surprising behavior. A message from dune in this case would have saved a lot of time. |
|
A few months. Definitely before the end of the year, but still. |
|
Ok, I wasn't aware this was such an issue. No objection to providing a better error message in the meantime then. |
I think this PR accomplishes this job. @diml do you find the feature to turn the error message off excessive? |
Signed-off-by: Rudi Grinberg <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]>
|
No, it seems fine. |
rgrinberg
force-pushed
the
inaccurate-dot-merlin
branch
from
bee2fbd to
2c32bac
Compare
rgrinberg
force-pushed
the
inaccurate-dot-merlin
branch
from
4da317b to
47dc1d7
Compare
Signed-off-by: Rudi Grinberg <[email protected]>
rgrinberg
force-pushed
the
inaccurate-dot-merlin
branch
from
47dc1d7 to
faeb9b7
Compare
CHANGES: - Warn when generated `.merlin` does not reflect the preprocessing specification. This occurs when multiple stanzas in the same directory use different preprocessing specifications. This warning can now be disabled with `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg) - Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956, @emillon) - Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg) - Add support for library variants and default implementations. (ocaml/dune#1900, @TheLortex) - Add experimental `$ dune init` command. This command is used to create or update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder) - Experimental Coq support (fix ocaml/dune#1466, @ejgallego) - Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix ocaml/dune#1973 @rgrinberg) - Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997, @rgrinberg) - Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008, @rgrinberg) - Warn instead of failing if an opam file fails to parse. This opam file can still be used to define scope. (ocaml/dune#2023, @rgrinberg) - Do not crash if unable to read a directory when traversing to find root (ocaml/dune#2024, @rgrinberg) - Do not exit dune if some source directories are unreadable. Instead, warn the user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg) - Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent `binaries` fields would be ignored, but instead they should be combined. (ocaml/dune#2029, @rgrinberg) - Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg)
CHANGES: - Warn when generated `.merlin` does not reflect the preprocessing specification. This occurs when multiple stanzas in the same directory use different preprocessing specifications. This warning can now be disabled with `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg) - Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956, @emillon) - Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg) - Add support for library variants and default implementations. (ocaml/dune#1900, @TheLortex) - Add experimental `$ dune init` command. This command is used to create or update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder) - Experimental Coq support (fix ocaml/dune#1466, @ejgallego) - Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix ocaml/dune#1973 @rgrinberg) - Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997, @rgrinberg) - Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008, @rgrinberg) - Warn instead of failing if an opam file fails to parse. This opam file can still be used to define scope. (ocaml/dune#2023, @rgrinberg) - Do not crash if unable to read a directory when traversing to find root (ocaml/dune#2024, @rgrinberg) - Do not exit dune if some source directories are unreadable. Instead, warn the user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg) - Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent `binaries` fields would be ignored, but instead they should be combined. (ocaml/dune#2029, @rgrinberg) - Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg) - Format rules: if a dune file uses OCaml syntax, do not format it. (ocaml/dune#2014, fix ocaml/dune#2012, @emillon)
This ticket will over-approximate the ppx's by merging the list of ppx's from different libs or executables.
Now there are still some cases where we silently drop the ppx's. For those cases I propose the following:
(allow-approximate-dot-merlin)stanza in that dir.@diml I just noticed that we don't depend on the preprocessing binary when we generate the .merlin, is that correct? Shouldn't it be added as a dependency to the .merlin rule?