Skip to content

various improvements over incremental build #4747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions jscomp/bsb/bsb_ninja_file_groups.ml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ let emit_module_build
(package_specs : Bsb_package_specs.t)
(is_dev : bool)
oc
js_post_build_cmd
namespace
(module_info : Bsb_db.module_info)
=
Expand Down Expand Up @@ -131,15 +130,6 @@ let emit_module_build
~rule:(if is_dev then rules.mi_dev else rules.mi)
;
end;

let shadows : Bsb_ninja_targets.shadow list =
match js_post_build_cmd with
| None -> []
| Some cmd ->
[{key = Bsb_ninja_global_vars.postbuild;
op = Overwrite ("&& " ^ cmd ^ Ext_string.single_space ^ String.concat Ext_string.single_space output_js)}]

in
let rule =
if has_intf_file then
(if is_dev then rules.mj_dev
Expand All @@ -151,7 +141,6 @@ let emit_module_build
in
Bsb_ninja_targets.output_build oc
~outputs:[output_cmj]
~shadows
~implicit_outputs:
(if has_intf_file then output_js else output_cmi::output_js )
~inputs:[output_mlast]
Expand All @@ -170,7 +159,6 @@ let handle_files_per_dir
oc
~(rules : Bsb_ninja_rule.builtin)
~package_specs
~js_post_build_cmd
~(files_to_install : Hash_set_string.t)
~(namespace : string option)
(group: Bsb_file_groups.file_group )
Expand All @@ -192,7 +180,6 @@ let handle_files_per_dir
package_specs
group.dev_index
oc
js_post_build_cmd
namespace module_info
)

