reasonml / ReasonNativeProject

Reason native compilation starter project
MIT License
203 stars 45 forks source link

Fix sub declaration #43

Closed satansdeer closed 6 years ago

satansdeer commented 6 years ago

Running make build was causing:

Error: Library "sub" is private, it cannot be a dependency of a public library.
You need to give "sub" a public name.
make: *** [build] Error 1

Added missing public_name for sub. Had to add empty sub.opam to avoid an error:

jbuilder build
File "lib/sub/jbuild", line 5, characters 1-18:
Error: The current scope doesn't define package "sub".
The only packages for which you can declare elements to be installed in this directory are:
- ReasonNativeProject (because of ReasonNativeProject.opam)
make: *** [build] Error 1

Had to remove the lib/sub/jbuild declaration that was making sub a library and used it as local module instead by setting inlude_subdirs unqualified.

jordwalke commented 6 years ago

The CI is failing. If you get it passing, I'll merge this.

jordwalke commented 6 years ago

I've never heard of the include_subdirs unqualified. I guess it prevents forming a Sub library?

satansdeer commented 6 years ago

Yeah, i've read the docs for dune and it seems that if you want to have a local module - you have to use include_subdirs and not define a lib.

By default include_subdirs has value no which prevents dune from using modules from subfolders. The value unqualified allows it to use those modules.

satansdeer commented 6 years ago

I think it's important to have this project in valid state and available for newcomers to build.

jordwalke commented 6 years ago

Yeah, I'll merge this it's just that the include_subdirs is a pretty uncommon feature. It would have been better to have it actually be a properly namespaced sub-library. You can see another simple example here: https://github.com/esy-ocaml/hello-reason

Thank you for the fix.