Skip to content

Fix builds failing with Unbound module Stdlib when a project uses menhir with package management - #16374

Draft
shonfeder wants to merge 6 commits into
mainfrom
shonfeder/15642
Draft

Fix builds failing with Unbound module Stdlib when a project uses menhir with package management#16374
shonfeder wants to merge 6 commits into
mainfrom
shonfeder/15642

Conversation

@shonfeder

@shonfeder shonfeder commented Sep 12, 2026

Copy link
Copy Markdown
Member

Description

The relocatable ocaml compiler relies on platform-specific heuristics to find the compiler executable, which it then uses to locate the Stdlib. When the relocatable ocaml compiler is managed as a regular package by dune package management, we had been (unintentionally) relying on behavior that is only guaranteed on Linux to make up for a rule that did not specify its dependency on the Stdlib. This allowed #15642 to surface on macOS.

To fix this error across all platforms, two fixes are required:

  1. The stdlib must be added as an explicit dependency for the rule that generates interfaces.
  2. In case the the path to the stdlib is located inside the _build directory (as it is when using dune package management with the relocatable compiler), we need to reclassify the path as Local, to ensure the stdlib will get linked into the sandbox where it is needed.

Please see the commit messages and comments for further details.

Related Issue and Motivation

Fixes #15642

Builds on and supersedes #15679

Checklist

  • Tests added, if applicable.
  • Change log entry added for any user-facing changes.
  • Documentation added for any user-facing changes.

Alizter and others added 2 commits September 12, 2026 00:11
@Alizter @shonfeder
test(pkg): cover a sandboxed compiler installed inside _build
b29b45c 
The relocatable ocaml compiler relies on platform-specific heuristics to
find the compiler executable, which it then uses to locate the Stdlib.
When the relocatable ocaml compiler is managed as a regular package by
dune package management, we had been relying on behavior that is only
guaranteed on Linux to make up for a rule that did not specify its
dependency on the Stdlib. This allowed
#15642 to surface on macOS. The test
added here reproduces the error case by using a wrapper around the
compiler to reproduce the problematic Stdlib location logic.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
@shonfeder
fix(menhir): declare the stdlib as a dependency of type inference
f1f9082 
Add the compiler's standard library directory as a hidden dependency of
the sandboxed `ocamlc -i` action that Menhir uses to infer interfaces.

This alone does not fix the reported failure, because the stdlib path is
classified as an external path, and the sandbox links only build paths
into the sandbox directory (see `Sandbox.link_deps`). Thus, the test
still shows "Unbound module Stdlib" after this commit.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
@shonfeder
shonfeder requested a review from Alizter September 12, 2026 04:23
shonfeder
shonfeder commented Sep 12, 2026
View reviewed changes
Comment thread src/dune_rules/lib_config.ml
Comment on lines +77 to +79
(let path = Path.of_string (Ocaml_config.standard_library ocaml_config) in
let localized = Path.Expert.try_localize_external path in
if Path.is_in_build_dir localized then localized else path)

@shonfeder shonfeder Sep 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not crazy about the duplication here and in the other location, but not sure that this is worth expanding the API of Path.Expert for, since the logic is pretty clear. I'd welcome suggestions for a cleaner fix for this!

@shonfeder shonfeder mentioned this pull request Sep 12, 2026
Closed
rgrinberg
rgrinberg requested changes Sep 12, 2026
View reviewed changes

@rgrinberg rgrinberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please find a way to deduplicate the path handling. It is too easy to get wrong otherwise.

@shonfeder
refactor(pkg): avoid constructing the std_lib path in ocaml_toolchain.ml
cce86f9 
The standard lib path is already constructed when creating the
`Lib_config.t` via `Lib_config.create`. But the current previous was
also constructing it a second time to derive the `Meta.builtins`.

This refactor simplifies the dataflow, and will enable us to avoid
brittle code duplication when we fix the classification of the stdlib's path
when the should be `Local`. Done in the following commit.

Signed-off-by: Shon Feder <shon.feder@gmail.com>
@shonfeder
fix(lib_config): try to localize the stdlib_dir
ea0336a 
This will ensure that the path to the stdlib is correctly classified when
the relocatable compiler is provided as a normal package, thus requiring
a location inside the _build directory. Without classifying it correctly
we can fail to install the stdlib into sandboxes where it is needed.

Signed-off-by: Shon Feder <shon.feder@gmail.com>
@shonfeder
test(menhir): cover relocatable compilers in the workspace root
bc4d8cf 
A relocatable compiler that is installed under the workspace root but
outside _build (e.g., as with local opam switchs) will report a standard
library that `Path.Expert.try_localize_external` would try to map to a
source-tree path. Since sandboxed actions refuse source-tree dependencies,
the stdlib must stay external in that layout. Thus the conditional logic
introduced in the previous commit. This test would fails with an
internal error if the conditional guard on the reclassification were
removed, and it protects against regressions for future changes to that
bit of logic.

Signed-off-by: Shon Feder <shon.feder@gmail.com>
@shonfeder
doc(changes): sandboxed relocatable compiler fix
166ddbd 
Signed-off-by: Shon Feder <shon.feder@gmail.com>
@shonfeder

shonfeder commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

I found a small refactor that cleans up the duplication in constructing the path. It's a general improvement as it remove a redundant path construction done on every construction of the toolchain.

However, I've also found an unrelated new error that will affect portable compilers < 5.0 triggered by this fix. I'm working on fixing it.

@shonfeder
shonfeder marked this pull request as draft September 14, 2026 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgrinberg rgrinberg rgrinberg requested changes
@Alizter Alizter Awaiting requested review from Alizter

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Build fails with Unbound module Stdlib when a project uses menhir with package management

3 participants

@shonfeder @rgrinberg @Alizter