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

Fix bigarray config (#5494) #5526

Merged
merged 10 commits into from
Jun 13, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 27 additions & 0 deletions src/dune_rules/lib.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,23 @@ end = struct
{ resolved; selects; re_exports }
end

let remove_library deps target =
List.filter_map deps ~f:(fun (dep : Lib_dep.t) ->
match dep with
| Re_export (_, name) | Direct (_, name) ->
Option.some_if (not (Lib_name.equal target name)) dep
| Select select ->
let choices =
List.filter_map select.choices ~f:(fun choice ->
if Lib_name.Set.mem choice.forbidden target then None
else
Some
{ choice with
required = Lib_name.Set.remove choice.required target
})
in
Some (Select { select with choices }))

let resolve_complex_deps db deps ~private_deps : resolved_deps Memo.t =
let resolve_select { Lib_dep.Select.result_fn; choices; loc } =
let open Memo.O in
Expand Down Expand Up @@ -1207,6 +1224,16 @@ end = struct
in
let open Memo.O in
let open Resolved_deps_builder in
let deps =
let ocaml_version = db.lib_config.ocaml_version in
let bigarray_in_std_libraries =
Ocaml.Version.has_bigarray_library ocaml_version
in
if bigarray_in_std_libraries then deps
else
let bigarray = Lib_name.of_string "bigarray" in
Copy link
Member

Choose a reason for hiding this comment

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

This change is incorrect and introduces a bug that forbids local libraries named bigarray from being used on OCaml < 5.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, I don't know how we can test if there a local library called bigarray.

Copy link
Collaborator

Choose a reason for hiding this comment

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

that has been fixed in #8902 .

remove_library deps bigarray
in
let+ builder =
Memo.List.fold_left deps ~init:empty ~f:(fun acc dep ->
match (dep : Lib_dep.t) with
Expand Down
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/a/a.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let _c = Bigarray.C_layout_typ

let () = Printf.eprintf "Welcome to a\n%!"
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/a/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(executable
(name a)
(libraries bigarray))
5 changes: 5 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/b/b.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
let _c1 = B_lib.v

let _c2 = Bigarray.C_layout_typ

let () = Printf.eprintf "Welcome to b\n%!"
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/bigarray.t/b/b_lib.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let v = Bigarray.C_layout_typ
10 changes: 10 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/b/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
(library
(name b_lib)
(libraries
(re_export bigarray))
(modules b_lib))

(executable
(name b)
(libraries b_lib)
(modules b))
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/c/c.bigarray.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let _c = Bigarray.C_layout_typ

let () = Printf.eprintf "Welcome to c WITH bigarray support\n%!"
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/bigarray.t/c/c.dummy.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let () = Printf.eprintf "Welcome to c with nothing inferred\n%!"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let () = Printf.eprintf "Welcome to c WITHOUT bigarray support\n%!"
9 changes: 9 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/c/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
(executable
(name c)
(libraries
(select
c.ml
from
(!bigarray -> c.nobigarray.ml)
(bigarray -> c.bigarray.ml)
(-> c.dummy.ml))))
3 changes: 3 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/dune-project
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(lang dune 3.0)

(implicit_transitive_deps false)
33 changes: 33 additions & 0 deletions test/blackbox-tests/test-cases/bigarray.t/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
This tests the support for the bigarray atom in the dune libraries stanza.

History:
- OCaml 4.05 ([ocaml/ocaml#997](https://github.com/ocaml/ocaml/pull/997) and
[ocaml/ocaml#1077](https://github.com/ocaml/ocaml/pull/1077)) add
`Unix.map_file` allowing the `map_file` functions in `Bigarray` to be
deprecated. Bigarray remains a separate library.
- OCaml 4.07 ([ocaml/ocaml#1685](https://github.com/ocaml/ocaml/pull/1685))
adds `Stdlib.Bigarray`, but without the `map_file` functions (since these
required `Unix.file_descr`. The separate library remains with those
functions (but still marked as deprecated). Code can be updated to use
Unix.map_file and then Stdlib.Bigarray and not require the separate library
at all, but the separate remains compatible with OCaml 4.06.
- OCaml 4.08 ([ocaml/ocaml#2263](https://github.com/ocaml/ocaml/pull/2263))
deletes the `map_file` functions completely, requiring _all_ code to be
updated to use `Unix.map_file`, if appropriate. From this release, it is
unnecessary to link with the separate Bigarray library.
- OCaml 5.00 ([ocaml/ocaml#10896](https://github.com/ocaml/ocaml/pull/10896)
removes the separate Bigarray library.

Code may be written which is designed to support both OCaml 4.06 and earlier and
also OCaml 5.0+. In such cases, it is appropriate to have `(libraries bigarray)`
even though there is no Bigarray library in OCaml 5.

This test uses `(libraries bigarray)` (the program uses `Bigarray`)
$ dune exec a/a.exe
Welcome to a
This test uses `(libraries (re_export bigarray))` similarly
$ dune exec b/b.exe
Welcome to b
This test uses a `(select )` construct and should always select bigarray support
$ dune exec c/c.exe
Welcome to c WITH bigarray support