From 3e33d2353593f3bc82d3284d0851260eb36fade5 Mon Sep 17 00:00:00 2001 From: Jeremie Dimino Date: Wed, 8 Aug 2018 15:15:06 +0100 Subject: [PATCH 1/4] Expose a bug involving environments and build contexts Signed-off-by: Jeremie Dimino --- test/blackbox-tests/dune.inc | 10 +++++++++ .../test-cases/envs-and-contexts/dune-project | 1 + .../envs-and-contexts/dune-workspace | 10 +++++++++ .../test-cases/envs-and-contexts/run.t | 21 +++++++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 test/blackbox-tests/test-cases/envs-and-contexts/dune-project create mode 100644 test/blackbox-tests/test-cases/envs-and-contexts/dune-workspace create mode 100644 test/blackbox-tests/test-cases/envs-and-contexts/run.t diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index b8c3cdae..b3167fc5 100644 --- a/test/blackbox-tests/dune.inc +++ b/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)) @@ -778,6 +786,7 @@ (alias dune-project-edition) (alias dup-fields) (alias env) + (alias envs-and-contexts) (alias exclude-missing-module) (alias exec-cmd) (alias exec-missing) @@ -873,6 +882,7 @@ (alias dune-project-edition) (alias dup-fields) (alias env) + (alias envs-and-contexts) (alias exclude-missing-module) (alias exec-cmd) (alias exec-missing) diff --git a/test/blackbox-tests/test-cases/envs-and-contexts/dune-project b/test/blackbox-tests/test-cases/envs-and-contexts/dune-project new file mode 100644 index 00000000..6687faf2 --- /dev/null +++ b/test/blackbox-tests/test-cases/envs-and-contexts/dune-project @@ -0,0 +1 @@ +(lang dune 1.1) \ No newline at end of file diff --git a/test/blackbox-tests/test-cases/envs-and-contexts/dune-workspace b/test/blackbox-tests/test-cases/envs-and-contexts/dune-workspace new file mode 100644 index 00000000..4557c44c --- /dev/null +++ b/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))) diff --git a/test/blackbox-tests/test-cases/envs-and-contexts/run.t b/test/blackbox-tests/test-cases/envs-and-contexts/run.t new file mode 100644 index 00000000..7e1eb276 --- /dev/null +++ b/test/blackbox-tests/test-cases/envs-and-contexts/run.t @@ -0,0 +1,21 @@ +Regression test for https://github.com/ocaml/dune/issues/1016#issuecomment-411390740 + + $ dune printenv + Environment for context dev: + ( + (flags + (-w + @a-4-29-40-41-42-44-45-48-58-59-60-40 + -strict-sequence + -strict-formats + -short-paths + -keep-locs)) + (ocamlc_flags (-g)) + (ocamlopt_flags (-g)) + ) + Environment for context release: + ( + (flags (-w -40)) + (ocamlc_flags (-g)) + (ocamlopt_flags (-g)) + ) From 6a9c32dcb4a2f0d28a48b67a138818188cbe20f3 Mon Sep 17 00:00:00 2001 From: Jeremie Dimino Date: Wed, 8 Aug 2018 15:23:36 +0100 Subject: [PATCH 2/4] Fix bug exposed by previous commit Signed-off-by: Jeremie Dimino --- CHANGES.md | 3 +++ src/workspace.ml | 2 +- test/blackbox-tests/test-cases/envs-and-contexts/run.t | 10 ++-------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a28e1a7c..952c61cf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,9 @@ next - Fix #1107. `-opaque` wasn't correctly being added to modules without an interface. (#1108, fix #1107, @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) ------------------ diff --git a/src/workspace.ml b/src/workspace.ml index 9a6cccd8..1723ae3a 100644 --- a/src/workspace.ml +++ b/src/workspace.ml @@ -186,7 +186,7 @@ let t ?x ?profile:cmdline_profile () = 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 } + { t with contexts = ctx :: t.contexts; merlin_context = Some name } | _ -> { t with contexts = ctx :: t.contexts }) in diff --git a/test/blackbox-tests/test-cases/envs-and-contexts/run.t b/test/blackbox-tests/test-cases/envs-and-contexts/run.t index 7e1eb276..2584a89d 100644 --- a/test/blackbox-tests/test-cases/envs-and-contexts/run.t +++ b/test/blackbox-tests/test-cases/envs-and-contexts/run.t @@ -3,19 +3,13 @@ Regression test for https://github.com/ocaml/dune/issues/1016#issuecomment-41139 $ dune printenv Environment for context dev: ( - (flags - (-w - @a-4-29-40-41-42-44-45-48-58-59-60-40 - -strict-sequence - -strict-formats - -short-paths - -keep-locs)) + (flags (dev-flags)) (ocamlc_flags (-g)) (ocamlopt_flags (-g)) ) Environment for context release: ( - (flags (-w -40)) + (flags (release-flags)) (ocamlc_flags (-g)) (ocamlopt_flags (-g)) ) From 2913d3e501adae4330269778d696f98f6005b8f6 Mon Sep 17 00:00:00 2001 From: Jeremie Dimino Date: Wed, 8 Aug 2018 15:55:52 +0100 Subject: [PATCH 3/4] Refactor a bit some code in workspace.ml Signed-off-by: Jeremie Dimino --- src/workspace.ml | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/workspace.ml b/src/workspace.ml index 1723ae3a..aec4cf56 100644 --- a/src/workspace.ml +++ b/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 -> - { t with contexts = ctx :: t.contexts; merlin_context = Some name } + Some name | _ -> - { t with contexts = ctx :: t.contexts }) + acc) in let contexts = match contexts with From 8ea075dd6b2faad54f2518fe5d56de99b4269262 Mon Sep 17 00:00:00 2001 From: Jeremie Dimino Date: Wed, 8 Aug 2018 17:29:53 +0100 Subject: [PATCH 4/4] Disable last added test Signed-off-by: Jeremie Dimino --- test/blackbox-tests/dune.inc | 4 +--- test/blackbox-tests/gen_tests.ml | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index b3167fc5..a2971213 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -786,7 +786,6 @@ (alias dune-project-edition) (alias dup-fields) (alias env) - (alias envs-and-contexts) (alias exclude-missing-module) (alias exec-cmd) (alias exec-missing) @@ -882,7 +881,6 @@ (alias dune-project-edition) (alias dup-fields) (alias env) - (alias envs-and-contexts) (alias exclude-missing-module) (alias exec-cmd) (alias exec-missing) @@ -944,6 +942,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))) \ No newline at end of file diff --git a/test/blackbox-tests/gen_tests.ml b/test/blackbox-tests/gen_tests.ml index 85c413b4..02925545 100644 --- a/test/blackbox-tests/gen_tests.ml +++ b/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 (