merlin: communicate STDLIB directive#4211
Conversation
|
There is a CI failure which I don't understand, any ideas? ./dune.exe runtest
File "test/blackbox-tests/test-cases/merlin/github4125.t/run.t", line 1, characters 0-0:
git (internal) (exit 1)
(cd _build/.sandbox/259d57d1d05fbb64c642522c5909613e/default && /usr/bin/git --no-pager diff --no-index --color=always -u ../../../default/test/blackbox-tests/test-cases/merlin/github4125.t/run.t test/blackbox-tests/test-cases/merlin/github4125.t/run.t.corrected)
diff --git a/../../../default/test/blackbox-tests/test-cases/merlin/github4125.t/run.t b/test/blackbox-tests/test-cases/merlin/github4125.t/run.t.corrected
index 8df63f0..1639ec6 100644
--- a/../../../default/test/blackbox-tests/test-cases/merlin/github4125.t/run.t
+++ b/test/blackbox-tests/test-cases/merlin/github4125.t/run.t.corrected
@@ -8,7 +8,7 @@
$ dune ocaml-merlin --dump-config="$(pwd)" |
> sed 's#'$(opam config var --switch default prefix)'#OPAM_PREFIX#'
Foo
- ((STDLIB OPAM_PREFIX/lib/ocaml)
+ ((STDLIB /usr/lib/ocaml)
(EXCLUDE_QUERY_DIR)
(B
$TESTCASE_ROOT/_build/cross/.foo.objs/byte)
Makefile:73: recipe for target 'test' failed
make: *** [test] Error 1
'make test' failed with code 2 |
|
It could be because in the CI ocaml is installed globally while on your machine ocaml was installed by opam. As a result the paths to the stdlib directory differs. You could try rewriting BTW, instead of |
Yes, but if I do that then the test will may/will fail when run in some developer's machine. Perhaps I should try to do both? I will investigate a bit more.
Thanks for the tip. I used |
I pushed a fix for the test that appears to work always: |
|
I would expect just On the other hand, if the opam switch currently in use is not |
That's actually the case in this particular test :) It uses the |
|
Ah, I see. That seems right then |
ghost
left a comment
There was a problem hiding this comment.
LGTM. A changelog entry seems relevant for this BTW.
I'm not a merlin expert, but looking at the messages in the linked tickets, that does seem like the right thing to do to me.
|
Friendly ping @voodoos |
b92ad89 to
8624a8c
Compare
voodoos
left a comment
There was a problem hiding this comment.
Looks good to me.
I looked at Context.stdlib_dir and it is always populated with the path from the ocamlc -config of the current compiler.
If this is the expected behavior then it's all good (I am asking confirmation because I am not an expert, but this looks like the correct thing to do).
Also it my be good to add a call to merlin in one of the tests to check that it understands the directive as expected, something like:
ocamlmerlin single dump-configuration -filename foo.ml < foo.ml | jq ".value.merlin.stdlib"
Yes, that's the expected behaviour. The point is that the
OK, will do. |
I tried doing this, but for some reason I always get the error: |
|
Do we backport this to 2.8.3? |
Ok it works when adding a The best is probably not to test it as it won't be reproducible if Dune is in a larger workspace. (However it works as expected: |
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
Thanks, got it. I will merge once the CI is green. |
I would say yes. |
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
Signed-off-by: Nicolás Ojeda Bär <[email protected]>
…ne-action-plugin, dune-private-libs and dune-glob (2.8.3) CHANGES: - Make `patdiff` show refined diffs (ocaml/dune#4257, fixes ocaml/dune#4254, @hakuch) - Fixed a bug that could result in needless recompilation under Windows due to case differences in the result of `Sys.getcwd` (observed under `emacs`). (ocaml/dune#3966, @nojb). - Restore compatibility with Coq < 8.10 for coq-lang < 0.3 , document that `(using coq 0.3)` does require Coq 8.10 at least (ocaml/dune#4224, fixes ocaml/dune#4142, @ejgallego) - Add a META rule for 'compiler-libs.native-toplevel' (ocaml/dune#4175, @AltGr) - No longer call `chmod` on symbolic links (fixes ocaml/dune#4195, @dannywillems) - Dune no longer automatically create or edit `dune-project` files (ocaml/dune#4239, fixes ocaml/dune#4108, @jeremiedimino) - Have `dune` communicate the location of the standard library directory to `merlin` (ocaml/dune#4211, fixes ocaml/dune#4188, @nojb) - Workaround incorrect exception raised by Unix.utimes (OCaml PR#8857) in Path.touch on Windows (ocaml/dune#4223, @dra27) - `dune ocaml-merlin` is now able to provide configuration for source files in the `_build` directory. (ocaml/dune#4274, @voodoos) - Automatically delete left-over Merlin files when rebuilding for the first time a project previously built with Dune `<= 2.7`. (ocaml/dune#4261, @voodoos, @aalekseyev) - Fix `ppx.exe` being compiled for the wrong target when cross-compiling (ocaml/dune#3751, fixes ocaml/dune#3698, @toots) - `dune top` correctly escapes the generated toplevel directives, and make it easier for `dune top` to locate C stubs associated to concerned libraries. (ocaml/dune#4242, fixes ocaml/dune#4231, @nojb) - Do not pass include directories containing native objects when compiling bytecode (ocaml/dune#4200, @nojb)
Make it so that
dunecommunicates the standard library directory tomerlin. This makes it possible to usemerlinwith an OCaml compiler other than the one it was compiled with.Fixes #4188