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

Merged
merged 121 commits into from
Nov 12, 2024

Conversation

lukewilliamboswell
Copy link
Collaborator

@lukewilliamboswell lukewilliamboswell commented Jul 2, 2024

This PR decouples roc from the host toolchains, by moving host rebuilding behind a compiler flag so it is no longer the default behaviour. It is still available where required using --build-host however will give a warning;

WARNING: platforms are responsible for building hosts, this flag will be removed when internal test platforms have a build script

This PR effectively removes platform rebuilding from the cli for end users -- while keeping some rebuilding functionality just for internal cli tests.

Application authors who use a release of a platform are not affected by this change. These users will continue to use a prebuilt platform.

After this change, platform authors will need to include a build script in their platform to build host binaries.

This change will make running platform example less convenient if they are not using a URL release, as the platform binaries will need to be built first in a separate step. For example this workflow;

  1. Build the platform binary roc build.roc using host toolchain e.g. cargo or zig
  2. Run the example application roc examples/hello-world.roc

WIP

WIP

WIP

ignore some cli tests that are going to move

try fix for linux valgrind tests

try a legacy host for valgrind

fix roc_glue tests
@lukewilliamboswell lukewilliamboswell changed the title WIP replace cli flag --prebuilt-platform with --build-host Replace cli flag --prebuilt-platform with --build-host Aug 14, 2024
@lukewilliamboswell
Copy link
Collaborator Author

For this CI failure in nix-ubuntu... there is something going on between Nix and basic-cli. I'm going to ignore these tests (for linux) for this PR because we will be removing these in a follow-up as they are the only remaining basic-cli dependencies and prevent roc from moving ahead of the platform.

stack backtrace:
   0: rust_begin_unwind
             at /rustc/bf3c6c5bed498f41ad815641319a1ad9bcecb8e8/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/bf3c6c5bed498f41ad815641319a1ad9bcecb8e8/library/core/src/panicking.rs:72:14
   2: assert_no_unexpected_error
             at /home/runner/work/roc/roc/crates/cli_test_utils/src/command.rs:137:5
   3: assert_success_with_no_unexpected_errors
             at /home/runner/work/roc/roc/crates/cli_test_utils/src/command.rs:83:9
   4: cli_test_utils::command::CmdOut::assert_clean_success
             at /home/runner/work/roc/roc/crates/cli_test_utils/src/command.rs:68:9
   5: check_build_and_run
             at /home/runner/work/roc/roc/crates/cli_test_utils/src/exec_cli.rs:84:9
   6: cli_tests::cli_tests::test_platform_basic_cli::module_params_pass_task
             at ./tests/cli_tests.rs:436:13
   7: cli_tests::cli_tests::test_platform_basic_cli::module_params_pass_task::{{closure}}
             at ./tests/cli_tests.rs:425:37
   8: core::ops::function::FnOnce::call_once
             at /rustc/bf3c6c5bed498f41ad815641319a1ad9bcecb8e8/library/core/src/ops/function.rs:250:5
   9: core::ops::function::FnOnce::call_once
             at /rustc/bf3c6c5bed498f41ad815641319a1ad9bcecb8e8/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    cli_tests::test_platform_basic_cli::combine_tasks_with_record_builder
    cli_tests::test_platform_basic_cli::module_params_different_types
    cli_tests::test_platform_basic_cli::module_params_pass_task

@Anton-4 Anton-4 self-assigned this Nov 12, 2024
smores56
smores56 previously approved these changes Nov 12, 2024
Copy link
Collaborator

@smores56 smores56 left a comment

Choose a reason for hiding this comment

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

Some minor stuff, but the shape is good.

This PR is a behemoth and has been cooking for many months. I'd vote that we continue to address feedback in a different PR, as every time I come back to this PR after a merge request, thousands of lines have to be re-read or re-skimmed.

That said, this was a Herculean task and everyone involved in this PR should be proud of their work!

rust-toolchain.toml Outdated Show resolved Hide resolved
crates/compiler/build/src/program.rs Outdated Show resolved Hide resolved
crates/compiler/build/src/program.rs Outdated Show resolved Hide resolved
crates/compiler/build/src/program.rs Show resolved Hide resolved
crates/compiler/build/src/program.rs Show resolved Hide resolved
@Anton-4
Copy link
Collaborator

Anton-4 commented Nov 12, 2024

I'd vote that we continue to address feedback in a different PR

I already started with some cleanup before I read this, CI should be good though 🤞

@lukewilliamboswell lukewilliamboswell merged commit 7180930 into main Nov 12, 2024
18 checks passed
@lukewilliamboswell lukewilliamboswell deleted the rebuild-platform branch November 12, 2024 19:18
@Anton-4
Copy link
Collaborator

Anton-4 commented Nov 12, 2024

Much love and gratitude @lukewilliamboswell @smores56 @r-bar ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants