-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
Replace cli flag --prebuilt-platform
with --build-host
#6859
base: main
Are you sure you want to change the base?
Changes from all commits
dc0902b
753cfb4
85447ec
96c559e
4ccbc41
287d00d
f515311
bd68787
61a5a0a
d747cc9
dfa3254
bc29782
89d63f2
3662c90
4ec213a
550bade
49f5ea8
28ceb0e
2928969
6276cc7
4863234
014514a
64cc816
9ecb209
0767115
804cb6e
c8522c2
d2fbf86
7ce9ecd
894e4d1
5c8eb4f
6ab0de1
0256c4e
8ad8198
fa40166
c3785a2
767ec7a
ac78d39
8612220
b0c1d88
e7a90f1
cd955fd
93a4c58
0e917be
4e6178c
f92974c
012a2fc
a11f51a
ec8f6e8
251e1ed
ef87653
c6a67c9
5dfcdae
73846ed
beb2dcb
bb57c4a
dc138b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,6 @@ pub const CMD_VERSION: &str = "version"; | |
pub const CMD_FORMAT: &str = "format"; | ||
pub const CMD_TEST: &str = "test"; | ||
pub const CMD_GLUE: &str = "glue"; | ||
pub const CMD_GEN_STUB_LIB: &str = "gen-stub-lib"; | ||
pub const CMD_PREPROCESS_HOST: &str = "preprocess-host"; | ||
|
||
pub const FLAG_EMIT_LLVM_IR: &str = "emit-llvm-ir"; | ||
|
@@ -70,7 +69,8 @@ pub const FLAG_TARGET: &str = "target"; | |
pub const FLAG_TIME: &str = "time"; | ||
pub const FLAG_VERBOSE: &str = "verbose"; | ||
pub const FLAG_LINKER: &str = "linker"; | ||
pub const FLAG_PREBUILT: &str = "prebuilt-platform"; | ||
pub const FLAG_BUILD_HOST: &str = "build-host"; | ||
pub const FLAG_SUPPRESS_BUILD_HOST_WARNING: &str = "suppress-build-host-warning"; | ||
pub const FLAG_CHECK: &str = "check"; | ||
pub const FLAG_STDIN: &str = "stdin"; | ||
pub const FLAG_STDOUT: &str = "stdout"; | ||
|
@@ -140,9 +140,15 @@ pub fn build_app() -> Command { | |
.value_parser(["surgical", "legacy"]) | ||
.required(false); | ||
|
||
let flag_prebuilt = Arg::new(FLAG_PREBUILT) | ||
.long(FLAG_PREBUILT) | ||
.help("Assume the platform has been prebuilt and skip rebuilding the platform\n(This is enabled implicitly when using `roc build` with a --target other than `--target <current machine>`, unless the target is wasm.)") | ||
let flag_build_host = Arg::new(FLAG_BUILD_HOST) | ||
.long(FLAG_BUILD_HOST) | ||
.help("WARNING: platforms are responsible for building hosts, this flag will be removed when internal test platforms have a build script") | ||
.action(ArgAction::SetTrue) | ||
.required(false); | ||
|
||
let flag_suppress_build_host_warning = Arg::new(FLAG_SUPPRESS_BUILD_HOST_WARNING) | ||
.long(FLAG_SUPPRESS_BUILD_HOST_WARNING) | ||
.help("WARNING: platforms are responsible for building hosts, this flag will be removed when internal test platforms have a build script") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO -- description needs updating |
||
.action(ArgAction::SetTrue) | ||
.required(false); | ||
|
||
|
@@ -199,7 +205,8 @@ pub fn build_app() -> Command { | |
.arg(flag_profiling.clone()) | ||
.arg(flag_time.clone()) | ||
.arg(flag_linker.clone()) | ||
.arg(flag_prebuilt.clone()) | ||
.arg(flag_build_host.clone()) | ||
.arg(flag_suppress_build_host_warning.clone()) | ||
.arg(flag_fuzz.clone()) | ||
.arg(flag_wasm_stack_size_kb) | ||
.arg( | ||
|
@@ -251,7 +258,8 @@ pub fn build_app() -> Command { | |
.arg(flag_profiling.clone()) | ||
.arg(flag_time.clone()) | ||
.arg(flag_linker.clone()) | ||
.arg(flag_prebuilt.clone()) | ||
.arg(flag_build_host.clone()) | ||
.arg(flag_suppress_build_host_warning.clone()) | ||
.arg(flag_fuzz.clone()) | ||
.arg( | ||
Arg::new(FLAG_VERBOSE) | ||
|
@@ -282,7 +290,8 @@ pub fn build_app() -> Command { | |
.arg(flag_profiling.clone()) | ||
.arg(flag_time.clone()) | ||
.arg(flag_linker.clone()) | ||
.arg(flag_prebuilt.clone()) | ||
.arg(flag_build_host.clone()) | ||
.arg(flag_suppress_build_host_warning.clone()) | ||
.arg(flag_fuzz.clone()) | ||
.arg(roc_file_to_run.clone()) | ||
.arg(args_for_app.clone().last(true)) | ||
|
@@ -297,7 +306,8 @@ pub fn build_app() -> Command { | |
.arg(flag_profiling.clone()) | ||
.arg(flag_time.clone()) | ||
.arg(flag_linker.clone()) | ||
.arg(flag_prebuilt.clone()) | ||
.arg(flag_build_host.clone()) | ||
.arg(flag_suppress_build_host_warning.clone()) | ||
.arg(flag_fuzz.clone()) | ||
.arg(roc_file_to_run.clone()) | ||
.arg(args_for_app.clone().last(true)) | ||
|
@@ -388,23 +398,6 @@ pub fn build_app() -> Command { | |
.default_value(DEFAULT_ROC_FILENAME) | ||
) | ||
) | ||
.subcommand(Command::new(CMD_GEN_STUB_LIB) | ||
.about("Generate a stubbed shared library that can be used for linking a platform binary.\nThe stubbed library has prototypes, but no function bodies.\n\nNote: This command will be removed in favor of just using `roc build` once all platforms support the surgical linker") | ||
.arg( | ||
Arg::new(ROC_FILE) | ||
.help("The .roc file for an app using the platform") | ||
.value_parser(value_parser!(PathBuf)) | ||
.required(true) | ||
) | ||
.arg( | ||
Arg::new(FLAG_TARGET) | ||
.long(FLAG_TARGET) | ||
.help("Choose a different target") | ||
.default_value(Into::<&'static str>::into(Target::default())) | ||
.value_parser(build_target_values_parser.clone()) | ||
.required(false), | ||
) | ||
) | ||
.subcommand(Command::new(CMD_PREPROCESS_HOST) | ||
.about("Runs the surgical linker preprocessor to generate `.rh` and `.rm` files.") | ||
.arg( | ||
|
@@ -449,7 +442,8 @@ pub fn build_app() -> Command { | |
.arg(flag_profiling) | ||
.arg(flag_time) | ||
.arg(flag_linker) | ||
.arg(flag_prebuilt) | ||
.arg(flag_build_host) | ||
.arg(flag_suppress_build_host_warning) | ||
.arg(flag_fuzz) | ||
.arg(roc_file_to_run) | ||
.arg(args_for_app.trailing_var_arg(true)) | ||
|
@@ -715,7 +709,6 @@ pub fn build( | |
roc_cache_dir: RocCacheDir<'_>, | ||
link_type: LinkType, | ||
) -> io::Result<i32> { | ||
use roc_build::program::build_file; | ||
use BuildConfig::*; | ||
|
||
let path = matches.get_one::<PathBuf>(ROC_FILE).unwrap(); | ||
|
@@ -863,17 +856,10 @@ pub fn build( | |
LinkingStrategy::Surgical | ||
}; | ||
|
||
let prebuilt = { | ||
let cross_compile = target != Target::default(); | ||
let targeting_wasm = matches!(target.architecture(), Architecture::Wasm32); | ||
|
||
matches.get_flag(FLAG_PREBUILT) || | ||
// When compiling for a different target, assume a prebuilt platform. | ||
// Otherwise compilation would most likely fail because many toolchains | ||
// assume you're compiling for the current machine. We make an exception | ||
// for Wasm, because cross-compiling is the norm in that case. | ||
(cross_compile && !targeting_wasm) | ||
}; | ||
// All hosts should be prebuilt, this flag keeps the rebuilding behvaiour | ||
// as required for internal tests | ||
let build_host = matches.get_flag(FLAG_BUILD_HOST); | ||
let suppress_build_host_warning = matches.get_flag(FLAG_SUPPRESS_BUILD_HOST_WARNING); | ||
|
||
let fuzz = matches.get_flag(FLAG_FUZZ); | ||
if fuzz && !matches!(code_gen_backend, CodeGenBackend::Llvm(_)) { | ||
|
@@ -901,15 +887,16 @@ pub fn build( | |
|
||
let load_config = standard_load_config(target, build_ordering, threading); | ||
|
||
let res_binary_path = build_file( | ||
let res_binary_path = roc_build::program::build_file( | ||
&arena, | ||
target, | ||
path.to_owned(), | ||
code_gen_options, | ||
emit_timings, | ||
link_type, | ||
linking_strategy, | ||
prebuilt, | ||
build_host, | ||
suppress_build_host_warning, | ||
wasm_dev_stack_bytes, | ||
roc_cache_dir, | ||
load_config, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
app [main] { pf: platform "main.roc" } | ||
|
||
main = Task.ok {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,8 +112,6 @@ comptime { | |
const Unit = extern struct {}; | ||
|
||
pub fn main() !u8 { | ||
const stderr = std.io.getStdErr().writer(); | ||
|
||
// The size might be zero; if so, make it at least 8 so that we don't have a nullptr | ||
const size = @max(@as(usize, @intCast(roc__mainForHost_1_exposed_size())), 8); | ||
const raw_output = roc_alloc(@as(usize, @intCast(size)), @alignOf(u64)).?; | ||
|
@@ -123,26 +121,15 @@ pub fn main() !u8 { | |
roc_dealloc(raw_output, @alignOf(u64)); | ||
} | ||
|
||
var timer = std.time.Timer.start() catch unreachable; | ||
|
||
roc__mainForHost_1_exposed_generic(output); | ||
|
||
const closure_data_pointer = @as([*]u8, @ptrCast(output)); | ||
|
||
call_the_closure(closure_data_pointer); | ||
|
||
const nanos = timer.read(); | ||
const seconds = (@as(f64, @floatFromInt(nanos)) / 1_000_000_000.0); | ||
|
||
stderr.print("runtime: {d:.3}ms\n", .{seconds * 1000}) catch unreachable; | ||
|
||
Comment on lines
-134
to
-138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We now compare the expected output from both stdout and stderr, instead of just stdout as previously, to verify it behaves as expected. Removing the timing information from zig test platforms simplifies the comparison -- and this wasn't being used for anything. There are now other more mature examples of platform implementation for people interested to use as examples. |
||
return 0; | ||
} | ||
|
||
fn to_seconds(tms: std.os.timespec) f64 { | ||
return @as(f64, @floatFromInt(tms.tv_sec)) + (@as(f64, @floatFromInt(tms.tv_nsec)) / 1_000_000_000.0); | ||
} | ||
|
||
fn call_the_closure(closure_data_pointer: [*]u8) void { | ||
const allocator = std.heap.page_allocator; | ||
|
||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this flag to make testing easier -- we could go without it, but we would need to strip the warning from stderr when building the platform.