Fix [locate] to look for non-pp source files. #1219

ddickstein · 2020-12-06T16:28:11Z

When ppx is used, the cmt sourcefile that is recorded is the *.pp.mli? file,
and we compare its digest to the digests of the files in the source directory.
Often the .pp.mli? file isn't there, and even when it is we probably do not
want to prefer it over the normal .mli? file for locate. This patch checks if
the file is a .pp.mli? file, and if so uses the digest of the regular .mli?
file instead.

ddickstein · 2020-12-06T16:31:06Z

This patch resolves #894.

I've attached a small dune project that reproduces the problem. Inside bin/main.ml, if you try jumping to definition on Good it will work, but on Bad it will fail w/ "Several source files in your path have the same name, and merlin doesn't know which is the right one." With this patch, jumping to definition on Bad now works too. I also confirmed that this resolves the failure flagged in the issue with jumping to the definition of Core.List.

merlin-jump-bug.tar.gz

ddickstein · 2020-12-06T16:36:49Z

Note: If you modify lib/core_kernelish/dune by removing (preprocess (pps ppx_jane -check)), you will then be able to jump to definition of Bad in bin/main.ml without this patch.

trefis · 2020-12-07T09:05:25Z

Disclaimer: I haven't looked at your reproduction case yet.
In the general case, I don't think your idea of jumping to the .ml corresponding to a .pp.ml would be correct: some preprocessing will have happened, things will have moved. So you are going to jump to the right (perhaps) file but not to the right location.
But I guess that primarily, I'm confused regarding why a .pp.ml would be fed to the compiler in the first place.

ddickstein · 2021-01-02T17:27:28Z

@trefis have you had a chance to take a closer look at the repro case? Is a .pp.ml being fed to the compiler a dune bug?

Also, on the dune discord channel @rgrinberg said he expects preprocessing to preserve locations, and from a bit of experimenting with throwing some definitions mixed w/ ppx's into the files I seem to be jumping to the right place. But if there's a better way to approach this I'm happy to revise the patch accordingly.

rgrinberg · 2021-01-02T22:39:59Z

Is a .pp.ml being fed to the compiler a dune bug?

This behavior is intended. When dune preprocesses a module, it saves the result into module.pp.ml. This is done to share the preprocessing step between ocamlc & ocamldep.

trefis · 2021-01-04T08:41:21Z

@rgrinberg and how does that work w.r.t. error messages locations? Do you introduce line directives in the .pp.ml?
Couldn't you simply save a serialized AST instead of a text file?

rgrinberg · 2021-04-20T02:32:12Z

Do you introduce line directives in the .pp.ml

Well behaving preprocessors will insert such directives.

Couldn't you simply save a serialized AST instead of a text file?

We do if the preprocessor is a ppx. The ppx driver will emit the serialized AST.

Sorry for the late reply. Slipped by my inbox.

trefis · 2021-04-20T09:51:24Z

We do if the preprocessor is a ppx. The ppx driver will emit the serialized AST.

Even if it's not, I think a majority of the reasonable preprocessors are able to emit a serialized AST.

I finally looked at @ddickstein's example (sorry for the delay!) and there is indeed a .pp.ml, which contains a serialized ast!
So the locations should indeed be correct and the approach proposed here seems reasonable.

I just pushed a (probably incorrectly) minimized version of your reproduction case to master, as a test (which you can build with dune build @dot-pp-dot-ml), which exposes the behavior you describe before the PR, but gives the following error with the patch proposed in this PR:

I/O error: $TESTCASE_ROOT/_build/libb/dep.ml: No such file or directory

I say "probably incorrectly minimized" because it's lacking one level of indirection (:exploding_head: :gun:) so we're probably going to be missing on crucial digest along the way.
But right now we're not even reaching that point. I'm not quite sure what is happening, it could be that the call to Digest.file is failing (but it'd be surprising, because the file we get an error on does exist at the time).
I don't quite have the time to debug this right now, so you guys should feel free to have a go at it.

rgrinberg · 2021-05-02T02:21:30Z

I finally had a look. Sorry for the delay.

which exposes the behavior you describe before the PR, but gives the following error with the patch proposed in this PR:

Probably because your test doesn't copy the source files to the build directory like dune does.

Regarding the PR, it seems like an improvement but it should probably be resilient to the source being absent from the build dir and fail more gracefully.

This commit addresses the request in ocaml#1219 (comment) It also adds a test analogous to dot-pp-dot-ml.t that builds with dune and demonstrates that the fix works. I'm still unsure what's wrong with the original test, but since it's hand-constructed I'm not sure that it's simulating a real case. Finally, it updates both tests to use different values for x in dep.ml to avoid triggering an unrelated Merlin bug - ocaml#1403 - which should be resolved separately.

ddickstein · 2021-11-14T19:36:54Z

The fix doesn't seem to work on MacOS -- not yet sure why.

Any advice on how to get the new test to play nicely on Windows?

Also, if "A" and "B" in the test are replaced with (), you'll see #1403

ddickstein · 2021-11-29T00:27:41Z

Update: fixed the issue on MacOS -- now only failing on Windows.

ddickstein · 2021-11-29T02:19:56Z

@trefis / @voodoos Do you have any suggestions on how to get the Windows build working? I tried to use act to run the workflows locally but I ran into a "Parameter token or opts.auth is required" error that I think may be due to my repo being a fork and not having access to the Github secrets.

voodoos · 2021-11-29T10:18:09Z

Do you have any suggestions on how to get the Windows build working?

Not sure what is the issue here, my guess would be that there is an problem with the path starting by /tmp in:

 (cd _build/default && C:\Windows\system32\cmd.exe /c "/tmp/project/prep.exe libb/dep.ml") > _build/default/libb/dep.pp.ml

However we don't have a strict policy for Windows testing yet. If patching the test is too complicated you can simply disable it (see #1298).

voodoos · 2021-12-16T10:30:17Z

Could you add a changelog entry ?

ddickstein · 2021-12-18T05:52:25Z

@voodoos sure - how's "locate: look for original source files before looking for preprocessed files (#1219, fixes #894)"

voodoos · 2021-12-20T09:33:09Z

@voodoos sure - how's "locate: look for original source files before looking for preprocessed files (#1219, fixes #894)"

Sounds good to me

voodoos · 2021-12-20T12:25:11Z

And you can add your github handle to it, we usually credit explicitly external contributors :-)

This commit addresses the request in ocaml#1219 (comment) It also adds a test analogous to dot-pp-dot-ml.t that builds with dune and demonstrates that the fix works. I'm still unsure what's wrong with the original test, but since it's hand-constructed I'm not sure that it's simulating a real case. Finally, it updates both tests to use different values for x in dep.ml to avoid triggering an unrelated Merlin bug - ocaml#1403 - which should be resolved separately.

ddickstein · 2021-12-20T20:11:31Z

I'm not sure why debian-11-4.13_x86_32 is now failing. Is that architecture test newly added or is the test flaky?

voodoos · 2021-12-21T17:22:49Z

Looking again at the test I think we should avoid escaping the sandbox.

However I tried to rework it but it doesn't work quite as expected.
My idea is to have .merlin files to use dot-merlin-reader and build with --root=..
But the last test does not pass.
Interestingly, it doesn't work either when reproducing the test myself in another folder 🤔

I won't have the time to look more closely for the next week, here is a gist with my attempt if you want to play with it:
https://gist.github.com/voodoos/4192730f056ffb5ef939e900255ed93d

When ppx is used, the cmt sourcefile that is recorded is the *.pp.mli? file, and we compare its digest to the digests of the files in the source directory. Often the .pp.mli? file isn't there, and even when it is we probably do not want to prefer it over the normal .mli? file for locate. This patch checks if the file is a .pp.mli? file, and if so uses the digest of the regular .mli? file instead.

This commit addresses the request in ocaml#1219 (comment) It also adds a test analogous to dot-pp-dot-ml.t that builds with dune and demonstrates that the fix works. I'm still unsure what's wrong with the original test, but since it's hand-constructed I'm not sure that it's simulating a real case. Finally, it updates both tests to use different values for x in dep.ml to avoid triggering an unrelated Merlin bug - ocaml#1403 - which should be resolved separately.

Clean up the test output for MacOS. The $TMPDIR rewrite wasn't working properly because $TMPDIR was /var/folders/... but /var is a symlink to /private/var so the output was appearing as /private$TMPDIR/... The fix in this feature does not seem to work on MacOS - I'm not yet sure why.

At least one problem with the Windows build is that '.' is not recognized as an internal or external command, operable program or batch file. Hopefully by invoking prep.exe with an absolute path instead of as ./prep.exe that problem will be resolved.

When sourcefile is a path it isn't a valid basename for Misc.exact_file_exists. This feature fixes it, which seems to resolve the test failure on MacOS.

The Windows test is failing due to some issue with the usage of $TMPDIR, but I don't have an easy way to test the failure myself. I'm disabling the test for now and someone who can run the Windows workflow locally can investigate.

voodoos · 2022-01-13T14:44:40Z

@ddickstein I just force-pushed to your branch to be able to benefit from recent changes to merlin-wrapper.

I rewrote the test to stay in the sandbox (that is not using the /tmp directory) and use the dune configuration reader.

However the last test is still not giving the expected result. Would you mind reading again the test case to check that I did not add any mistake ? I am wondering if that PR actually solves the issue or not...

  $ $MERLIN single locate -look-for ml -position 2:15 \
  > -filename test.ml < ./test.ml
  {
    "class": "return",
    "value": "Several source files in your path have the same name, and merlin doesn't know which is the right one: $TESTCASE_ROOT/libb/dep.ml, $TESTCASE_ROOT/liba/dep.ml",
    "notifications": []
  }

When this variable is set, $TESTCASE_ROOT appears in the build directory, which causes the search for the source file to fail because the path does not contain "$TESTCASE_ROOT" literally.

The merlin-wrapper changes were needed for MacOS when the test escaped the sandbox. I suspect they are no longer needed.

The debian-11-4.13_x86_32 test generates the following extra output to stderr: ``` $ BUILD_PATH_PREFIX_MAP= dune build + ocamlopt liba/liba.cmxs + /usr/bin/ld: liba/liba.a(liba__Dep.o): warning: relocation in read-only section `.text' + /usr/bin/ld: warning: creating DT_TEXTREL in a shared object + ocamlopt libb/libb.cmxs + /usr/bin/ld: libb/libb.a(libb__Dep.o): warning: relocation in read-only section `.text' + /usr/bin/ld: warning: creating DT_TEXTREL in a shared object ``` The output of `dune build` isn't interesting for the purposes of this test, so we suppress it to accommodate this test.

voodoos

Looks good to me, thank you !
I can merge by squashing everything or if you want you can rebase first and squash some of the commits. Like the iterations on the tests, the back and forth in merlin-wrapper etc.

ddickstein · 2022-01-25T15:10:03Z

Squashing everything seems fine to me. Thank you!

…

On Tue, Jan 25, 2022 at 10:03 AM Ulysse ***@***.***> wrote: ***@***.**** approved this pull request. Looks good to me. I can merge by squashing everything or if you want you can rebase first and squash some of the commits. Like the iterations on the tests, the back and forth in merlin-wrapper etc. — Reply to this email directly, view it on GitHub <#1219 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGD5T43W4TTO6DI7TYVDWDUX23VRANCNFSM4UPNU7KQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

* Fix [locate] to look for non-pp source files. When ppx is used, the cmt sourcefile that is recorded is the *.pp.mli? file, and we compare its digest to the digests of the files in the source directory. Often the .pp.mli? file isn't there, and even when it is we probably do not want to prefer it over the normal .mli? file for locate. This patch checks if the file is a .pp.mli? file, and if so uses the digest of the regular .mli? file instead. When sourcefile is a path it isn't a valid basename for Misc.exact_file_exists. This feature fixes it, which resolves test failures on MacOS. * Adds a test analogous to dot-pp-dot-ml.t that builds with dune and demonstrates that the fix works. In order for the test to work we had to unset BUILD_PATH_PREFIX_MAP= When this variable is set, $TESTCASE_ROOT appears in the build directory, which causes the search for the source file to fail because the path does not contain "$TESTCASE_ROOT" literally. Co-authored-by: Ulysse Gérard <[email protected]>

@ddickstein

CHANGES: Tue Apr 5 20:59:42 CEST 2020 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - filter dups in source paths (ocaml/merlin#1218) - improve load path performance (ocaml/merlin#1323) - fix handlink of ppx's under Windows (ocaml/merlin#1413) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - expose all destruct exceptions in the api (ocaml/merlin#1437) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) + editor modes - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425) - cover locate calls on module aliases with and without dune - Add a test expliciting the interaction between locate and Dune's generated source files (ocaml/merlin#1444)

@ddickstein

CHANGES: Tue Apr 5 21:12:42 CEST 2022 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - fix handlink of ppx's under Windows (ocaml/merlin#1413) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - improve load path performance (ocaml/merlin#1323) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) + editor modes - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425)

@ddickstein

CHANGES: Tue Apr 5 21:17:21 PM CET 2022 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - fix handling of ppx's under Windows (ocaml/merlin#1413) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) + editor modes - update quick setup instructions for emacs (ocaml/merlin#1380, @ScriptDevil) - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425)

@ddickstein

CHANGES for 414: Tue Apr 5 20:51:42 CEST 2022 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - filter dups in source paths (ocaml/merlin#1218) - improve load path performance (ocaml/merlin#1323) - fix handlink of ppx's under Windows (ocaml/merlin#1413) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - expose all destruct exceptions in the api (ocaml/merlin#1437) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) - use the new "shapes" generated by the compiler to perform precise jump-to-definition (ocaml/merlin#1431) + editor modes - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425) - cover locate calls on module aliases with and without dune - Add a test expliciting the interaction between locate and Dune's generated source files (ocaml/merlin#1444) CHANGES for 413: Tue Apr 5 20:59:42 CEST 2022 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - filter dups in source paths (ocaml/merlin#1218) - improve load path performance (ocaml/merlin#1323) - fix handlink of ppx's under Windows (ocaml/merlin#1413) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - expose all destruct exceptions in the api (ocaml/merlin#1437) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) + editor modes - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425) - cover locate calls on module aliases with and without dune - Add a test expliciting the interaction between locate and Dune's generated source files (ocaml/merlin#1444) CHANGES for 412: Tue Apr 5 21:12:42 CEST 2022 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - fix handlink of ppx's under Windows (ocaml/merlin#1413) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - improve load path performance (ocaml/merlin#1323) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) + editor modes - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425) CHANGES for 411: Tue Apr 5 21:17:21 PM CET 2022 + merlin binary - don't reset the environment when running merlin in single mode so that the parent environement is forwarded the the child processes (ocaml/merlin#1425) - locate: look for original source files before looking for preprocessed files (ocaml/merlin#1219 by @ddickstein, fixes ocaml/merlin#894) - fix handling of ppx's under Windows (ocaml/merlin#1413) - handle `=` syntax in compiler flags (ocaml/merlin#1409) - fix superfluous break in error reporting (ocaml/merlin#1432) - recognise binding operators in locate and occurrences (ocaml/merlin#1398, @mattiase) - remove dependency on Result (ocaml/merlin#1441, @kit-ty-kate) + editor modes - update quick setup instructions for emacs (ocaml/merlin#1380, @ScriptDevil) - fix an issue in Neovim where the current line jumps to the top of the window on repeated calls to `MerlinTypeOf` (ocaml/merlin#1433 by @ddickstein, fixes ocaml/merlin#1221) - add module, module type, and class imenu items for emacs (ocaml/merlin#1244, @ivg) - add prefix argument to force or prevent opening in a new buffer in locate command (ocaml/merlin#1426, @panglesd) - add type-on-hover functionality for vim (ocaml/merlin#1439, @nilsbecker) - add a dedicated buffer `*merlin-errors*` containing the last viewed error (ocaml/merlin#1414, @panglesd) + test suite - make `merlin-wrapper` create a default `.merlin` file only when there is no `dune-project` to let tests use `dune ocaml-merlin` reader. (ocaml/merlin#1425)

ddickstein mentioned this pull request Dec 7, 2020

Difficulty vendoring base and core ocaml/dune#3971

Open

trefis force-pushed the master branch from cf6700e to a8fa9db Compare July 13, 2021 17:14

ddickstein force-pushed the fix-locate-ppx branch from 8c700d5 to 05e7a56 Compare November 14, 2021 19:23

ddickstein force-pushed the fix-locate-ppx branch from 05e7a56 to 04eff2b Compare November 14, 2021 19:27

tatchi mentioned this pull request Nov 24, 2021

Search for a symbol in workspace looks into .pp.ml files ocaml/ocaml-lsp#548

Closed

voodoos requested a review from trefis December 6, 2021 09:30

ddickstein added a commit to ddickstein/merlin that referenced this pull request Dec 20, 2021

Update changelog for PR ocaml#1219

f15a020

ddickstein added a commit to ddickstein/merlin that referenced this pull request Dec 20, 2021

Update changelog for PR ocaml#1219

0e4432d

voodoos mentioned this pull request Jan 10, 2022

Allow the use of dune-base configuration reader in tests #1425

Merged

ddickstein and others added 8 commits January 13, 2022 15:38

Invoke prep.exe with absolute path

f77006c

At least one problem with the Windows build is that '.' is not recognized as an internal or external command, operable program or batch file. Hopefully by invoking prep.exe with an absolute path instead of as ./prep.exe that problem will be resolved.

Fixed move_to_root so basename is never a path

e83683e

When sourcefile is a path it isn't a valid basename for Misc.exact_file_exists. This feature fixes it, which seems to resolve the test failure on MacOS.

Disable Windows test for dot-pp-dot-ml-dune.t

10d23bb

The Windows test is failing due to some issue with the usage of $TMPDIR, but I don't have an easy way to test the failure myself. I'm disabling the test for now and someone who can run the Windows workflow locally can investigate.

Update changelog for PR ocaml#1219

2cc8940

Do not escape the sandbox in the test

5357fec

voodoos force-pushed the fix-locate-ppx branch from 0e4432d to 5357fec Compare January 13, 2022 14:41

ddickstein added 3 commits January 22, 2022 02:00

Unset BUILD_PATH_PREFIX_MAP= when running dune build in dot-pp test.

91ab2e8

When this variable is set, $TESTCASE_ROOT appears in the build directory, which causes the search for the source file to fail because the path does not contain "$TESTCASE_ROOT" literally.

Unwind changes to merlin-wrapper

760e7dd

The merlin-wrapper changes were needed for MacOS when the test escaped the sandbox. I suspect they are no longer needed.

voodoos approved these changes Jan 25, 2022

View reviewed changes

voodoos merged commit 93b16f3 into ocaml:master Jan 25, 2022

ddickstein deleted the fix-locate-ppx branch January 25, 2022 22:05

voodoos mentioned this pull request Apr 5, 2022

[new release] merlin 4.5-414/413/412/411 and dot-merlin-reader 4.2 ocaml/opam-repository#21121

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix [locate] to look for non-pp source files. #1219

Fix [locate] to look for non-pp source files. #1219

ddickstein commented Dec 6, 2020

ddickstein commented Dec 6, 2020 •

edited

Loading

ddickstein commented Dec 6, 2020 •

edited

Loading

trefis commented Dec 7, 2020

ddickstein commented Jan 2, 2021 •

edited

Loading

rgrinberg commented Jan 2, 2021

trefis commented Jan 4, 2021

rgrinberg commented Apr 20, 2021

trefis commented Apr 20, 2021

rgrinberg commented May 2, 2021

ddickstein commented Nov 14, 2021 •

edited

Loading

ddickstein commented Nov 29, 2021

ddickstein commented Nov 29, 2021 •

edited

Loading

voodoos commented Nov 29, 2021

voodoos commented Dec 16, 2021

ddickstein commented Dec 18, 2021

voodoos commented Dec 20, 2021

voodoos commented Dec 20, 2021

ddickstein commented Dec 20, 2021

voodoos commented Dec 21, 2021 •

edited

Loading

voodoos commented Jan 13, 2022 •

edited

Loading

voodoos left a comment •

edited

Loading

ddickstein commented Jan 25, 2022 via email

Fix [locate] to look for non-pp source files. #1219

Fix [locate] to look for non-pp source files. #1219

Conversation

ddickstein commented Dec 6, 2020

ddickstein commented Dec 6, 2020 • edited Loading

ddickstein commented Dec 6, 2020 • edited Loading

trefis commented Dec 7, 2020

ddickstein commented Jan 2, 2021 • edited Loading

rgrinberg commented Jan 2, 2021

trefis commented Jan 4, 2021

rgrinberg commented Apr 20, 2021

trefis commented Apr 20, 2021

rgrinberg commented May 2, 2021

ddickstein commented Nov 14, 2021 • edited Loading

ddickstein commented Nov 29, 2021

ddickstein commented Nov 29, 2021 • edited Loading

voodoos commented Nov 29, 2021

voodoos commented Dec 16, 2021

ddickstein commented Dec 18, 2021

voodoos commented Dec 20, 2021

voodoos commented Dec 20, 2021

ddickstein commented Dec 20, 2021

voodoos commented Dec 21, 2021 • edited Loading

voodoos commented Jan 13, 2022 • edited Loading

voodoos left a comment • edited Loading

Choose a reason for hiding this comment

ddickstein commented Jan 25, 2022 via email

ddickstein commented Dec 6, 2020 •

edited

Loading

ddickstein commented Dec 6, 2020 •

edited

Loading

ddickstein commented Jan 2, 2021 •

edited

Loading

ddickstein commented Nov 14, 2021 •

edited

Loading

ddickstein commented Nov 29, 2021 •

edited

Loading

voodoos commented Dec 21, 2021 •

edited

Loading

voodoos commented Jan 13, 2022 •

edited

Loading

voodoos left a comment •

edited

Loading