Skip to content
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

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
dc0902b
WIP
lukewilliamboswell Jul 2, 2024
753cfb4
fix: building valgrind surgical host
lukewilliamboswell Jul 9, 2024
85447ec
replace stray strings with impl on Target
lukewilliamboswell Jul 9, 2024
96c559e
minor fixes
lukewilliamboswell Jul 9, 2024
4ccbc41
replace gen-stub-lib in nix CI tests
lukewilliamboswell Jul 9, 2024
287d00d
remove gen_stub_lib
lukewilliamboswell Jul 9, 2024
f515311
clippy fix
lukewilliamboswell Jul 9, 2024
bd68787
add --build-host for benchmark tests
lukewilliamboswell Jul 9, 2024
61a5a0a
checkpoint: refactor prebuilt host artifacts
r-bar Jul 9, 2024
d747cc9
Merge remote-tracking branch 'remotes/luke/rebuild-platform' into reb…
r-bar Jul 9, 2024
dfa3254
checkpoint: it compiles
r-bar Jul 11, 2024
bc29782
checkpoint: Run struct refactor for helpers
r-bar Jul 22, 2024
89d63f2
Fix ownership issues with runners
smores56 Aug 14, 2024
3662c90
WIP get things compiling... hacks everywhere beware
lukewilliamboswell Aug 15, 2024
4ec213a
Get tests working
smores56 Aug 15, 2024
550bade
Merge branch 'main' into rebuild-platform
smores56 Aug 15, 2024
49f5ea8
Merge branch 'main' into rebuild-platform
smores56 Aug 15, 2024
28ceb0e
Fix formatting and clippy issues
smores56 Aug 15, 2024
2928969
restore one test
lukewilliamboswell Aug 15, 2024
6276cc7
WIP more cli_run tests passing
lukewilliamboswell Aug 15, 2024
4863234
Merge branch 'rebuild-platform' of github.com:roc-lang/roc into rebui…
lukewilliamboswell Aug 15, 2024
014514a
WIP more passing roc_cli tests
lukewilliamboswell Aug 15, 2024
64cc816
WIP all the roc_cli tests passing, woo
lukewilliamboswell Aug 15, 2024
9ecb209
add suppress warning flag, remove test test_roc_app_slim
lukewilliamboswell Aug 16, 2024
0767115
WIP refactor to build test platform once per test run
lukewilliamboswell Aug 16, 2024
804cb6e
WIP upgrade cli test packages fixture tests
lukewilliamboswell Aug 16, 2024
c8522c2
WIP upgrade cli tests transitive-deps
lukewilliamboswell Aug 16, 2024
d2fbf86
format test files, upgrade some tests
lukewilliamboswell Aug 16, 2024
7ce9ecd
WIP improve cli tests
lukewilliamboswell Aug 16, 2024
894e4d1
make clippy happy again
lukewilliamboswell Aug 16, 2024
5c8eb4f
rename cli to basic-cli
lukewilliamboswell Aug 16, 2024
6ab0de1
fix benchmark tests
lukewilliamboswell Aug 16, 2024
0256c4e
move false interpreter into cli tests
lukewilliamboswell Aug 16, 2024
8ad8198
move effects cli test
lukewilliamboswell Aug 16, 2024
fa40166
move tui example into cli tests
lukewilliamboswell Aug 16, 2024
c3785a2
fix more tests
lukewilliamboswell Aug 16, 2024
767ec7a
cli run tests passing on macos
lukewilliamboswell Aug 16, 2024
ac78d39
Merge remote-tracking branch 'remote/main' into rebuild-platform
lukewilliamboswell Aug 17, 2024
8612220
fix plumbing for roc_glue
lukewilliamboswell Aug 18, 2024
b0c1d88
Merge remote-tracking branch 'remote/main' into rebuild-platform
lukewilliamboswell Aug 18, 2024
e7a90f1
fix clippy
lukewilliamboswell Aug 18, 2024
cd955fd
remove check_compile_error, fix cli tests
lukewilliamboswell Aug 18, 2024
93a4c58
fix cli tests - mainly wasm and i386
lukewilliamboswell Aug 18, 2024
0e917be
add LEGACY LINKER conditional for test runs
lukewilliamboswell Aug 18, 2024
4e6178c
move valgrind configuration into Run builder
lukewilliamboswell Aug 18, 2024
f92974c
use legacy linker in tests
lukewilliamboswell Aug 18, 2024
012a2fc
Merge remote-tracking branch 'remote/main' into rebuild-platform
lukewilliamboswell Sep 3, 2024
a11f51a
fixup
lukewilliamboswell Sep 3, 2024
ec8f6e8
clippy fix
Anton-4 Sep 3, 2024
251e1ed
misc improvements
Anton-4 Sep 3, 2024
ef87653
migrate cli test combine-tasks.roc off basic-cli
lukewilliamboswell Sep 4, 2024
c6a67c9
remove basic-cli tests from roc_cli
lukewilliamboswell Sep 4, 2024
5dfcdae
migrate inspect-logging.roc into cli tests and use effects platform
lukewilliamboswell Sep 4, 2024
73846ed
add test for module_params pass_task.roc
lukewilliamboswell Sep 4, 2024
beb2dcb
remove unneeded basic-cli reference in formatting test
lukewilliamboswell Sep 4, 2024
bb57c4a
remove unneded basic-cli reference in test_reporting
lukewilliamboswell Sep 4, 2024
dc138b8
remove last usage of basic-cli platform from the tests
lukewilliamboswell Sep 4, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/nix_macos_apple_silicon.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
run: nix develop -c ./ci/roc_test_builtins.sh

