Skip to content

Commit

Permalink
fix(melange): treat private libraries with (package ..) as public l…
Browse files Browse the repository at this point in the history
…ibs (#10415)

Signed-off-by: Antonio Nuno Monteiro <[email protected]>
  • Loading branch information
anmonteiro authored Apr 12, 2024
1 parent c2a00c9 commit 4e236e4
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 39 deletions.
3 changes: 3 additions & 0 deletions doc/changes/10415.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- melange: treat private libraries with `(package ..)` as public libraries,
fixing an issue where `import` paths were wrongly emitted. (#10415,
@anmonteiro)
1 change: 1 addition & 0 deletions src/dune_rules/cinaps.ml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ let gen_rules sctx t ~dir ~scope =
~requires_link
~flags:(Ocaml_flags.of_list [ "-w"; "-24" ])
~js_of_ocaml:None
~melange_package_name:None
~package:None
in
let* (_ : Exe.dep_graphs) =
Expand Down
8 changes: 4 additions & 4 deletions src/dune_rules/compilation_context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type t =
; sandbox : Sandbox_config.t
; package : Package.t option
; vimpl : Vimpl.t option
; public_lib_name : Lib_name.t option
; melange_package_name : Lib_name.t option
; modes : Lib_mode.Map.Set.t
; bin_annot : bool
; ocamldep_modules_data : Ocamldep.Modules_data.t
Expand All @@ -108,7 +108,7 @@ let js_of_ocaml t = t.js_of_ocaml
let sandbox t = t.sandbox
let set_sandbox t sandbox = { t with sandbox }
let package t = t.package
let public_lib_name t = t.public_lib_name
let melange_package_name t = t.melange_package_name
let vimpl t = t.vimpl
let modes t = t.modes
let bin_annot t = t.bin_annot
Expand All @@ -130,7 +130,7 @@ let create
?stdlib
~js_of_ocaml
~package
?public_lib_name
~melange_package_name
?vimpl
?modes
?bin_annot
Expand Down Expand Up @@ -197,7 +197,7 @@ let create
; sandbox
; package
; vimpl
; public_lib_name
; melange_package_name
; modes
; bin_annot
; ocamldep_modules_data
Expand Down
4 changes: 2 additions & 2 deletions src/dune_rules/compilation_context.mli
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ val create
-> ?stdlib:Ocaml_stdlib.t
-> js_of_ocaml:Js_of_ocaml.In_context.t option
-> package:Package.t option
-> ?public_lib_name:Lib_name.t
-> melange_package_name:Lib_name.t option
-> ?vimpl:Vimpl.t
-> ?modes:Mode_conf.Set.Details.t Lib_mode.Map.t
-> ?bin_annot:bool
Expand Down Expand Up @@ -65,7 +65,7 @@ val sandbox : t -> Sandbox_config.t
val set_sandbox : t -> Sandbox_config.t -> t
val package : t -> Package.t option
val vimpl : t -> Vimpl.t option
val public_lib_name : t -> Lib_name.t option
val melange_package_name : t -> Lib_name.t option
val modes : t -> Lib_mode.Map.Set.t
val for_wrapped_compat : t -> t
val for_root_module : t -> Module.t -> t
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/exe_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ let executables_rules
~preprocessing:pp
~js_of_ocaml
~opaque:Inherit_from_settings
~melange_package_name:None
~package:exes.package
in
let lib_config = ocaml.lib_config in
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/inline_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ include Sub_system.Register_end_point (struct
~requires_link:(Memo.lazy_ (fun () -> runner_libs))
~flags
~js_of_ocaml:(Some js_of_ocaml)
~melange_package_name:None
~package
in
let linkages =
Expand Down
14 changes: 8 additions & 6 deletions src/dune_rules/lib_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -528,14 +528,16 @@ let cctx (lib : Library.t) ~sctx ~source_modules ~dir ~expander ~scope ~compile_
in
let package = Library.package lib in
let js_of_ocaml = Js_of_ocaml.In_context.make ~dir lib.buildable.js_of_ocaml in
(* XXX(anmonteiro): `public_lib_name` is used to derive Melange's
(* XXX(anmonteiro): `melange_package_name` is used to derive Melange's
`--bs-package-name` argument. We only use the library name for public
libraries because we need melange to preserve relative paths for private
libs (i.e. not pass the `--bs-package-name` arg). *)
let public_lib_name =
libraries / private libraries with `(package ..)` because we need Melange
to preserve relative paths for private libs (i.e. not pass the
`--bs-package-name` arg). *)
let melange_package_name =
match lib.visibility with
| Public p -> Some (Public_lib.name p)
| Private _ -> None
| Private (Some pkg) -> Some (Lib_name.mangled (Package.name pkg) (snd lib.name))
| Private None -> None
in
Compilation_context.create
()
Expand All @@ -552,7 +554,7 @@ let cctx (lib : Library.t) ~sctx ~source_modules ~dir ~expander ~scope ~compile_
?stdlib:lib.stdlib
~package
?vimpl
?public_lib_name
~melange_package_name
~modes
;;

Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/mdx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ let mdx_prog_gen t ~sctx ~dir ~scope ~mdx_prog =
~requires_link
~opaque:(Explicit false)
~js_of_ocaml:None
~melange_package_name:None
~package:None
()
in
Expand Down
24 changes: 16 additions & 8 deletions src/dune_rules/melange/melange_rules.ml
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
open Import
open Memo.O

let output_of_lib ~target_dir lib =
let info = Lib.info lib in
match Lib_info.status info with
| Private _ -> `Private_library_or_emit target_dir
| Installed | Installed_private | Public _ ->
let lib_name = Lib_info.name info in
let src_dir = Lib_info.src_dir info in
let output_of_lib =
let public_lib ~info ~target_dir lib_name =
`Public_library
( src_dir
( Lib_info.src_dir info
, Path.Build.L.relative target_dir [ "node_modules"; Lib_name.to_string lib_name ]
)
in
fun ~target_dir lib ->
let info = Lib.info lib in
match Lib_info.status info with
| Private (_, None) -> `Private_library_or_emit target_dir
| Private (_, Some pkg) ->
public_lib
~info
~target_dir
(Lib_name.mangled (Package.name pkg) (Lib_name.to_local_exn (Lib.name lib)))
| Installed | Installed_private | Public _ ->
public_lib ~info ~target_dir (Lib_info.name info)
;;

let lib_output_path ~output_dir ~lib_dir src =
Expand Down Expand Up @@ -298,6 +305,7 @@ let setup_emit_cmj_rules
~preprocessing:pp
~js_of_ocaml:None
~opaque:Inherit_from_settings
~melange_package_name:None
~package:mel.package
~modes:
{ ocaml = { byte = None; native = None }; melange = Some (Requested mel.loc) }
Expand Down
2 changes: 1 addition & 1 deletion src/dune_rules/module_compilation.ml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let melange_args (cctx : Compilation_context.t) (cm_kind : Lib_mode.Cm_kind.t) m
let package_output =
Module.file ~ml_kind:Impl module_ |> Option.value_exn |> Path.parent_exn
in
match Compilation_context.public_lib_name cctx with
match Compilation_context.melange_package_name cctx with
| None -> [], package_output
| Some lib_name ->
let dir =
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/ppx_driver.ml
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ let build_ppx_driver sctx ~scope ~target ~pps ~pp_names =
~requires_link
~opaque
~js_of_ocaml:None
~melange_package_name:None
~package:None
~bin_annot:false
()
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/toplevel.ml
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ module Stanza = struct
~requires_link
~flags
~js_of_ocaml:None
~melange_package_name:None
~package:None
~preprocessing
in
Expand Down
1 change: 1 addition & 0 deletions src/dune_rules/utop.ml
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ let setup sctx ~dir =
~requires_compile:requires
~flags
~js_of_ocaml:None
~melange_package_name:None
~package:None
~preprocessing
in
Expand Down
30 changes: 15 additions & 15 deletions test/blackbox-tests/test-cases/melange/emit-private.t
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Test dependency on a private library in the same package as melange.emit

$ cat >dune-project <<EOF
> (lang dune 3.8)
> (package (name a))
> (package (name pkg))
> (using melange 0.1)
> EOF

Expand All @@ -12,7 +12,7 @@ Test dependency on a private library in the same package as melange.emit
> (library
> (name a)
> (modes melange)
> (package a))
> (package pkg))
> EOF

$ cat > a/foo.ml <<EOF
Expand All @@ -22,33 +22,33 @@ Test dependency on a private library in the same package as melange.emit
$ dune build

$ dune install --prefix $PWD/prefix --display short
Installing $TESTCASE_ROOT/prefix/lib/a/META
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/a.ml
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/foo.ml
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/melange/.public_cmi_melange/a.cmi
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/melange/.public_cmi_melange/a.cmt
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/melange/.public_cmi_melange/a__Foo.cmi
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/melange/.public_cmi_melange/a__Foo.cmt
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/melange/a.cmj
Installing $TESTCASE_ROOT/prefix/lib/a/__private__/a/melange/a__Foo.cmj
Installing $TESTCASE_ROOT/prefix/lib/a/dune-package
Installing $TESTCASE_ROOT/prefix/lib/pkg/META
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/a.ml
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/foo.ml
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/melange/.public_cmi_melange/a.cmi
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/melange/.public_cmi_melange/a.cmt
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/melange/.public_cmi_melange/a__Foo.cmi
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/melange/.public_cmi_melange/a__Foo.cmt
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/melange/a.cmj
Installing $TESTCASE_ROOT/prefix/lib/pkg/__private__/a/melange/a__Foo.cmj
Installing $TESTCASE_ROOT/prefix/lib/pkg/dune-package

$ cat > b/dune <<EOF
> (melange.emit
> (target dist)
> (alias dist)
> (libraries a)
> (emit_stdlib false)
> (package a))
> (package pkg))
> EOF

$ cat > b/bar.ml <<EOF
> let x = Js.log A.Foo.x
> EOF

$ OCAMLPATH=$PWD/prefix/lib/:$OCAMLPATH dune build @dist --display=short 2>&1 | grep -v melange
melc b/dist/a/a.js
melc b/dist/a/foo.js
melc b/dist/node_modules/pkg.__private__.a/a.js
melc b/dist/node_modules/pkg.__private__.a/foo.js
melc b/dist/b/bar.js

$ node _build/default/b/dist/b/bar.js
Expand Down
6 changes: 3 additions & 3 deletions test/blackbox-tests/test-cases/melange/private-lib-dep.t
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ Melange public library depends on private library
$ dune build @melange
$ node _build/default/output/entry.js 2>&1 | grep 'Cannot find module'
Error: Cannot find module 'priv/priv.js'
$ node _build/default/output/entry.js
public lib uses private
$ cat _build/default/output/node_modules/foo/foo.js
// Generated by Melange
'use strict';
let Priv = require("priv/priv.js");
let Priv = require("foo.__private__.priv/priv.js");
const x = "public lib uses " + Priv.x;
Expand Down

0 comments on commit 4e236e4

Please sign in to comment.