Fix nested dune workspace

[actionshrimp]

The first commit fixes a bug in the toEqualUri jest helper (happy to split this out into a separate PR if that makes sense).

Second commit fixes an issue with merlin config setup when a subfolder inside a project has its own dune-project or dune-workspace file.

The current implementation searches upwards until the first dune-project or dune-workspace file is found, but there may be such files further up the tree, for example if there's a dependency bought in as a submodule with its own dune-project file - currently this breaks ocaml-lsp in files in such a submodule.

Now we search for the innermost dune file to get the 'workdir' referenced in the function's comment, but the outermost dune-projectordune-workspace` file. (The newly added test fails under the old implementation).

For the tests to work, we have to temporarily rename the dune-project from the ocaml-lsp project itself in test setup/teardown so it doesn't interfere with the context (perhaps there's a better way to do this :)).

[rgrinberg]

So is this bug also present in dot-merlin-reader? cc @voodoos

[rgrinberg]

happy to split this out into a separate PR if that makes sense

It's appreciated but not required.

[voodoos]

It should not impact dot-merlin-reader since this is the tool we use to read .merlin files not interact with dune.
However the algorithm we use to determine is we are running in a dune project or not is the same.

But limiting the recursion to the first encounter with a dune-project / dune-workspace was intentional. We expect dune ocaml-merlin, when launched, to discover by itself the real root of the project.

I will try to reproduce in the Merlin setting.

[voodoos]

Looking at the tests files I don't see any "outer" dune project ? There is a dune-workspace file in the declaration_files/inner directory but nothing in the declaration_files directory.

It looks like the "outer" project used in the test is ocaml-lsp itself. Is that correct ?
let outerDuneProjectPath = path.join(__dirname, "../../../../dune-project");

Aren't the e2e tests running in a sandbox ?

[actionshrimp]

Hey @voodoos , thanks for taking a look .

I added the inner/dune-workspace file as part of this PR, but there was a declaration_files/dune-project file already in the repo for the 'outer' (https://github.com/ocaml/ocaml-lsp/blob/master/ocaml-lsp-server/test/e2e/__tests__/declaration_files/dune-project).

The outerDuneProjectPath (a bad name, sorry) does indeed reference ocaml-lsp's dune-project file - I hide it from the LSP as part of the test setup as it interfered when running locally. Perhaps there's a more sandboxed way of running the tests I'm unaware of though.

[rgrinberg]

Aren't the e2e tests running in a sandbox ?

The e2e tests run outside of dune because they are written in typescript.

[voodoos]

I just tested on a very simple nested project structure:

And OCaml-LSP works as intended on my setup after building the project.

[voodoos]

Ok I see it works in my example because I have a dune-project nested in another one (or a dune-workspace).

However when nesting dune-workspace dune will not look at the outermost one:

Dune requires at least one dune-workspace file or dune-project file to operate. This file may appear in the current or a parent directory. It’s used to mark the root of the current workspace; however, dune-workspace and dune-project files are treated slightly differently. Since there can be only a single workspace active at a given time, Dune stops its search for the root at the first dune-workspace file it encounters. On the other hand, Dune projects are composable, and there can be multiple projects in a single workspace. For this reason, when Dune finds a dune-project file it will continue searching in parent directories, in case this project is part of a bigger workspace.

I am not able to build your example with dune... it says Library "inner" not found.

As far as I understand, there is no issue with OCaml-LSP here but I might be wrong.
Can you make a working reproduction in a separate repository ?

[actionshrimp]

Thanks for taking a look again. I did manage to repro in this repo: https://github.com/actionshrimp/lsp-dune-test-repo (simulating a project that brings in a submodule as a dependency that has its own dune-workspace file). With that setup, LSP doesn't seem to like files in the inner project until the dune-workspace file is removed.

However in the process of playing around with that I noticed a much simpler way of resolving the issue, which is to just add a dune-workspace file at the repo root as well. That seems to make everything work as intended, even in the presence of the inner dune-workspace file - I'd previously thought the dune-project and dune-workspace were more or less interchangable as project root markers.

Apologies for the confusion and I'll close this!