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 evaluate-time error with MDX stanza and unavailable packages #3650

Merged
merged 3 commits into from
Jul 28, 2020
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 CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ next
- Insert correct extension name when editing `dune-project` files. Previously,
dune would just insert the stanza name. (#3649, fixes #3624, @rgrinberg)

- Fix crash when evaluating an `mdx` stanza that depends on unavailable
packages. (#3650, @CraigFe)

2.6.1 (02/07/2020)
------------------

Expand Down
2 changes: 1 addition & 1 deletion src/dune/gen_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ end = struct
Cinaps.gen_rules sctx cinaps ~dir ~scope;
empty_none
| Mdx.T mdx ->
Mdx.gen_rules ~sctx ~dir mdx;
Mdx.gen_rules ~sctx ~dir ~expander mdx;
empty_none
| _ -> empty_none

Expand Down
22 changes: 11 additions & 11 deletions src/dune/mdx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ end
type t =
{ loc : Loc.t
; files : Predicate_lang.Glob.t
; packages : Package.Name.t list
; packages : (Loc.t * Package.Name.t) list
; preludes : Prelude.t list
}

Expand All @@ -139,7 +139,8 @@ let decode =
(let+ loc = loc
and+ files =
field "files" Predicate_lang.Glob.decode ~default:default_files
and+ packages = field ~default:[] "packages" (repeat Package.Name.decode)
and+ packages =
field ~default:[] "packages" (repeat (located Package.Name.decode))
and+ preludes = field ~default:[] "preludes" (repeat Prelude.decode) in
{ loc; files; packages; preludes })

Expand Down Expand Up @@ -169,7 +170,7 @@ let files_to_mdx t ~sctx ~dir =

(** Generates the rules for a single [src] file covered covered by the given
[stanza]. *)
let gen_rules_for_single_file stanza ~sctx ~dir ~mdx_prog src =
let gen_rules_for_single_file stanza ~sctx ~dir ~expander ~mdx_prog src =
let loc = stanza.loc in
let files = Files.from_source_file src in
(* Add the rule for generating the .mdx.deps file with ocaml-mdx deps *)
Expand All @@ -180,17 +181,15 @@ let gen_rules_for_single_file stanza ~sctx ~dir ~mdx_prog src =
let deps = Build.map (Deps.read files) ~f:(Deps.to_dep_set ~dir) in
let dyn_deps = Build.map deps ~f:(fun d -> ((), d)) in
let pkg_deps =
let context = Super_context.context sctx in
let packages = Super_context.packages sctx in
stanza.packages
|> List.map ~f:(fun pkg ->
let pkg = Package.Name.Map.find_exn packages pkg in
Build.alias (Build_system.Alias.package_install ~context ~pkg))
|> List.map ~f:(fun (loc, pkg) ->
Dep_conf.Package
(Package.Name.to_string pkg |> String_with_vars.make_text loc))
in
let prelude_args =
List.concat_map stanza.preludes ~f:(Prelude.to_args ~dir)
in
Build.(with_no_targets (all_unit pkg_deps))
Build.(with_no_targets (Dep_conf_eval.unnamed ~expander pkg_deps))
>>> Build.with_no_targets (Build.dyn_deps dyn_deps)
>>> Command.run ~dir:(Path.build dir) mdx_prog
( [ Command.Args.A "test" ] @ prelude_args
Expand All @@ -204,10 +203,11 @@ let gen_rules_for_single_file stanza ~sctx ~dir ~mdx_prog src =
(Build.with_no_targets diff_action)

(** Generates the rules for a given mdx stanza *)
let gen_rules t ~sctx ~dir =
let gen_rules t ~sctx ~dir ~expander =
let files_to_mdx = files_to_mdx t ~sctx ~dir in
let mdx_prog =
Super_context.resolve_program sctx ~dir ~loc:(Some t.loc)
~hint:"opam install mdx" "ocaml-mdx"
in
List.iter files_to_mdx ~f:(gen_rules_for_single_file t ~sctx ~dir ~mdx_prog)
List.iter files_to_mdx
~f:(gen_rules_for_single_file t ~sctx ~dir ~expander ~mdx_prog)
3 changes: 2 additions & 1 deletion src/dune/mdx.mli
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ type t
type Stanza.t += T of t

(** Genrates the rules to handle the given mdx stanza *)
val gen_rules : t -> sctx:Super_context.t -> dir:Path.Build.t -> unit
val gen_rules :
t -> sctx:Super_context.t -> dir:Path.Build.t -> expander:Expander.t -> unit
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```ocaml
This isn't valid OCaml, so running MDX would fail.
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(library
(name public_lib)
(public_name pkg))
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(mdx
(files README.md)
(packages pkg))
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
(lang dune 2.4)
(name pkg)

(using mdx 0.1)
Empty file.
17 changes: 15 additions & 2 deletions test/blackbox-tests/test-cases/mdx-stanza.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,21 @@ or mli to-be-mdxed file depends upon
You can make local packages available to mdx by using the `packages` field of
the stanza

$ dune runtest --root local-packages
Entering directory 'local-packages'
$ dune runtest --root local-package
Entering directory 'local-package'

Dune does not fail if the `packages` are not available at evaluation time
(regression test fixed by ocaml/dune#3650)

$ cd local-package-unrelated && dune build -p unrelated-package; cd ../

Dune fails if the `packages` are not avaliable at execution time

$ cd local-package-unrelated && dune runtest -p unrelated-package; cd ../
File "dune", line 3, characters 11-14:
3 | (packages pkg))
^^^
Error: Package pkg does not exist

You can set MDX preludes using the preludes field of the stanza

Expand Down