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

Use public names when printing dune files #2558

Merged
merged 4 commits into from
Aug 20, 2019

Conversation

rgrinberg
Copy link
Member

Fix #2425 and the bug @NathanReb encountered with @avsm.

@rgrinberg rgrinberg requested a review from a user August 19, 2019 10:50
src/dune/lib.ml Outdated Show resolved Hide resolved
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good apart from a test that needs updating

test/blackbox-tests/test-cases/vlib-default-impl/run.t Outdated Show resolved Hide resolved
@rgrinberg rgrinberg force-pushed the default-impl-name branch 2 times, most recently from 6706628 to ee08f69 Compare August 20, 2019 03:27
Default implementations should always use the public name

Signed-off-by: Rudi Grinberg <[email protected]>
The library that was used for default-implementation didn't exist
previously and our error checking was lax.

Signed-off-by: Rudi Grinberg <[email protected]>
@rgrinberg
Copy link
Member Author

Thanks for the review. I addressed your comments.

@rgrinberg rgrinberg merged commit 80fc7ea into ocaml:master Aug 20, 2019
@rgrinberg rgrinberg deleted the default-impl-name branch August 20, 2019 03:53
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Aug 20, 2019
CHANGES:

- Remove the optimisation of passing `-nodynlink` for executalbes when
  not necessary. It seems to be breaking things (see ocaml/dune#2527, @diml)

- Fix invalid library names in `dune-package` files. Only public names should
  exist in such files. (ocaml/dune#2558, fix ocaml/dune#2425, @rgrinberg)
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

Successfully merging this pull request may close these issues.

default_implementation should enforce public_name
1 participant