Ryan Barth (Jul 11 2024 at 08:54): Ryan Barth (Jul 11 2024 at 08:54):Hey folks! I am helping out @Luke Boswell with the --build-host flag PR. This PR also cleans up a bunch of logic around enumerating / detecting all the various host and build types the compiler will support. Note the most recent changes can be found here, I cannot push to project branches, sorry.
I am going through cleaning up the failures with the CLI tests. Luke previously set all these to force recompilation of the host to make them close to parity with the existing behavior. This results a few dominant failure modes that spark questions on what we want the user experience (and compiler dev experience) to be.
The --build-host flag is set and a pre-compiled host already exists. Do we wish to:
a. Silently rebuild and overwrite the compiled host.
b. Fail and stop compilation
c. Warn and use the compiled host.
d. Warn and rebuild and overwrite the compiled host. (effectively current behavior)
Tests do not clean up after themselves currently. Some use a host with source within the compiler repo. With the current set of changes since we are trying to lean away from always rebuilding would you prefer:
a. If I always rebuild at the start of the test run (happy medium)
b. Always use a compiled artifact if it exists (potentially use a stale artifact if it is not manually cleaned between runs, fast)
c. Always rebuild at the start of each test (current behavior, slow)
Some tests use basic-cli as their platform. With the --build-host in the most recent version of the PR this always results in the compiler exiting with an error message. 2 questions here:
a. Do we want the same behavior as \#1?
b. Is it an antipattern to use these external platforms? Do we want to change them? Something feels strange about it.
Brendan Hansknecht (Jul 11 2024 at 15:18):For 1, at least long term, the flag should go away
Brendan Hansknecht (Jul 11 2024 at 15:19):Host has to be built first and roc will never build it is the olan
Anton (Jul 12 2024 at 10:40):olan -> plan (probably)
Anton (Jul 12 2024 at 10:48):For 3, whenever you use a platform using a URL and also set the --build-host flag, I think we should warn and use the compiled host.
Anton (Jul 12 2024 at 10:54):For 3 b), I get the antipattern feeling. But if there are breaking changes in Roc we definitely want to have a new basic-cli release available, because once that Roc code is merged and is used in the next nightly, users need to be able to use basic-cli and the most popular platforms in general, otherwise they can't do much with Roc :p
Anton (Jul 12 2024 at 11:02):2 a) seems like a nice long term solution, I don't know how much it takes to set up, feel free to give that a try.
We don't do 2 c) everywhere currently, but if 2 a) is complicated 2 c) is acceptable for now.
Anton (Jul 12 2024 at 11:06):For 1, I agree with Brendan, d) seems the most logical for now.
Anton (Jul 12 2024 at 11:07):Thanks for setting up all these nicely structured options @Ryan Barth :)
Luke Boswell (Aug 16 2024 at 05:22):I apologise in advance, this PR is going to be quite large.
I am basically moving the cli tests around, and going through each one by one. I have to do this to verify the host rebuilding is being done correctly. And while I'm at it I'm fixing obvious things like formatting etc.
I think the end result should be tests that can run happily in parallel with only building the platform hosts once per run.
Luke Boswell (Aug 18 2024 at 07:07):Ok so https://github.com/roc-lang/roc/pull/6859 is pretty much good to go, well a review at least. It's a fairly large change, though mostly moving deck chairs around the titanic. We've refactored a bit to help set us up for future build pipeline improvements, and possibly sped up the cli tests by only building hosts once per test run and removing the serial restrictions.
I think there is only one issue remaining -- and I think I know what is going on. We must have broken the plumbing for using valgrind in the -p roc_cli tests. I know exactly where the previous implementation was, which we've moved into a cli_utils::helpers::Run builder API, but it must not be correct.
I'm not set up well for linux development. I have a machine that I can tests on, but it's basically a remote server and I don't normally use an editor from within a shell/ssh session. So things are much slower for me.
I'm hoping someone running linux can help me fix this?
Luke Boswell (Aug 27 2024 at 02:31):Any assistance here with figuring out what is going on with the valgrind setup/changes for cli tests would be helpful. It can wait until after we land Task as builtin though as that is a priority I think.
Anton (Aug 27 2024 at 08:30):I can look at it after we're finished with Task as builtin
Sam Mohr (Aug 30 2024 at 21:10):So would a first PR be to move any cli_run.rs tests into roc-lang/examples?
Sam Mohr (Aug 30 2024 at 21:11):I can work on that
Brendan Hansknecht (Aug 30 2024 at 21:38):Only those that depend on external platforms shound need to move
Brendan Hansknecht (Aug 30 2024 at 21:38):We still want some level of testing in the main repo
Luke Boswell (Aug 30 2024 at 21:38):Please dont, I've already done it / am halfway though it.
Sam Mohr (Aug 30 2024 at 21:39):You're lucky I'm distracted by other changes, I'm hands off
Luke Boswell (Aug 30 2024 at 21:40):After builtin task lands (which may have already, I need to catch up) I'll clean up/merge into the rebuild host PR and that has touched all the cli tests.
Sam Mohr (Aug 30 2024 at 21:40):It has!!!
Luke Boswell (Aug 30 2024 at 21:40):If we are agreed on removing the basic cli tests I think it will make that merge easier for me cause I can just delete them as I go through and check them.
Sam Mohr (Aug 30 2024 at 21:40):And so has module params
Sam Mohr (Aug 30 2024 at 21:41):We can at least delete anything in examples
Luke Boswell (Aug 30 2024 at 21:42):I've been moving things elsewhere, like making sure there is a PR to add to examples repo before I delete them so we dont lose them.
Brendan Hansknecht (Aug 30 2024 at 22:21):I guess my one comment is that we should make sure we are getting signals from examples before just deleting everything.
Luke Boswell (Aug 30 2024 at 22:22):Signals?
Brendan Hansknecht (Aug 30 2024 at 22:24):Like it be nice to have a nightly job that runs examples repo tests. So we know when we accidently break them
Luke Boswell (Aug 30 2024 at 22:25):Anton has been testing the examples every day manually I believe.
Sam Mohr (Aug 30 2024 at 22:25):That would be good for us to know for all of the basic-* platforms
Sam Mohr (Aug 30 2024 at 22:25):Wow, that's some effort
Luke Boswell (Aug 30 2024 at 22:25):Yeah, he's a machine
Luke Boswell (Oct 09 2024 at 01:39):Making progress with our rebuild host PR https://github.com/roc-lang/roc/pull/6859
:smiley:
Screenshot 2024-10-09 at 12.39.05.png
Luke Boswell (Oct 09 2024 at 03:38):I think the next issue is that I misinterpreted our behaviour using valgrind for cli tests.
The PR is currently running valgrind on roc run ... etc, when I think we should instead be running after roc build and only on the executable produced by roc with valgrind.
This isn't easy for me to make the change on my mac as we only run valgrind on linux.
@Anton -- I just want to confirm is this the part that you have been working on and have changes you want to include?
Anton (Oct 09 2024 at 07:56):@Anton -- I just want to confirm is this the part that you have been working on and have changes you want to include?
Yes, correct, I expect to continue work on that today :)
Luke Boswell (Oct 18 2024 at 19:02):@Anton I'm loving the cleanup our cli tests are getting in this PR. :grinning:
Sam Mohr (Oct 18 2024 at 20:41):Anton, you are a machine
Luke Boswell (Nov 11 2024 at 03:00):I've merged in the purity-inference tests and latest main.
We had previously removed all of basic-cli from this branch, however I wasn't able to get some of the new tests working when merging. I've left these using basic-cli, and plan to remove the last basic-cli tests in a follow-up PR. This will be required for us to move to the release management process where roc can move ahead of the platforms with breaking changes.
I noticed that we had multiple crates all dependeding on a couple of common platforms in roc_cli. I have moved these platforms into a separate standalone crate which really simplifies their use in various tests, and now for these we aren't using the host rebuilding flag at all!
I can see a clear path to completely remove the host rebuilding now, but it's a pretty mechanical change to move all the smaller test platforms around, and I'd like to leave that for a future PR, this is already complicated enough merging in new changes.
The irony isn't lost on me that we put all this work in to try and keep the host rebuilding functionality for tests... and now we have an easy way to not need it at all.
Luke Boswell (Nov 11 2024 at 05:48):I think I need to take the last few commits and make these a separate branch. I hadn't realised I was missing the surgical host, and I've found it difficult to get rust to produce one of these for the purpose of testing. I've made a lot of progress, but I'm currently unsure it will even be possible -- due to crate dependencies etc.
Luke Boswell (Nov 11 2024 at 22:06):If I'm lucky, we should pass CI on this run :fingers_crossed:
Luke Boswell (Nov 11 2024 at 22:08):I've been skimming through the changes (there's a lot) and I can only see one TODO we left in there which was to investigate if it's possible to have a --dev with --target wasm32
Luke Boswell (Nov 11 2024 at 22:09):I'm willing to leave this for now, I doubt anyone is going to hit this branch in the build pipeline.
Luke Boswell (Nov 12 2024 at 00:44):Screenshot 2024-11-12 at 11.44.00.png
:tada:
Luke Boswell (Nov 12 2024 at 00:45):So pumped about this :smiley:
Thank you to everyone who put a massive effort in to make this happen. @Anton @Ryan Barth @Sam Mohr
Richard Feldman (Nov 12 2024 at 01:20):yoooooooooooo, this is huge!!! Amazing work, everyone!!! :heart_eyes::heart_eyes::heart_eyes::heart_eyes::heart_eyes::heart_eyes::heart_eyes:
Last updated: Jul 06 2025 at 12:14 UTC