- name: make a libapp.so for the next step
run: nix develop -c cargo run -- gen-stub-lib examples/platform-switching/rocLovesRust.roc
run: nix develop -c cargo run -- build --lib examples/platform-switching/rust-platform/libapp.roc

- name: check that the platform`s produced dylib is loadable
run: cd examples/platform-switching/rust-platform && nix develop -c cargo test --release --locked
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/nix_macos_x86_64.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
run: nix develop -c ./ci/roc_test_builtins.sh

- name: make a libapp.so for the next step
run: nix develop -c cargo run -- gen-stub-lib examples/platform-switching/rocLovesRust.roc
run: nix develop -c cargo run -- build --lib examples/platform-switching/rust-platform/libapp.roc

- name: check that the platform`s produced dylib is loadable
run: cd examples/platform-switching/rust-platform && nix develop -c cargo test --release --locked
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,12 @@ roc_repl_expect = { path = "../repl_expect" }

[dev-dependencies]
cli_utils = { path = "../cli_utils" }
roc_test_utils = { path = "../test_utils" }
roc_command_utils = { path = "../utils/command" }

criterion.workspace = true
indoc.workspace = true
parking_lot.workspace = true
pretty_assertions.workspace = true
serial_test.workspace = true

[[bench]]
name = "time_bench"
Expand Down
4 changes: 2 additions & 2 deletions crates/cli/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,15 @@ mod tests {
use std::io::Write;
use tempfile::{tempdir, TempDir};

const FORMATTED_ROC: &str = r#"app [main] { pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.15.0/SlwdbJ-3GR7uBWQo6zlmYWNYOxnvo8r6YABXD-45UOw.tar.br" }
const FORMATTED_ROC: &str = r#"app [main] { pf: platform "platform/main.roc" }

import pf.Stdout
import pf.Task

main =
Stdout.line! "I'm a Roc application!""#;

const UNFORMATTED_ROC: &str = r#"app [main] { pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.15.0/SlwdbJ-3GR7uBWQo6zlmYWNYOxnvo8r6YABXD-45UOw.tar.br" }
const UNFORMATTED_ROC: &str = r#"app [main] { pf: platform "platform/main.roc" }


import pf.Stdout
Expand Down
69 changes: 28 additions & 41 deletions crates/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Copy link
Collaborator Author

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.

pub const FLAG_CHECK: &str = "check";
pub const FLAG_STDIN: &str = "stdin";
pub const FLAG_STDOUT: &str = "stdout";
Expand Down Expand Up @@ -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")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO -- description needs updating

.action(ArgAction::SetTrue)
.required(false);

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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(_)) {
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 10 additions & 21 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ use roc_build::link::LinkType;
use roc_build::program::{check_file, CodeGenBackend};
use roc_cli::{
build_app, format_files, format_src, test, BuildConfig, FormatMode, CMD_BUILD, CMD_CHECK,
CMD_DEV, CMD_DOCS, CMD_FORMAT, CMD_GEN_STUB_LIB, CMD_GLUE, CMD_PREPROCESS_HOST, CMD_REPL,
CMD_RUN, CMD_TEST, CMD_VERSION, DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_DEV, FLAG_LIB, FLAG_MAIN,
FLAG_NO_LINK, FLAG_OUTPUT, FLAG_PP_DYLIB, FLAG_PP_HOST, FLAG_PP_PLATFORM, FLAG_STDIN,
FLAG_STDOUT, FLAG_TARGET, FLAG_TIME, GLUE_DIR, GLUE_SPEC, ROC_FILE,
CMD_DEV, CMD_DOCS, CMD_FORMAT, CMD_GLUE, CMD_PREPROCESS_HOST, CMD_REPL, CMD_RUN, CMD_TEST,
CMD_VERSION, DIRECTORY_OR_FILES, FLAG_CHECK, FLAG_DEV, FLAG_LIB, FLAG_MAIN, FLAG_NO_LINK,
FLAG_OUTPUT, FLAG_PP_DYLIB, FLAG_PP_HOST, FLAG_PP_PLATFORM, FLAG_STDIN, FLAG_STDOUT,
FLAG_TARGET, FLAG_TIME, GLUE_DIR, GLUE_SPEC, ROC_FILE,
};
use roc_docs::generate_docs_html;
use roc_error_macros::user_error;
use roc_gen_dev::AssemblyBackendMode;
use roc_gen_llvm::llvm::build::LlvmBackendMode;
use roc_load::{FunctionKind, LoadingProblem, Threading};
use roc_load::{LoadingProblem, Threading};
use roc_packaging::cache::{self, RocCacheDir};
use roc_target::Target;
use std::fs::{self, FileType};
Expand Down Expand Up @@ -120,21 +120,6 @@ fn main() -> io::Result<()> {
Ok(1)
}
}
Some((CMD_GEN_STUB_LIB, matches)) => {
let input_path = matches.get_one::<PathBuf>(ROC_FILE).unwrap();
let target = matches
.get_one::<String>(FLAG_TARGET)
.and_then(|s| Target::from_str(s).ok())
.unwrap_or_default();
let function_kind = FunctionKind::from_env();
roc_linker::generate_stub_lib(
input_path,
RocCacheDir::Persistent(cache::roc_cache_dir().as_path()),
target,
function_kind,
);
Ok(0)
}
Some((CMD_PREPROCESS_HOST, matches)) => {
let preprocess_host_err =
{ |msg: String| user_error!("\n\n ERROR PRE-PROCESSING HOST: {}\n\n", msg) };
Expand Down Expand Up @@ -169,10 +154,14 @@ fn main() -> io::Result<()> {

let verbose_and_time = matches.get_one::<bool>(roc_cli::FLAG_VERBOSE).unwrap();

let preprocessed_path = platform_path.with_file_name(target.prebuilt_surgical_host());
let metadata_path = platform_path.with_file_name(target.metadata_file_name());

roc_linker::preprocess_host(
target,
host_path,
platform_path,
metadata_path.as_path(),
preprocessed_path.as_path(),
dylib_path,
*verbose_and_time,
*verbose_and_time,
Expand Down
12 changes: 0 additions & 12 deletions crates/cli/tests/algorithms/quicksort-platform/host.zig
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,13 @@ pub export fn main() u8 {

var roc_list = RocList{ .elements = numbers, .length = NUM_NUMS, .capacity = NUM_NUMS };

var timer = std.time.Timer.start() catch unreachable;

// actually call roc to populate the callresult
const callresult: RocList = roc__mainForHost_1_exposed(roc_list);

// stdout the result
const length = @min(20, callresult.length);
var result = callresult.elements[0..length];

const nanos = timer.read();
const seconds = (@as(f64, @floatFromInt(nanos)) / 1_000_000_000.0);

for (result, 0..) |x, i| {
if (i == 0) {
stdout.print("[{}, ", .{x}) catch unreachable;
Expand All @@ -149,12 +144,5 @@ pub export fn main() u8 {
}
}

const stderr = std.io.getStdErr().writer();
stderr.print("runtime: {d:.3}ms\n", .{seconds * 1000}) catch unreachable;

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);
}
3 changes: 3 additions & 0 deletions crates/cli/tests/benchmarks/platform/app.roc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
app [main] { pf: platform "main.roc" }

main = Task.ok {}
13 changes: 0 additions & 13 deletions crates/cli/tests/benchmarks/platform/host.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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)).?;
Expand All @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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;

Expand Down
18 changes: 0 additions & 18 deletions crates/cli/tests/cli/countdown.roc

This file was deleted.

Loading
Loading