From 164dea20e07ddf8cdd8a93f7b8a7736202c83574 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Wed, 17 Jul 2024 09:48:31 -0300 Subject: [PATCH 1/6] force regenerate build.ninja --- jscomp/bsb_exe/rescript_main.ml | 9 +--- .../build_tests/build_warn_as_error/input.js | 43 ++++++++++++++++--- jscomp/build_tests/in_source/input.js | 2 +- jscomp/build_tests/xpkg/input.js | 2 +- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index 7498116934..3b9a45fed2 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -32,8 +32,6 @@ let do_install = ref false let warning_as_error = ref None -let force_regenerate = ref false - type spec = Bsb_arg.spec let call_spec f : spec = Unit (Unit_call f) @@ -130,11 +128,6 @@ let build_subcommand ~start argv argv_len = (* This should be put in a subcommand previously it works with the implication `bsb && bsb -install` *) - ( "-regen", - unit_set_spec force_regenerate, - "*internal* \n\ - Always regenerate build.ninja no matter bsconfig.json is changed or \ - not" ); ("-no-deps", unit_set_spec no_deps_mode, "*internal* Needed for watcher to build without dependencies on file change"); ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Warning numbers and whether to turn them into errors, e.g., \"+8+32-102\"") |] @@ -155,7 +148,7 @@ let build_subcommand ~start argv argv_len = Bsb_ninja_regen.regenerate_ninja ~package_kind:Toplevel ~per_proj_dir:Bsb_global_paths.cwd - ~forced:!force_regenerate + ~forced:true ~warn_legacy_config:true ~warn_as_error in diff --git a/jscomp/build_tests/build_warn_as_error/input.js b/jscomp/build_tests/build_warn_as_error/input.js index a00405f010..7f5c9b22b0 100644 --- a/jscomp/build_tests/build_warn_as_error/input.js +++ b/jscomp/build_tests/build_warn_as_error/input.js @@ -1,17 +1,48 @@ var p = require("child_process"); var assert = require("assert"); -var { rescript_exe } = require("#cli/bin_path"); +var rescript_exe = require("../../../scripts/bin_path").rescript_exe; -var o = p.spawnSync(rescript_exe, ["build", "-warn-error", "+110"], { +var o1 = p.spawnSync(rescript_exe, ["build"], { encoding: "utf8", cwd: __dirname, }); -var error_message = o.stdout +var first_message = o1.stdout .split("\n") .map(s => s.trim()) - .includes("Warning number 110 (configured as error)"); + .find(s => s == "Warning number 110"); -if (!error_message) { - assert.fail(o.stdout); +if (!first_message) { + assert.fail(o1.stdout); +} + +// Second build using -warn-error +110 +var o2 = p.spawnSync(rescript_exe, ["build", "-warn-error", "+110"], { + encoding: "utf8", + cwd: __dirname, +}); + +var second_message = o2.stdout + .split("\n") + .map(s => s.trim()) + .find(s => s == "Warning number 110 (configured as error)"); + +if (!second_message) { + assert.fail(o2.stdout); +} + +// Third build, without -warn-error +110 +// The result should not be a warning as error +var o3 = p.spawnSync(rescript_exe, ["build"], { + encoding: "utf8", + cwd: __dirname, +}); + +var third_message = o3.stdout + .split("\n") + .map(s => s.trim()) + .find(s => s == "Dependency Finished"); + +if (!third_message) { + assert.fail(o3.stdout); } diff --git a/jscomp/build_tests/in_source/input.js b/jscomp/build_tests/in_source/input.js index 533a61a521..ae169924d7 100644 --- a/jscomp/build_tests/in_source/input.js +++ b/jscomp/build_tests/in_source/input.js @@ -6,7 +6,7 @@ var { rescript_exe } = require("#cli/bin_path"); assert.throws( () => { - var output = child_process.execSync(`${rescript_exe} build -regen`, { + var output = child_process.execSync(`${rescript_exe} build`, { cwd: __dirname, encoding: "utf8", }); diff --git a/jscomp/build_tests/xpkg/input.js b/jscomp/build_tests/xpkg/input.js index d499746478..3d6ec0bd4e 100644 --- a/jscomp/build_tests/xpkg/input.js +++ b/jscomp/build_tests/xpkg/input.js @@ -2,7 +2,7 @@ var p = require("child_process"); var assert = require("assert"); var { rescript_exe } = require("#cli/bin_path"); try { - var output = p.spawnSync(`${rescript_exe} build -regen`, { + var output = p.spawnSync(`${rescript_exe} build`, { shell: true, encoding: "utf8", }); From 02b9be1a9ee4eabdfb917666fc9cebc9623b7318 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Wed, 17 Jul 2024 09:50:39 -0300 Subject: [PATCH 2/6] fix bin path --- jscomp/build_tests/build_warn_as_error/input.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jscomp/build_tests/build_warn_as_error/input.js b/jscomp/build_tests/build_warn_as_error/input.js index 7f5c9b22b0..fd39a5d5af 100644 --- a/jscomp/build_tests/build_warn_as_error/input.js +++ b/jscomp/build_tests/build_warn_as_error/input.js @@ -1,6 +1,6 @@ var p = require("child_process"); var assert = require("assert"); -var rescript_exe = require("../../../scripts/bin_path").rescript_exe; +var { rescript_exe } = require("#cli/bin_path"); var o1 = p.spawnSync(rescript_exe, ["build"], { encoding: "utf8", From ab7730c68696503c82a831ff3bbaafefabbcdc52 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Wed, 17 Jul 2024 10:00:35 -0300 Subject: [PATCH 3/6] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e76730088..e5f51bb436 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ - Remove support for compiling `.ml` files, and general cleanup. https://github.com/rescript-lang/rescript-compiler/pull/6852 - Remove `rescript convert` subcommand. https://github.com/rescript-lang/rescript-compiler/pull/6860 - Remove support for `@bs.send.pipe`. https://github.com/rescript-lang/rescript-compiler/pull/6858 +- Remove `regen` argument. Now ReScript regenerate `build.ninja` for each build. https://github.com/rescript-lang/rescript-compiler/pull/6877 #### :bug: Bug Fix @@ -47,6 +48,7 @@ - Fix issue where using partial application `...` can generate code that uses `Curry` at runtime. https://github.com/rescript-lang/rescript-compiler/pull/6872 - Avoid generation of `Curry` with reverse application `|>`. https://github.com/rescript-lang/rescript-compiler/pull/6876 - Fix issue where the internal ppx for pipe `->` would not use uncurried application in uncurried mode. https://github.com/rescript-lang/rescript-compiler/pull/6878 +- Fix build after calling without `-warn-error`, see https://github.com/rescript-lang/rescript-compiler/issues/6868 for more details. https://github.com/rescript-lang/rescript-compiler/pull/6877 #### :house: Internal From 53d7f20d44b1d17e0dd3d334f77086f12eeb8a56 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 20 Jul 2024 13:02:27 -0300 Subject: [PATCH 4/6] force regenerate when `warn-error` argument change --- jscomp/bsb/bsb_ninja_check.ml | 15 +++++++++++---- jscomp/bsb/bsb_ninja_check.mli | 3 +++ jscomp/bsb/bsb_ninja_regen.ml | 5 +++-- jscomp/bsb_exe/rescript_main.ml | 9 ++++++++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/jscomp/bsb/bsb_ninja_check.ml b/jscomp/bsb/bsb_ninja_check.ml index 09ad563da5..9c652f56d8 100644 --- a/jscomp/bsb/bsb_ninja_check.ml +++ b/jscomp/bsb/bsb_ninja_check.ml @@ -43,6 +43,7 @@ type check_result = | Bsb_bsc_version_mismatch | Bsb_forced | Bsb_package_kind_inconsistent + | Bsb_regenerate_required | Other of string let pp_check_result fmt (check_resoult : check_result) = @@ -55,6 +56,7 @@ let pp_check_result fmt (check_resoult : check_result) = | Bsb_bsc_version_mismatch -> "Bsc or bsb version mismatch" | Bsb_forced -> "Bsb forced rebuild" | Bsb_package_kind_inconsistent -> "The package was built in different mode" + | Bsb_regenerate_required -> "Bsb need regenerate build.ninja" | Other s -> s) let rec check_aux cwd (xs : string list) = @@ -91,13 +93,14 @@ let record_global_atime buf name = Ext_buffer.add_string_char buf (hex_of_float stamp) '\n' let record ~(package_kind : Bsb_package_kind.t) ~per_proj_dir ~file - ~(config : Bsb_config_types.t) (file_or_dirs : string list) : unit = + ~(config : Bsb_config_types.t) ~(warn_as_error: string option) (file_or_dirs : string list) : unit = let buf = Ext_buffer.create 1_000 in Ext_buffer.add_string_char buf Bs_version.version '\n'; Ext_buffer.add_string_char buf per_proj_dir '\n'; Ext_buffer.add_string_char buf (Bsb_package_kind.encode_no_nl package_kind) '\n'; + Ext_buffer.add_string_char buf (match warn_as_error with | Some s -> s | None -> "0") '\n'; Ext_list.iter file_or_dirs (fun f -> Ext_buffer.add_string_char buf f '\t'; Ext_buffer.add_string_char buf @@ -119,21 +122,25 @@ let record ~(package_kind : Bsb_package_kind.t) ~per_proj_dir ~file Even forced, we still need walk through a little bit in case we found a different version of compiler *) -let check ~(package_kind : Bsb_package_kind.t) ~(per_proj_dir : string) ~forced - ~file : check_result = +let check ~(package_kind : Bsb_package_kind.t) ~(per_proj_dir : string) ~forced ~(warn_as_error: string option) ~file : check_result = match open_in_bin file with (* Windows binary mode*) | exception _ -> Bsb_file_not_exist | ic -> ( match List.rev (Ext_io.rev_lines_of_chann ic) with | exception _ -> Bsb_file_corrupted - | version :: source_directory :: package_kind_str :: dir_or_files -> ( + | version :: source_directory :: package_kind_str :: previous_warn_as_error :: dir_or_files -> ( + let warn_as_error_changed = match warn_as_error with + | None -> false + | Some current -> current <> previous_warn_as_error in + if version <> Bs_version.version then Bsb_bsc_version_mismatch else if per_proj_dir <> source_directory then Bsb_source_directory_changed else if forced then Bsb_forced (* No need walk through *) else if Bsb_package_kind.encode_no_nl package_kind <> package_kind_str then Bsb_package_kind_inconsistent + else if warn_as_error_changed then Bsb_regenerate_required else try check_aux per_proj_dir dir_or_files with e -> diff --git a/jscomp/bsb/bsb_ninja_check.mli b/jscomp/bsb/bsb_ninja_check.mli index f2f48c644e..adff881079 100644 --- a/jscomp/bsb/bsb_ninja_check.mli +++ b/jscomp/bsb/bsb_ninja_check.mli @@ -38,6 +38,7 @@ type check_result = | Bsb_bsc_version_mismatch | Bsb_forced | Bsb_package_kind_inconsistent + | Bsb_regenerate_required | Other of string val pp_check_result : Format.formatter -> check_result -> unit @@ -47,6 +48,7 @@ val record : per_proj_dir:string -> file:string -> config:Bsb_config_types.t -> + warn_as_error:string option -> string list -> unit (** [record cwd file relevant_file_or_dirs] @@ -64,6 +66,7 @@ val check : package_kind:Bsb_package_kind.t -> per_proj_dir:string -> forced:bool -> + warn_as_error: string option -> file:string -> check_result (** check if [build.ninja] should be regenerated *) diff --git a/jscomp/bsb/bsb_ninja_regen.ml b/jscomp/bsb/bsb_ninja_regen.ml index d2953b93ee..cabf8d84bd 100644 --- a/jscomp/bsb/bsb_ninja_regen.ml +++ b/jscomp/bsb/bsb_ninja_regen.ml @@ -36,7 +36,7 @@ let regenerate_ninja ~(package_kind : Bsb_package_kind.t) ~forced ~per_proj_dir let lib_bs_dir = per_proj_dir // lib_artifacts_dir in let output_deps = lib_bs_dir // bsdeps in let check_result = - Bsb_ninja_check.check ~package_kind ~per_proj_dir ~forced ~file:output_deps + Bsb_ninja_check.check ~package_kind ~per_proj_dir ~forced ~warn_as_error ~file:output_deps in let config_filename, config_json = Bsb_config_load.load_json ~per_proj_dir ~warn_legacy_config @@ -45,6 +45,7 @@ let regenerate_ninja ~(package_kind : Bsb_package_kind.t) ~forced ~per_proj_dir | Good -> None (* Fast path, no need regenerate ninja *) | Bsb_forced | Bsb_bsc_version_mismatch | Bsb_package_kind_inconsistent | Bsb_file_corrupted | Bsb_file_not_exist | Bsb_source_directory_changed + | Bsb_regenerate_required | Other _ -> Bsb_log.info "@{BSB check@} build spec : %a @." Bsb_ninja_check.pp_check_result check_result; @@ -94,7 +95,7 @@ let regenerate_ninja ~(package_kind : Bsb_package_kind.t) ~forced ~per_proj_dir config; (* PR2184: we still need record empty dir since it may add files in the future *) - Bsb_ninja_check.record ~package_kind ~per_proj_dir ~config + Bsb_ninja_check.record ~package_kind ~per_proj_dir ~config ~warn_as_error ~file:output_deps (config.filename :: config.file_groups.globbed_dirs); Some config diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index 3b9a45fed2..7498116934 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -32,6 +32,8 @@ let do_install = ref false let warning_as_error = ref None +let force_regenerate = ref false + type spec = Bsb_arg.spec let call_spec f : spec = Unit (Unit_call f) @@ -128,6 +130,11 @@ let build_subcommand ~start argv argv_len = (* This should be put in a subcommand previously it works with the implication `bsb && bsb -install` *) + ( "-regen", + unit_set_spec force_regenerate, + "*internal* \n\ + Always regenerate build.ninja no matter bsconfig.json is changed or \ + not" ); ("-no-deps", unit_set_spec no_deps_mode, "*internal* Needed for watcher to build without dependencies on file change"); ("-warn-error", string_call (fun s -> warning_as_error := Some s), "Warning numbers and whether to turn them into errors, e.g., \"+8+32-102\"") |] @@ -148,7 +155,7 @@ let build_subcommand ~start argv argv_len = Bsb_ninja_regen.regenerate_ninja ~package_kind:Toplevel ~per_proj_dir:Bsb_global_paths.cwd - ~forced:true + ~forced:!force_regenerate ~warn_legacy_config:true ~warn_as_error in From d83ed33adb4899d6a502d0ccf12187f7ef871d4c Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 20 Jul 2024 13:04:30 -0300 Subject: [PATCH 5/6] restore `-regen` argument --- jscomp/build_tests/in_source/input.js | 2 +- jscomp/build_tests/xpkg/input.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jscomp/build_tests/in_source/input.js b/jscomp/build_tests/in_source/input.js index ae169924d7..533a61a521 100644 --- a/jscomp/build_tests/in_source/input.js +++ b/jscomp/build_tests/in_source/input.js @@ -6,7 +6,7 @@ var { rescript_exe } = require("#cli/bin_path"); assert.throws( () => { - var output = child_process.execSync(`${rescript_exe} build`, { + var output = child_process.execSync(`${rescript_exe} build -regen`, { cwd: __dirname, encoding: "utf8", }); diff --git a/jscomp/build_tests/xpkg/input.js b/jscomp/build_tests/xpkg/input.js index 3d6ec0bd4e..d499746478 100644 --- a/jscomp/build_tests/xpkg/input.js +++ b/jscomp/build_tests/xpkg/input.js @@ -2,7 +2,7 @@ var p = require("child_process"); var assert = require("assert"); var { rescript_exe } = require("#cli/bin_path"); try { - var output = p.spawnSync(`${rescript_exe} build`, { + var output = p.spawnSync(`${rescript_exe} build -regen`, { shell: true, encoding: "utf8", }); From 6c5f795c08de59a02020e9faac18044a905aa17a Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Sat, 20 Jul 2024 13:34:36 -0300 Subject: [PATCH 6/6] regenerate if previous warn error != 0 --- jscomp/bsb/bsb_ninja_check.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jscomp/bsb/bsb_ninja_check.ml b/jscomp/bsb/bsb_ninja_check.ml index 9c652f56d8..b11bf66c12 100644 --- a/jscomp/bsb/bsb_ninja_check.ml +++ b/jscomp/bsb/bsb_ninja_check.ml @@ -131,7 +131,7 @@ let check ~(package_kind : Bsb_package_kind.t) ~(per_proj_dir : string) ~forced | exception _ -> Bsb_file_corrupted | version :: source_directory :: package_kind_str :: previous_warn_as_error :: dir_or_files -> ( let warn_as_error_changed = match warn_as_error with - | None -> false + | None -> previous_warn_as_error <> "0" | Some current -> current <> previous_warn_as_error in if version <> Bs_version.version then Bsb_bsc_version_mismatch