Merge pull request #3650 from CraigFe/mdx-support-package-selection

Fix evaluate-time error with MDX stanza and unavailable packages

CHANGES.md

@@ -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)
 ------------------
 

src/dune/gen_rules.ml

@@ -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
 

src/dune/mdx.ml

@@ -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
   }
 
@@ -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 })
 
@@ -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 *)
@@ -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
@@ -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)

src/dune/mdx.mli

@@ -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

test/blackbox-tests/test-cases/mdx-stanza.t/local-package-unrelated/README.md

@@ -0,0 +1,3 @@
+```ocaml
+This isn't valid OCaml, so running MDX would fail.
+```

test/blackbox-tests/test-cases/mdx-stanza.t/local-package-unrelated/lib/dune

@@ -0,0 +1,3 @@
+(library
+ (name public_lib)
+ (public_name pkg))

test/blackbox-tests/test-cases/mdx-stanza.t/local-package/dune

@@ -0,0 +1,3 @@
+(mdx
+ (files README.md)
+ (packages pkg))

test/blackbox-tests/test-cases/mdx-stanza.t/local-package/dune-project

@@ -0,0 +1,4 @@
+(lang dune 2.4)
+(name pkg)
+
+(using mdx 0.1)

test/blackbox-tests/test-cases/mdx-stanza.t/run.t

@@ -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