Conversation
4b0a84e to
edf488c
Compare
edf488c to
51eb54c
Compare
|
Seems that this PR got forgotten. It needs rebasing following recent changes to context.ml |
src/dune_rules/context.ml
Outdated
| (Config.local_install_dir ~context:name) | ||
| "lib/stublibs") | ||
| ; extend_var "OCAMLPATH" ~path_sep:ocamlpath_sep local_lib_root | ||
| ; extend_var_string_list "OCAMLPATH" ~path_sep:ocamlpath_sep ocamlpath |
There was a problem hiding this comment.
This extend is a bit difficult to reason about; we read the OCaml OCAMLPATH variable and use it as part of the library search path computation, and then we extend it.
I feel like at this point, we explicitly set OCAMLPATH to the library search path we just constructed (variable findlib_paths), so that ocamlfind and dune are in complete agreement.
|
BTW, @bobot what is the |
|
more precisely, I don't understand why "default" means here. |
|
Hi, I tested the new order you suggested on real world ocaml (where findlib is vendored) and it didn't fix the issue, so I put that on hold. I'll try again with the recent fixes. |
51eb54c to
68f5f22
Compare
|
This fixed an issue where a vendored ocamlfind couldn't resolve Thanks for the rebase @emillon. Please let me know if I can help moving forward on this in any way! |
c3d1832 to
d11296b
Compare
|
@rgrinberg @bobot I rebased these changes since this fixes some use cases with opam-monorepo. More precisely, when both ocamlfind and js_of_ocaml are part of the workspace, they have trouble finding libraries without this patch. I tried to implement @jeremiedimino's suggestion above but this causes some tests to fail. What do you think about this patch? |
d11296b to
9abc1cf
Compare
What are the failures? |
|
@emillon what's the status of this PR? It would be nice to get this in. |
Fixes ocaml#4196 This adds `default_ocamlpath` to the `OCAMLPATH` variable passed to subprocesses to fix the situation where `ocamlfind` is part of the workspace. Signed-off-by: Etienne Millon <me@emillon.org>
0c72cb2 to
501db58
Compare
|
I have rebased. I have not tried the suggestions yet. |
Fixes #4196
This adds
default_ocamlpathto theOCAMLPATHvariable passed to subprocesses to fix the situation whereocamlfindis part of the workspace.