From 8458bf3b1580e1d3520265efd21791e2548e793e Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sat, 10 Mar 2018 20:33:59 +0700
Subject: [PATCH 01/14] Move env related functions to Env module

---
 src/action.ml   | 10 +++----
 src/context.ml  | 79 ++++++++++---------------------------------------
 src/context.mli | 13 +-------
 src/env.ml      | 51 +++++++++++++++++++++++++++++++
 src/env.mli     | 13 ++++++++
 5 files changed, 85 insertions(+), 81 deletions(-)
 create mode 100644 src/env.ml
 create mode 100644 src/env.mli

diff --git a/src/action.ml b/src/action.ml
index 24fb19b9..153a700b 100644
--- a/src/action.ml
+++ b/src/action.ml
@@ -1,8 +1,6 @@
 open Import
 open Sexp.Of_sexp
 
-module Env_var_map = Context.Env_var_map
-
 module Outputs = struct
   include Action_intf.Outputs
 
@@ -717,7 +715,7 @@ let exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args =
      invalid_prefix ("_build/" ^ target.name);
      invalid_prefix ("_build/install/" ^ target.name);
   end;
-  let env = Context.extend_env ~vars:env_extra ~env:ectx.env in
+  let env = Env.extend_env ~vars:env_extra ~env:ectx.env in
   Process.run Strict ~dir:(Path.to_string dir) ~env ~stdout_to ~stderr_to
     ~purpose:ectx.purpose
     (Path.reach_for_running ~from:dir prog) args
@@ -743,7 +741,7 @@ let rec exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
     exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to
   | Setenv (var, value, t) ->
     exec t ~ectx ~dir ~stdout_to ~stderr_to
-      ~env_extra:(Env_var_map.add env_extra var value)
+      ~env_extra:(Env.Map.add env_extra var value)
   | Redirect (Stdout, fn, Echo s) ->
     Io.write_file (Path.to_string fn) s;
     Fiber.return ()
@@ -894,13 +892,13 @@ and exec_list l ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
 let exec ~targets ?context t =
   let env =
     match (context : Context.t option) with
-    | None -> Lazy.force Context.initial_env
+    | None -> Lazy.force Env.initial_env
     | Some c -> c.env
   in
   let targets = Path.Set.to_list targets in
   let purpose = Process.Build_job targets in
   let ectx = { purpose; context; env } in
-  exec t ~ectx ~dir:Path.root ~env_extra:Env_var_map.empty
+  exec t ~ectx ~dir:Path.root ~env_extra:Env.Map.empty
     ~stdout_to:None ~stderr_to:None
 
 let sandbox t ~sandboxed ~deps ~targets =
diff --git a/src/context.ml b/src/context.ml
index fbd2e8fe..b26d9562 100644
--- a/src/context.ml
+++ b/src/context.ml
@@ -18,17 +18,6 @@ module Kind = struct
                            ])
 end
 
