Several driver fixes#1461
Open
jonludlam wants to merge 9 commits into
Open
Conversation
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>
… deps
The -L/-P/-I arguments to `odoc link` were computed per library from its
transitively-closed dependency set, so libraries in the same package could get
different link scopes. Compute them once per package instead, shared by all its
libraries and pages:
- -L is the directly-declared META dependencies of the package's libraries
(own libraries included) plus anything from odoc-config.sexp; no transitive
closure is taken here.
- -P is every package providing one of those libraries, plus the package
itself and any odoc-config.sexp packages.
Dependencies a package uses but doesn't declare in META (e.g. tyxml.functor,
reached only via tyxml's META) are recovered by digest rather than by closure.
Pkg_args and the per-unit pkg_args field are link-step only, as compile derives
its own includes per module.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Spell out, from both the author's and the driver's point of view, which libraries and packages are in scope for references, and therefore what you can link to. In odoc_for_authors, the "Reference Scope" section gains a "What is in scope" subsection listing the referenceable libraries, packages and pages. In driver.mld, the "Link-time dependencies" section describes how the reference driver computes the per-package [-P]/[-L]/[-I] scope: the package's own libraries, their direct META dependencies, the stdlib, and anything added via odoc-config.sexp, but not the full transitive closure. It also notes the one exception to the "same arguments for the whole package" rule -- alternative implementations of a virtual library are kept off each other's paths, since their same-name, same-hash modules can't be disambiguated by digest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two libraries define a module `Common` with different interface digests, and a third module `include`s one of them. When both are on the include path, odoc resolves the source links of the included values by module name alone and picks the wrong same-named implementation. The recorded output shows the current (buggy) behaviour: the source link points to `libb`'s `common.ml` rather than `liba`'s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`lookup_impl` resolved implementations by module name only, returning the first match. When two implementations with the same module name but different interface digests are on the include path (e.g. a package's `.client` and `.server` libraries), it could pick the wrong one, and shape-based source-location resolution would then silently drop the source links. Disambiguate by digest, as `lookup_unit` already does for interfaces: pass `lookup_impl` the referring unit's imports map and pick the implementation whose digest matches. A unit is linked from its `.cmti`, so its own import table only covers interface-level dependencies; augment it in `build_link_env_for_unit` with the matching implementation's imports (which carry the implementation-only dependencies) and a self-entry keyed by the unit's own digest, so the unit's own shape and its impl-only dependencies resolve to the right same-named implementation. Two implementations of the same virtual library share one interface digest and so can't be told apart this way; the driver keeps them off each other's include paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er's paths Two implementations of a virtual library (e.g. a package's `.c` and `.ocaml` libraries) provide modules with the same name and the same interface digest. With both on a unit's include path those modules collide, and — unlike ordinary same-name modules — they can't be told apart by digest, so source-location resolution picks the wrong implementation and drops the source links. Exclude a library's sibling implementations from its `-L`/`-I`. The sibling relationship is derived from the same duplicate-interface-hash grouping that `remap_virtual` uses, now factored into a shared `virtual_modules` and exposed as `Packages.virtual_siblings`; `link_args_of` drops those siblings from the link paths of the library's units. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a dependency's interface hash is provided by more than one unit — for instance a virtual library's interface and each of its implementations, which share an interface hash — includes_of_deps added every provider's directory to the compile -I set. Prefer instead a provider whose library the unit actually depends on (its own library or a direct/recovered dependency, tracked in the new lib_deps field), falling back to an arbitrary provider when none is a dependency. Same-hash providers are interchangeable for compilation, so this only trims the include set and keeps unrelated sibling implementations off it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.