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(melange): treat private libraries with (package ..) as public libs #10415

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
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
anmonteiro marked this conversation as resolved.
Show resolved Hide resolved
?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
Loading