-module Env_var = struct
-  type t = string
-  let compare a b =
-    if Sys.win32 then
-      String.compare (String.lowercase a) (String.lowercase b)
-    else
-      String.compare a b
-end
-
-module Env_var_map = Map.Make(Env_var)
-
 type t =
   { name                    : string
   ; kind                    : Kind.t
@@ -45,7 +34,7 @@ type t =
   ; ocamldep                : Path.t
   ; ocamlmklib              : Path.t
   ; env                     : string array
-  ; env_extra               : string Env_var_map.t
+  ; env_extra               : string Env.Map.t
   ; findlib                 : Findlib.t
   ; findlib_toolchain       : string option
   ; arch_sixtyfour          : bool
@@ -103,7 +92,7 @@ let sexp_of_t t =
     ; "ocamlopt", option path t.ocamlopt
     ; "ocamldep", path t.ocamldep
     ; "ocamlmklib", path t.ocamlmklib
-    ; "env", list (pair string string) (Env_var_map.to_list t.env_extra)
+    ; "env", list (pair string string) (Env.Map.to_list t.env_extra)
     ; "findlib_path", list path (Findlib.path t.findlib)
     ; "arch_sixtyfour", bool t.arch_sixtyfour
     ; "natdynlink_supported", bool t.natdynlink_supported
@@ -130,44 +119,12 @@ let opam_config_var ~env ~cache var =
         Some s
       | Error _ -> None
 
-let get_env env var =
-  let rec loop i =
-    if i = Array.length env then
-      None
-    else
-      let entry = env.(i) in
-      match String.lsplit2 entry ~on:'=' with
-      | Some (key, value) when Env_var.compare key var = Eq ->
-        Some value
-      | _ -> loop (i + 1)
-  in
-  loop 0
-
 let which ~cache ~path x =
   Hashtbl.find_or_add cache x ~f:(Bin.which ~path)
 
-let extend_env ~vars ~env =
-  if Env_var_map.is_empty vars then
-    env
-  else
-    let imported =
-      Array.to_list env
-      |> List.filter ~f:(fun s ->
-        match String.index s '=' with
-        | None -> true
-        | Some i ->
-          let key = String.sub s ~pos:0 ~len:i in
-          not (Env_var_map.mem vars key))
-    in
-    List.rev_append
-      (List.map (Env_var_map.to_list vars)
-         ~f:(fun (k, v) -> sprintf "%s=%s" k v))
-      imported
-    |> Array.of_list
-
 let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       ~targets () =
-  let env = extend_env ~env:base_env ~vars:env_extra in
+  let env = Env.extend_env ~env:base_env ~vars:env_extra in
   let opam_var_cache = Hashtbl.create 128 in
   (match kind with
    | Opam { root; _ } ->
@@ -291,12 +248,12 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       && version >= (4, 03, 0)
       && version <  (4, 05, 0) then
         let value =
-          match get_env env "OCAMLPARAM" with
+          match Env.get_env env "OCAMLPARAM" with
           | None -> "color=always,_"
           | Some s -> "color=always," ^ s
         in
-        extend_env ~env ~vars:((Env_var_map.singleton "OCAMLPARAM" value)),
-        (Env_var_map.add env_extra "OCAMLPARAM" value)
+        Env.extend_env ~env ~vars:((Env.Map.singleton "OCAMLPARAM" value)),
+        (Env.Map.add env_extra "OCAMLPARAM" value)
       else
         env,env_extra
     in
@@ -313,7 +270,7 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       ; for_host = host
       ; build_dir
       ; path
-      ; toplevel_path = Option.map (get_env env "OCAML_TOPLEVEL_PATH") ~f:Path.absolute
+      ; toplevel_path = Option.map (Env.get_env env "OCAML_TOPLEVEL_PATH") ~f:Path.absolute
 
       ; ocaml_bin  = dir
       ; ocaml      = (match which "ocaml" with Some p -> p | None -> prog_not_found_in_path "ocaml")
@@ -384,18 +341,14 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
 
 let opam_config_var t var = opam_config_var ~env:t.env ~cache:t.opam_var_cache var
 
-let initial_env = lazy (
-  Lazy.force Colors.setup_env_for_colors;
-  Unix.environment ())
-
 let default ?(merlin=true) ~targets () =
-  let env = Lazy.force initial_env in
+  let env = Lazy.force Env.initial_env in
   let path =
-    match get_env env "PATH" with
+    match Env.get_env env "PATH" with
     | Some s -> Bin.parse_path s
     | None -> []
   in
-  create ~kind:Default ~path ~base_env:env ~env_extra:Env_var_map.empty
+  create ~kind:Default ~path ~base_env:env ~env_extra:Env.Map.empty
     ~name:"default" ~merlin ~targets ()
 
 let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
@@ -413,8 +366,8 @@ let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
     let vars =
       Usexp.parse_string ~fname:"<opam output>" ~mode:Single s
       |> Sexp.Of_sexp.(list (pair string string))
-      |> Env_var_map.of_list_multi
-      |> Env_var_map.mapi ~f:(fun var values ->
+      |> Env.Map.of_list_multi
+      |> Env.Map.mapi ~f:(fun var values ->
         match List.rev values with
         | [] -> assert false
         | [x] -> x
@@ -430,11 +383,11 @@ let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
           x)
     in
     let path =
-      match Env_var_map.find vars "PATH" with
+      match Env.Map.find vars "PATH" with
       | None -> Bin.path
       | Some s -> Bin.parse_path s
     in
-    let env = Lazy.force initial_env in
+    let env = Lazy.force Env.initial_env in
     create ~kind:(Opam { root; switch }) ~targets
       ~path ~base_env:env ~env_extra:vars ~name ~merlin ()
 
@@ -472,7 +425,7 @@ let env_for_exec t =
   let cwd = Sys.getcwd () in
   let extend_var var v =
     let v = Filename.concat cwd (Path.to_string v) in
-    match get_env t.env var with
+    match Env.get_env t.env var with
     | None -> (var, v)
     | Some prev -> (var, sprintf "%s%c%s" v sep prev)
   in
@@ -491,7 +444,7 @@ let env_for_exec t =
         (Config.local_install_man_dir ~context:t.name)
     ]
   in
-  extend_env ~env:t.env ~vars:(Env_var_map.of_list_exn vars)
+  Env.extend_env ~env:t.env ~vars:(Env.Map.of_list_exn vars)
 
 let compiler t (mode : Mode.t) =
   match mode with
diff --git a/src/context.mli b/src/context.mli
index 7dd66bea..921a6160 100644
--- a/src/context.mli
+++ b/src/context.mli
@@ -30,13 +30,6 @@ module Kind : sig
   type t = Default | Opam of Opam.t
 end
 
-module Env_var : sig
-  type t = string
-  val compare : t -> t -> Ordering.t
-end
-
-module Env_var_map : Map.S with type key := Env_var.t
-
 type t =
   { name : string
   ; kind : Kind.t
@@ -73,7 +66,7 @@ type t =
     env : string array
 
   ; (** Diff between the base environment and [env] *)
-    env_extra : string Env_var_map.t
+    env_extra : string Env.Map.t
 
   ; findlib : Findlib.t
   ; findlib_toolchain : string option
@@ -135,8 +128,6 @@ val create
 
 val which : t -> string -> Path.t option
 
-val extend_env : vars:string Env_var_map.t -> env:string array -> string array
-
 val opam_config_var : t -> string -> string option Fiber.t
 
 val install_prefix : t -> Path.t Fiber.t
@@ -144,8 +135,6 @@ val install_ocaml_libdir : t -> Path.t option Fiber.t
 
 val env_for_exec : t -> string array
 
-val initial_env : string array Lazy.t
-
 (** Return the compiler needed for this compilation mode *)
 val compiler : t -> Mode.t -> Path.t option
 
diff --git a/src/env.ml b/src/env.ml
new file mode 100644
index 00000000..af18dbab
--- /dev/null
+++ b/src/env.ml
@@ -0,0 +1,51 @@
+open Import
+
+let initial_env = lazy (
+  Lazy.force Colors.setup_env_for_colors;
+  Unix.environment ())
+
+module Var = struct
+  type t = string
+  let compare a b =
+    if Sys.win32 then
+      String.compare (String.lowercase a) (String.lowercase b)
+    else
+      String.compare a b
+
+end
+
+let get_env env var =
+  let rec loop i =
+    if i = Array.length env then
+      None
+    else
+      let entry = env.(i) in
+      match String.lsplit2 entry ~on:'=' with
+      | Some (key, value) when Var.compare key var = Eq ->
+        Some value
+      | _ -> loop (i + 1)
+  in
+  loop 0
+
+module Map = Map.Make(Var)
+
+let extend_env ~vars ~env =
+  if Map.is_empty vars then
+    env
+  else
+    let imported =
+      Array.to_list env
+      |> List.filter ~f:(fun s ->
+        match String.index s '=' with
+        | None -> true
+        | Some i ->
+          let key = String.sub s ~pos:0 ~len:i in
+          not (Map.mem vars key))
+    in
+    List.rev_append
+      (List.map (Map.to_list vars)
+         ~f:(fun (k, v) -> sprintf "%s=%s" k v))
+      imported
+    |> Array.of_list
+
+
diff --git a/src/env.mli b/src/env.mli
new file mode 100644
index 00000000..4f8ca1dc
--- /dev/null
+++ b/src/env.mli
@@ -0,0 +1,13 @@
+open Import
+
+module Var : sig
+  type t = string
+end
+
+module Map : Map.S with type key = Var.t
+
+val initial_env : string array Lazy.t
+
+val extend_env : vars:string Map.t -> env:string array -> string array
+
+val get_env : string array -> string -> string option

From 35d4153641d3f5a05654fe407152d0a747521f29 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sat, 10 Mar 2018 21:15:02 +0700
Subject: [PATCH 02/14] Make Env.t abstract

---
 src/action.ml      |  9 +++---
 src/context.ml     | 34 +++++++++++---------
 src/context.mli    |  2 +-
 src/env.ml         | 79 ++++++++++++++++++++++++++++++++--------------
 src/env.mli        | 11 +++++--
 src/jbuild_load.ml |  3 +-
 6 files changed, 90 insertions(+), 48 deletions(-)

diff --git a/src/action.ml b/src/action.ml
index 153a700b..d6b25de3 100644
--- a/src/action.ml
+++ b/src/action.ml
@@ -697,7 +697,7 @@ end
 type exec_context =
   { context : Context.t option
   ; purpose : Process.purpose
-  ; env     : string array
+  ; env     : Env.t
   }
 
 let exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args =
@@ -715,8 +715,9 @@ let exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args =
      invalid_prefix ("_build/" ^ target.name);
      invalid_prefix ("_build/install/" ^ target.name);
   end;
-  let env = Env.extend_env ~vars:env_extra ~env:ectx.env in
-  Process.run Strict ~dir:(Path.to_string dir) ~env ~stdout_to ~stderr_to
+  let env = Env.extend ectx.env ~vars:env_extra in
+  Process.run Strict ~dir:(Path.to_string dir) ~env:(Env.to_unix env)
+    ~stdout_to ~stderr_to
     ~purpose:ectx.purpose
     (Path.reach_for_running ~from:dir prog) args
 
@@ -892,7 +893,7 @@ and exec_list l ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
 let exec ~targets ?context t =
   let env =
     match (context : Context.t option) with
-    | None -> Lazy.force Env.initial_env
+    | None -> Env.initial ()
     | Some c -> c.env
   in
   let targets = Path.Set.to_list targets in
diff --git a/src/context.ml b/src/context.ml
index b26d9562..42aeb868 100644
--- a/src/context.ml
+++ b/src/context.ml
@@ -33,7 +33,7 @@ type t =
   ; ocamlopt                : Path.t option
   ; ocamldep                : Path.t
   ; ocamlmklib              : Path.t
-  ; env                     : string array
+  ; env                     : Env.t
   ; env_extra               : string Env.Map.t
   ; findlib                 : Findlib.t
   ; findlib_toolchain       : string option
@@ -110,7 +110,8 @@ let opam_config_var ~env ~cache var =
     match Bin.opam with
     | None -> Fiber.return None
     | Some fn ->
-      Process.run_capture (Accept All) (Path.to_string fn) ~env
+      Process.run_capture (Accept All) (Path.to_string fn)
+        ~env:(Env.to_unix env)
         ["config"; "var"; var]
       >>| function
       | Ok s ->
@@ -124,7 +125,7 @@ let which ~cache ~path x =
 
 let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       ~targets () =
-  let env = Env.extend_env ~env:base_env ~vars:env_extra in
+  let env = Env.extend base_env ~vars:env_extra in
   let opam_var_cache = Hashtbl.create 128 in
   (match kind with
    | Opam { root; _ } ->
@@ -145,7 +146,7 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       match Sys.getenv "OCAMLFIND_CONF" with
       | s -> Fiber.return (Path.absolute s)
       | exception Not_found ->
-        Process.run_capture_line ~env Strict
+        Process.run_capture_line ~env:(Env.to_unix env) Strict
           (Path.to_string fn) ["printconf"; "conf"]
         >>| Path.absolute)
   in
@@ -207,7 +208,9 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
             | None -> args
             | Some s -> "-toolchain" :: s :: args
           in
-          Process.run_capture_lines ~env Strict (Path.to_string fn) args
+          Process.run_capture_lines
+            ~env:(Env.to_unix env)
+            Strict (Path.to_string fn) args
           >>| List.map ~f:Path.absolute
         | None ->
           (* If there no ocamlfind in the PATH, check if we have opam
@@ -230,7 +233,7 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
     Fiber.fork_and_join
       findlib_path
       (fun () ->
-         Process.run_capture_lines ~env Strict
+         Process.run_capture_lines ~env:(Env.to_unix env) Strict
            (Path.to_string ocamlc) ["-config"]
          >>| fun lines ->
          let open Result.O in
@@ -248,11 +251,11 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       && version >= (4, 03, 0)
       && version <  (4, 05, 0) then
         let value =
-          match Env.get_env env "OCAMLPARAM" with
+          match Env.get_var env "OCAMLPARAM" with
           | None -> "color=always,_"
           | Some s -> "color=always," ^ s
         in
-        Env.extend_env ~env ~vars:((Env.Map.singleton "OCAMLPARAM" value)),
+        Env.extend env ~vars:(Env.Map.singleton "OCAMLPARAM" value),
         (Env.Map.add env_extra "OCAMLPARAM" value)
       else
         env,env_extra
@@ -270,7 +273,8 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
       ; for_host = host
       ; build_dir
       ; path
-      ; toplevel_path = Option.map (Env.get_env env "OCAML_TOPLEVEL_PATH") ~f:Path.absolute
+      ; toplevel_path =
+          Option.map (Env.get_var env "OCAML_TOPLEVEL_PATH") ~f:Path.absolute
 
       ; ocaml_bin  = dir
       ; ocaml      = (match which "ocaml" with Some p -> p | None -> prog_not_found_in_path "ocaml")
@@ -342,9 +346,9 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
 let opam_config_var t var = opam_config_var ~env:t.env ~cache:t.opam_var_cache var
 
 let default ?(merlin=true) ~targets () =
-  let env = Lazy.force Env.initial_env in
+  let env = Env.initial () in
   let path =
-    match Env.get_env env "PATH" with
+    match Env.get_var env "PATH" with
     | Some s -> Bin.parse_path s
     | None -> []
   in
@@ -387,7 +391,7 @@ let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
       | None -> Bin.path
       | Some s -> Bin.parse_path s
     in
-    let env = Lazy.force Env.initial_env in
+    let env = Env.initial () in
     create ~kind:(Opam { root; switch }) ~targets
       ~path ~base_env:env ~env_extra:vars ~name ~merlin ()
 
@@ -412,7 +416,7 @@ let install_ocaml_libdir t =
     (* If ocamlfind is present, it has precedence over everything else. *)
     match which t "ocamlfind" with
     | Some fn ->
-      (Process.run_capture_line ~env:t.env Strict
+      (Process.run_capture_line ~env:(Env.to_unix t.env) Strict
          (Path.to_string fn) ["printconf"; "destdir"]
        >>| fun s ->
        Some (Path.absolute s))
@@ -425,7 +429,7 @@ let env_for_exec t =
   let cwd = Sys.getcwd () in
   let extend_var var v =
     let v = Filename.concat cwd (Path.to_string v) in
-    match Env.get_env t.env var with
+    match Env.get_var t.env var with
     | None -> (var, v)
     | Some prev -> (var, sprintf "%s%c%s" v sep prev)
   in
@@ -444,7 +448,7 @@ let env_for_exec t =
         (Config.local_install_man_dir ~context:t.name)
     ]
   in
-  Env.extend_env ~env:t.env ~vars:(Env.Map.of_list_exn vars)
+  Env.to_unix (Env.extend t.env ~vars:(Env.Map.of_list_exn vars))
 
 let compiler t (mode : Mode.t) =
   match mode with
diff --git a/src/context.mli b/src/context.mli
index 921a6160..40027bf5 100644
--- a/src/context.mli
+++ b/src/context.mli
@@ -63,7 +63,7 @@ type t =
   ; ocamlmklib : Path.t
 
   ; (** Environment variables *)
-    env : string array
+    env : Env.t
 
   ; (** Diff between the base environment and [env] *)
     env_extra : string Env.Map.t
diff --git a/src/env.ml b/src/env.ml
index af18dbab..fbdccea0 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -1,9 +1,5 @@
 open Import
 
-let initial_env = lazy (
-  Lazy.force Colors.setup_env_for_colors;
-  Unix.environment ())
-
 module Var = struct
   type t = string
   let compare a b =
@@ -14,7 +10,39 @@ module Var = struct
 
 end
 
-let get_env env var =
+module Map = Map.Make(Var)
+
+type t =
+  { base : string array
+  ; extra : string Map.t
+  ; combined : string array Lazy.t
+  }
+
+let make ~base ~extra =
+  { base
+  ; extra
+  ; combined = lazy (
+      if Map.is_empty extra then
+        base
+      else
+        let imported =
+          Array.to_list base
+          |> List.filter ~f:(fun s ->
+            match String.index s '=' with
+            | None -> true
+            | Some i ->
+              let key = String.sub s ~pos:0 ~len:i in
+              not (Map.mem extra key))
+        in
+        List.rev_append
+          (List.map (Map.to_list extra)
+             ~f:(fun (k, v) -> sprintf "%s=%s" k v))
+          imported
+        |> Array.of_list
+    )
+  }
+
+let get_env_base env var =
   let rec loop i =
     if i = Array.length env then
       None
@@ -27,25 +55,28 @@ let get_env env var =
   in
   loop 0
 
-module Map = Map.Make(Var)
+let get_var t v =
+  match Map.find t.extra v with
+  | None -> get_env_base t.base v
+  | Some _ as v -> v
 
-let extend_env ~vars ~env =
-  if Map.is_empty vars then
-    env
-  else
-    let imported =
-      Array.to_list env
-      |> List.filter ~f:(fun s ->
-        match String.index s '=' with
-        | None -> true
-        | Some i ->
-          let key = String.sub s ~pos:0 ~len:i in
-          not (Map.mem vars key))
-    in
-    List.rev_append
-      (List.map (Map.to_list vars)
-         ~f:(fun (k, v) -> sprintf "%s=%s" k v))
-      imported
-    |> Array.of_list
+let to_unix t = Lazy.force t.combined
 
+let initial =
+  let i =
+    lazy (
+      make
+        ~base:(Lazy.force Colors.setup_env_for_colors;
+               Unix.environment ())
+        ~extra:Map.empty
+    ) in
+  fun () -> Lazy.force i
 
+let extend t ~vars =
+  make ~base:t.base
+    ~extra:(
+      Map.merge t.extra vars ~f:(fun _ v1 v2 ->
+        match v2 with
+        | Some _ -> v2
+        | None -> v1)
+    )
diff --git a/src/env.mli b/src/env.mli
index 4f8ca1dc..81b85e62 100644
--- a/src/env.mli
+++ b/src/env.mli
@@ -2,12 +2,17 @@ open Import
 
 module Var : sig
   type t = string
+  val compare : t -> t -> Ordering.t
 end
 
+type t
+
 module Map : Map.S with type key = Var.t
 
-val initial_env : string array Lazy.t
+val initial : unit -> t
 
-val extend_env : vars:string Map.t -> env:string array -> string array
+val to_unix : t -> string array
 
-val get_env : string array -> string -> string option
+val get_var : t -> Var.t -> string option
+
+val extend : t -> vars:string Map.t -> t
diff --git a/src/jbuild_load.ml b/src/jbuild_load.ml
index f7882a40..3564edcf 100644
--- a/src/jbuild_load.ml
+++ b/src/jbuild_load.ml
@@ -148,7 +148,8 @@ end
            in
          ]}
       *)
-      Process.run Strict ~dir:(Path.to_string dir) ~env:context.env
+      Process.run Strict ~dir:(Path.to_string dir)
+        ~env:(Env.to_unix context.env)
         (Path.to_string context.ocaml)
         args
       >>= fun () ->

From 206cc69fd9620373caf6e1eb6268ae91a6fddc75 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sat, 10 Mar 2018 21:58:58 +0700
Subject: [PATCH 03/14] Simplify Context.create

There's no need for a base_env parameter as it's always Env.initial
---
 src/context.ml | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/src/context.ml b/src/context.ml
index 42aeb868..ccf6dc01 100644
--- a/src/context.ml
+++ b/src/context.ml
@@ -123,9 +123,9 @@ let opam_config_var ~env ~cache var =
 let which ~cache ~path x =
   Hashtbl.find_or_add cache x ~f:(Bin.which ~path)
 
-let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
+let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
       ~targets () =
-  let env = Env.extend base_env ~vars:env_extra in
+  let env = Env.extend (Env.initial ()) ~vars:env_extra in
   let opam_var_cache = Hashtbl.create 128 in
   (match kind with
    | Opam { root; _ } ->
@@ -346,13 +346,7 @@ let create ~(kind : Kind.t) ~path ~base_env ~env_extra ~name ~merlin
 let opam_config_var t var = opam_config_var ~env:t.env ~cache:t.opam_var_cache var
 
 let default ?(merlin=true) ~targets () =
-  let env = Env.initial () in
-  let path =
-    match Env.get_var env "PATH" with
-    | Some s -> Bin.parse_path s
-    | None -> []
-  in
-  create ~kind:Default ~path ~base_env:env ~env_extra:Env.Map.empty
+  create ~kind:Default ~path:Bin.path ~env_extra:Env.Map.empty
     ~name:"default" ~merlin ~targets ()
 
 let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
@@ -391,9 +385,8 @@ let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
       | None -> Bin.path
       | Some s -> Bin.parse_path s
     in
-    let env = Env.initial () in
     create ~kind:(Opam { root; switch }) ~targets
-      ~path ~base_env:env ~env_extra:vars ~name ~merlin ()
+      ~path ~env_extra:vars ~name ~merlin ()
 
 let create ?merlin def =
   match (def : Workspace.Context.t) with

From 4b191b2b0357c5ceaa292774d73dad4512100a9a Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sun, 11 Mar 2018 11:17:09 +0700
Subject: [PATCH 04/14] s/Env.get_var/Env.get/

---
 src/context.ml | 6 +++---
 src/env.ml     | 2 +-
 src/env.mli    | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/context.ml b/src/context.ml
index ccf6dc01..ff59ee66 100644
--- a/src/context.ml
+++ b/src/context.ml
@@ -251,7 +251,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
       && version >= (4, 03, 0)
       && version <  (4, 05, 0) then
         let value =
-          match Env.get_var env "OCAMLPARAM" with
+          match Env.get env "OCAMLPARAM" with
           | None -> "color=always,_"
           | Some s -> "color=always," ^ s
         in
@@ -274,7 +274,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
       ; build_dir
       ; path
       ; toplevel_path =
-          Option.map (Env.get_var env "OCAML_TOPLEVEL_PATH") ~f:Path.absolute
+          Option.map (Env.get env "OCAML_TOPLEVEL_PATH") ~f:Path.absolute
 
       ; ocaml_bin  = dir
       ; ocaml      = (match which "ocaml" with Some p -> p | None -> prog_not_found_in_path "ocaml")
@@ -422,7 +422,7 @@ let env_for_exec t =
   let cwd = Sys.getcwd () in
   let extend_var var v =
     let v = Filename.concat cwd (Path.to_string v) in
-    match Env.get_var t.env var with
+    match Env.get t.env var with
     | None -> (var, v)
     | Some prev -> (var, sprintf "%s%c%s" v sep prev)
   in
diff --git a/src/env.ml b/src/env.ml
index fbdccea0..cf533782 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -55,7 +55,7 @@ let get_env_base env var =
   in
   loop 0
 
-let get_var t v =
+let get t v =
   match Map.find t.extra v with
   | None -> get_env_base t.base v
   | Some _ as v -> v
diff --git a/src/env.mli b/src/env.mli
index 81b85e62..0bb9ef9a 100644
--- a/src/env.mli
+++ b/src/env.mli
@@ -13,6 +13,6 @@ val initial : unit -> t
 
 val to_unix : t -> string array
 
-val get_var : t -> Var.t -> string option
+val get : t -> Var.t -> string option
 
 val extend : t -> vars:string Map.t -> t

From 3193e4902dc39f94fadf1f8e377608ac314b1d67 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sun, 11 Mar 2018 11:20:37 +0700
Subject: [PATCH 05/14] Make Process.run take Env.t directly

---
 src/action.ml      |  2 +-
 src/context.ml     | 13 +++++--------
 src/jbuild_load.ml |  2 +-
 src/process.ml     |  2 +-
 src/process.mli    |  8 ++++----
 5 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/action.ml b/src/action.ml
index d6b25de3..16f5ca86 100644
--- a/src/action.ml
+++ b/src/action.ml
@@ -716,7 +716,7 @@ let exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args =
      invalid_prefix ("_build/install/" ^ target.name);
   end;
   let env = Env.extend ectx.env ~vars:env_extra in
-  Process.run Strict ~dir:(Path.to_string dir) ~env:(Env.to_unix env)
+  Process.run Strict ~dir:(Path.to_string dir) ~env
     ~stdout_to ~stderr_to
     ~purpose:ectx.purpose
     (Path.reach_for_running ~from:dir prog) args
diff --git a/src/context.ml b/src/context.ml
index ff59ee66..518c2f62 100644
--- a/src/context.ml
+++ b/src/context.ml
@@ -110,8 +110,7 @@ let opam_config_var ~env ~cache var =
     match Bin.opam with
     | None -> Fiber.return None
     | Some fn ->
-      Process.run_capture (Accept All) (Path.to_string fn)
-        ~env:(Env.to_unix env)
+      Process.run_capture (Accept All) (Path.to_string fn) ~env
         ["config"; "var"; var]
       >>| function
       | Ok s ->
@@ -146,7 +145,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
       match Sys.getenv "OCAMLFIND_CONF" with
       | s -> Fiber.return (Path.absolute s)
       | exception Not_found ->
-        Process.run_capture_line ~env:(Env.to_unix env) Strict
+        Process.run_capture_line ~env Strict
           (Path.to_string fn) ["printconf"; "conf"]
         >>| Path.absolute)
   in
@@ -208,9 +207,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
             | None -> args
             | Some s -> "-toolchain" :: s :: args
           in
-          Process.run_capture_lines
-            ~env:(Env.to_unix env)
-            Strict (Path.to_string fn) args
+          Process.run_capture_lines ~env Strict (Path.to_string fn) args
           >>| List.map ~f:Path.absolute
         | None ->
           (* If there no ocamlfind in the PATH, check if we have opam
@@ -233,7 +230,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
     Fiber.fork_and_join
       findlib_path
       (fun () ->
-         Process.run_capture_lines ~env:(Env.to_unix env) Strict
+         Process.run_capture_lines ~env Strict
            (Path.to_string ocamlc) ["-config"]
          >>| fun lines ->
          let open Result.O in
@@ -409,7 +406,7 @@ let install_ocaml_libdir t =
     (* If ocamlfind is present, it has precedence over everything else. *)
     match which t "ocamlfind" with
     | Some fn ->
-      (Process.run_capture_line ~env:(Env.to_unix t.env) Strict
+      (Process.run_capture_line ~env:t.env Strict
          (Path.to_string fn) ["printconf"; "destdir"]
        >>| fun s ->
        Some (Path.absolute s))
diff --git a/src/jbuild_load.ml b/src/jbuild_load.ml
index 3564edcf..2bf6e9dd 100644
--- a/src/jbuild_load.ml
+++ b/src/jbuild_load.ml
@@ -149,7 +149,7 @@ end
          ]}
       *)
       Process.run Strict ~dir:(Path.to_string dir)
-        ~env:(Env.to_unix context.env)
+        ~env:context.env
         (Path.to_string context.ocaml)
         args
       >>= fun () ->
diff --git a/src/process.ml b/src/process.ml
index 39fbb651..3a6ae0e4 100644
--- a/src/process.ml
+++ b/src/process.ml
@@ -243,7 +243,7 @@ let run_internal ?dir ?(stdout_to=Terminal) ?(stderr_to=Terminal) ?env ~purpose
       Unix.create_process prog argv
         Unix.stdin stdout stderr
     | Some env ->
-      Unix.create_process_env prog argv env
+      Unix.create_process_env prog argv (Env.to_unix env)
         Unix.stdin stdout stderr
   in
   let pid =
diff --git a/src/process.mli b/src/process.mli
index 0a7ea370..ad406f8c 100644
--- a/src/process.mli
+++ b/src/process.mli
@@ -41,7 +41,7 @@ val run
   :  ?dir:string
   -> ?stdout_to:std_output_to
   -> ?stderr_to:std_output_to
-  -> ?env:string array
+  -> ?env:Env.t
   -> ?purpose:purpose
   -> (unit, 'a) failure_mode
   -> string
@@ -51,7 +51,7 @@ val run
 (** Run a command and capture its output *)
 val run_capture
   :  ?dir:string
-  -> ?env:string array
+  -> ?env:Env.t
   -> ?purpose:purpose
   -> (string, 'a) failure_mode
   -> string
@@ -59,7 +59,7 @@ val run_capture
   -> 'a Fiber.t
 val run_capture_line
   :  ?dir:string
-  -> ?env:string array
+  -> ?env:Env.t
   -> ?purpose:purpose
   -> (string, 'a) failure_mode
   -> string
@@ -67,7 +67,7 @@ val run_capture_line
   -> 'a Fiber.t
 val run_capture_lines
   :  ?dir:string
-  -> ?env:string array
+  -> ?env:Env.t
   -> ?purpose:purpose
   -> (string list, 'a) failure_mode
   -> string

From f1baaa23a85292428afda77786a7e49c41a92ed0 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sun, 11 Mar 2018 11:45:47 +0700
Subject: [PATCH 06/14] Add Env.add

Just add 1 binding to Env
---
 src/env.ml  | 3 +++
 src/env.mli | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/src/env.ml b/src/env.ml
index cf533782..84173b93 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -80,3 +80,6 @@ let extend t ~vars =
         | Some _ -> v2
         | None -> v1)
     )
+
+let add t ~var ~value =
+  make ~base:t.base ~extra:(Map.add t.extra var value)
diff --git a/src/env.mli b/src/env.mli
index 0bb9ef9a..02c67ec6 100644
--- a/src/env.mli
+++ b/src/env.mli
@@ -16,3 +16,5 @@ val to_unix : t -> string array
 val get : t -> Var.t -> string option
 
 val extend : t -> vars:string Map.t -> t
+
+val add : t -> var:Var.t -> value:string -> t

From e604c695104bff520164de4f1248e79bbc9bce67 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Sun, 11 Mar 2018 12:13:04 +0700
Subject: [PATCH 07/14] Remove env_extra in Action

It's simply to just use Env.t everywhere. Also, there's no need to have the env
in the execution context since it's not used for anything. Only the env that is
passed directly is used.
---
 src/action.ml | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/src/action.ml b/src/action.ml
index 16f5ca86..2e5bd7e7 100644
--- a/src/action.ml
+++ b/src/action.ml
@@ -697,10 +697,9 @@ end
 type exec_context =
   { context : Context.t option
   ; purpose : Process.purpose
-  ; env     : Env.t
   }
 
-let exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args =
+let exec_run_direct ~ectx ~dir ~env ~stdout_to ~stderr_to prog args =
   begin match ectx.context with
    | None
    | Some { Context.for_host = None; _ } -> ()
@@ -715,7 +714,6 @@ let exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args =
      invalid_prefix ("_build/" ^ target.name);
      invalid_prefix ("_build/install/" ^ target.name);
   end;
-  let env = Env.extend ectx.env ~vars:env_extra in
   Process.run Strict ~dir:(Path.to_string dir) ~env
     ~stdout_to ~stderr_to
     ~purpose:ectx.purpose
@@ -732,17 +730,17 @@ let exec_echo stdout_to str =
      | None -> print_string str; flush stdout
      | Some (_, oc) -> output_string oc str)
 
-let rec exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
+let rec exec t ~ectx ~dir ~env ~stdout_to ~stderr_to =
   match t with
   | Run (Error e, _) ->
     Prog.Not_found.raise e
   | Run (Ok prog, args) ->
-    exec_run ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args
+    exec_run ~ectx ~dir ~env ~stdout_to ~stderr_to prog args
   | Chdir (dir, t) ->
-    exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to
+    exec t ~ectx ~dir ~env ~stdout_to ~stderr_to
   | Setenv (var, value, t) ->
     exec t ~ectx ~dir ~stdout_to ~stderr_to
-      ~env_extra:(Env.Map.add env_extra var value)
+      ~env:(Env.add env ~var ~value)
   | Redirect (Stdout, fn, Echo s) ->
     Io.write_file (Path.to_string fn) s;
     Fiber.return ()
@@ -754,13 +752,13 @@ let rec exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
       | Stderr -> (get_std_output stdout_to, out)
       | Outputs -> (out, out)
     in
-    exec_run_direct ~ectx ~dir ~env_extra ~stdout_to ~stderr_to prog args
+    exec_run_direct ~ectx ~dir ~env ~stdout_to ~stderr_to prog args
   | Redirect (outputs, fn, t) ->
-    redirect ~ectx ~dir outputs fn t ~env_extra ~stdout_to ~stderr_to
+    redirect ~ectx ~dir outputs fn t ~env ~stdout_to ~stderr_to
   | Ignore (outputs, t) ->
-    redirect ~ectx ~dir outputs Config.dev_null t ~env_extra ~stdout_to ~stderr_to
+    redirect ~ectx ~dir outputs Config.dev_null t ~env ~stdout_to ~stderr_to
   | Progn l ->
-    exec_list l ~ectx ~dir ~env_extra ~stdout_to ~stderr_to
+    exec_list l ~ectx ~dir ~env ~stdout_to ~stderr_to
   | Echo str -> exec_echo stdout_to str
   | Cat fn ->
     Io.with_file_in (Path.to_string fn) ~f:(fun ic ->
@@ -813,9 +811,9 @@ let rec exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
     let path, arg =
       Utils.system_shell_exn ~needed_to:"interpret (system ...) actions"
     in
-    exec_run ~ectx ~dir ~env_extra ~stdout_to ~stderr_to path [arg; cmd]
+    exec_run ~ectx ~dir ~env ~stdout_to ~stderr_to path [arg; cmd]
   | Bash cmd ->
-    exec_run ~ectx ~dir ~env_extra ~stdout_to ~stderr_to
+    exec_run ~ectx ~dir ~env ~stdout_to ~stderr_to
       (Utils.bash_exn ~needed_to:"interpret (bash ...) actions")
       ["-e"; "-u"; "-o"; "pipefail"; "-c"; cmd]
   | Write_file (fn, s) ->
@@ -867,7 +865,7 @@ let rec exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
       Print_diff.print file1 file2
     end
 
-and redirect outputs fn t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
+and redirect outputs fn t ~ectx ~dir ~env ~stdout_to ~stderr_to =
   let fn = Path.to_string fn in
   let oc = Io.open_out fn in
   let out = Some (fn, oc) in
@@ -877,18 +875,18 @@ and redirect outputs fn t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
     | Stderr -> (stdout_to, out)
     | Outputs -> (out, out)
   in
-  exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to >>| fun () ->
+  exec t ~ectx ~dir ~env ~stdout_to ~stderr_to >>| fun () ->
   close_out oc
 
-and exec_list l ~ectx ~dir ~env_extra ~stdout_to ~stderr_to =
+and exec_list l ~ectx ~dir ~env ~stdout_to ~stderr_to =
   match l with
   | [] ->
     Fiber.return ()
   | [t] ->
-    exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to
+    exec t ~ectx ~dir ~env ~stdout_to ~stderr_to
   | t :: rest ->
-    exec t ~ectx ~dir ~env_extra ~stdout_to ~stderr_to >>= fun () ->
-    exec_list rest ~ectx ~dir ~env_extra ~stdout_to ~stderr_to
+    exec t ~ectx ~dir ~env ~stdout_to ~stderr_to >>= fun () ->
+    exec_list rest ~ectx ~dir ~env ~stdout_to ~stderr_to
 
 let exec ~targets ?context t =
   let env =
@@ -898,9 +896,8 @@ let exec ~targets ?context t =
   in
   let targets = Path.Set.to_list targets in
   let purpose = Process.Build_job targets in
-  let ectx = { purpose; context; env } in
-  exec t ~ectx ~dir:Path.root ~env_extra:Env.Map.empty
-    ~stdout_to:None ~stderr_to:None
+  let ectx = { purpose; context } in
+  exec t ~ectx ~dir:Path.root ~env ~stdout_to:None ~stderr_to:None
 
 let sandbox t ~sandboxed ~deps ~targets =
   Progn

From 26d6b03c66971145398215a7e1b70336b4f4f24f Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Mon, 12 Mar 2018 09:52:52 +0700
Subject: [PATCH 08/14] Make unix representation more efficient

By not allocation a closure on every update
---
 src/env.ml | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/env.ml b/src/env.ml
index 84173b93..4f2bed39 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -15,31 +15,13 @@ module Map = Map.Make(Var)
 type t =
   { base : string array
   ; extra : string Map.t
-  ; combined : string array Lazy.t
+  ; mutable unix : string array option
   }
 
 let make ~base ~extra =
   { base
   ; extra
-  ; combined = lazy (
-      if Map.is_empty extra then
-        base
-      else
-        let imported =
-          Array.to_list base
-          |> List.filter ~f:(fun s ->
-            match String.index s '=' with
-            | None -> true
-            | Some i ->
-              let key = String.sub s ~pos:0 ~len:i in
-              not (Map.mem extra key))
-        in
-        List.rev_append
-          (List.map (Map.to_list extra)
-             ~f:(fun (k, v) -> sprintf "%s=%s" k v))
-          imported
-        |> Array.of_list
-    )
+  ; unix = None
   }
 
 let get_env_base env var =
@@ -60,7 +42,30 @@ let get t v =
   | None -> get_env_base t.base v
   | Some _ as v -> v
 
-let to_unix t = Lazy.force t.combined
+let to_unix t =
+  match t.unix with
+  | Some v -> v
+  | None ->
+    let res =
+      if Map.is_empty t.extra then
+        t.base
+      else
+        let imported =
+          Array.to_list t.base
+          |> List.filter ~f:(fun s ->
+            match String.index s '=' with
+            | None -> true
+            | Some i ->
+              let key = String.sub s ~pos:0 ~len:i in
+              not (Map.mem t.extra key))
+        in
+        List.rev_append
+          (List.map (Map.to_list t.extra)
+             ~f:(fun (k, v) -> sprintf "%s=%s" k v))
+          imported
+        |> Array.of_list in
+    t.unix <- Some res;
+    res
 
 let initial =
   let i =

From 79d47ab68e00599690cc8c653c8b74e20f349fc6 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Mon, 12 Mar 2018 10:43:43 +0700
Subject: [PATCH 09/14] Simplify Env.t

Use assoc list
---
 src/env.ml  | 115 ++++++++++++++++++++++++++++------------------------
 src/env.mli |   4 ++
 2 files changed, 66 insertions(+), 53 deletions(-)

diff --git a/src/env.ml b/src/env.ml
index 4f2bed39..d968cc09 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -8,83 +8,92 @@ module Var = struct
     else
       String.compare a b
 
+  let equal a b =
+    match compare a b with
+    | Ordering.Eq -> true
+    | _ -> false
+
+  let hash =
+    if Sys.win32 then
+      fun x -> Hashtbl.hash (String.lowercase x)
+    else
+      Hashtbl.hash
 end
 
 module Map = Map.Make(Var)
 
+module Table = Hashtbl.Make(Var)
+
 type t =
-  { base : string array
-  ; extra : string Map.t
+  { vars : (Var.t * string) list
   ; mutable unix : string array option
   }
 
-let make ~base ~extra =
-  { base
-  ; extra
+let make vars =
+  { vars
   ; unix = None
   }
 
-let get_env_base env var =
-  let rec loop i =
-    if i = Array.length env then
-      None
-    else
-      let entry = env.(i) in
-      match String.lsplit2 entry ~on:'=' with
-      | Some (key, value) when Var.compare key var = Eq ->
-        Some value
-      | _ -> loop (i + 1)
-  in
-  loop 0
-
-let get t v =
-  match Map.find t.extra v with
-  | None -> get_env_base t.base v
-  | Some _ as v -> v
+let get t k =
+  List.find_map t.vars ~f:(fun (k', v) ->
+    match Var.compare k k' with
+    | Ordering.Eq -> Some v
+    | _ -> None)
 
 let to_unix t =
   match t.unix with
   | Some v -> v
   | None ->
     let res =
-      if Map.is_empty t.extra then
-        t.base
-      else
-        let imported =
-          Array.to_list t.base
-          |> List.filter ~f:(fun s ->
-            match String.index s '=' with
-            | None -> true
-            | Some i ->
-              let key = String.sub s ~pos:0 ~len:i in
-              not (Map.mem t.extra key))
-        in
-        List.rev_append
-          (List.map (Map.to_list t.extra)
-             ~f:(fun (k, v) -> sprintf "%s=%s" k v))
-          imported
-        |> Array.of_list in
+      let seen = Table.create 16 in
+      t.vars
+      |> List.fold_left ~init:[] ~f:(fun uniques (k, v) ->
+        if Table.mem seen k then (
+          uniques
+        ) else (
+          Table.add seen ~key:k ~data:();
+          (k, v) :: uniques
+        ))
+      |> List.rev_map ~f:(fun (k, v) -> sprintf "%s=%s" k v)
+      |> Array.of_list in
     t.unix <- Some res;
     res
 
+let of_unix arr =
+  Array.to_list arr
+  |> List.map ~f:(fun s ->
+    match String.lsplit2 s ~on:'=' with
+    | None -> (s, "")
+    | Some (k, v) -> (k, v))
+
 let initial =
   let i =
     lazy (
-      make
-        ~base:(Lazy.force Colors.setup_env_for_colors;
-               Unix.environment ())
-        ~extra:Map.empty
+      make (Lazy.force Colors.setup_env_for_colors;
+            Unix.environment ()
+            |> of_unix)
     ) in
   fun () -> Lazy.force i
 
-let extend t ~vars =
-  make ~base:t.base
-    ~extra:(
-      Map.merge t.extra vars ~f:(fun _ v1 v2 ->
-        match v2 with
-        | Some _ -> v2
-        | None -> v1)
-    )
-
 let add t ~var ~value =
-  make ~base:t.base ~extra:(Map.add t.extra var value)
+  { vars = (var, value) :: t.vars
+  ; unix = None
+  }
+
+let extend t ~vars =
+  { vars = Map.foldi ~init:t.vars ~f:(fun k v t -> (k, v) :: t) vars
+  ; unix = None
+  }
+
+let sexp_of_t t =
+  let open Sexp.To_sexp in
+  (list (pair string string)) t.vars
+
+let diff x y =
+  let to_map b = Map.of_list_reduce b ~f:(fun old _new -> old) in
+  Map.merge (to_map x.vars) (to_map y.vars) ~f:(fun _k vx vy ->
+    match vy with
+    | Some _ -> None
+    | None -> vx)
+  |> Map.to_list
+  |> make
diff --git a/src/env.mli b/src/env.mli
index 02c67ec6..7a91a654 100644
--- a/src/env.mli
+++ b/src/env.mli
@@ -18,3 +18,7 @@ val get : t -> Var.t -> string option
 val extend : t -> vars:string Map.t -> t
 
 val add : t -> var:Var.t -> value:string -> t
+
+val diff : t -> t -> t
+
+val sexp_of_t : t -> Sexp.t

From f4955cdc224c154735be349acd93626935d217e3 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Mon, 12 Mar 2018 10:44:04 +0700
Subject: [PATCH 10/14] Remove env_extra from Context.t

Rely on diffing
---
 src/context.ml  | 21 ++++++++-------------
 src/context.mli |  3 ---
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/src/context.ml b/src/context.ml
index 518c2f62..794464c8 100644
--- a/src/context.ml
+++ b/src/context.ml
@@ -34,7 +34,6 @@ type t =
   ; ocamldep                : Path.t
   ; ocamlmklib              : Path.t
   ; env                     : Env.t
-  ; env_extra               : string Env.Map.t
   ; findlib                 : Findlib.t
   ; findlib_toolchain       : string option
   ; arch_sixtyfour          : bool
@@ -92,7 +91,7 @@ let sexp_of_t t =
     ; "ocamlopt", option path t.ocamlopt
     ; "ocamldep", path t.ocamldep
     ; "ocamlmklib", path t.ocamlmklib
-    ; "env", list (pair string string) (Env.Map.to_list t.env_extra)
+    ; "env", Env.sexp_of_t (Env.diff t.env (Env.initial ()))
     ; "findlib_path", list path (Findlib.path t.findlib)
     ; "arch_sixtyfour", bool t.arch_sixtyfour
     ; "natdynlink_supported", bool t.natdynlink_supported
@@ -122,9 +121,7 @@ let opam_config_var ~env ~cache var =
 let which ~cache ~path x =
   Hashtbl.find_or_add cache x ~f:(Bin.which ~path)
 
-let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
-      ~targets () =
-  let env = Env.extend (Env.initial ()) ~vars:env_extra in
+let create ~(kind : Kind.t) ~path ~env ~name ~merlin ~targets () =
   let opam_var_cache = Hashtbl.create 128 in
   (match kind with
    | Opam { root; _ } ->
@@ -238,7 +235,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
            (Ocaml_config.Vars.of_lines lines >>= Ocaml_config.make))
     >>= fun (findlib_path, ocfg) ->
     let version = Ocaml_config.version ocfg in
-    let env, env_extra =
+    let env =
       (* See comment in ansi_color.ml for setup_env_for_colors. For
          OCaml < 4.05, OCAML_COLOR is not supported so we use
          OCAMLPARAM. OCaml 4.02 doesn't support 'color' in OCAMLPARAM,
@@ -252,10 +249,9 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
           | None -> "color=always,_"
           | Some s -> "color=always," ^ s
         in
-        Env.extend env ~vars:(Env.Map.singleton "OCAMLPARAM" value),
-        (Env.Map.add env_extra "OCAMLPARAM" value)
+        Env.add env ~var:"OCAMLPARAM" ~value
       else
-        env,env_extra
+        env
     in
     let stdlib_dir = Path.of_string (Ocaml_config.standard_library ocfg) in
     let natdynlink_supported = Ocaml_config.natdynlink_supported ocfg in
@@ -281,7 +277,6 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
       ; ocamlmklib = get_ocaml_tool_exn "ocamlmklib"
 
       ; env
-      ; env_extra
       ; findlib = Findlib.create ~stdlib_dir ~path:findlib_path
       ; findlib_toolchain
       ; arch_sixtyfour
@@ -343,7 +338,7 @@ let create ~(kind : Kind.t) ~path ~env_extra ~name ~merlin
 let opam_config_var t var = opam_config_var ~env:t.env ~cache:t.opam_var_cache var
 
 let default ?(merlin=true) ~targets () =
-  create ~kind:Default ~path:Bin.path ~env_extra:Env.Map.empty
+  create ~kind:Default ~path:Bin.path ~env:(Env.initial ())
     ~name:"default" ~merlin ~targets ()
 
 let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
@@ -382,8 +377,8 @@ let create_for_opam ?root ~targets ~switch ~name ?(merlin=false) () =
       | None -> Bin.path
       | Some s -> Bin.parse_path s
     in
-    create ~kind:(Opam { root; switch }) ~targets
-      ~path ~env_extra:vars ~name ~merlin ()
+    let env = Env.extend (Env.initial ()) ~vars in
+    create ~kind:(Opam { root; switch }) ~targets ~path ~env ~name ~merlin ()
 
 let create ?merlin def =
   match (def : Workspace.Context.t) with
diff --git a/src/context.mli b/src/context.mli
index 40027bf5..974b0bd7 100644
--- a/src/context.mli
+++ b/src/context.mli
@@ -65,9 +65,6 @@ type t =
   ; (** Environment variables *)
     env : Env.t
 
-  ; (** Diff between the base environment and [env] *)
-    env_extra : string Env.Map.t
-
   ; findlib : Findlib.t
   ; findlib_toolchain : string option
 

From 1f99d0fb702858daa054dd3b5e2db69ce7162e09 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Mon, 12 Mar 2018 10:46:58 +0700
Subject: [PATCH 11/14] Simplify Env.get

we already have Var.equal now
---
 src/env.ml | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/env.ml b/src/env.ml
index d968cc09..772c7772 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -35,10 +35,7 @@ let make vars =
   }
 
 let get t k =
-  List.find_map t.vars ~f:(fun (k', v) ->
-    match Var.compare k k' with
-    | Ordering.Eq -> Some v
-    | _ -> None)
+  List.find_map t.vars ~f:(fun (k', v) -> Option.some_if (Var.equal k k') v)
 
 let to_unix t =
   match t.unix with

From 161be3af1ce61c91b2cd81628f1c95a407213e3c Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Mon, 12 Mar 2018 10:47:56 +0700
Subject: [PATCH 12/14] Stage Var.compare

---
 src/env.ml | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/env.ml b/src/env.ml
index 772c7772..f0160155 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -2,11 +2,12 @@ open Import
 
 module Var = struct
   type t = string
-  let compare a b =
-    if Sys.win32 then
-      String.compare (String.lowercase a) (String.lowercase b)
-    else
-      String.compare a b
+  let compare =
+    if Sys.win32 then (
+      fun a b -> String.compare (String.lowercase a) (String.lowercase b)
+    ) else (
+      String.compare
+    )
 
   let equal a b =
     match compare a b with

From 4a68db622d3f4e68bb37847792c537e900e09ed5 Mon Sep 17 00:00:00 2001
From: Rudi Grinberg <rudi.grinberg@gmail.com>
Date: Mon, 12 Mar 2018 16:56:19 +0700
Subject: [PATCH 13/14] Change representation to map from assoc list

---
 src/env.ml | 50 +++++++++++++-------------------------------------
 1 file changed, 13 insertions(+), 37 deletions(-)

diff --git a/src/env.ml b/src/env.ml
index f0160155..7c7947d5 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -8,25 +8,12 @@ module Var = struct
     ) else (
       String.compare
     )
-
-  let equal a b =
-    match compare a b with
-    | Ordering.Eq -> true
-    | _ -> false
-
-  let hash =
-    if Sys.win32 then
-      fun x -> Hashtbl.hash (String.lowercase x)
-    else
-      Hashtbl.hash
 end
 
 module Map = Map.Make(Var)
 
-module Table = Hashtbl.Make(Var)
-
 type t =
-  { vars : (Var.t * string) list
+  { vars : string Map.t
   ; mutable unix : string array option
   }
 
@@ -35,24 +22,16 @@ let make vars =
   ; unix = None
   }
 
-let get t k =
-  List.find_map t.vars ~f:(fun (k', v) -> Option.some_if (Var.equal k k') v)
+let get t k = Map.find t.vars k
 
 let to_unix t =
   match t.unix with
   | Some v -> v
   | None ->
     let res =
-      let seen = Table.create 16 in
-      t.vars
-      |> List.fold_left ~init:[] ~f:(fun uniques (k, v) ->
-        if Table.mem seen k then (
-          uniques
-        ) else (
-          Table.add seen ~key:k ~data:();
-          (k, v) :: uniques
-        ))
-      |> List.rev_map ~f:(fun (k, v) -> sprintf "%s=%s" k v)
+      Map.foldi ~init:[] ~f:(fun k v acc ->
+        (sprintf "%s=%s" k v)::acc
+      ) t.vars
       |> Array.of_list in
     t.unix <- Some res;
     res
@@ -61,8 +40,11 @@ let of_unix arr =
   Array.to_list arr
   |> List.map ~f:(fun s ->
     match String.lsplit2 s ~on:'=' with
-    | None -> (s, "")
+    | None ->
+      Sexp.code_error "Env.of_unix doesn't support env vars without '='"
+        ["var", Sexp.To_sexp.string s]
     | Some (k, v) -> (k, v))
+  |> Map.of_list_exn
 
 let initial =
   let i =
@@ -74,24 +56,18 @@ let initial =
   fun () -> Lazy.force i
 
 let add t ~var ~value =
-  { vars = (var, value) :: t.vars
-  ; unix = None
-  }
+  make (Map.add t.vars var value)
 
 let extend t ~vars =
-  { vars = Map.foldi ~init:t.vars ~f:(fun k v t -> (k, v) :: t) vars
-  ; unix = None
-  }
+  make (Map.union t.vars vars ~f:(fun _ _ v -> Some v))
 
 let sexp_of_t t =
   let open Sexp.To_sexp in
-  (list (pair string string)) t.vars
+  (list (pair string string)) (Map.to_list t.vars)
 
 let diff x y =
-  let to_map b = Map.of_list_reduce b ~f:(fun old _new -> old) in
-  Map.merge (to_map x.vars) (to_map y.vars) ~f:(fun _k vx vy ->
+  Map.merge x.vars y.vars ~f:(fun _k vx vy ->
     match vy with
     | Some _ -> None
     | None -> vx)
-  |> Map.to_list
   |> make

From eef3633d9a42dfed03e751e6af3d2917ad566a38 Mon Sep 17 00:00:00 2001
From: Jeremie Dimino <jdimino@janestreet.com>
Date: Mon, 12 Mar 2018 11:22:54 +0000
Subject: [PATCH 14/14] Tweak error messages

---
 src/env.ml | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/env.ml b/src/env.ml
index 7c7947d5..7b625243 100644
--- a/src/env.ml
+++ b/src/env.ml
@@ -41,10 +41,18 @@ let of_unix arr =
   |> List.map ~f:(fun s ->
     match String.lsplit2 s ~on:'=' with
     | None ->
-      Sexp.code_error "Env.of_unix doesn't support env vars without '='"
+      Sexp.code_error "Env.of_unix: entry without '=' found in the environ"
         ["var", Sexp.To_sexp.string s]
     | Some (k, v) -> (k, v))
-  |> Map.of_list_exn
+  |> Map.of_list
+  |> function
+  | Ok x -> x
+  | Error (var, v1, v2) ->
+    Sexp.code_error "Env.of_unix: duplicated variable found in the environment"
+      [ "var"   , Sexp.To_sexp.string var
+      ; "value1", Sexp.To_sexp.string v1
+      ; "value2", Sexp.To_sexp.string v2
+      ]
 
 let initial =
   let i =