Several driver fixes - #1461
Open
jonludlam wants to merge 5 commits into
Open
Conversation
panglesd
reviewed
Jul 15, 2026
panglesd
left a comment
panglesd
left a comment
Contributor
There was a problem hiding this comment.
The improvement to the docs is very nice and welcome!
Member
Author
|
Let's hold off on merging this just yet - I want to double check the changes to the docs. |
jonludlam
force-pushed
the
several-driver-fixes
branch
from
July 17, 2026 16:53
3ee017b to
42da53b
Compare
read_libraries_from_pkg_defs only looked up the archive under the [byte]/[native] predicates. ppx derivers such as ppxlib.traverse and ppxlib.metaquot declare their archive only under the ppx_driver predicate, so they were dropped here and later re-discovered by the no-META fallback, which names a library after its .cma file (e.g. ppxlib_traverse instead of ppxlib.traverse). Also try the [byte; ppx_driver] and [native; ppx_driver] predicates, mirroring Ocamlfind.archives which the non-voodoo driver already uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previously the include set passed to `odoc compile`/`compile-impl` was derived from the compiled library's whole dependency set (every library it requires). That implies doing the transitive closure over all libraries' dependencies. In voodoo mode, we only have the META files for the package being documented, so this isn't possible. Instead, derive the include set from the unit's actual per-module dependencies. Each dependency carries the interface hash of the module it refers to; look that hash up among the units being compiled plus the partials of already-compiled dependencies, and take the directory of the providing unit. This is more precise (far fewer `-I` flags) and doesn't depend on META completeness. Applies to both interfaces and implementations; `impl_extra` now carries the implementation's dependency list. In the non-voodoo driver, where the library-level set was already complete, source-link output is byte-for-byte identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compute the link phase's -I search path per library -- the union of its units' dependency cones, the same directories the compiler used -- rather than a package-wide set. -L and -P remain package-wide and unvarying (the reference scope). Because a library's -I holds only its own cone, alternative implementations of a virtual library never share a search path, so no sibling-exclusion special-case is needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document, in driver.mld and the author-facing odoc_for_authors.mld, the reference-scope convention for opam-installed packages: the per-package -P/-L reference scope, the per-unit -I include path, and the odoc-config.sexp stanzas. Also capture the broader driver guidance learned from building the odd driver -- the compile/link include model, virtual-library handling, and what a driver should compile.
jonludlam
force-pushed
the
several-driver-fixes
branch
from
July 17, 2026 17:01
42da53b to
6de44e2
Compare
Member
Author
|
OK, I've updated the docs some more. This should be ready for review now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A number of related fixes, mostly to the driver. There's one fix to odoc in the mix too. The commits should be independently reviewable.