-
Notifications
You must be signed in to change notification settings - Fork 251
[dune] Merlin's PPX invocations does not match Dune's #1292
Description
Context
Merlin associates with each ppx command line a workdir value which represents the directory in witch the ppx executable will be started by Merlin. This configuration value is not given to Merlin but inferred from the folder containing the configuration. That is, for .merlin based configuration, that folder was always the one containing the .merlin.
This was not a perfect solution as it does not reflect the true running folder of the ppxes when Dune builds a project, but it was a best effort solution which seems to work in most cases.
Issue
This best-effort broke when switching to the Dune-based configuration reader because Merlin considered the folder in which the external configuration reader is started to be the workdir. However, unlike dot-merlin-reader, the dune ocaml-merlin process starting folder might not be the one with the dune file.
The issue notably appears when using ppx_expect and was reported by @jeremiedimino. After investigating it turns out that ppx_expect should not attempt to compute the hash off the file when called by Merlin and Jérémie opened an issue.
Other ppxes that need to interact with files from the sources might be equally broken right now.
Food for thought
This issue is a good opportunity to make Merlin ppx behavior closer to Dune's in term of workdir and execution environment. However this need changes to both sides and might not happen immediately. This raises two questions:
- How can we have Merlin replicate more precisely the environnement and conditions of executions of ppxes by Dune ?
- Meanwhile, should we issue a quick fix to restore the previous, less-broken, behavior ?
For 1. Dune would need to forward more information to merlin, especially the workdir which appears to always be the root of the current context folder in _build. We also need Dune to provide the filename to use for locations.
For 2. the fix is ready but it is adding more noise to the codebase for use cases that may be illegitimate (for example ppx_expect should not do what it is doing when invoked in Merlin). However the patch does "solve" the current issue and could always be reverted when a cleaner approach is merged.