Merge pull request #1114 from diml/envs-and-contexts

Fix a bad interaction between `(env ...)` and `(merlin)` in the workspace file

CHANGES.md

@@ -16,6 +16,9 @@ next
   applies when the `name` is derived from the `public_name`. (#1110, fix #1102,
   @rgrinberg)
 
+- Fix a bug causing the toplevel `env` stanza in the workspace file to
+  be ignored when at least one context had `(merlin)` (#1114, @diml)
+
 1.1.0 (06/08/2018)
 ------------------
 

src/workspace.ml

@@ -167,28 +167,22 @@ let t ?x ?profile:cmdline_profile () =
   multi_field "context" (Context.t ~profile ~x)
   >>| fun contexts ->
   let defined_names = ref String.Set.empty in
-  let { merlin_context; contexts; env } =
-    let init =
-      { merlin_context = None
-      ; contexts       = []
-      ; env
-      }
-    in
-    List.fold_left contexts ~init ~f:(fun t ctx ->
+  let merlin_context =
+    List.fold_left contexts ~init:None ~f:(fun acc ctx ->
       let name = Context.name ctx in
       if String.Set.mem !defined_names name then
         Loc.fail (Context.loc ctx)
           "second definition of build context %S" name;
       defined_names := String.Set.union !defined_names
                          (String.Set.of_list (Context.all_names ctx));
-      match ctx, t.merlin_context with
+      match ctx, acc with
       | Opam { merlin = true; _ }, Some _ ->
         Loc.fail (Context.loc ctx)
           "you can only have one context for merlin"
       | Opam { merlin = true; _ }, None ->
-        { merlin_context = Some name; contexts = ctx :: t.contexts; env = None }
+        Some name
       | _ ->
-        { t with contexts = ctx :: t.contexts })
+        acc)
   in
   let contexts =
     match contexts with

test/blackbox-tests/dune.inc

@@ -151,6 +151,14 @@
    test-cases/env
    (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))
 
+(alias
+ (name envs-and-contexts)
+ (deps (package dune) (source_tree test-cases/envs-and-contexts))
+ (action
+  (chdir
+   test-cases/envs-and-contexts
+   (progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))
+
 (alias
  (name exclude-missing-module)
  (deps (package dune) (source_tree test-cases/exclude-missing-module))
@@ -944,6 +952,6 @@
   (alias windows-diff)
   (alias workspaces)))
 
-(alias (name runtest-disabled) (deps))
+(alias (name runtest-disabled) (deps (alias envs-and-contexts)))
 
 (alias (name runtest-js) (deps (alias js_of_ocaml)))

test/blackbox-tests/gen_tests.ml

@@ -139,6 +139,9 @@ let exclusions =
   ; make "github764" ~skip_platforms:[Win]
   ; make "gen-opam-install-file" ~external_deps:true
   ; make "scope-ppx-bug" ~external_deps:true
+  (* The next test is disabled as it relies on configured opam
+     swtiches and it's hard to get that working properly *)
+  ; make "envs-and-contexts" ~external_deps:true ~enabled:false
   ]
 
 let all_tests = lazy (

test/blackbox-tests/test-cases/envs-and-contexts/dune-project

@@ -0,0 +1 @@
+(lang dune 1.1)

test/blackbox-tests/test-cases/envs-and-contexts/dune-workspace

@@ -0,0 +1,10 @@
+(lang dune 1.1)
+
+(context (opam (switch default) (name dev) (profile dev) (merlin)))
+(context (opam (switch default) (name release) (profile release)))
+
+(env
+ (dev
+  (flags dev-flags))
+ (release
+  (flags release-flags)))

test/blackbox-tests/test-cases/envs-and-contexts/run.t

@@ -0,0 +1,15 @@
+Regression test for https://github.com/ocaml/dune/issues/1016#issuecomment-411390740
+
+  $ dune printenv
+  Environment for context dev:
+    (
+     (flags (dev-flags))
+     (ocamlc_flags (-g))
+     (ocamlopt_flags (-g))
+    )
+  Environment for context release:
+    (
+     (flags (release-flags))
+     (ocamlc_flags (-g))
+     (ocamlopt_flags (-g))
+    )