Skip to content
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.

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

(no_dynlink): do not build .cmxs #11176

Merged
merged 5 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 2 additions & 0 deletions doc/changes/11176.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- #11176: when a library declares `(no_dynlink)`, then the `.cmxs` file for it
is no longer built. (@nojb)
1 change: 1 addition & 0 deletions src/dune_rules/lib_info.ml
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ let orig_src_dir t = t.orig_src_dir
let best_src_dir t = Option.value ~default:t.src_dir t.orig_src_dir
let set_version t version = { t with version }
let entry_modules t = t.entry_modules
let dynlink_supported t = Mode.Dict.get t.plugins Native <> []

let eval_native_archives_exn (type path) (t : path t) ~modules =
match t.native_archives, modules with
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/lib_info.mli
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ val enabled : _ t -> Enabled_status.t Memo.t
val orig_src_dir : 'path t -> 'path option
val version : _ t -> Package_version.t option
val dune_version : _ t -> Dune_lang.Syntax.Version.t option
val dynlink_supported : _ t -> bool

(** Directory where the source files for the library are located. Returns the
original src dir when it exists *)
Expand Down
4 changes: 3 additions & 1 deletion src/dune_rules/lib_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,9 @@ let setup_build_archives (lib : Library.t) ~top_sorted_modules ~cctx ~expander ~
Super_context.add_rule sctx ~dir ~loc:lib.buildable.loc rule)))
in
Memo.when_
(Dynlink_supported.By_the_os.get natdynlink_supported && modes.ocaml.native)
(Lib_info.dynlink_supported lib_info
&& Dynlink_supported.By_the_os.get natdynlink_supported
&& modes.ocaml.native)
(fun () -> build_shared ~native_archives ~sctx lib ~dir ~flags)
;;

Expand Down
32 changes: 32 additions & 0 deletions test/blackbox-tests/test-cases/no_dynlink.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
This test checks that if a library is declared with `(no_dynlink)`, then the
corresponding `.cmxs` file is *not* built.

$ cat >dune-project <<EOF
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a few sentences here to describe the test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, thanks.

> (lang dune 3.17)
> EOF

First we check the behaviour when `(no_dynlink)` is not present.

$ cat >dune <<EOF
> (library
> (name mylib))
> EOF

$ touch a.ml

$ dune build _build/default/mylib.cmxs

Now with `(no_dynlink)`.

$ cat >dune <<EOF
> (library
> (name mylib)
> (no_dynlink))
> EOF

$ dune clean

$ dune build _build/default/mylib.cmxs
Copy link
Member

Choose a reason for hiding this comment

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

Could you add some tests for the .install file as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. Fixed, thanks.

Error: Don't know how to build _build/default/mylib.cmxs
Hint: did you mean _build/default/mylib.cma or _build/default/mylib.cmxa?
[1]
Loading