From 78b4849ffd468d6e2e9f4484ab2aca2c789ced1e Mon Sep 17 00:00:00 2001
From: Benjamin San Souci <benjamin.sansouci@gmail.com>
Date: Mon, 12 Feb 2018 14:17:10 -0800
Subject: [PATCH] Add -build-artifacts-dir for configurable out-of-source
 artifacts

This works by creating a dir of the name given and then doing the same
thing as it used to do: `given_dir/lib/bs` and `given_dir/lib/ocaml`
etc...

The bs-dependencies' artifacts are in `dir/node_modules/dep_name/lib`

This does not seem to significantly increase build times, but it'd be
nice to make sure it doesn't.

cc @chenglou @schmavery
---
 jscomp/all.depend                   |  3 ++-
 jscomp/bsb/bsb_build_util.ml        | 18 +++++++++++++++++
 jscomp/bsb/bsb_build_util.mli       |  4 ++++
 jscomp/bsb/bsb_clean.ml             |  4 ++--
 jscomp/bsb/bsb_config.ml            |  4 ++++
 jscomp/bsb/bsb_config.mli           |  3 +++
 jscomp/bsb/bsb_config_parse.ml      | 14 +++++++++++---
 jscomp/bsb/bsb_main.ml              | 30 ++++++++++++++++++-----------
 jscomp/bsb/bsb_ninja_gen.ml         |  8 +++++---
 jscomp/bsb/bsb_ninja_global_vars.ml |  4 +++-
 jscomp/bsb/bsb_ninja_regen.ml       | 11 ++++++-----
 jscomp/bsb/bsb_package_specs.ml     | 17 ++++++++--------
 jscomp/bsb/bsb_parse_sources.ml     |  2 +-
 jscomp/bsb/bsb_world.ml             |  7 ++++---
 jscomp/core/js_packages_info.ml     |  4 ++--
 jscomp/core/lam_compile_main.ml     |  2 +-
 16 files changed, 94 insertions(+), 41 deletions(-)

diff --git a/jscomp/all.depend b/jscomp/all.depend
index 290253c7e7..b19f8e727b 100644
--- a/jscomp/all.depend
+++ b/jscomp/all.depend
@@ -824,7 +824,8 @@ bsb/bsb_build_util.cmx : ext/string_map.cmx ext/string_hashtbl.cmx \
     ext/literals.cmx ext/ext_sys.cmx ext/ext_string.cmx ext/ext_path.cmx \
     ext/ext_list.cmx ext/ext_json_types.cmx ext/ext_json_parse.cmx \
     ext/ext_json.cmx ext/ext_array.cmx bsb/bsb_pkg.cmx bsb/bsb_log.cmx \
-    bsb/bsb_exception.cmx bsb/bsb_build_schemas.cmx bsb/bsb_build_util.cmi
+    bsb/bsb_exception.cmx bsb/bsb_config.cmx bsb/bsb_build_schemas.cmx \
+    bsb/bsb_build_util.cmi
 bsb/bsb_clean.cmx : ext/ext_path.cmx bsb/bsb_unix.cmx bsb/bsb_log.cmx \
     bsb/bsb_config.cmx bsb/bsb_build_util.cmx bsb/bsb_clean.cmi
 bsb/bsb_config.cmx : ext/ext_path.cmx bsb/bsb_config.cmi
diff --git a/jscomp/bsb/bsb_build_util.ml b/jscomp/bsb/bsb_build_util.ml
index 3f726575a7..a3d1d498d9 100644
--- a/jscomp/bsb/bsb_build_util.ml
+++ b/jscomp/bsb/bsb_build_util.ml
@@ -242,3 +242,21 @@ let rec walk_all_deps_aux visited paths top dir cb =
 let walk_all_deps dir cb = 
   let visited = String_hashtbl.create 0 in 
   walk_all_deps_aux visited [] true dir cb 
