Skip to content

fix(odoc): handle build-dir cmti paths in @doc-new fallback (#15290) #15293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Alizter wants to merge 2 commits into
base: main
Choose a base branch
from
+57 −3
Open

fix(odoc): handle build-dir cmti paths in @doc-new fallback (#15290) #15293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ba5526
test(pkg): repro @doc-new crash on locked META-only lib (#15290)
Alizter Jun 23, 2026
0c83f2c
fix(odoc): handle build-dir cmti paths in @doc-new fallback (#15290)
Alizter Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/dune_rules/odoc/odoc_new.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,11 +1178,10 @@ let modules_of_dir d : (Module_name.t * (Path.t * [ `Cmti | `Cmt | `Cmi ])) list
List.find_map extensions ~f:(fun (ext', ty) ->
Option.some_if (Filename.Extension.equal ext ext') ty)
in
Fs_memo.dir_contents (Path.as_outside_build_dir_exn d)
Fs.dir_contents d
>>| function
| Error _ -> []
| Ok dc ->
let list = Fs_memo.Dir_contents.to_list dc in
| Ok list ->
List.filter_map list ~f:(fun (x, ty) ->
match ty, Filename.extension x |> Filename.Extension.Or_empty.extension with
| Unix.S_REG, Some ext when Option.is_some (ty_of_ext ext) ->
Expand Down
55 changes: 55 additions & 0 deletions test/blackbox-tests/test-cases/pkg/doc-new-with-locked-lib.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
Reproduction for https://github.com/ocaml/dune/issues/15290

@shonfeder shonfeder Jul 2, 2026

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.

I guess this is now a regression test, rather than a repro?

@Alizter Alizter Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Indeed.


Building @doc-new with package management enabled crashes when a locked
package installs a library via a META file (without a dune-package), because
Comment on lines +3 to +4

@shonfeder shonfeder Jul 1, 2026

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.

I guess this needs to be updated?

@Alizter Alizter Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this prose will need to be updated.

the odoc rules classify it as a "fallback" directory and try to read the
cmti directory as if it were outside the build tree.

$ mkdir external_sources

$ cat >external_sources/META <<EOF
> version = "0.0.1"
> description = ""
> archive(byte) = "fakefmt.cma"
> EOF

$ touch external_sources/fakefmt.cma

$ cat >external_sources/fakefmt.install <<EOF
> lib: [
> "META"
> "fakefmt.cma"
> ]
> EOF

$ make_dune_project 3.25

$ cat > dune-workspace <<EOF
> (lang dune 3.25)
> (pkg enabled)
> EOF

$ make_lockdir
$ make_lockpkg fakefmt <<EOF
> (version 0.0.1)
> (source (copy $PWD/external_sources))
> EOF

$ cat > dune-project <<EOF
> (lang dune 3.25)
> (package (name foo) (depends fakefmt))
> EOF

$ cat > dune <<EOF
> (library
> (public_name foo)
> (libraries fakefmt))
> EOF

$ touch foo.ml

$ dune build @doc-new
File "fakefmt.mld", line 7, characters 0-11:
Warning: '{!modules ...}' should not be empty.
File "page-fakefmt.odoc":
Warning: Failed to lookup child page dummy

@shonfeder shonfeder Jul 2, 2026

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.

Is this the right/expected outcome? Some remarks explaining what this is demonstrating would be helful.

@Alizter Alizter Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is the correct output as this is coming straight from odoc. There is probably a better way we can assert that odoc ran other than capturing its output.

Loading