[herd] Expose herd as a dune library#1905
Conversation
HadrienRenaud
left a comment
There was a problem hiding this comment.
Hi @fsestini, this looks good to me, I don't see any negative to this.
By curiosity, did you notice a difference in compilation time (from clean or just with small edits)?
Thanks @HadrienRenaud . I did not felt a noticeable difference as I was developing, but did not measure it. I've now run a simple |
|
|
||
| (executable | ||
| (name herd) | ||
| (public_name herd7) |
There was a problem hiding this comment.
Are there any plans on having more opam metadata files on the top level? it might be worth adding (package herdtools7) if that's the case
There was a problem hiding this comment.
Are there any plans on having more opam metadata files on the top level? it might be worth adding
(package herdtools7)if that's the case
Added: 92904d8
| (libraries unix herdtools) | ||
| (wrapped true) | ||
| (modules | ||
| (:standard \ herd cli lexConf_herd itimer)) |
There was a problem hiding this comment.
This is more of a question than a concern. Shouldn't lexConf_herd be part of the library? For example www also needs it. Is there any herd interface that wouldn't need it?
There was a problem hiding this comment.
Yes, you're right.
This PR implements step 4 of the plan documented in issue #1782: it makes herd available as a dune library,
herd_core, so that other OCaml tools in the repository can depend on its internals directly.Note that the broader plan to make herd's core code properly library-like and client-agnostic is still work in progress. In other words, this PR does not claim to provide a polished or stable API, nor does it address all remaining issues and TODOs described in the aforementioned issue #1782.
That said, the API that herd does currently offer (mainly through its modules
ParseTestandTop_herd) is already useful enough for other internal tools to take advantage of it. See, for example, my upcoming tool litmus2desc (#1903), which runs herd directly in OCaml via theherd_corelibrary added here.Since the proposed
herd_corelibrary is intended for internal use only (at least of now), I think it is reasonable to expose it now as an incremental step, while continuing the remaining refactoring work tracked in #1782.Let me know your thoughts.
PS: This PR also updates
herd/rc11.mlto use explicit namespacing for record field accessors. Adding the module to the wrappedherd_corelibrary made the compiler report warnings 40 and 42 for various record accesses. The PR only adds explicit module names to those field accesses, it does not change any of the logic. As an aside,rc11.mldoes not currently appear to be used anywhere in herd.