Skip to content

Implicit transitive deps experiment (do not merge)#14

Closed
mbarbin wants to merge 10 commits intomainfrom
implicit-transitive-deps-experiment
Closed

Implicit transitive deps experiment (do not merge)#14
mbarbin wants to merge 10 commits intomainfrom
implicit-transitive-deps-experiment

Conversation

@mbarbin
Copy link
Copy Markdown
Owner

@mbarbin mbarbin commented May 29, 2025

This further experiments with (implicit_transitive_deps _) settings. Do not merge.

@mbarbin mbarbin changed the title Implicit transitive deps experiment Implicit transitive deps experiment (do not merge) May 29, 2025
@mbarbin
Copy link
Copy Markdown
Owner Author

mbarbin commented May 29, 2025

From that experiment all seems to work as expected with the new option except one part:

commit 97b3d69 ("Why does this build?").

I am compiling dune build -p pplumbing at that commit with 5.3.0, and I'd expect the build to fail because err.ml makes use of Pp without having the pp library listed in its dependencies, and the (implicit_transitive_deps false) at that commit.

cc @nojb (this does not relate to the new option, but is a question about (implicit_transitive_deps false) in general).

@nojb
Copy link
Copy Markdown

nojb commented May 29, 2025

I am compiling dune build -p pplumbing at that commit with 5.3.0, and I'd expect the build to fail because err.ml makes use of Pp without having the pp library listed in its dependencies, and the (implicit_transitive_deps false) at that commit.

I would expect so as well at first sight. Can you build at that commit and post the build log (_build/log) here?

@mbarbin
Copy link
Copy Markdown
Owner Author

mbarbin commented May 29, 2025

Ah @nojb I think I understand now, this is I believe thanks to https://github.com/ocaml/dune/blob/82bbef1c62e6f402e22823481c3bde35429a2bad/otherlibs/stdune/src/dune#L11 in stdune which does (re_export pp).

Seems all fine then, I'm happy to consider this testing session satisfactory.

@nojb
Copy link
Copy Markdown

nojb commented May 29, 2025

which does (re_export pp).

Yes, this is the explanation. Thanks!

@mbarbin
Copy link
Copy Markdown
Owner Author

mbarbin commented Aug 26, 2025

Experimentation is complete - we'll go with #15 when ready.

@mbarbin mbarbin closed this Aug 26, 2025
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.

2 participants