+
+let build_artifacts_dir = ref None
+
+let get_build_artifacts_location cwd =
+  (* If the project's parent folder is not node_modules, we know it's the top level one. *)
+  if (Filename.basename (Filename.dirname cwd)) <> "node_modules" then 
+    match !build_artifacts_dir with 
+    | None -> cwd
+    | Some dir -> dir
+  else begin
+    match !build_artifacts_dir with 
+    | None -> cwd
+      (* (Filename.dirname (Filename.dirname cwd)) // Bsb_config.lib_lit // Bsb_config.node_modules // project_name *)
+    | Some dir -> 
+      let project_name = Filename.basename cwd in
+      dir // Bsb_config.lib_lit // Bsb_config.node_modules // project_name
+  end
+
diff --git a/jscomp/bsb/bsb_build_util.mli b/jscomp/bsb/bsb_build_util.mli
index 320923a54e..a78f5971a7 100644
--- a/jscomp/bsb/bsb_build_util.mli
+++ b/jscomp/bsb/bsb_build_util.mli
@@ -69,3 +69,7 @@ type package_context = {
 }
 
 val walk_all_deps : string -> (package_context -> unit) -> unit
+
+val build_artifacts_dir : (string option) ref
+
+val get_build_artifacts_location : string -> string
diff --git a/jscomp/bsb/bsb_clean.ml b/jscomp/bsb/bsb_clean.ml
index 7d679c09ad..519737defe 100644
--- a/jscomp/bsb/bsb_clean.ml
+++ b/jscomp/bsb/bsb_clean.ml
@@ -55,7 +55,7 @@ let clean_bs_garbage bsc_dir proj_dir =
 let clean_bs_deps bsc_dir proj_dir =
   Bsb_build_util.walk_all_deps  proj_dir  (fun { cwd} ->
       (* whether top or not always do the cleaning *)
-      clean_bs_garbage bsc_dir cwd
+      clean_bs_garbage bsc_dir (Bsb_build_util.get_build_artifacts_location cwd)
     )
 
-let clean_self bsc_dir proj_dir = clean_bs_garbage bsc_dir proj_dir
\ No newline at end of file
+let clean_self bsc_dir proj_dir = clean_bs_garbage bsc_dir (Bsb_build_util.get_build_artifacts_location proj_dir)
diff --git a/jscomp/bsb/bsb_config.ml b/jscomp/bsb/bsb_config.ml
index 20450a22f6..bbf4892222 100644
--- a/jscomp/bsb/bsb_config.ml
+++ b/jscomp/bsb/bsb_config.ml
@@ -47,6 +47,9 @@ let rev_lib_bs_prefix p = rev_lib_bs // p
 
 let ocaml_bin_install_prefix p = lib_ocaml // p
 
+let lazy_build_artifacts_dir = "$build_artifacts_dir"
+let build_artifacts_dir path = lazy_build_artifacts_dir // path
+
 let lazy_src_root_dir = "$src_root_dir" 
 let proj_rel path = lazy_src_root_dir // path
 
@@ -61,3 +64,4 @@ let proj_rel path = lazy_src_root_dir // path
 
 let cmd_package_specs = ref None 
 
+let node_modules = "node_modules"
diff --git a/jscomp/bsb/bsb_config.mli b/jscomp/bsb/bsb_config.mli
index c097b87450..bfe341a248 100644
--- a/jscomp/bsb/bsb_config.mli
+++ b/jscomp/bsb/bsb_config.mli
@@ -25,7 +25,9 @@
 
 val ocaml_bin_install_prefix : string -> string
 val proj_rel : string -> string
+val build_artifacts_dir : string -> string
 
+val lib_lit : string
 val lib_js : string 
 val lib_amd : string 
 val lib_bs : string
@@ -40,3 +42,4 @@ val rev_lib_bs_prefix : string -> string
 
 (** default not install, only when -make-world, its dependencies will be installed  *)
 
+val node_modules : string
diff --git a/jscomp/bsb/bsb_config_parse.ml b/jscomp/bsb/bsb_config_parse.ml
index 986c8ad878..4c6d224d2b 100644
--- a/jscomp/bsb/bsb_config_parse.ml
+++ b/jscomp/bsb/bsb_config_parse.ml
@@ -32,7 +32,7 @@ let resolve_package cwd  package_name =
   let x =  Bsb_pkg.resolve_bs_package ~cwd package_name  in
   {
     Bsb_config_types.package_name ;
-    package_install_path = x // Bsb_config.lib_ocaml
+    package_install_path = (Bsb_build_util.get_build_artifacts_location x) // Bsb_config.lib_ocaml
   }
 
 
@@ -187,7 +187,15 @@ let interpret_json
        ()
      | None 
      | Some _ ->
-       built_in_package := Some (resolve_package cwd Bs_version.package_name);
+     (* Inline this instead of calling resolve_package because bs-platform is a special pre-built package
+        whose artifacts are in `node_modules/bs-platform/lib` and which we don't want to rebuild. *)
+       let package_name = Bs_version.package_name in
+       let x =  Bsb_pkg.resolve_bs_package ~cwd package_name  in
+       built_in_package := Some (
+        {
+          Bsb_config_types.package_name ;
+          package_install_path = x // Bsb_config.lib_ocaml
+        });
     ) ;
     let package_specs =     
       match String_map.find_opt Bsb_build_schemas.package_specs map with 
@@ -274,7 +282,7 @@ let interpret_json
              namespace; 
             }  x in 
         if generate_watch_metadata then
