Skip to content

Commit

Permalink
Fix the priority order for environment variables
Browse files Browse the repository at this point in the history
    between workspace and context. Use `env-vars` from the `dune-workspace` for
    dune.

Signed-off-by: François Bobot <francois.bobot@cea.fr>
  • Loading branch information
bobot committed Jul 30, 2019
1 parent ed64846 commit 88a6d8e
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 14 deletions.
16 changes: 9 additions & 7 deletions doc/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -501,13 +501,15 @@ context or can be the description of an opam switch, as follows:
are meant to be executed on the host machine, such as preprocessors.

- ``(paths (<var1> <val1>) .. (<varN> <valN>))`` allows to set the value of any
``PATH``-like variables in this context. If ``PATH`` itself is modified in
this way, its value will be used to resolve binaries in the workspace,
including finding the compiler and related tools. These variables will also be
passed as part of the environment to any program launched by ``dune``. For
each variable, the value is specified using the :ref:`ordered-set-language`.
Relative paths are interpreted with respect to the workspace root, see
:ref:`finding-root`.
``PATH``-like variables in this context. These variables are passed as part of
the environment to any program launched by ``dune``. For each variable, the
value is specified using the :ref:`ordered-set-language`. Relative paths are
interpreted with respect to the workspace root, see :ref:`finding-root`.

If environmeent variables, such as ``PATH``, are modified in the
`dune-workspace` through `env-vars` or `paths`, its value will be used to
resolve binaries in the workspace, including finding the compiler and related
tools.

Both ``(default ...)`` and ``(opam ...)`` accept a ``targets`` field in order to
setup cross compilation. See :ref:`advanced-cross-compilation` for more
Expand Down
15 changes: 9 additions & 6 deletions src/context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ module Env_nodes = struct

let extra_env ~profile env_nodes =
Env.extend_env
(Dune_env.Stanza.find env_nodes.context ~profile).env_vars
(Dune_env.Stanza.find env_nodes.workspace ~profile).env_vars
(Dune_env.Stanza.find env_nodes.context ~profile).env_vars
end

type t =
Expand Down Expand Up @@ -407,7 +407,6 @@ let create ~(kind : Kind.t) ~path ~env ~env_nodes ~name ~merlin ~targets
Option.value ~default:Env.empty
(Option.map findlib_config ~f:Findlib.Config.env)
)
|> Env.extend_env (Env_nodes.extra_env ~profile env_nodes)
in
let stdlib_dir = Path.of_string (Ocaml_config.standard_library ocfg) in
let natdynlink_supported = Ocaml_config.natdynlink_supported ocfg in
Expand Down Expand Up @@ -646,9 +645,15 @@ let instantiate_context env (workspace : Workspace.t)
; workspace = workspace.env
}
in
let env =
Env.extend_env env
(Env_nodes.extra_env
~profile:(Workspace.Context.profile context) env_nodes)
in
let env = extend_paths ~env (Workspace.Context.paths context) in
match context with
| Default { targets; name; host_context = _; profile; env = _
; toolchain ; paths; loc = _ } ->
; toolchain ; paths = _; loc = _ } ->
let merlin =
workspace.merlin_context = Some (Workspace.Context.name context)
in
Expand All @@ -657,13 +662,11 @@ let instantiate_context env (workspace : Workspace.t)
| Some _ -> toolchain
| None -> Env.get env "OCAMLFIND_TOOLCHAIN"
in
let env = extend_paths ~env paths in
default ~env ~env_nodes ~profile ~targets ~name ~merlin ~host_context
~host_toolchain
| Opam { base = { targets; name; host_context = _; profile; env = _
; toolchain; paths; loc = _ }
; toolchain; paths = _; loc = _ }
; switch; root; merlin } ->
let env = extend_paths ~env paths in
create_for_opam ~root ~env_nodes ~env ~profile ~switch ~name ~merlin
~targets ~host_context ~host_toolchain:toolchain

Expand Down
1 change: 1 addition & 0 deletions src/stdune/env.mli
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ val get : t -> Var.t -> string option
val extend : t -> vars:string Map.t -> t

val extend_env : t -> t -> t
(** [extend_env x y] gives priority to the values of y *)

val add : t -> var:Var.t -> value:string -> t

Expand Down
8 changes: 8 additions & 0 deletions src/workspace.ml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ module Context = struct
| Default x -> x.targets
| Opam x -> x.base.targets

let profile = function
| Default x -> x.profile
| Opam x -> x.base.profile

let paths = function
| Default x -> x.paths
| Opam x -> x.base.paths

let all_names t =
let n = name t in
n :: List.filter_map (targets t) ~f:(function
Expand Down
3 changes: 3 additions & 0 deletions src/workspace.mli
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ module Context : sig

val env : t -> Dune_env.Stanza.t

val profile : t -> string
val paths : t -> (string * Ordered_set_lang.t) list

val host_context : t -> string option
end

Expand Down
2 changes: 1 addition & 1 deletion test/blackbox-tests/test-cases/env-variables/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ used.
$ dune exec --root precedence ./printenv.exe VARIABLE_FROM_BOTH
Entering directory 'precedence'
Entering directory 'precedence'
VARIABLE_FROM_BOTH=from_workspace
VARIABLE_FROM_BOTH=from_context

When a variable is repeated, an error message is displayed:

Expand Down

0 comments on commit 88a6d8e

Please sign in to comment.