ocamlbuild icon indicating copy to clipboard operation
ocamlbuild copied to clipboard

Bug in [Ocaml_utils.ocaml_lib]?

Open pveber opened this issue 10 years ago • 1 comments

The implementation of Ocaml_utils.ocaml_lib does not seem right to me: as indicated by the corresponding comment, L113 can never have an effect. The function flag_and_dep is only called if extern is true, but L113 does something only if extern is false.

So at least this is dead code, but I think the line should be evaluated in all cases. If a target t.cmo has tag use_foo for compiling it, we maybe need to set the -I flag (this is done L133), but we also have to make sure the library is built before. However, ocamlbuild here will only know via ocamldep that it needs some modules of the foo library, and will typically compile t.cmo by adding the corresponding cmo instead of foo.cma. This is wrong since it may lead to link-time errors saying that two units define the same module.

Another use case where this is annoying is that I want to build a library cma in a different location than the cmo it contains (this is necessary when building several (packed) libraries having module names in common). In that case, ocamlbuild has no means to track the dependency and fails to compile t.cmo complaining modules from foo are unbound.

Long story short: I think dep should be called unconditionally. On a related note I find the meaning of the extern argument unclear, and maybe a good start on this would be to try and clarify it. I'd be happy to work on this, but as this function must used quite a lot, I may need some guidance to propose something useful.

pveber avatar Feb 25 '16 18:02 pveber

I don't know much about how the library subsystem is working, and I've been very happy to avoid it and use ocamlfind packages instead. It should still be useful for people building projects that include several separate but inter-related libraries in one unique ocamlbuild project. If you (or someone else) are willing to "take responsibility" for it, I would be happy to let you (them) -- the requirement is not to break other people's stuff, and let us review pull requests to make sure the new code is in good shape.

gasche avatar Feb 25 '16 20:02 gasche