-          Bsb_watcher_gen.generate_sourcedirs_meta cwd res ;     
+          Bsb_watcher_gen.generate_sourcedirs_meta (Bsb_build_util.get_build_artifacts_location cwd) res ;     
         begin match List.sort Ext_file_pp.interval_compare  res.intervals with
           | [] -> ()
           | queue ->
diff --git a/jscomp/bsb/bsb_main.ml b/jscomp/bsb/bsb_main.ml
index d45bd05f8c..281ceedcc3 100644
--- a/jscomp/bsb/bsb_main.ml
+++ b/jscomp/bsb/bsb_main.ml
@@ -82,7 +82,10 @@ let bsb_main_flags : (string * Arg.spec * string) list=
     "-where",
     Arg.Unit (fun _ -> 
         print_endline (Filename.dirname Sys.executable_name)),
-    " Show where bsb.exe is located"
+    " Show where bsb.exe is located";
+    
+    "-build-artifacts-dir", Arg.String (fun s -> Bsb_build_util.build_artifacts_dir := Some (cwd // s)),
+    " Sets the directory in which all the build artifacts will go into.";
   ]
 
 
@@ -94,22 +97,22 @@ let exec_command_then_exit  command =
   exit (Sys.command command ) 
 
 (* Execute the underlying ninja build call, then exit (as opposed to keep watching) *)
-let ninja_command_exit  vendor_ninja ninja_args  =
+let ninja_command_exit cwd vendor_ninja ninja_args  =
   let ninja_args_len = Array.length ninja_args in
   if Ext_sys.is_windows_or_cygwin then
     let path_ninja = Filename.quote vendor_ninja in 
     exec_command_then_exit @@ 
-    (if ninja_args_len = 0 then      
+    (if ninja_args_len = 0 then
        Ext_string.inter3
-         path_ninja "-C" Bsb_config.lib_bs
+         path_ninja "-C" (cwd // Bsb_config.lib_bs)
      else   
        let args = 
          Array.append 
-           [| path_ninja ; "-C"; Bsb_config.lib_bs|]
+           [| path_ninja ; "-C"; cwd // Bsb_config.lib_bs|]
            ninja_args in 
        Ext_string.concat_array Ext_string.single_space args)
   else
-    let ninja_common_args = [|"ninja.exe"; "-C"; Bsb_config.lib_bs |] in 
+    let ninja_common_args = [|"ninja.exe"; "-C"; cwd // Bsb_config.lib_bs |] in 
     let args = 
       if ninja_args_len = 0 then ninja_common_args else 
         Array.append ninja_common_args ninja_args in 
@@ -141,19 +144,24 @@ let watch_exit () =
 
 (* see discussion #929, if we catch the exception, we don't have stacktrace... *)
 let () =
-
   let vendor_ninja = bsc_dir // "ninja.exe" in  
   try begin 
     match Sys.argv with 
-    | [| _ |] ->  (* specialize this path [bsb.exe] which is used in watcher *)
+    | [| _ |]
+    | [| _; "-build-artifacts-dir"; _ |] ->  (* specialize this path [bsb.exe] which is used in watcher *)
       begin
+        (match Sys.argv with 
+          | [|_; "-build-artifacts-dir"; dir|] -> 
+            Bsb_build_util.build_artifacts_dir := Some (cwd // dir)
+          | _ -> ());
+        
         let _config_opt =  
           Bsb_ninja_regen.regenerate_ninja ~override_package_specs:None ~not_dev:false 
             ~generate_watch_metadata:true
             ~forced:false 
             cwd bsc_dir 
         in 
-        ninja_command_exit  vendor_ninja [||] 
+        ninja_command_exit (Bsb_build_util.get_build_artifacts_location cwd) vendor_ninja [||] 
       end
     | argv -> 
       begin
@@ -191,7 +199,7 @@ let () =
                        [bsb -regen ]
                     *)
                   end else if make_world then begin
-                    ninja_command_exit  vendor_ninja [||] 
+                    ninja_command_exit (Bsb_build_util.get_build_artifacts_location cwd) vendor_ninja [||] 
                   end
               end;
           end
@@ -204,7 +212,7 @@ let () =
             if !make_world then
               Bsb_world.make_world_deps cwd config_opt ;
             if !watch_mode then watch_exit ()
-            else ninja_command_exit  vendor_ninja ninja_args 
+            else ninja_command_exit (Bsb_build_util.get_build_artifacts_location cwd) vendor_ninja ninja_args 
           end
       end
   end
diff --git a/jscomp/bsb/bsb_ninja_gen.ml b/jscomp/bsb/bsb_ninja_gen.ml
index f52ae604b6..88e33567d0 100644
--- a/jscomp/bsb/bsb_ninja_gen.ml
+++ b/jscomp/bsb/bsb_ninja_gen.ml
@@ -77,7 +77,8 @@ let output_ninja_and_namespace_map
   let custom_rules = Bsb_rule.reset generators in 
   let bsc = bsc_dir // bsc_exe in   (* The path to [bsc.exe] independent of config  *)
   let bsdep = bsc_dir // bsb_helper_exe in (* The path to [bsb_heler.exe] *)
-  let cwd_lib_bs = cwd // Bsb_config.lib_bs in 
+  let build_artifacts_dir = Bsb_build_util.get_build_artifacts_location cwd in
+  let cwd_lib_bs = build_artifacts_dir // Bsb_config.lib_bs in 
   let ppx_flags = Bsb_build_util.flag_concat dash_ppx ppx_flags in
   let bsc_flags =  String.concat Ext_string.single_space bsc_flags in
   let refmt_flags = String.concat Ext_string.single_space refmt_flags in
@@ -150,7 +151,8 @@ let output_ninja_and_namespace_map
         Bsb_ninja_global_vars.bs_package_includes, bs_package_includes;
         Bsb_ninja_global_vars.bs_package_dev_includes, bs_package_dev_includes;  
         Bsb_ninja_global_vars.namespace , namespace_flag ; 
-        Bsb_build_schemas.bsb_dir_group, "0"  (*TODO: avoid name conflict in the future *)
+        Bsb_build_schemas.bsb_dir_group, "0";  (*TODO: avoid name conflict in the future *)
+        Bsb_ninja_global_vars.build_artifacts_dir, build_artifacts_dir;
       |] oc in
   let all_includes acc  = 
     match external_includes with 
@@ -242,7 +244,7 @@ let output_ninja_and_namespace_map
        ~output:Literals.build_ninja 
    | Some ns -> 
      let namespace_dir =     
-       cwd // Bsb_config.lib_bs  in
+       build_artifacts_dir // Bsb_config.lib_bs  in
      Bsb_namespace_map_gen.output 
        ~dir:namespace_dir ns
        bs_file_groups
diff --git a/jscomp/bsb/bsb_ninja_global_vars.ml b/jscomp/bsb/bsb_ninja_global_vars.ml
index e02278f7a0..313cc82296 100644
--- a/jscomp/bsb/bsb_ninja_global_vars.ml
+++ b/jscomp/bsb/bsb_ninja_global_vars.ml
@@ -50,4 +50,6 @@ let postbuild = "postbuild"
 let namespace = "namespace" 
 
 
-let warnings = "warnings"
\ No newline at end of file
+let warnings = "warnings"
+
+let build_artifacts_dir = "build_artifacts_dir"
diff --git a/jscomp/bsb/bsb_ninja_regen.ml b/jscomp/bsb/bsb_ninja_regen.ml
index 701af01157..ee7379ea7d 100644
--- a/jscomp/bsb/bsb_ninja_regen.ml
+++ b/jscomp/bsb/bsb_ninja_regen.ml
@@ -38,10 +38,11 @@ let regenerate_ninja
     ~generate_watch_metadata 
     ~forced cwd bsc_dir
   : _ option =
-  let output_deps = cwd // Bsb_config.lib_bs // bsdeps in
+  let build_artifacts_dir = Bsb_build_util.get_build_artifacts_location cwd in
+  let output_deps = build_artifacts_dir // Bsb_config.lib_bs // bsdeps in
   let check_result  =
     Bsb_ninja_check.check 
-      ~cwd  
+      ~cwd:build_artifacts_dir  
       ~forced ~file:output_deps in
   let () = 
     Bsb_log.info
@@ -56,9 +57,9 @@ let regenerate_ninja
     | Other _ -> 
       if check_result = Bsb_bsc_version_mismatch then begin 
         Bsb_log.info "@{<info>Different compiler version@}: clean current repo";
-        Bsb_clean.clean_self bsc_dir cwd; 
+        Bsb_clean.clean_self bsc_dir build_artifacts_dir; 
       end ; 
-      Bsb_build_util.mkp (cwd // Bsb_config.lib_bs); 
+      Bsb_build_util.mkp (build_artifacts_dir // Bsb_config.lib_bs); 
       let config = 
         Bsb_config_parse.interpret_json 
           ~override_package_specs
@@ -68,7 +69,7 @@ let regenerate_ninja
           cwd in 
       begin 
         Bsb_merlin_gen.merlin_file_gen ~cwd
-          (bsc_dir // bsppx_exe) config;       
+          (bsc_dir // bsppx_exe) config;      
         Bsb_ninja_gen.output_ninja_and_namespace_map 
           ~cwd ~bsc_dir ~not_dev config ;         
         (* PR2184: we still need record empty dir 
diff --git a/jscomp/bsb/bsb_package_specs.ml b/jscomp/bsb/bsb_package_specs.ml
index e269fca59b..17798cb638 100644
--- a/jscomp/bsb/bsb_package_specs.ml
+++ b/jscomp/bsb/bsb_package_specs.ml
@@ -126,12 +126,7 @@ let bs_package_output = "-bs-package-output"
     {[ -bs-package-output commonjs:lib/js/jscomp/test ]}
 *)
 let package_flag ({format; in_source } : spec) dir =
-  Ext_string.inter2
-    bs_package_output 
-    (Ext_string.concat3
-       format
-       Ext_string.single_colon
-       (if in_source then dir else
+  let dir = Bsb_config.build_artifacts_dir (if in_source then dir else
           (if format = Literals.amdjs then 
              amd_js_prefix dir 
            else if format = Literals.commonjs then 
@@ -142,7 +137,13 @@ let package_flag ({format; in_source } : spec) dir =
              es6_global_prefix dir   
            else if format = Literals.amdjs_global then 
              amdjs_global_prefix dir 
-           else assert false))
+           else assert false)) in
+  Ext_string.inter2
+    bs_package_output 
+    (Ext_string.concat3
+       format
+       Ext_string.single_colon
+       dir
     )
 
 let package_flag_of_package_specs (package_specs : t) 
@@ -173,7 +174,7 @@ let package_output ({format; in_source } : spec) output=
          amdjs_global_prefix
        else assert false)
   in
-  (Bsb_config.proj_rel @@ prefix output )
+  (Bsb_config.build_artifacts_dir @@ prefix output )
 
 (**
     [get_list_of_output_js specs "src/hi/hello"]
diff --git a/jscomp/bsb/bsb_parse_sources.ml b/jscomp/bsb/bsb_parse_sources.ml
index 19770baf64..4c47cfddbc 100644
--- a/jscomp/bsb/bsb_parse_sources.ml
+++ b/jscomp/bsb/bsb_parse_sources.ml
@@ -393,7 +393,7 @@ let rec
          let parent = Filename.concat cxt.root cxt.cwd in 
          let lib_parent = 
            Filename.concat (Filename.concat cxt.root Bsb_config.lib_bs) 
-             cxt.cwd in 
+             (Bsb_build_util.get_build_artifacts_location cxt.cwd) in 
          if not (String_map.mem (Ext_string.capitalize_ascii basename) cur_sources) then 
            begin 
              Unix.unlink (Filename.concat parent f);
diff --git a/jscomp/bsb/bsb_world.ml b/jscomp/bsb/bsb_world.ml
index 33f9168bdf..038d5aedd3 100644
--- a/jscomp/bsb/bsb_world.ml
+++ b/jscomp/bsb/bsb_world.ml
@@ -78,6 +78,7 @@ let build_bs_deps cwd deps =
     (fun {top; cwd} ->
        if not top then
          begin 
+           let build_artifacts_dir = Bsb_build_util.get_build_artifacts_location cwd in
            let config_opt = Bsb_ninja_regen.regenerate_ninja ~not_dev:true
                ~generate_watch_metadata:false
                ~override_package_specs:(Some deps) 
@@ -85,7 +86,7 @@ let build_bs_deps cwd deps =
                cwd bsc_dir  in (* set true to force regenrate ninja file so we have [config_opt]*)
            let command = 
             {Bsb_unix.cmd = vendor_ninja;
-              cwd = cwd // Bsb_config.lib_bs;
+              cwd = build_artifacts_dir // Bsb_config.lib_bs;
               args  = [|vendor_ninja|]
              } in     
            let eid =
@@ -98,7 +99,7 @@ let build_bs_deps cwd deps =
               Note that we can check if ninja print "no work to do", 
               then don't need reinstall more
            *)
-           install_targets cwd config_opt;
+           install_targets build_artifacts_dir config_opt;
          end
     )
 
@@ -114,4 +115,4 @@ let make_world_deps cwd (config : Bsb_config_types.t option) =
       *)
       Bsb_config_parse.package_specs_from_bsconfig ()
     | Some {package_specs} -> package_specs in
-  build_bs_deps cwd deps
\ No newline at end of file
+  build_bs_deps cwd deps
diff --git a/jscomp/core/js_packages_info.ml b/jscomp/core/js_packages_info.ml
index 6416529ddb..1604a20658 100644
--- a/jscomp/core/js_packages_info.ml
+++ b/jscomp/core/js_packages_info.ml
@@ -145,7 +145,7 @@ let get_js_path module_system
 *)
 let get_output_dir ~package_dir module_system 
     (info: t ) =
-  Filename.concat package_dir 
+  Ext_path.combine package_dir 
     (get_js_path module_system info)
 
 
@@ -172,7 +172,7 @@ let add_npm_package_path s (packages_info : t)  : t =
 
 
 
-let (//) = Filename.concat 
+let (//) = Ext_path.combine 
 
 
 
diff --git a/jscomp/core/lam_compile_main.ml b/jscomp/core/lam_compile_main.ml
index e719faf03a..e872f6f9f3 100644
--- a/jscomp/core/lam_compile_main.ml
+++ b/jscomp/core/lam_compile_main.ml
@@ -375,7 +375,7 @@ let compile  ~filename (output_prefix : string) env _sigs
     )
 ;;
 
-let (//) = Filename.concat  
+let (//) = Ext_path.combine  
 
 let lambda_as_module 
     finalenv