realworldocaml / mdx

Execute code blocks inside your documentation
ISC License
269 stars 45 forks source link

ocaml-mdx silently depends on ocaml-mdx-test #138

Open NathanReb opened 5 years ago

NathanReb commented 5 years ago

There seems to be an intricate relationship between the ocaml-mdx and the ocaml-mdx-test binaries.

The former seems to just be some proxy that re-executes the latter with its own command line arguments.

I'm not familiar enough with mdx to understand why it works like that but I'd be curious to know.

The pattern is currently pretty fragile and more specifically, it doesn't work very well with dune, or without opam at least.

I'm using a vendored mdx so when I tell dune a rule depends on ocaml-mdx it will know how to build it. Because dune's not aware that ocaml-mdx depends on ocaml-mdx-test though, running ocaml-mdx just crashes with the following error:

ocaml-mdx: internal error, uncaught exception:
           Unix.Unix_error(Unix.ENOENT, "execvp", "../../../install/default/bin/ocaml-mdx-test")

While we could probably hack with dune rules to make that dependency explicit to dune, I'm not sure that's the right solution here.

Is there any reason why those binaries are split in such a way?

NathanReb commented 5 years ago

It could also be fixed by changing the rules generated by ocaml-mdx rule to make that dependency clear if we absolutely need those two separated binaries!

Let me know what you think is best, I'm happy to contribute a patch after that!

avsm commented 5 years ago

Interesting -- it would be far easier if these were the same binary indeed

samoht commented 5 years ago

The binaries are split because bytecode is needed for the toplevel libraries (as ocamlnat doesn't work realiably yet). The main code doing the parsing needs to be in native otherwise it's too slow (it was like this initially but people noticed it was slow).

So adding an explicit dependency rule makes sense here.

maiste commented 2 years ago

Hi, Currently working on building a locked file with opam monorepo for irmin, I have to face the same issue as @NathanReb did.

(cd _build/default && ../install/default/bin/ocaml-mdx test -o .mdx/README.md.corrected README.md)
ocaml-mdx: internal error, uncaught exception:
           Unix.Unix_error(Unix.ENOENT, "create_process", "../install/default/bin/ocaml-mdx-test")

When pulling the duniverse and then building it, the test file doesn't seem to be built so the ocaml-mdx is not able to find the file at the right place.

Is there a way to fix (even a temporary one) this issue with any hack?

NathanReb commented 2 years ago

Yes this is a known limitation when using the MDX stanza v0.1 in a monorepo project.

Upgrading to the stanza v0.2 (requires dune.3.0) will fix this problem.

Alternatively, this can be worked around by adding (packages mdx) to your mdx stanza.