Expand Down
1 change: 0 additions & 1 deletion jscomp/bsb/bsb_ninja_file_groups.mli
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ val handle_files_per_dir :
out_channel ->
rules:Bsb_ninja_rule.builtin ->
package_specs:Bsb_package_specs.t ->
js_post_build_cmd:string option ->
files_to_install:Hash_set_string.t ->
namespace:string option ->
Bsb_file_groups.file_group -> unit
25 changes: 10 additions & 15 deletions jscomp/bsb/bsb_ninja_gen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,11 @@ let output_ninja_and_namespace_map
let cwd_lib_bs = per_proj_dir // lib_artifacts_dir in
let ppx_flags = Bsb_build_util.ppx_flags ppx_files in
let oc = open_out_bin (cwd_lib_bs // Literals.build_ninja) in
let g_pkg_flg , g_ns_flg =
match namespace with
| None ->
Ext_string.inter2 "-bs-package-name" package_name, Ext_string.empty
| Some s ->
Ext_string.inter4
"-bs-package-name" package_name
"-bs-ns" s
,
Ext_string.inter2 "-bs-ns" s in
let g_pkg_flg =
Ext_string.inter2 "-bs-package-name" package_name
in
let warnings = Bsb_warning.to_bsb_string ~toplevel warning in
let bsc_flags = (get_bsc_flags bsc_flags) in
let () =
Ext_option.iter pp_file (fun flag ->
Bsb_ninja_targets.output_kv Bsb_ninja_global_vars.pp_flags
Expand All @@ -150,15 +145,15 @@ let output_ninja_and_namespace_map
Bsb_ninja_global_vars.bsc, (Ext_filename.maybe_quote Bsb_global_paths.vendor_bsc);
(* The path to [bsb_heler.exe] *)
Bsb_ninja_global_vars.bsdep, (Ext_filename.maybe_quote Bsb_global_paths.vendor_bsdep) ;
Bsb_ninja_global_vars.warnings, Bsb_warning.to_bsb_string ~toplevel warning ;
Bsb_ninja_global_vars.bsc_flags, (get_bsc_flags bsc_flags) ;
Bsb_ninja_global_vars.warnings, warnings;
Bsb_ninja_global_vars.bsc_flags, bsc_flags;
Bsb_ninja_global_vars.ppx_flags, ppx_flags;

Bsb_ninja_global_vars.g_dpkg_incls,
(Bsb_build_util.include_dirs_by
bs_dev_dependencies
(fun x -> x.package_install_path));
Bsb_ninja_global_vars.g_ns , g_ns_flg ;

|] oc
in
let bs_groups : Bsb_db.t = {lib = Map_string.empty; dev = Map_string.empty} in
Expand Down Expand Up @@ -199,12 +194,13 @@ let output_ninja_and_namespace_map
Bsb_ninja_rule.make_custom_rules
~refmt
~has_gentype:(gentype_config <> None)
~has_postbuild:(js_post_build_cmd <> None)
~has_postbuild:js_post_build_cmd
~has_ppx:(ppx_files <> [])
~has_pp:(pp_file <> None)
~has_builtin:(built_in_dependency <> None)
~reason_react_jsx
~package_specs
~namespace
~digest
generators in
emit_bsc_lib_includes bs_dependencies source_dirs.lib external_includes namespace oc;
Expand All @@ -214,7 +210,6 @@ let output_ninja_and_namespace_map
(fun files_per_dir ->
Bsb_ninja_file_groups.handle_files_per_dir oc
~rules
~js_post_build_cmd
~package_specs
~files_to_install
~namespace files_per_dir)
Expand Down
2 changes: 1 addition & 1 deletion jscomp/bsb/bsb_ninja_global_vars.ml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ let refmt_flags = "refmt_flags"

let postbuild = "postbuild"

let g_ns = "g_ns"


let warnings = "warnings"

Expand Down
39 changes: 27 additions & 12 deletions jscomp/bsb/bsb_ninja_rule.ml
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ let print_rule (oc : out_channel)
begin match description with
| None -> ()
| Some description ->
output_string oc " description = " ; output_string oc description
end ;
output_string oc "\n"
output_string oc " description = " ; output_string oc description;
output_string oc "\n"
end




Expand Down Expand Up @@ -114,27 +115,35 @@ type builtin = {

let make_custom_rules
~(has_gentype : bool)
~(has_postbuild : bool)
~(has_postbuild : string option)
~(has_ppx : bool)
~(has_pp : bool)
~(has_builtin : bool)
~(reason_react_jsx : Bsb_config_types.reason_react_jsx option)
~(digest : string)
~(refmt : string option) (* set refmt path when needed *)
~(package_specs: Bsb_package_specs.t)
~namespace
(custom_rules : command Map_string.t) :
builtin =
(** FIXME: We don't need set [-o ${out}] when building ast
since the default is already good -- it does not*)
let buf = Ext_buffer.create 100 in
let ns_flag =
match namespace with None -> ""
| Some n -> " -bs-ns " ^ n in
let mk_ml_cmj_cmd
~(read_cmi : [`yes | `is_cmi | `no])
~is_dev
~postbuild : string =
Ext_buffer.clear buf;
Ext_buffer.add_string buf "$bsc";
Ext_buffer.add_ninja_prefix_var buf Bsb_ninja_global_vars.g_pkg_flg;
Ext_buffer.add_string buf (Bsb_package_specs.package_flag_of_package_specs package_specs "$in_d");

Ext_buffer.add_string buf ns_flag;
if read_cmi <> `is_cmi then begin
Ext_buffer.add_ninja_prefix_var buf Bsb_ninja_global_vars.g_pkg_flg;
Ext_buffer.add_string buf (Bsb_package_specs.package_flag_of_package_specs package_specs "$in_d")
end;
if read_cmi = `yes then
Ext_buffer.add_string buf " -bs-read-cmi";
if is_dev then
Expand All @@ -148,8 +157,13 @@ let make_custom_rules
if has_gentype then
Ext_buffer.add_ninja_prefix_var buf Bsb_ninja_global_vars.gentypeconfig;
Ext_buffer.add_string buf " -o $out $in";
if postbuild then
Ext_buffer.add_string buf " $postbuild";
begin match postbuild with
| None -> ()
| Some cmd ->
Ext_buffer.add_string buf " && ";
Ext_buffer.add_string buf cmd ;
Ext_buffer.add_string buf " $out_last"
end ;
Ext_buffer.contents buf
in
let mk_ast ~(has_pp : bool) ~has_ppx ~has_reason_react_jsx : string =
Expand All @@ -172,7 +186,7 @@ let make_custom_rules
);
if has_ppx then
Ext_buffer.add_ninja_prefix_var buf Bsb_ninja_global_vars.ppx_flags;
Ext_buffer.add_string buf " $bsc_flags -o $out -bs-syntax-only -bs-binary-ast $in";
Ext_buffer.add_string buf " $bsc_flags -o $out -bs-ast $in";
Ext_buffer.contents buf
in
let build_ast =
Expand All @@ -192,17 +206,18 @@ let make_custom_rules
else "cp $in $out"
)
"copy_resource" in

let build_bin_deps =
define
~restat:()
~command:
("$bsdep -hash " ^ digest ^" $g_ns $in")
("$bsdep -hash " ^ digest ^ ns_flag ^ " $in")
"deps" in
let build_bin_deps_dev =
define
~restat:()
~command:
("$bsdep -g -hash " ^ digest ^" $g_ns $in")
("$bsdep -g -hash " ^ digest ^ ns_flag ^ " $in")
"deps_dev" in
let aux ~name ~read_cmi ~postbuild =
define
Expand All @@ -229,7 +244,7 @@ let make_custom_rules
~name:"mij" ~postbuild:has_postbuild in
let mi, mi_dev =
aux
~read_cmi:`is_cmi ~postbuild:false
~read_cmi:`is_cmi ~postbuild:None
~name:"mi" in
let build_package =
define
Expand Down
3 changes: 2 additions & 1 deletion jscomp/bsb/bsb_ninja_rule.mli
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,15 @@ type command = string
*)
val make_custom_rules :
has_gentype:bool ->
has_postbuild:bool ->
has_postbuild:string option ->
has_ppx:bool ->
has_pp:bool ->
has_builtin:bool ->
reason_react_jsx : Bsb_config_types.reason_react_jsx option ->
digest:string ->
refmt:string option ->
package_specs:Bsb_package_specs.t ->
namespace:string option ->
command Map_string.t ->
builtin

8 changes: 4 additions & 4 deletions jscomp/bsb/bsb_ninja_targets.ml
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ let output_build
~rule
oc =
let rule = Bsb_ninja_rule.get_name rule oc in (* Trigger building if not used *)
output_string oc "o ";
output_string oc "o";
Ext_list.iter outputs (fun s -> output_string oc Ext_string.single_space ; output_string oc s );
if implicit_outputs <> [] then begin
output_string oc " | ";
output_string oc " |";
Ext_list.iter implicit_outputs (fun s -> output_string oc Ext_string.single_space ; output_string oc s)
end;
output_string oc " : ";
Expand All @@ -74,7 +74,7 @@ let output_build
;
if order_only_deps <> [] then
begin
output_string oc " || ";
output_string oc " ||";
Ext_list.iter order_only_deps (fun s -> output_string oc Ext_string.single_space ; output_string oc s)
end
;
Expand Down Expand Up @@ -146,6 +146,6 @@ let output_kv key value oc =
output_string oc "\n"

let output_kvs kvs oc =
Ext_array.iter kvs (fun (k,v) -> output_kv k v oc)
Ext_array.iter kvs (fun (k,v) -> if v <> "" then output_kv k v oc)


26 changes: 26 additions & 0 deletions jscomp/build_tests/post-build/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
*.exe
*.obj
*.out
*.compile
*.native
*.byte
*.cmo
*.annot
*.cmi
*.cmx
*.cmt
*.cmti
*.cma
*.a
*.cmxa
*.obj
*~
*.annot
*.cmj
*.bak
lib/bs
*.mlast
*.mliast
.vscode
.merlin
.bsb.lock
13 changes: 13 additions & 0 deletions jscomp/build_tests/post-build/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@


# Build
```
npm run build
```

# Watch

```
npm run watch
```

12 changes: 12 additions & 0 deletions jscomp/build_tests/post-build/bsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "post-build",
"version": "0.1.0",
"sources": {
"dir" : "src",
"subdirs" : true
},
"js-post-build": {"cmd":"cat"},
"warnings": {
"error" : "+101"
}
}
8 changes: 8 additions & 0 deletions jscomp/build_tests/post-build/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
var child_process = require('child_process')
var assert = require('assert')

var out = child_process.spawnSync(`bsb`,{encoding : 'utf8'})

if(out.status !== 0 ){
assert.fail(out.stdout + out.stderr)
}
17 changes: 17 additions & 0 deletions jscomp/build_tests/post-build/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "post-build",
"version": "0.1.0",
"scripts": {
"clean": "bsb -clean-world",
"build": "bsb -make-world",
"watch": "bsb -make-world -w"
},
"keywords": [
"BuckleScript"
],
"author": "",
"license": "MIT",
"devDependencies": {
"bs-platform": "^8.3.0-dev.2"
}
}
3 changes: 3 additions & 0 deletions jscomp/build_tests/post-build/src/demo.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@


let () = Js.log "Hello, BuckleScript"
1 change: 1 addition & 0 deletions jscomp/build_tests/post-build/src/hello.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let a = 3
4 changes: 2 additions & 2 deletions jscomp/main/js_main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ let buckle_script_flags : (string * Bsc_args.spec * string) array =
"-bs-package-output", string_call Js_packages_state.update_npm_package_path,
"*internal* Set npm-output-path: [opt_module]:path, for example: 'lib/cjs', 'amdjs:lib/amdjs', 'es6:lib/es6' ";

"-bs-binary-ast", set Js_config.binary_ast,
"*internal* Generate binary .mli_ast and ml_ast";
"-bs-ast", unit_call(fun _ -> Js_config.binary_ast := true; Js_config.syntax_only := true),
"*internal* Generate binary .mli_ast and ml_ast and stop";

"-bs-syntax-only", set Js_config.syntax_only,
"Only check syntax";
Expand Down
Loading