Skip to content

Force regenerate build.ninja #6877

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 7 commits into from
Jul 21, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,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

Expand Down
15 changes: 11 additions & 4 deletions jscomp/bsb/bsb_ninja_check.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand All @@ -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) =
Expand Down Expand Up @@ -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
Expand All @@ -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 -> previous_warn_as_error <> "0"
| 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 ->
Expand Down
3 changes: 3 additions & 0 deletions jscomp/bsb/bsb_ninja_check.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -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 *)
5 changes: 3 additions & 2 deletions jscomp/bsb/bsb_ninja_regen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 "@{<info>BSB check@} build spec : %a @."
Bsb_ninja_check.pp_check_result check_result;
Expand Down Expand Up @@ -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
41 changes: 36 additions & 5 deletions jscomp/build_tests/build_warn_as_error/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,47 @@ var p = require("child_process");
var assert = require("assert");
var { rescript_exe } = require("#cli/bin_path");

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);
}