ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.61k stars 400 forks source link

beta18 changes how jbuilder handles conflicts between installed and symlinked libraries #562

Closed hcarty closed 6 years ago

hcarty commented 6 years ago

Consider a project myproject which uses mylib and otherlib. otherlib uses mylib internally. mylib and otherlib are installed via opam.

When developing myproject, new changes are required in mylib. Because jbuilder/dune is super awesome, we can symlink the dev mylib source tree under the myproject source tree, hack away, and everything almost works great. BUT this is problematic because now the mylib used by myproject and the mylib used by otherlib are different! So that difference needs to be dealt with.

Under beta17, jbuilder would report that the internal/symlinked version of mylib conflicts with the mylib installed under opam as required by otherlib. An error message is displayed and the build fails with a happy little error message describing the issue. We can fix the issue by symlinking otherlib too, appeasing jbuilder and getting the results we want.

Under beta18, jbuilder does not report the conflict. The internal version of mylib is built but jbuilder silently compiles myproject against the opam-installed version of mylib. This causes the myproject build to fail in an unexpected way because it's trying to use the new mylib interface which only exists in the internal mylib, not the opam `mylib.

The beta17 behavior was much more friendly - it took me a while to dig out the issue after upgrading jbuilder to beta18.

ghost commented 6 years ago

The case you describe should still be an error with beta18, I'll have a look. Basically one case we tried to support better is for instance the case of linting in Base:

With beta17, one would have to put in the same workspace Base and all the dependencies of the ppx rewriters in order to lint Base. With beta18, it's not necessary.

Though we still check for conflicts when the two versions ends up being used by the same library or executable, since this cannot work.

ghost commented 6 years ago

BTW, can you give a concrete example (toy example or real project) where you think an error should be reported and isn't? Just to be sure I'm thinking about the right thing

hcarty commented 6 years ago

@diml Sorry for the delay! See https://github.com/hcarty/jbuilder-issue-562 for an example that illustrates the problem. Instructions are in that repo's README.md. It's very minimal so please let me know if there's more information I can provide.

ghost commented 6 years ago

Thanks. Looking at the example, it seems to me that #430 would avoid the issue entirely. i.e. c uses a directly but it's not written in the jbuild file. If you add a to the jbuild then you should get an error.

I'm not sure what's the best solution here. Possibly we could make the new behavior opt-in, i.e. in cases such as Base you'd have to write something in the jbuild file of the linter in order to allow it to use the external version of Base.

ghost commented 6 years ago

I implemented the latter solution in #587

ghost commented 6 years ago

fixed in beta19