Skip to content

Add a missing dependency in the configuration detection - #267

Open
shym wants to merge 2 commits into
mirage:mainfrom
shym:dep-configurator
Open

Add a missing dependency in the configuration detection#267
shym wants to merge 2 commits into
mirage:mainfrom
shym:dep-configurator

Conversation

@shym

@shym shym commented Jun 18, 2025

Copy link
Copy Markdown

This PR fixes an issue due to a missing dependency on the configuration for the configuration detection.

The configuration detection executable uses dune-configurator and so it uses the configuration passed in (currently) .dune/configurator.v2 in particular to know which C compiler it should invoke. This adds an explicit dependency to this file (with a glob in case the format changes) to make sure a change of compiler triggers a redetection.

The issue arose when building a MirageOS unikernel with a Unikraft backend. The OCaml/Unikraft packaging makes it easy to switch between arm64 and x86_64 architecture, where only the %{lib}%/findlib.conf.d/unikraft.conf file is updated to point to one or the other toolchain. With such a change of target, dune was missing the dependency information to rerun cfg.exe with the proper C compiler.

This PR also takes the opportunity to centralise the detection in one directory.

The bug investigation is joint work with @Firobe.

shym added 2 commits June 18, 2025 12:04
Make sure the configuration is detected, via `config/cfg.exe`, only once
in a build by running it in `config` rather than in every directory
where the flags are used
The configuration detection executable uses `dune-configurator` and so
it uses the configuration passed in (currently) `.dune/configurator.v2`
in particular to know which C compiler it should invoke. This adds an
explicit dependency to this file (with a glob in case the format
changes) to make sure a change of compiler triggers a redetection.
@hannesm

hannesm commented Jun 18, 2025

Copy link
Copy Markdown
Member

Thanks, though I'm curious -- I copied this detection logic from elsewhere (I don't remember where), and I've never seen this

 (deps
  (glob_files %{workspace_root}/.dune/configurator*))

Is this a new thing? Is this documented somewhere? I couldn't find anything e.g. in https://dune.readthedocs.io/en/stable/dune-libs.html#configurator

Also, I'm not sure about workspace_root (never used that), neither what kind this ".dune/configurator*" is? An executable? A directory?

I find this rather mysterious, and would like to understand why this is needed and which other projects need these magic lines?

@hannesm

hannesm commented Jun 19, 2025

Copy link
Copy Markdown
Member

I digged into the mirage issue tracker, and is this one relevant mirage/mirage#1195 (comment) -- where some changes were needed to some of the packages that used the dune-configurator to play nicely with opam-monorepo / the MirageOS 4.0 story of building unikernels using cross-compilation?

@shym

shym commented Jun 19, 2025

Copy link
Copy Markdown
Author

Thanks, though I'm curious -- I copied this detection logic from elsewhere (I don't remember where), and I've never seen this

 (deps
  (glob_files %{workspace_root}/.dune/configurator*))

Is this a new thing? Is this documented somewhere? I couldn't find anything e.g. in https://dune.readthedocs.io/en/stable/dune-libs.html#configurator

It’s based on dune-configurator code rather than its documentation, I admit.

Also, I'm not sure about workspace_root (never used that),

Maybe an example would answer this question?
For a unikernel using mirage-crypto, the value of workspace_root in a dune file in mirage-crypto would be . (aka _build/solo5 or _build/unikraft), while the value of project_root would be duniverse/mirage-crypto (with _build/<toolchain> prefix).

neither what kind this ".dune/configurator*" is? An executable? A directory?

dune dumps the configuration (where is ocamlc and an easy-to-parse version of ocamlc -config) it has detected in _build/<context>/.dune/configurator and _build/<context>/.dune/configurator.v2 files so that it’s available if needed. You can see how these files (.dune/configurator and .dune/configurator.v2) are built by:

$ dune show rules .dune/configurator{,.v2} --display=quiet

(their contents are not meant to be human-readable, though...)

Executables using the dune-configurator library will use such a file if $INSIDE_DUNE is defined (it points to the _build/<context> directory when dune runs the executable).
As those files is really at a fixed location relative to _build/<context>, %{workspace_root} must be used to locate them.
The idea of using glob_files is also to make the dependency resilient in case the file disappear, is renamed, or etc.: if there is no such file, it will ignore the dependency happily, which is usually fine when you’re not switching between architectures as far as mirage-crypto is concerned.

I find this rather mysterious, and would like to understand why this is needed and which other projects need these magic lines?

I think that the dependency is usually missing but the use case is rare enough that it has never been blocking, I suppose? I’ve been wondering whether I should open a PR to change the documentation about this.

I digged into the mirage issue tracker, and is this one relevant mirage/mirage#1195 (comment) -- where some changes were needed to some of the packages that used the dune-configurator to play nicely with opam-monorepo / the MirageOS 4.0 story of building unikernels using cross-compilation?

I didn’t see what you were referring to in that issue.

@dinosaure

Copy link
Copy Markdown
Member

I think that the dependency is usually missing but the use case is rare enough that it has never been blocking, I suppose? I’ve been wondering whether I should open a PR to change the documentation about this.

I think that this is where the proposed approach depends quite subtly on what dune does. In itself, this is not a problem, but it could be either:

  1. notify the dune developers that such an approach exists and that they should take this usage into account and avoid breaking things in the future
  2. agree with the dune developers on an approach that takes cross-compilation into account as we do today, as well as maintainability issues on the dune side

@hannesm

hannesm commented Oct 20, 2025

Copy link
Copy Markdown
Member

From the linked dune issue, it sounds like there's a different solution the dune developers have in mind, but also they haven't done any action to move this forward. What is the state of this PR and dune changes?

@shym

shym commented Oct 20, 2025

Copy link
Copy Markdown
Author

On the dune side, I expect that this is waiting on someone having some spare time to implement that. I hope to get round to do this, but I’ve no idea when I’ll find some time to look into that.
So I see 2 possibilities:

  • merge the PR as is, knowing it’s a hack that should be rewritten whenever dune provides cleaner support for this,
  • keep this PR open, or turn it into an issue, to keep some track of the underlying problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants