Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hcarty opened this issue Feb 28, 2018 · 6 comments

Comments

@hcarty
Copy link
Member

hcarty commented Feb 28, 2018

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 ghost self-assigned this Mar 1, 2018
@ghost
Copy link

ghost commented Mar 1, 2018

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:

  • Base is linted using a ppx rewriter
  • The ppx rewriter uses Base and a few other ppx libraries

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
Copy link

ghost commented Mar 1, 2018

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
Copy link
Member Author

hcarty commented Mar 5, 2018

@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
Copy link

ghost commented Mar 5, 2018

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
Copy link

ghost commented Mar 5, 2018

I implemented the latter solution in #587

@ghost
Copy link

ghost commented Mar 14, 2018

fixed in beta19

@ghost ghost closed this as completed Mar 14, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant