basic-cli purity inference · contributing

Just wanted to give an update on the progress with purity inference upgrade for basic-cli PR#257

The host, platform, and all of the examples have been upgraded -- it's working well :smiley:

thread 'main' panicked at crates/compiler/gen_llvm/src/llvm/build.rs:5748:19:
Error in alias analysis: error in module ModName("UserApp"), function definition FuncName("\x11\x00\x00\x00\x00\x00\x00\x80\xe0{\x02\xeaI\xd4\xd7?"), definition of value binding ValueId(54): expected type 'union { (((heap_cell,), ()),), ((),) }', found type 'union { (), (), ((heap_cell,),), (), (), (), () }'

thread 'main' panicked at crates/compiler/gen_llvm/src/llvm/build.rs:5748:19:
Error in alias analysis: error in module ModName("UserApp"), function definition FuncName("\x11\x00\x00\x00\x04\x00\x00\x80\x16\x02|\xef\xcd\x95Y\xbd"), definition of value binding ValueId(42): expected type 'union { (union { ((heap_cell,), ()), (union { (), (), ((heap_cell,),), (), (), (), () },) },), ((),) }', found type 'union { ((heap_cell,), ()), (union { (), (), ((heap_cell,),), (), (), (), () },) }'

thread '<unnamed>' panicked at crates/compiler/mono/src/ir.rs:6191:56:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I've also switched us over to the Weaver package and removed the Arg parser from basic-cli.

We can put this back if anyone has an issue with this, but we discussed eventually pointing people towards the packages as that keeps the platform simple, and I think now is a good time.

Next up is updating/checking all of the expect tests to get CI back online. I've also temporarily disabled the pull_request trigger in the yaml files so I don't get spammed on every push, but we can restore this once we have that working locally.

Anton (Nov 09 2024 at 11:22):

Could we stick to very simple arg handling like here and link to weaver in a comment? I want to avoid being blocked on needing to upgrade weaver in case of breaking roc changes.

Luke Boswell (Nov 09 2024 at 11:24):

Agus Zubiaga (Nov 09 2024 at 12:10):

Agus Zubiaga (Nov 09 2024 at 23:55):

It looks like the tcp-client example issue is caused by the return issue that Sam is already looking into. I inlined all try without using return and it run properly:
Screenshot 2024-11-09 at 8.55.01 PM.png

Agus Zubiaga (Nov 09 2024 at 23:56):

@Luke Boswell so if you made an issue for that one, I think we can close it as a dupe

Agus Zubiaga (Nov 09 2024 at 23:57):

Agus Zubiaga (Nov 09 2024 at 23:58):

Agus Zubiaga (Nov 09 2024 at 23:59):

We should hold the release until we fix it, because not having try is really painful

Luke Boswell (Nov 10 2024 at 00:17):

Sam Mohr (Nov 10 2024 at 01:22):

Sam Mohr (Nov 10 2024 at 01:24):

--- STDOUT: test_gen::test_gen gen_return::early_return_annotated_function ---

running 1 test

--- STDERR: test_gen::test_gen gen_return::early_return_annotated_function ---

An internal compiler expectation was broken.

This is definitely a compiler bug.

Please file an issue here: <https://github.com/roc-lang/roc/issues/new/choose>

single register layouts are not complex symbols

Location: crates/compiler/gen_dev/src/generic64/x86_64.rs:355:17

Sam Mohr (Nov 10 2024 at 01:25):

Sam Mohr (Nov 10 2024 at 01:26):

Off to hang with a friend, I'll update when I get to a keyboard in like 5 hours or so

Luke Boswell (Nov 22 2024 at 03:32):

If you are able, can you please give basic-cli purity inference upgrade a test, or to skim through the changes and see if we've missed anything obvious.

CI failures for native are to be expected until the new nightly which includes Sam's #7204 fix

I'm hoping the euro-roc-stars can take it from here... I think I'm pretty much done for today. Still hopeful we can land this in time for AoC :fingers_crossed:

Luke Boswell (Nov 23 2024 at 02:30):

Ok, I've only started to investigate... but I think that latest fix @Sam Mohr or something has broken hello world on basic-cli purity inference.

It doesn't make any sense to me right now, but hello world exits with a non-zero exit code.

Trying to figure out how that could happen. It's odd, that it's happening on literally the most simple program.

Sam Mohr (Nov 23 2024 at 02:31):

Luke Boswell (Nov 23 2024 at 02:35):

Luke Boswell (Nov 23 2024 at 02:37):

works

mainForHost! : I32 => Result {} I32
mainForHost! = \_ ->
    when main! {} is
        Ok {} -> Ok {}
        Err (Exit code msg) ->

broken

mainForHost! : I32 => Result {} I32
mainForHost! = \_ ->
    main! {}
    |> \result ->
        when result is
            Ok {} -> Ok {}

Sam Mohr (Nov 23 2024 at 03:55):

Sam Mohr (Nov 23 2024 at 04:53):

So when I log before and after running mainForHost! in the Rust host code, I get this:

before running mainForHost!
Hello, World!
main! returned Ok {}
after mainForHost! exit_code = 1875841744

Sam Mohr (Nov 23 2024 at 04:56):

So this means we successfully run mainForHost! and main! inside it, but the exit code is wrong.

Sam Mohr (Nov 23 2024 at 04:56):

Which smells to me like we're reading past our memory when detecting the exit code

Sam Mohr (Nov 23 2024 at 05:12):

Brendan Hansknecht (Nov 23 2024 at 06:59):

Also, this smells of an old bug we never figured out but seemed to have gone away

Luke Boswell (Nov 23 2024 at 07:00):

Brendan Hansknecht (Nov 23 2024 at 07:10):

Luke Boswell (Nov 24 2024 at 00:02):

Now to figure out why linux isn't happy... anyone with a linux machine feel like helping? feel free to jump in

Sam Mohr (Nov 24 2024 at 00:13):

Luke Boswell (Nov 24 2024 at 00:32):

I found a bunch of places where the host fx type was returning a RocResult, but the PlatformTask was something like {} => {}.

Luke Boswell (Nov 24 2024 at 00:33):

The current issue looks to be the non-zero exit code... and I suspect it may be related to the types for our Fx's not matching perfectly with what roc expects. My current hypothesis is that is we return a unit () to roc i.e. {} in rust that maybe that is sized or aligned differently.

Luke Boswell (Nov 24 2024 at 00:35):

Need to dig into the LLVM IR maybe. Possible workaround might be to use a RocResult for everything across the host boundary which will ensure the correct alignment etc, and then just use a crash "unreachable" in the platform for those effects that cannot fail (or rust host never provides an RocResult::Err(..) value).

Luke Boswell (Nov 24 2024 at 00:35):

Linux definitely seems to be more picky about this stuff than macos for some reason.

Luke Boswell (Nov 24 2024 at 00:56):

Do we generate size functions for fx's? It would be good to add a debug_assert that calls that and verifies the size of the return value in each effect is what roc expects.

Brendan Hansknecht (Nov 24 2024 at 01:51):

Brendan Hansknecht (Nov 24 2024 at 01:52):

Switching from effect to task (aka no result to result) lead to a lot of random issues

Brendan Hansknecht (Nov 24 2024 at 01:52):

Brendan Hansknecht (Nov 24 2024 at 01:53):

Luke Boswell (Nov 24 2024 at 02:17):

Just looking at the LLVM IR, not even the host, roc isn't generating anything for the effect PlatformTasks.currentArchOS!, which is really strange. I modified hello-world.roc to use this effect but for some reason roc isnt generating anything for it.

I've tried all kind of different things, like changing the interface, with and without Result, making it larger, random things... real head scratcher

Luke Boswell (Nov 30 2024 at 00:09):

@Brendan Hansknecht are you able to look at this? any thoughts on why this wouldn't be generating the effect?

Brendan Hansknecht (Nov 30 2024 at 03:47):

Brendan Hansknecht (Nov 30 2024 at 18:15):

Brendan Hansknecht (Nov 30 2024 at 18:16):

$ roc examples/hello-world.roc
{arch: AARCH64, os: MACOS}
[crates/roc_host/src/lib.rs:350:5] exit_code = 0

Brendan Hansknecht (Nov 30 2024 at 18:16):

To get things building correctly, I had to rm platform/*.{a,rh} then run ./jump-start.sh again.

Brendan Hansknecht (Nov 30 2024 at 18:17):

Brendan Hansknecht (Nov 30 2024 at 18:19):

Continues to just work when changing glue::ReturnArchOS to only contain arch and os.

Luke Boswell (Nov 30 2024 at 18:47):

Brendan Hansknecht (Nov 30 2024 at 18:48):

Luke Boswell (Nov 30 2024 at 18:50):

Brendan Hansknecht (Nov 30 2024 at 18:51):

Luke Boswell (Nov 30 2024 at 18:51):

Brendan Hansknecht (Nov 30 2024 at 19:10):

Brendan Hansknecht (Nov 30 2024 at 19:11):

Luke Boswell (Nov 30 2024 at 19:12):

Brendan Hansknecht (Nov 30 2024 at 19:48):

Almost all passing. build-and-test-native is failing with a segfault in examples/arg.roc. Anyone have a native (not using nix) linux setup they can run the tests on? I assume running with valgrind will reveal the issue.

Brendan Hansknecht (Nov 30 2024 at 19:49):

jan kili (Nov 30 2024 at 19:56):

In a half hour, I can run on native 2021-era Fedora and/or Debian 12, though I may be unclear how to run it

Brendan Hansknecht (Nov 30 2024 at 20:02):

Actually, I realized I can just download the nightly to get out of nix. Don't have to deal with all of the dependency and what not. So I can test.

Brendan Hansknecht (Nov 30 2024 at 20:05):

Just built everything outside of nix with roc nightly and musl, no segfault or failures.

Brendan Hansknecht (Nov 30 2024 at 20:24):

Brendan Hansknecht (Nov 30 2024 at 20:26):

Everything is passing in nix, so I think this is the last piece to have fully working basic cli with purity inference.

jan kili (Nov 30 2024 at 21:55):

Update: on a relatively clean native Debian 12 setup, I haven't figured out basic-cli local execution yet - ~~debugging with Brendan in DMs~~ suggestions welcome
After successful ./jump-start.sh:

jan@lenny:~/_roc/basic-cli$ roc build.roc
LinkingStrategy was set to Surgical (default), but I tried to find the surgical host at any of these paths Either the generic host files or the surgical host files must exist. File status: Generic host (platform/host.rh): missing, Generic metadata (platform/metadata_host.rm): missing, Surgical host (platform/linux-x64.rh): missing, Surgical metadata (platform/metadata_linux-x64.rm): missing but it does not exist.
jan@lenny:~/_roc/basic-cli$ roc examples/hello-world.roc
LinkingStrategy was set to Surgical (default), but I tried to find the surgical host at any of these paths Either the generic host files or the surgical host files must exist. File status: Generic host (examples/../platform/host.rh): missing, Generic metadata (examples/../platform/metadata_host.rm): missing, Surgical host (examples/../platform/linux-x64.rh): missing, Surgical metadata (examples/../platform/metadata_linux-x64.rm): missing but it does not exist.
jan@lenny:~/_roc/basic-cli$

Luke Boswell (Nov 30 2024 at 22:01):

Strange. I thought roc build.roc would have given you the surgical host... does it work if you add --linker legacy?

Luke Boswell (Nov 30 2024 at 22:02):

jan kili (Nov 30 2024 at 22:02):

jan@lenny:~/_roc/basic-cli$ ls -hl platform/
total 15M
drwxr-xr-x 2 jan jan 4.0K Nov 30 14:38 Arg
-rw-r--r-- 1 jan jan 3.3K Nov 30 14:38 Arg.roc
-rw-r--r-- 1 jan jan 4.1K Nov 30 14:38 Cmd.roc
-rw-r--r-- 1 jan jan 2.6K Nov 30 14:38 Dir.roc
-rw-r--r-- 1 jan jan 4.5K Nov 30 14:38 EnvDecoding.roc
-rw-r--r-- 1 jan jan 7.2K Nov 30 14:38 Env.roc
-rw-r--r-- 1 jan jan 1.3K Nov 30 14:38 FileMetadata.roc
-rw-r--r-- 1 jan jan 9.8K Nov 30 14:38 File.roc
-rw-r--r-- 1 jan jan  399 Nov 30 14:38 glue.roc
-rw-r--r-- 1 jan jan 4.8K Nov 30 14:38 Http.roc
-rw-r--r-- 1 jan jan  813 Nov 30 14:38 InternalCommand.roc
-rw-r--r-- 1 jan jan 1.4K Nov 30 14:38 InternalFile.roc
-rw-r--r-- 1 jan jan 1.8K Nov 30 14:38 InternalHttp.roc
-rw-r--r-- 1 jan jan 3.6K Nov 30 14:38 InternalPath.roc
-rw-r--r-- 1 jan jan  263 Nov 30 14:38 libapp.roc
-rwxr-xr-x 1 jan jan  63K Nov 30 14:54 libapp.so
-rw-r--r-- 1 jan jan  14M Nov 30 14:54 libhost.a
-rw-r--r-- 1 jan jan  600 Nov 30 14:38 Locale.roc
-rw-r--r-- 1 jan jan 1.3K Nov 30 14:38 main.roc
-rw-r--r-- 1 jan jan  31K Nov 30 14:38 Path.roc
-rw-r--r-- 1 jan jan 3.1K Nov 30 14:38 PlatformTasks.roc
-rw-r--r-- 1 jan jan  310 Nov 30 14:38 Sleep.roc
-rw-r--r-- 1 jan jan 2.5K Nov 30 14:38 Stderr.roc
-rw-r--r-- 1 jan jan 4.1K Nov 30 14:38 Stdin.roc
-rw-r--r-- 1 jan jan 2.5K Nov 30 14:38 Stdout.roc
-rw-r--r-- 1 jan jan 6.4K Nov 30 14:38 Tcp.roc
-rw-r--r-- 1 jan jan 1.2K Nov 30 14:38 Tty.roc
-rw-r--r-- 1 jan jan  16K Nov 30 14:38 Url.roc
-rw-r--r-- 1 jan jan 2.4K Nov 30 14:38 Utc.roc
jan@lenny:~/_roc/basic-cli$

Luke Boswell (Nov 30 2024 at 22:03):

jan kili (Nov 30 2024 at 22:03):

jan@lenny:~/_roc/basic-cli$ roc build.roc --linker=legacy
INFO: Checking provided roc; executing `roc version`:
roc nightly pre-release, built from commit d72da8e on Fr 29 Nov 2024 09:11:57 UTC
INFO: Getting the native operating system and architecture ...
INFO: Building stubbed app shared library ...
0 errors and 0 warnings found in 336 ms
 while successfully building:

    platform/libapp.so
INFO: Building rust host ...
warning: skipping duplicate package `host` found at `/home/jan/.cargo/git/checkouts/roc-f2007fde281427d6/8c73786/examples/glue/rust-platform`
warning: skipping duplicate package `host` found at `/home/jan/.cargo/git/checkouts/roc-f2007fde281427d6/8c73786/examples/cli/false-interpreter/platform`
warning: skipping duplicate package `host` found at `/home/jan/.cargo/git/checkouts/roc-f2007fde281427d6/8c73786/crates/glue/tests/fixture-templates/rust`
    Finished `release` profile [optimized] target(s) in 0.37s
INFO: Failed to get env var CARGO_BUILD_TARGET with error VarNotFound. Assuming default CARGO_BUILD_TARGET (native)...
INFO: Moving the prebuilt binary from target/release/libhost.a to platform/linux-x64.a ...
INFO: Preprocessing surgical host ...
INFO: Successfully built platform files!
jan@lenny:~/_roc/basic-cli$

jan kili (Nov 30 2024 at 22:03):

jan@lenny:~/_roc/basic-cli$ roc examples/hello-world.roc
Hello, World!
jan@lenny:~/_roc/basic-cli$

Luke Boswell (Nov 30 2024 at 22:04):

Another data point for defaulting to linker legacy if it's available and the flag isn't provided

Luke Boswell (Nov 30 2024 at 22:05):

jan kili (Nov 30 2024 at 22:05):

jan@lenny:~/_roc/basic-cli$ does that mean purity inference is fully working in basic-cli@0.17.0?
bash: does: command not found
jan@lenny:~/_roc/basic-cli$

jan kili (Nov 30 2024 at 22:19):

:stuck_out_tongue_wink: but really though, happy to run any other commands to help validate PI-in-BC

Brendan Hansknecht (Nov 30 2024 at 22:45):

Brendan Hansknecht (Nov 30 2024 at 22:46):

jan kili (Nov 30 2024 at 22:51):

jan@lenny:~/_roc/basic-cli$ cat platform/src/lib.rs | grep -oP 'roc_fx_[^(\s]*' | sort | uniq -u | grep -q .
cat: platform/src/lib.rs: No such file or directory
jan@lenny:~/_roc/basic-cli$ curl -fOL https://github.com/roc-lang/roc/releases/download/nightly/roc_nightly-linux_x86_64-TESTING.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     9    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 404
jan@lenny:~/_roc/basic-cli$ curl -fOL https://github.com/roc-lang/roc/releases/download/nightly/roc_nightly-linux_x86_64-latest.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 34.1M  100 34.1M    0     0  14.0M      0  0:00:02  0:00:02 --:--:-- 21.2M
jan@lenny:~/_roc/basic-cli$ mv $(ls | grep "roc_nightly.*tar\.gz") roc_nightly.tar.gz
jan@lenny:~/_roc/basic-cli$ tar -xzf roc_nightly.tar.gz
jan@lenny:~/_roc/basic-cli$ rm roc_nightly.tar.gz
jan@lenny:~/_roc/basic-cli$ mv roc_nightly* roc_nightly
jan@lenny:~/_roc/basic-cli$ ./roc_nightly/roc version
roc nightly pre-release, built from commit d72da8e on Fr 29 Nov 2024 09:11:57 UTC
jan@lenny:~/_roc/basic-cli$ sudo apt install -y expect ncat
[sudo] password for jan:
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
  liblua5.3-0 libtcl8.6 tcl-expect tcl8.6
Suggested packages:
  tk8.6 tcl-tclreadline
The following NEW packages will be installed:
  expect liblua5.3-0 libtcl8.6 ncat tcl-expect tcl8.6
0 upgraded, 6 newly installed, 0 to remove and 0 not upgraded.
Need to get 1,961 kB of archives.
After this operation, 6,186 kB of additional disk space will be used.
Get:1 http://deb.debian.org/debian bookworm/main amd64 libtcl8.6 amd64 8.6.13+dfsg-2 [1,035 kB]
Get:2 http://deb.debian.org/debian bookworm/main amd64 tcl8.6 amd64 8.6.13+dfsg-2 [120 kB]
Get:3 http://deb.debian.org/debian bookworm/main amd64 tcl-expect amd64 5.45.4-2+b1 [133 kB]
Get:4 http://deb.debian.org/debian bookworm/main amd64 expect amd64 5.45.4-2+b1 [166 kB]
Get:5 http://deb.debian.org/debian bookworm/main amd64 liblua5.3-0 amd64 5.3.6-2 [123 kB]
Get:6 http://deb.debian.org/debian bookworm/main amd64 ncat amd64 7.93+dfsg1-1 [384 kB]
Fetched 1,961 kB in 0s (6,665 kB/s)
Selecting previously unselected package libtcl8.6:amd64.
(Reading database ... 185306 files and directories currently installed.)
Preparing to unpack .../0-libtcl8.6_8.6.13+dfsg-2_amd64.deb ...
Unpacking libtcl8.6:amd64 (8.6.13+dfsg-2) ...
Selecting previously unselected package tcl8.6.
Preparing to unpack .../1-tcl8.6_8.6.13+dfsg-2_amd64.deb ...
Unpacking tcl8.6 (8.6.13+dfsg-2) ...
Selecting previously unselected package tcl-expect:amd64.
Preparing to unpack .../2-tcl-expect_5.45.4-2+b1_amd64.deb ...
Unpacking tcl-expect:amd64 (5.45.4-2+b1) ...
Selecting previously unselected package expect.
Preparing to unpack .../3-expect_5.45.4-2+b1_amd64.deb ...
Unpacking expect (5.45.4-2+b1) ...
Selecting previously unselected package liblua5.3-0:amd64.
Preparing to unpack .../4-liblua5.3-0_5.3.6-2_amd64.deb ...
Unpacking liblua5.3-0:amd64 (5.3.6-2) ...
Selecting previously unselected package ncat.
Preparing to unpack .../5-ncat_7.93+dfsg1-1_amd64.deb ...
Unpacking ncat (7.93+dfsg1-1) ...
Setting up libtcl8.6:amd64 (8.6.13+dfsg-2) ...
Setting up liblua5.3-0:amd64 (5.3.6-2) ...
Setting up tcl8.6 (8.6.13+dfsg-2) ...
Setting up tcl-expect:amd64 (5.45.4-2+b1) ...
Setting up ncat (7.93+dfsg1-1) ...
update-alternatives: using /usr/bin/ncat to provide /bin/nc (nc) in auto mode
Setting up expect (5.45.4-2+b1) ...
Processing triggers for man-db (2.11.2-2) ...
Processing triggers for libc-bin (2.36-9+deb12u9) ...
jan@lenny:~/_roc/basic-cli$ expect -v
expect version 5.45.4

jan kili (Nov 30 2024 at 22:57):

jan@lenny:~/_roc/basic-cli$ ROC=./roc_nightly/roc EXAMPLES_DIR=./examples/ ./ci/all_tests.sh > pasteme.txt 2>&1
jan@lenny:~/_roc/basic-cli$

jan kili (Nov 30 2024 at 22:59):

jan@lenny:~/_roc/basic-cli$ grep -zqv "crashed" pasteme.txt
jan@lenny:~/_roc/basic-cli$ sudo apt-get install -y musl-tools
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
  musl musl-dev
Recommended packages:
  linux-musl-dev
The following NEW packages will be installed:
  musl musl-dev musl-tools
0 upgraded, 3 newly installed, 0 to remove and 0 not upgraded.
Need to get 1,036 kB of archives.
After this operation, 3,954 kB of additional disk space will be used.
Get:1 http://deb.debian.org/debian bookworm/main amd64 musl amd64 1.2.3-1 [406 kB]
Get:2 http://deb.debian.org/debian bookworm/main amd64 musl-dev amd64 1.2.3-1 [587 kB]
Get:3 http://deb.debian.org/debian bookworm/main amd64 musl-tools amd64 1.2.3-1 [42.3 kB]
Fetched 1,036 kB in 0s (3,959 kB/s)
Selecting previously unselected package musl:amd64.
(Reading database ... 185670 files and directories currently installed.)
Preparing to unpack .../musl_1.2.3-1_amd64.deb ...
Unpacking musl:amd64 (1.2.3-1) ...
Selecting previously unselected package musl-dev:amd64.
Preparing to unpack .../musl-dev_1.2.3-1_amd64.deb ...
Unpacking musl-dev:amd64 (1.2.3-1) ...
Selecting previously unselected package musl-tools.
Preparing to unpack .../musl-tools_1.2.3-1_amd64.deb ...
Unpacking musl-tools (1.2.3-1) ...
Setting up musl:amd64 (1.2.3-1) ...
Setting up musl-dev:amd64 (1.2.3-1) ...
Setting up musl-tools (1.2.3-1) ...
Processing triggers for man-db (2.11.2-2) ...
jan@lenny:~/_roc/basic-cli$ rustup target add x86_64-unknown-linux-musl
info: downloading component 'rust-std' for 'x86_64-unknown-linux-musl'
info: installing component 'rust-std' for 'x86_64-unknown-linux-musl'
 33.0 MiB /  33.0 MiB (100 %)  15.0 MiB/s in  2s ETA:  0s
jan@lenny:~/_roc/basic-cli$ ROC=./roc_nightly/roc CARGO_BUILD_TARGET=x86_64-unknown-linux-musl ./roc_nightly/roc build.roc
INFO: Checking provided roc; executing `roc version`:
roc nightly pre-release, built from commit d72da8e on Fr 29 Nov 2024 09:11:57 UTC
INFO: Getting the native operating system and architecture ...
INFO: Building stubbed app shared library ...
0 errors and 0 warnings found in 425 ms
 while successfully building:

    platform/libapp.so
INFO: Building rust host ...
warning: skipping duplicate package `host` found at `/home/jan/.cargo/git/checkouts/roc-f2007fde281427d6/8c73786/examples/glue/rust-platform`
warning: skipping duplicate package `host` found at `/home/jan/.cargo/git/checkouts/roc-f2007fde281427d6/8c73786/examples/cli/false-interpreter/platform`
warning: skipping duplicate package `host` found at `/home/jan/.cargo/git/checkouts/roc-f2007fde281427d6/8c73786/crates/glue/tests/fixture-templates/rust`
   Compiling libc v0.2.155
   Compiling cfg-if v1.0.0
...
   Compiling roc_host_bin v0.0.1 (/home/jan/_roc/basic-cli/crates/roc_host_bin)
   Compiling host v0.0.1 (/home/jan/_roc/basic-cli/crates/roc_host_lib)
    Finished `release` profile [optimized] target(s) in 31.42s
INFO: Moving the prebuilt binary from target/x86_64-unknown-linux-musl/release/libhost.a to platform/linux-x64.a ...
INFO: Preprocessing surgical host ...
INFO: Successfully built platform files!
jan@lenny:~/_roc/basic-cli$

jan kili (Nov 30 2024 at 23:01):

jan@lenny:~/_roc/basic-cli$ NO_BUILD=1 ROC=./roc_nightly/roc EXAMPLES_DIR=./examples/ ./ci/all_tests.sh > pasteme2.txt 2>&1
jan@lenny:~/_roc/basic-cli$

jan kili (Nov 30 2024 at 23:01):

jan@lenny:~/_roc/basic-cli$ grep -zqv "crashed" pasteme2.txt
jan@lenny:~/_roc/basic-cli$ I think I did it!
bash: I: command not found
jan@lenny:~/_roc/basic-cli$

Luke Boswell (Nov 30 2024 at 23:47):

Brendan Hansknecht (Dec 01 2024 at 01:23):

So no repro yet? I don't think I saw a segfualt anywhere above. I wonder if it is Ubuntu 20.04 specific. @Anton any chance you can try to repro on something identical to CI?

Luke Boswell (Dec 01 2024 at 02:45):

I can't repro it on my linux machine running native either. I'm going to restart that CI and see if that changes anything

jan kili (Dec 01 2024 at 02:46):

Luke Boswell (Dec 01 2024 at 02:48):

I'd like to have it done, but I don't think we're in any rush to upgrade it to the latest supported release. Maybe make a pre-release sometime soon for those game to use it and not wanting to build from source.

Luke Boswell (Dec 01 2024 at 02:48):

If we rush it, we're just likely to cause a lot of confusion and mess things up for people.

Luke Boswell (Dec 01 2024 at 02:53):

I can make a pre-release "purity-inference" version of my AoC template in case anyone wants to use it and builds basic-cli from source.

Luke Boswell (Dec 01 2024 at 02:54):

Added bonus if someone uses it and finds the bug that's blocking CI... we have a reproduction that we can glean some information from.

Luke Boswell (Dec 01 2024 at 03:10):

I don't think I can make the template work with purity inference. @Agus Zubiaga I might need your assistance to figure out if it's a me thing or something is broken

Luke Boswell (Dec 01 2024 at 03:11):

$ roc check examples/2020/01.roc

── MODULE PARAMS MISMATCH in examples/2020/01.roc ──────────────────────────────

Something is off with the params provided by this import:

 9│>  import aoc.AoC {
10│>      stdin!: Stdin.bytes!,
11│>      stdout!: Stdout.write!,
12│>      time!: \{} -> Utc.now! {} |> Utc.toMillisSinceEpoch,
13│>  }

This is the type I inferred:

    { stdin! : {} => Result (List U8) [
        EndOfFile,
        StdinErr [
            AlreadyExists,
            BrokenPipe,
            Interrupted,
            NotFound,
            Other Str,
            OutOfMemory,
            PermissionDenied,
            Unsupported,
        ],
    ], … }

However, AoC expects:

    { stdin! : effectful function, … }

────────────────────────────────────────────────────────────────────────────────

1 error and 4 warnings found in 20 ms

Luke Boswell (Dec 01 2024 at 03:13):

Luke Boswell (Dec 01 2024 at 03:15):

module { stdin!, stdout!, time! } -> [Solution, solve!]

Solution err : {
    year : U64,
    day : U64,
    title : Str,
    part1 : Str -> Result Str err,
    part2 : Str -> Result Str err,
} where err implements Inspect

solve! : Solution err => Result {} _
solve! = \{ year, day, title, part1, part2 } ->
    Ok {}

$ roc check examples/2020/01.roc

── MODULE PARAMS MISMATCH in examples/2020/01.roc ──────────────────────────────

Something is off with the params provided by this import:

 9│>  import aoc.AoC {
10│>      stdin!: Stdin.bytes!,
11│>      stdout!: Stdout.write!,
12│>      time!: \{} -> Utc.now! {} |> Utc.toMillisSinceEpoch,
13│>  }

This is the type I inferred:

    {
        stdin! : {} => Result (List U8) [
            EndOfFile,
            StdinErr [
                AlreadyExists,
                BrokenPipe,
                Interrupted,
                NotFound,
                Other Str,
                OutOfMemory,
                PermissionDenied,
                Unsupported,
            ],
        ],
        stdout! : Str => Result {} [StdoutErr [
            AlreadyExists,
            BrokenPipe,
            Interrupted,
            NotFound,
            Other Str,
            OutOfMemory,
            PermissionDenied,
            Unsupported,
        ]],
        time! : {}e => I128,
    }

However, AoC expects:

    {
        stdin! : effectful function,
        stdout! : effectful function,
        time! : effectful function,
    }

────────────────────────────────────────────────────────────────────────────────

1 error and 8 warnings found in 20 ms

Agus Zubiaga (Dec 01 2024 at 03:17):

Interesting. That looks like we’re not constraining the module with params enough. effectful function means we only inferred it’s effectful from the name of the field, but we don’t know anything about the arguments or return type.

Agus Zubiaga (Dec 01 2024 at 03:18):

Unfortunately, I’m moving this whole weekend so I probably won’t have time to look at this until Monday

Agus Zubiaga (Dec 01 2024 at 03:19):

Luke Boswell (Dec 01 2024 at 03:19):

Agus Zubiaga (Dec 01 2024 at 03:19):

jan kili (Dec 01 2024 at 05:47):

I just jumped out of bed realizing everything I did above was on the main branch and y'all were probably wondering about the purity-inference branch.

jan kili (Dec 01 2024 at 05:48):

[jan@framey basic-cli]$ git status
On branch purity-inference
Your branch is up to date with 'origin/purity-inference'.

nothing to commit, working tree clean
[jan@framey basic-cli]$ roc build.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
Error:

        Function, roc__mainForHost_0_caller, was not defined by the app.

Potential causes:

        - because the platform was built with a non-compatible version of roc compared to the one you are running.

                solutions:
                        + Downgrade your roc version to the one that was used to build the platform.
                        + Or ask the platform author to release a new version of the platform using a current roc release.

        - This can also occur due to a bug in the compiler. In that case, file an issue here: https://github.com/roc-lang/roc/issues/new/choose
[jan@framey basic-cli]$

jan kili (Dec 01 2024 at 05:52):

[jan@framey basic-cli]$ ./jump-start.sh
+ '[' -z '' ']'
+ echo 'Warning: ROC environment variable is not set... I'\''ll try with just '\''roc'\''.'
Warning: ROC environment variable is not set... I'll try with just 'roc'.
+ ROC=roc
+ roc build --lib ./platform/libapp.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
0 errors and 0 warnings found in 360 ms
 while successfully building:

    ./platform/libapp
+ cargo build --release
    Finished `release` profile [optimized] target(s) in 0.16s
+ '[' -n '' ']'
+ cp target/release/libhost.a ./platform/libhost.a
+ roc build --linker=legacy build.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
ld: platform/linux-x64.a(host-e4e7228faf4c51e3.host.62e8b11d478b1302-cgu.0.rcgu.o): in function `rust_main':
host.62e8b11d478b1302-cgu.0:(.text.rust_main+0x88): undefined reference to `roc__mainForHost_0_caller'
ld: host.62e8b11d478b1302-cgu.0:(.text.rust_main+0xc2): undefined reference to `roc__mainForHost_0_caller'
thread 'main' panicked at crates/compiler/build/src/program.rs:1058:17:
not yet implemented: linker failed with exit code Some(1)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[jan@framey basic-cli]$ roc build.roc --linker=legacy
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
ld: platform/linux-x64.a(host-e4e7228faf4c51e3.host.62e8b11d478b1302-cgu.0.rcgu.o): in function `rust_main':
host.62e8b11d478b1302-cgu.0:(.text.rust_main+0x88): undefined reference to `roc__mainForHost_0_caller'
ld: host.62e8b11d478b1302-cgu.0:(.text.rust_main+0xc2): undefined reference to `roc__mainForHost_0_caller'
thread 'main' panicked at crates/compiler/build/src/program.rs:1058:17:
not yet implemented: linker failed with exit code Some(1)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[jan@framey basic-cli]$
[jan@framey basic-cli]$ roc build.roc --linker=legacy
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
ld: platform/linux-x64.a(host-e4e7228faf4c51e3.host.62e8b11d478b1302-cgu.0.rcgu.o): in function `rust_main':
host.62e8b11d478b1302-cgu.0:(.text.rust_main+0x88): undefined reference to `roc__mainForHost_0_caller'
ld: host.62e8b11d478b1302-cgu.0:(.text.rust_main+0xc2): undefined reference to `roc__mainForHost_0_caller'
thread 'main' panicked at crates/compiler/build/src/program.rs:1058:17:
not yet implemented: linker failed with exit code Some(1)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[jan@framey basic-cli]$

Brendan Hansknecht (Dec 01 2024 at 05:52):

That is likely different. Probably from a stale platform/linux_x64.a (not sure I got that file name quite right). If you rm platform/*.a then ./jump-start.sh again,what do you get?

jan kili (Dec 01 2024 at 05:53):

[jan@framey basic-cli]$ rm platform/*.a
[jan@framey basic-cli]$ ./jump-start.sh
+ '[' -z '' ']'
+ echo 'Warning: ROC environment variable is not set... I'\''ll try with just '\''roc'\''.'
Warning: ROC environment variable is not set... I'll try with just 'roc'.
+ ROC=roc
+ roc build --lib ./platform/libapp.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
0 errors and 0 warnings found in 351 ms
 while successfully building:

    ./platform/libapp
+ cargo build --release
    Finished `release` profile [optimized] target(s) in 0.18s
+ '[' -n '' ']'
+ cp target/release/libhost.a ./platform/libhost.a
+ roc build --linker=legacy build.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
0 errors and 0 warnings found in 986 ms
 while successfully building:

    build
[jan@framey basic-cli]$ roc build.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
Error:

        Function, roc__mainForHost_0_caller, was not defined by the app.

Potential causes:

        - because the platform was built with a non-compatible version of roc compared to the one you are running.

                solutions:
                        + Downgrade your roc version to the one that was used to build the platform.
                        + Or ask the platform author to release a new version of the platform using a current roc release.

        - This can also occur due to a bug in the compiler. In that case, file an issue here: https://github.com/roc-lang/roc/issues/new/choose
[jan@framey basic-cli]$ roc build.roc --linker=legacy
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
INFO: Checking provided roc; executing `roc version`:
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
roc nightly pre-release, built from commit d72da8e on Fr 29 Nov 2024 09:11:57 UTC
INFO: Getting the native operating system and architecture ...
INFO: Building stubbed app shared library ...
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
0 errors and 0 warnings found in 949 ms
 while successfully building:

    platform/libapp.so
INFO: Building rust host ...
    Finished `release` profile [optimized] target(s) in 0.20s
INFO: Failed to get env var CARGO_BUILD_TARGET with error VarNotFound. Assuming default CARGO_BUILD_TARGET (native)...
INFO: Moving the prebuilt binary from target/release/libhost.a to platform/linux-x64.a ...
INFO: Preprocessing surgical host ...
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
INFO: Successfully built platform files!
[jan@framey basic-cli]$ roc examples/hello-world.roc
roc: /lib64/libtinfo.so.6: no version information available (required by roc)
Hello, World!
[jan@framey basic-cli]$

Eli Dowling (Dec 01 2024 at 05:54):

I also looked into this, I was able to get purity inference basic cli to build fine (I had to use the jumpstart.s,) I'm on nixos.

jan kili (Dec 01 2024 at 05:57):

jan kili (Dec 01 2024 at 05:58):

Eli Dowling (Dec 01 2024 at 06:08):

Brendan Hansknecht (Dec 01 2024 at 06:18):

Eli Dowling (Dec 01 2024 at 06:36):

Yeah, the template issue.
I'm working on making a test and also fixing the type checking issue .
The fix was:

// The existing effectul function unification function:
        (EffectfulFunc, Func(args, closure, ret, fx)) => {
            let mut outcome = unify_pool(env, pool, Variable::EFFECTFUL, *fx, ctx.mode);

            outcome.union(merge(
                env,
                ctx,
                Structure(Func(*args, *closure, *ret, Variable::EFFECTFUL)),
            ));

            outcome
        }
//I added this becasue it seemed to not be unifying properly and just leaving the function as effectful func so I assumed it never processed the oppisite case
        (Func(args, closure, ret, fx), EffectfulFunc) => {
            let mut outcome = unify_pool(env, pool, *fx, Variable::EFFECTFUL, ctx.mode);

            outcome.union(merge(
                env,
                ctx,
                Structure(Func(*args, *closure, *ret, Variable::EFFECTFUL)),
            ));

            outcome
        }

Eli Dowling (Dec 01 2024 at 06:50):

I think the current method for detecting whether a module should be using purity inference or Task effects is kind of janky and error prone.
For example if I require a record containing an effectful function it will break.
But given I assume tasks will be removed soon I suppose it's okay

Eli Dowling (Dec 01 2024 at 07:23):

Agus Zubiaga (Dec 01 2024 at 11:09):

Agus Zubiaga (Dec 01 2024 at 11:13):

I guess the issue you found with the compiler not switching to purity inference mode was because you had no main.roc, so it wasn’t finding a hosted module on the platform with a !-suffixed function when you roc check a module

Agus Zubiaga (Dec 01 2024 at 11:19):

That’s a tricky case to detect reliably, because we need to switch to the right mode before canonicalization, but I don’t know if we have the all the context from just the parser

Agus Zubiaga (Dec 01 2024 at 11:21):

Maybe we can visit all the exprs looking for !-suffixed defs and record fields (not call exprs because those could be tasks with the ! operator)

Agus Zubiaga (Dec 01 2024 at 11:23):

Using the platform is way easier to tell, but of course, we have no platform in this case

Anton (Dec 01 2024 at 11:37):

It's using a github CI machine, I'm just going to try calling valgrind in the ci workflow

Anton (Dec 01 2024 at 12:33):

 + valgrind ./examples//args div -n 5 -d 20
==5956== Memcheck, a memory error detector
==5956== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==5956== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==5956== Command: ./examples//args div -n 5 -d 20
==5956==
received argument: div
==5956== Can't extend stack to 0x4803138 during signal delivery for thread 1:
==5956==   no stack segment
==5956==
==5956== Process terminating with default action of signal 11 (SIGSEGV)
==5956==  Access not within mapped region at address 0x4803138
==5956==    at 0x2C1045: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==5956==    by 0x24AC63: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==5956==    by 0x12950D: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==5956==    by 0x480423F: ???
==5956==    by 0x1294F2: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==5956==    by 0x480423F: ???
==5956==    by 0x12981C: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==5956==    by 0x3E8: ???
==5956==    by 0x7E: ???
==5956==  If you believe this happened as a result of a stack
==5956==  overflow in your program's main thread (unlikely but
==5956==  possible), you can try to increase the size of the
==5956==  main thread stack using the --main-stacksize= flag.
==5956==  The main thread stack size used in this run was 16777216.
==5956==
==5956== HEAP SUMMARY:
==5956==     in use at exit: 0 bytes in 0 blocks
==5956==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==5956==
==5956== All heap blocks were freed -- no leaks are possible
==5956==
==5956== For lists of detected and suppressed errors, rerun with: -s
==5956== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
./ci/all_tests.sh: line 60:  5956 Segmentation fault      (core dumped) valgrind $EXAMPLES_DIR/args div -n 5 -d 20

Anton (Dec 01 2024 at 12:34):

Anton (Dec 01 2024 at 12:38):

Anton (Dec 01 2024 at 12:55):

I made a small change and now I see __run_exit_handlers and exit in the valgrind output, so it is very likely #5924

Anton (Dec 01 2024 at 13:04):

Eli Dowling (Dec 01 2024 at 13:05):

Does this, issue just kind of go away when we remove Tasks though? Like we can then not bother detecting if it's purity inference for tasks

Eli Dowling (Dec 01 2024 at 13:07):

As in, the current solution is imperfect but also very temporary, so it's not a big deal.

Agus Zubiaga (Dec 01 2024 at 13:16):

Anton (Dec 04 2024 at 19:03):

From my experience with #5924, we'll probably be good if we add or remove a roc_fx function from crates/roc_host/src/lib.rs, anybody any ideas for functionality we can add or leave out?

Luke Boswell (Dec 04 2024 at 19:09):

Lol, I didn't realise this could be a workaround. I could add something random in that we don't need pretty easily.

Sam Mohr (Dec 09 2024 at 21:52):

I can start with hard linking, and then soft linking, and if those don't work we try something else?

Sam Mohr (Dec 09 2024 at 21:53):

Sam Mohr (Dec 09 2024 at 21:55):

Sam Mohr (Dec 09 2024 at 23:23):

Luke Boswell (Dec 10 2024 at 03:18):

I'm thinking of merging @Eli Dowling's PR https://github.com/roc-lang/basic-cli/pull/278 into the PI branch. We don't have CI oversight here, but we haven't got everything passing yet anyway, and I think we should include the changes.

Luke Boswell (Dec 10 2024 at 03:19):

I'm not about the merge it rn, but if there's no objections I can do it tomorrow.

Sam Mohr (Dec 10 2024 at 03:23):

Anton (Dec 10 2024 at 10:11):

Luke Boswell (Dec 10 2024 at 22:18):

I am just looking at CI and it looks like we've progressed significantly, it's only failing in native now.

Luke Boswell (Dec 10 2024 at 22:19):

@Brendan Hansknecht or anyone familiar with valgrind... can we glean anything from this?

+ valgrind ./examples//args div -n 5 -d 20
==4550== Memcheck, a memory error detector
==4550== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4550== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==4550== Command: ./examples//args div -n 5 -d 20
==4550==
received argument: div
==4550== Can't extend stack to 0x484c138 during signal delivery for thread 1:
==4550==   no stack segment
==4550==
==4550== Process terminating with default action of signal 11 (SIGSEGV)
==4550==  Access not within mapped region at address 0x484C138
==4550==    at 0x2B0004: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==4550==    by 0x48EE8A6: __run_exit_handlers (exit.c:108)
==4550==    by 0x48EEA5F: exit (exit.c:139)
==4550==    by 0x2556C4: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==4550==    by 0x255653: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==4550==    by 0x12859D: ??? (in /home/runner/work/basic-cli/basic-cli/examples/args)
==4550==  If you believe this happened as a result of a stack
==4550==  overflow in your program's main thread (unlikely but
==4550==  possible), you can try to increase the size of the
==4550==  main thread stack using the --main-stacksize= flag.
==4550==  The main thread stack size used in this run was 16777216.
==4550== Invalid write of size 8
==4550==    at 0x4831134: _vgnU_freeres (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_core-amd64-linux.so)
==4550==  Address 0x484cff8 is on thread 1's stack
==4550==
==4550==
==4550== Process terminating with default action of signal 11 (SIGSEGV)
==4550==  Access not within mapped region at address 0x484CFF8
==4550==    at 0x4831134: _vgnU_freeres (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_core-amd64-linux.so)
==4550==  If you believe this happened as a result of a stack
==4550==  overflow in your program's main thread (unlikely but
==4550==  possible), you can try to increase the size of the
==4550==  main thread stack using the --main-stacksize= flag.
==4550==  The main thread stack size used in this run was 16777216.
==4550==
==4550== HEAP SUMMARY:
==4550==     in use at exit: 0 bytes in 0 blocks
==4550==   total heap usage: 16 allocs, 16 frees, 3,330 bytes allocated
==4550==
==4550== All heap blocks were freed -- no leaks are possible
==4550==
==4550== For lists of detected and suppressed errors, rerun with: -s
==4550== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
./ci/all_tests.sh: line 60:  4550 Segmentation fault      (core dumped) valgrind $EXAMPLES_DIR/args div -n 5 -d 20
Error: Process completed with exit code 139.

Luke Boswell (Dec 10 2024 at 22:26):

oh... I think I found something :fingers_crossed: that is just completely wrong. The args example doesn't do that anymore!

Brendan Hansknecht (Dec 10 2024 at 22:30):

Luke Boswell (Dec 10 2024 at 22:31):

We were calling the args example valgrind $EXAMPLES_DIR/args div -n 5 -d 20 but args has been dramatically simplified so we don't have a dependency on Weaver any more

Luke Boswell (Dec 10 2024 at 22:34):

Luke Boswell (Dec 10 2024 at 22:40):

Luke Boswell (Dec 10 2024 at 22:56):

I'm pretty sure the issue is related to ubuntu-20.04, I can't repro on my 22.04. I might try disabling that machine... just to confirm CI passes for 22.04 also.

Brendan Hansknecht (Dec 10 2024 at 23:15):

Is the failure with both surgical and legacy linker? Can we change ci test check?

Luke Boswell (Dec 10 2024 at 23:57):

+ valgrind --main-stacksize=32777216 --track-origins=yes --leak-check=full --show-leak-kinds=all --verbose ./examples//args argument
==4639== Memcheck, a memory error detector
==4639== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4639== Using Valgrind-3.15.0-608cb11914-20190413 and LibVEX; rerun with -h for copyright info
==4639== Command: ./examples//args argument
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/libc-2.31.so
--4639--   Considering /usr/lib/debug/.build-id/07/02430aef5fa3dda43986563e9ffcc47efbd75e.debug ..
--4639--   .. build-id is valid
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/libm-2.31.so
--4639--   Considering /usr/lib/debug/.build-id/8d/2573eff281739f0e2b0eb710c860ce0b7261cf.debug ..
--4639--   .. build-id is valid
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/libpthread-2.31.so
--4639--   Considering /usr/lib/debug/.build-id/9a/65bb469e45a1c6fbcffae5b82a2fd7a69eb479.debug ..
--4639--   .. build-id is valid
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/libdl-2.31.so
--4639--   Considering /usr/lib/debug/.build-id/25/372f43dbcc661aa02020d0365c948e89f6e612.debug ..
--4639--   .. build-id is valid
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/librt-2.31.so
--4639--   Considering /usr/lib/debug/.build-id/fc/7c873442781f08af6bc88f1acac7ecccec7285.debug ..
--4639--   .. build-id is valid
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/libutil-2.31.so
--4639--   Considering /usr/lib/debug/.build-id/24/4c5140c0521c81914afe5d92ad5ccc954857d5.debug ..
--4639--   .. build-id is valid
--4639-- Reading syms from /usr/lib/x86_64-linux-gnu/libgcc_s.so.1
--4639--    object doesn't have a symbol table
--4639-- REDIR: 0x48fa480 (libc.so.6:memmove) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9780 (libc.so.6:strncpy) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa7b0 (libc.so.6:strcasecmp) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f90a0 (libc.so.6:strcat) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f97e0 (libc.so.6:rindex) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fbc50 (libc.so.6:rawmemchr) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x4916ce0 (libc.so.6:wmemchr) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x4916820 (libc.so.6:wcscmp) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa5e0 (libc.so.6:mempcpy) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa410 (libc.so.6:bcmp) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9710 (libc.so.6:strncmp) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9150 (libc.so.6:strcmp) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa540 (libc.so.6:memset) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x49167e0 (libc.so.6:wcschr) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9670 (libc.so.6:strnlen) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9230 (libc.so.6:strcspn) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa800 (libc.so.6:strncasecmp) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f91d0 (libc.so.6:strcpy) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa950 (libc.so.6:memcpy@@GLIBC_2.14) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x4917f50 (libc.so.6:wcsnlen) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x4916860 (libc.so.6:wcscpy) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9820 (libc.so.6:strpbrk) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9100 (libc.so.6:index) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9630 (libc.so.6:strlen) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x4902bb0 (libc.so.6:memrchr) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa850 (libc.so.6:strcasecmp_l) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa3d0 (libc.so.6:memchr) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x4916930 (libc.so.6:wcslen) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48f9ae0 (libc.so.6:strspn) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa750 (libc.so.6:stpncpy) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa6f0 (libc.so.6:stpcpy) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fbc90 (libc.so.6:strchrnul) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x48fa8a0 (libc.so.6:strncasecmp_l) redirected to 0x48311d0 (_vgnU_ifunc_wrapper)
--4639-- REDIR: 0x49e2730 (libc.so.6:__strrchr_avx2) redirected to 0x483ea10 (rindex)
--4639-- REDIR: 0x48f40e0 (libc.so.6:malloc) redirected to 0x483b780 (malloc)
--4639-- REDIR: 0x49e2900 (libc.so.6:__strlen_avx2) redirected to 0x483ef40 (strlen)
--4639-- REDIR: 0x48f46d0 (libc.so.6:free) redirected to 0x483c9d0 (free)
--4639-- REDIR: 0x49de230 (libc.so.6:__strncmp_avx2) redirected to 0x483f670 (strncmp)
--4639-- REDIR: 0x48f5b10 (libc.so.6:calloc) redirected to 0x483dce0 (calloc)
received argument: argument
==4639==
==4639== HEAP SUMMARY:
==4639==     in use at exit: 0 bytes in 0 blocks
==4639==   total heap usage: 8 allocs, 8 frees, 1,320 bytes allocated
==4639==
==4639== All heap blocks were freed -- no leaks are possible
==4639==
==4639== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

But, now CI is stopping on another thing... so need to investigate that also. It may be related.

+ ./roc_nightly/roc dev ./examples/command.roc
BAZ=DUCK
FOO=BAR
FOO=BAR
EXEC
Error: Process completed with exit code 1.

Luke Boswell (Dec 11 2024 at 00:24):

Ok, we're down to the very last step, and we're failing specifically with x86_64-unknown-linux-musl and not getting any args... this is a known issue https://github.com/roc-lang/basic-cli/issues/82 or at least feels very familiar

Sam Mohr (Dec 11 2024 at 00:27):

Brendan Hansknecht (Dec 11 2024 at 00:48):

Sam Mohr (Dec 11 2024 at 00:48):

Brendan Hansknecht (Dec 11 2024 at 00:50):

There has been an uber long standing bug related to this. Sometimes it seemed to be surgical linker only. Other times it was clearly with the legacy linker. I thought we finally fixed it with the exit code fixed. I think it was actually two separate bugs. The exit code bug that affected both linkers and this bug which is surgical linker specific. Really intriguing it only affects the arg example. I wonder what makes that special. I guess arg is also the only example to break on musl so maybe how loading args happens is just brittle/more complex.

Luke Boswell (Dec 11 2024 at 01:11):

I've been trying to get musl thing locally on my linux machine, and I can't even get rust to compile. I'm really confused

Sam Mohr (Dec 11 2024 at 01:12):

Luke Boswell (Dec 11 2024 at 01:18):

I'm trying to get CI green, trying to figure out if we can keep the whole musl build and test thing

Luke Boswell (Dec 11 2024 at 01:18):

If you have a linux machine, you could try running that step manually (build and run the tests using musl)

Luke Boswell (Dec 11 2024 at 01:19):

For some reason that last CI run got stuck at an earlier point, just building the host using musl. The previous run got further and was stuck at running the examples. So not sure why that happened, seems odd -- because the things I changed between runs were only in the running part and shouldn't have affected the building.

Sam Mohr (Dec 11 2024 at 02:17):

Brendan Hansknecht (Dec 11 2024 at 02:23):

From what I can tell, we need get the args in main and then store them in a global or pass them into rust_main.

Luke Boswell (Dec 11 2024 at 02:24):

Sam Mohr (Dec 11 2024 at 02:25):

So I know it's a breaking change, but we're already requiring breaking for converting to main!

Brendan Hansknecht (Dec 11 2024 at 02:25):

Luke Boswell (Dec 11 2024 at 02:25):

Brendan Hansknecht (Dec 11 2024 at 02:26):

oh, no change to the roc main! this is only the c program main that has to change

Sam Mohr (Dec 11 2024 at 02:26):

Luke Boswell (Dec 11 2024 at 02:26):

Also, I'm wanting to change PlatformTasks.roc to Host.roc ... unless there's strong objection in the next few minutes :smiley:

Sam Mohr (Dec 11 2024 at 02:26):

Brendan Hansknecht (Dec 11 2024 at 02:26):

Sam Mohr (Dec 11 2024 at 02:27):

I'm okay with either, but the benefit of Host.roc is that it implies you're calling the host/platform

Luke Boswell (Dec 11 2024 at 02:27):

I was on the fence, I like Effects.roc ... but it's a hosted module type (which I don't think we're removing anytime soon. So..

Sam Mohr (Dec 11 2024 at 02:27):

Effects.roc feels like its good because it's plural and sounds like the old name

Luke Boswell (Dec 11 2024 at 02:27):

It's where I like to keep all the functions and types that cross the boundary across to the host

Brendan Hansknecht (Dec 11 2024 at 02:28):

I mean it does hold all the effectful functions. also, why did we pick the name hosted? I never got that

Brendan Hansknecht (Dec 11 2024 at 02:28):

Also, no idea if fixing this will fix the valgrind bug with args and musl, but it seems likely

Luke Boswell (Dec 11 2024 at 02:28):

My thoughts are that Host is clearer for someone not very familiar with how it all connects together

Brendan Hansknecht (Dec 11 2024 at 02:29):

Luke Boswell (Dec 11 2024 at 02:32):

Actually... I vaguely remember dealing with something about args and rust static libraries. There was a really deep rabbit hole I went down... trying to remember where that was.

Luke Boswell (Dec 11 2024 at 02:32):

Brendan Hansknecht (Dec 11 2024 at 02:37):

That is why originally the lib version was wrapped by a c layer that was only a main function

Luke Boswell (Dec 11 2024 at 02:38):

Luke Boswell (Dec 11 2024 at 02:39):

Brendan Hansknecht (Dec 11 2024 at 02:42):

#[no_mangle]
pub extern "C" fn main(argc: isize, _argv: *const *const u8) -> i32 {
    dbg!(argc);
    roc_host::rust_main()
}

Brendan Hansknecht (Dec 11 2024 at 02:42):

Luke Boswell (Dec 11 2024 at 02:42):

Brendan Hansknecht (Dec 11 2024 at 02:44):

probably should just convert straight to a roc list of roc strings and pass that into roc_host::rust_main(). Then can store that in a global for later retrieval, that or change to passing it into mainforhost

Sam Mohr (Dec 11 2024 at 02:52):

Sam Mohr (Dec 11 2024 at 02:53):

Luke Boswell (Dec 11 2024 at 02:53):

use std::cell::RefCell;

thread_local! {
    static ARGS: RefCell<Vec<String>> = const { RefCell::new(vec![]) };
}

pub fn with<T>(f: impl FnOnce(&mut Vec<String>) -> T) -> T {
    ARGS.with(|args| f(&mut args.borrow_mut()))
}

Luke Boswell (Dec 11 2024 at 02:53):

#[no_mangle]
pub extern "C" fn main(argc: isize, argv: *const *const u8) {
    roc_host::args::with(|args| {
        for i in 0..argc {
            unsafe {
                let arg_ptr = *argv.offset(i);
                let arg = std::ffi::CStr::from_ptr(arg_ptr as *const i8)
                    .to_string_lossy()
                    .into_owned();

                args.push(arg);
            }
        }
    });

    roc_host::rust_main();
}

Sam Mohr (Dec 11 2024 at 02:54):

Luke Boswell (Dec 11 2024 at 02:54):

#[no_mangle]
pub extern "C" fn roc_fx_args() -> RocList<RocStr> {
    args::with(|args| {
        let vec_args: Vec<RocStr> = args.into_iter().map(|s| s.as_str().into()).collect();
        vec_args.as_slice().into()
    })
}

Luke Boswell (Dec 11 2024 at 02:54):

rustc: `RefCell<Vec<String>>` cannot be shared between threads safely
the trait `Sync` is not implemented for `RefCell<Vec<String>>`
if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead
shared static variables must have a type that implements `Sync`

Brendan Hansknecht (Dec 11 2024 at 02:55):

Brendan Hansknecht (Dec 11 2024 at 02:56):

Luke Boswell (Dec 11 2024 at 02:56):

rustc: cannot call non-const fn `RocList::<RocStr>::empty` in constants
calls in constants are limited to constant functions, tuple structs and tuple variants

Luke Boswell (Dec 11 2024 at 02:56):

Brendan Hansknecht (Dec 11 2024 at 02:57):

Sam Mohr (Dec 11 2024 at 02:57):

Brendan Hansknecht (Dec 11 2024 at 02:57):

original_list.set_readonly()
SendSafeRocList(original_list)

Luke Boswell (Dec 11 2024 at 02:58):

@Brendan Hansknecht it might be easier if you put something together... I'm going in circles a bit here

Brendan Hansknecht (Dec 11 2024 at 02:59):

Also, what are the thoughts on removing args and just passing the args into main? If we do that, we can skip all the globals in general?

Brendan Hansknecht (Dec 11 2024 at 03:00):

Luke Boswell (Dec 11 2024 at 03:00):

Brendan Hansknecht (Dec 11 2024 at 03:01):

main! = \{} ->
    Stdout.line! "Hello, World!"

main! = \_args ->
    Stdout.line! "Hello, World!"

Luke Boswell (Dec 11 2024 at 03:01):

We could have all the "global" environment type information passed in then, like the Env.platform! : {} => { arch : ARCH, os : OS } instead

Brendan Hansknecht (Dec 11 2024 at 03:01):

Luke Boswell (Dec 11 2024 at 03:01):

main! = \{args} ->
    Stdout.line! "Got $(Inspect.toStr args)"

Luke Boswell (Dec 11 2024 at 03:02):

Brendan Hansknecht (Dec 11 2024 at 03:02):

cause it may be wanted in a deeply nested state where it is convenient to just grab. Vs args are essentially only ever wanted at the top level

Brendan Hansknecht (Dec 11 2024 at 03:02):

Brendan Hansknecht (Dec 11 2024 at 03:35):

Sam Mohr (Dec 11 2024 at 03:36):

Sam Mohr (Dec 11 2024 at 03:40):

Luke Boswell (Dec 11 2024 at 03:40):

Brendan Hansknecht (Dec 11 2024 at 03:46):

Brendan Hansknecht (Dec 11 2024 at 03:47):

but SendSafeRocList is not Sync cause it also includes unique roc lists, which are Send but not Sync

Brendan Hansknecht (Dec 11 2024 at 03:47):

Luke Boswell (Dec 11 2024 at 03:47):

Can you rename it to SendAndSyncSafeRocList ? or DefinitelySendAndSyncSafeRocListBecauseItsReadOnly

Brendan Hansknecht (Dec 11 2024 at 03:47):

It isn't SendAndSyncSafeRocList cause a unique list is send but not sync. Only a readonly list is send and sync.

Brendan Hansknecht (Dec 11 2024 at 03:48):

Sam Mohr (Dec 11 2024 at 03:48):

You could just store the args as Vec<String> and convert every time, not as performant but it won't matter because it's just CLI args

Brendan Hansknecht (Dec 11 2024 at 03:48):

Sam Mohr (Dec 11 2024 at 03:48):

Luke Boswell (Dec 11 2024 at 03:49):

Brendan Hansknecht (Dec 11 2024 at 03:50):

let me make the proper fix, won't take long and is still wanted for other future usages.

Brendan Hansknecht (Dec 11 2024 at 04:04):

Brendan Hansknecht (Dec 11 2024 at 04:30):

Luke Boswell (Dec 11 2024 at 04:31):

Brendan Hansknecht (Dec 11 2024 at 04:32):

Luke Boswell (Dec 11 2024 at 04:33):

Feel free to revert or remove all the hacks in ci/all_tests.sh; we've left comments so it's easy to delete the relevant parts

Luke Boswell (Dec 11 2024 at 04:33):

Brendan Hansknecht (Dec 11 2024 at 04:35):

from local testing, everything looks to be passing with both linkers for both musl and glibc

Luke Boswell (Dec 11 2024 at 04:38):

Brendan Hansknecht (Dec 11 2024 at 04:38):

Thought it would be best to push now so either of you can take over later if I am not around

Sam Mohr (Dec 11 2024 at 04:47):

Luke Boswell (Dec 11 2024 at 04:47):

Richard Feldman (Dec 11 2024 at 05:14):

my original reasoning (although we never actually did this) is that the OS doesn't require that the args be valid UTF-8, and if you silently replace invalid UTF-8 sequences with the Unicode Replacement Character, then you've lost information and there becomes no way to access the actual original args that were passed into the process

Richard Feldman (Dec 11 2024 at 05:15):

anyway, the main point is that THIS IS AWESOME!!!! :heart_eyes: :heart_eyes: :heart_eyes:

Brendan Hansknecht (Dec 11 2024 at 05:19):

We were already doing this. Given we have ? and try now, maybe it would be best to change to returning a result.

Brendan Hansknecht (Dec 11 2024 at 05:20):

Richard Feldman (Dec 11 2024 at 05:26):

another thing we could do is to do that for you automatically, and then have a separate way to opt into going and getting the originals if you really want to

Richard Feldman (Dec 11 2024 at 05:26):

if there's demand in practice, we could introduce that as a nonbreaking change later

Luke Boswell (Dec 11 2024 at 06:13):

kk, I've kicked off CI now with the changes... and then I remember that we need to wait for the new nightly to land :tear:

@Anton we think this should be good to go. I've reverted the legacy linker and other debugging stuff we had in all_tests.sh so should be just the nightly I think :fingers_crossed:

Brendan Hansknecht (Dec 11 2024 at 06:14):

Brendan Hansknecht (Dec 11 2024 at 06:15):

Luke Boswell (Dec 11 2024 at 06:16):

Luke Boswell (Dec 11 2024 at 06:17):

Luke Boswell (Dec 11 2024 at 06:18):

Brendan Hansknecht (Dec 11 2024 at 06:21):

Yeah, and if this passes ci with both the legacy and surgical linker, it will mean that we fixed some major platform bugs and don't have a hard to debug/fix surgical linker bug.

Luke Boswell (Dec 11 2024 at 06:21):

Brendan Hansknecht (Dec 11 2024 at 06:21):

Brendan Hansknecht (Dec 11 2024 at 06:22):

Can we pull legacy linker changes to the test script and run it with surgical then?

Luke Boswell (Dec 11 2024 at 06:23):

Brendan Hansknecht (Dec 11 2024 at 06:23):

Luke Boswell (Dec 11 2024 at 06:23):

Nix macos failed though... seems odd it passed for me locally, probably a CI thing

Brendan Hansknecht (Dec 11 2024 at 06:24):

Luke Boswell (Dec 11 2024 at 06:26):

Brendan Hansknecht (Dec 11 2024 at 06:27):

Luke Boswell (Dec 11 2024 at 06:41):

Brendan Hansknecht (Dec 11 2024 at 06:48):

Luke Boswell (Dec 11 2024 at 06:48):

Luke Boswell (Dec 11 2024 at 06:53):

Luke Boswell (Dec 11 2024 at 07:00):

Ok, I've left a few comments, nothing blocking in my opinion so I'm going to merge this. Thank you :smiley:

Sam Mohr (Dec 11 2024 at 07:02):

Luke Boswell (Dec 11 2024 at 07:02):

Kilian Vounckx (Dec 11 2024 at 10:12):

This is awesome!! Good work to everyone who worked on this!
When will there be a release so it can be used without cloning the repo?

Anton (Dec 11 2024 at 10:36):

I'm going to bundle in the required changes for https://github.com/roc-lang/roc/pull/7321 in the next basic-cli release, that is a widely used function but probably few things will actually break because of that change. The new release will probably be available next week.

Kilian Vounckx (Dec 11 2024 at 10:37):

Sam Mohr (Dec 17 2024 at 19:00):

Before the PI release, can we change main! : {} => Result {} _ to main! : List Str => Result {} _ to put all of the breaking changes together

Sam Mohr (Dec 17 2024 at 19:01):

Luke Boswell (Dec 17 2024 at 19:02):

Brendan Hansknecht (Dec 17 2024 at 19:02):

Brendan Hansknecht (Dec 17 2024 at 19:03):

Luke Boswell (Dec 17 2024 at 19:03):

Or maybe it was my larger suggestion of providing all the envrionment stuff in an record that could be destructured.

Brendan Hansknecht (Dec 17 2024 at 19:04):

I think Richard commented about wanting it to be a zero arg function for max simplicity when teaching a new user. Though ignoring one arg sounds fine to teach a new user as well.

Luke Boswell (Dec 17 2024 at 19:06):

I can certainly make the change today. I would like to merge that refactor PR, and can add this in. I think were really close to getting cli/server/ssg all passong CI.

I might be getting ahead of myself, but I'm even thinking of bringing most of the basic-cli API across to ssg too.

Luke Boswell (Dec 17 2024 at 19:07):

Sam Mohr (Dec 17 2024 at 19:11):

That's my thought. And I think every beginner will know what args are (they're what you pass to roc to use it!)

Luke Boswell (Dec 17 2024 at 19:13):

Ayaz Hafiz (Dec 17 2024 at 19:16):

Sam Mohr (Dec 17 2024 at 19:16):

Brendan Hansknecht (Dec 17 2024 at 19:19):

I think it is fine to default to utf-8 and if we really want, we can add a fallback when a user requests/needs it.

Brendan Hansknecht (Dec 17 2024 at 19:19):

Luke Boswell (Dec 17 2024 at 19:30):

My concern is the silent nature of that, would people assume something and then it causes a bug. Maybe we detect non-utf8 args and crash, or instead provide zero args so it's easy to ignfore and then read args using an effect that gives List U8.

Sam Mohr (Dec 17 2024 at 19:31):

I'm down for the crash, but no one wants to handle non-UTF-8 except for a very small contingent

Luke Boswell (Dec 17 2024 at 19:33):

Brendan Hansknecht (Dec 17 2024 at 19:33):

Luke Boswell (Dec 17 2024 at 19:37):

We're aiming for higher level use cases. If someone needs non-utf8 args they're probably in the write or customise a platform space.

Luke Boswell (Dec 17 2024 at 19:39):

We still want to keep the effect for reading Args, so it's easier to get from within the app without having to thread apl the way through? Just checking.

Sam Mohr (Dec 17 2024 at 19:40):

Brendan Hansknecht (Dec 17 2024 at 19:41):

Brendan Hansknecht (Dec 17 2024 at 19:42):

Luke Boswell (Dec 17 2024 at 19:49):

May as well try just passing it in, and see how that goes. It's easy enough to add back if people have issues.

Luke Boswell (Dec 17 2024 at 19:49):

Sam Mohr (Dec 17 2024 at 19:52):

Richard Feldman (Dec 17 2024 at 20:42):

the only thing I strongly think we shouldn't do for sure is have the platform crash based on what args are provided :laughing:

Sam Mohr (Dec 17 2024 at 20:43):

Richard Feldman (Dec 17 2024 at 20:43):

we could do that, or if we wanted to be fancy we could do like main : Result (List Str) [NonUnicodeArgs] -> ...

Richard Feldman (Dec 17 2024 at 20:44):

Sam Mohr (Dec 17 2024 at 20:44):

Sam Mohr (Dec 17 2024 at 20:47):

Richard Feldman (Dec 17 2024 at 20:48):

the scenario I can think of where it could matter is that you have some non-utf8 filenames, and you want to pass those in as arguments

Sam Mohr (Dec 17 2024 at 20:49):

Richard Feldman (Dec 17 2024 at 20:49):

the thing that bothers me about that is that we're now creating a situation where application authors are by default telling their users "don't have this use case"

Sam Mohr (Dec 17 2024 at 20:49):

This doesn't make basic-cli unusable, it just means you need to pass the file via env var or something

Sam Mohr (Dec 17 2024 at 20:50):

Richard Feldman (Dec 17 2024 at 20:50):

what will happen in practice is that people will use the most convenient API, namely the List Str you get for free, and which doesn't handle this edge case, and then if end users have files they need to pass in that way, they just won't be able to do that

Richard Feldman (Dec 17 2024 at 20:51):

my hope with things like this is that if you're using a CLI arg parser like weaver anyway, then it can be actually very easy to handle that scenario

Richard Feldman (Dec 17 2024 at 20:51):

because weaver works on List U8 and provides a convenient interface to the app author anyway

Sam Mohr (Dec 17 2024 at 20:51):

Richard Feldman (Dec 17 2024 at 20:52):

and then if at some point I want to be like "oh just this one particular thing takes List U8 instead of Str" then I only have to change it in one place

Sam Mohr (Dec 17 2024 at 20:52):

But we could change basic-cli to use List (List U8) and Weaver to do the same, and have Weaver handle encoding errors

Richard Feldman (Dec 17 2024 at 20:52):

Sam Mohr (Dec 17 2024 at 20:53):

Okay, then I'll change this PR to make basic-cli do List (List U8) and update Weaver later

Richard Feldman (Dec 17 2024 at 20:53):

Sam Mohr (Dec 17 2024 at 20:54):

For now, people can do validArgs = List.map args \arg -> Str.fromUtf8 arg |> Result.withDefault ""

Sam Mohr (Dec 17 2024 at 20:54):

Sam Mohr (Dec 17 2024 at 20:56):

Sam Mohr (Dec 17 2024 at 20:59):

Since apps will be handling stuff with Weaver primarily, I vote just changing Arg.list! to {} => List (List U8)

Brendan Hansknecht (Dec 17 2024 at 21:04):

Sam Mohr (Dec 17 2024 at 21:05):

And to match with that, we should just convert Arg.list! to {} => List (List U8)

Brendan Hansknecht (Dec 17 2024 at 21:06):

Sam Mohr (Dec 17 2024 at 21:10):

What if you want the arg list halfway through the program? Should you have to thread the args through the program?

Sam Mohr (Dec 17 2024 at 21:13):

Brendan Hansknecht (Dec 17 2024 at 21:15):

I think it's fine. I personally just want one way. I think it is most common to use at the top, so forcing threading down sounds fine.

Sam Mohr (Dec 17 2024 at 21:15):

Richard Feldman (Dec 17 2024 at 21:22):

Richard Feldman (Dec 17 2024 at 21:23):

Sam Mohr (Dec 17 2024 at 21:23):

Sam Mohr (Dec 17 2024 at 21:24):

Sam Mohr (Dec 17 2024 at 21:25):

Richard Feldman (Dec 17 2024 at 21:25):

Sam Mohr (Dec 17 2024 at 21:26):

Richard Feldman (Dec 17 2024 at 21:26):

Sam Mohr (Dec 17 2024 at 21:26):

Richard Feldman (Dec 17 2024 at 21:26):

Sam Mohr (Dec 17 2024 at 21:26):

Richard Feldman (Dec 17 2024 at 21:27):

but the main reason was wanting to get to a state where no matter what target you run it on, all pure functions give the same answers given the same arguments

Sam Mohr (Dec 17 2024 at 21:27):

Richard Feldman (Dec 17 2024 at 21:27):

Sam Mohr (Dec 17 2024 at 21:28):

Richard Feldman (Dec 17 2024 at 21:28):

also OsStr.to_list : OsStr -> [Windows (List U16), Unix (List U8)] would be fine

Luke Boswell (Dec 17 2024 at 21:28):

Ok, if we can't crash (for reasons Richard outline above), what about my other option?

Luke Boswell (Dec 17 2024 at 21:29):

We check if it's valid UTF-8, if not, we provide an empty list for args. Then keep a Args.unix! : {} -> List (List U8) and a Args.windows! : {} -> List (List U16) .. at least until we implement roc-lang/path

Luke Boswell (Dec 17 2024 at 21:30):

That's probably just making things worse though... because people aren't going to pre-emptively check for empty args

Luke Boswell (Dec 17 2024 at 21:30):

Luke Boswell (Dec 17 2024 at 21:32):

Sam Mohr (Dec 17 2024 at 21:32):

Luke Boswell (Dec 17 2024 at 21:32):

Richard Feldman (Dec 17 2024 at 21:32):

Richard Feldman (Dec 17 2024 at 21:33):

the only way to change that would be to change something in builtins, which this wouldn't do regardless :big_smile:

Richard Feldman (Dec 17 2024 at 21:34):

the downside there is that if you actually have an end user who has a file with invalid unicode in its path, and wants to pass it into a cli made with basic-cli, it will be impossible

Richard Feldman (Dec 17 2024 at 21:35):

so basically if we go with the Unicode replacement character route, we're choosing to have those edge cases never work correctly in basic-cli

Luke Boswell (Dec 17 2024 at 21:35):

I dislike the silent nature of unicode replacment.. I'd prefer crashing and keeping main! : List Str => ...

Richard Feldman (Dec 17 2024 at 21:35):

which feels like a bit of a shame if people are just going to use a cli parser anyway, and could just say "hey this arg is a Path" (for example)

Sam Mohr (Dec 17 2024 at 21:35):

OsStr := { bytes : List U8, os: [Unix, Windows] }

OsStr.bytes = os_str ->
    when os is
        Unix -> os_str.bytes
        Windows ->
            os_str.bytes
                .chunks(2)
                .map(left, right -> left.shift_left(8) + right)

Richard Feldman (Dec 17 2024 at 21:36):

Luke Boswell (Dec 17 2024 at 21:36):

Sam Mohr (Dec 17 2024 at 21:36):

Richard Feldman (Dec 17 2024 at 21:37):

Luke Boswell (Dec 17 2024 at 21:37):

But in this case, people know they can't, and can reach for another program to convert the file names.

Richard Feldman (Dec 17 2024 at 21:37):

Sam Mohr (Dec 17 2024 at 21:37):

Just a point of pedantry, a Path is an OsStr, but an argument is not a Path necessarily, it's an OsStr. So OsStr is the real type at play

Richard Feldman (Dec 17 2024 at 21:38):

Sam Mohr (Dec 17 2024 at 21:40):

So I'm okay with main! : List (List U8) => Result {} _ because Weaver makes the user experience not really impeded, but if Weaver and basic-cli now depend of roc-lang/path for some OsStr type, now versioning is weird. This is a lot of weirdness eaten right at the start of "hello world" for a very niche problem

Sam Mohr (Dec 17 2024 at 21:41):

(not trying to do "hello world"-driven dev, just saying this would probably turn off beginners)

Richard Feldman (Dec 17 2024 at 21:42):

the problem with main! : List (List U8) => Result {} _ is that Windows will use u16s, not u8s

Richard Feldman (Dec 17 2024 at 21:42):

and weaver won't have any way of telling which one it's getting if we convert them all to u8

Richard Feldman (Dec 17 2024 at 21:43):

Richard Feldman (Dec 17 2024 at 21:44):

and the most straightforward thing you can do is give them to Arg.weave (or whatever)

Luke Boswell (Dec 17 2024 at 21:44):

Arg.display : Arg -> Result Str [InvalidUtf8]

Richard Feldman (Dec 17 2024 at 21:45):

Sam Mohr (Dec 17 2024 at 21:45):

Richard Feldman (Dec 17 2024 at 21:45):

although in this case it'd be InvalidUnicode because on Windows they aren't UTF-8 :big_smile:

Richard Feldman (Dec 17 2024 at 21:45):

Richard Feldman (Dec 17 2024 at 21:46):

Sam Mohr (Dec 17 2024 at 21:46):

Luke Boswell (Dec 17 2024 at 21:46):

Arg.to_raw : Arg -> [Unix (List U8), Windows (List U16)]

Luke Boswell (Dec 17 2024 at 21:47):

Sam Mohr (Dec 17 2024 at 21:47):

Richard Feldman (Dec 17 2024 at 21:47):

Sam Mohr (Dec 17 2024 at 21:47):

Richard Feldman (Dec 17 2024 at 21:48):

Sam Mohr (Dec 17 2024 at 21:48):

Richard Feldman (Dec 17 2024 at 21:48):

"main gets a List Arg and here is a nice way to turn that into a good experience for people"

Sam Mohr (Dec 17 2024 at 21:49):

Luke Boswell (Dec 17 2024 at 21:49):

I'd prefer we tell Weaver how to "decode" an Arg that comes from the platform. Other people may come up with Weaver 2.0's in future

Luke Boswell (Dec 17 2024 at 21:49):

Richard Feldman (Dec 17 2024 at 21:49):

Sam Mohr (Dec 17 2024 at 21:49):

Richard Feldman (Dec 17 2024 at 21:50):

heh, and actually in the static dispatch future, weave can just ask for something that has a to_raw method of the appropriate type :big_smile:

Richard Feldman (Dec 17 2024 at 21:50):

Sam Mohr (Dec 17 2024 at 21:51):

module [parseArgs] { decodeArg : a -> Result WeaverArg _ }

WeaverArg : [Unix (List U8), Windows (List U16)]

parseArgs : List WeaverArg -> Result _ _

Luke Boswell (Dec 17 2024 at 21:51):

Maybe we should spin this off into another thread... but, would we be picking a name for that?

Like, anything with the shape to_os_str : thing -> [Unix _, Windows _] where ... etc

Luke Boswell (Dec 17 2024 at 21:52):

I haven't fully digested static dispatch so ^^ may not be aligned with that worldview

Sam Mohr (Dec 17 2024 at 21:52):

import weaver.Cli { decodeArg : Arg.to_raw }

Sam Mohr (Dec 17 2024 at 21:53):

Richard Feldman (Dec 17 2024 at 21:53):

Richard Feldman (Dec 17 2024 at 21:54):

because that's the only place where it's relevant, and in the static dispatch world we can drop the extra weave argument

Sam Mohr (Dec 17 2024 at 21:54):

Luke Boswell (Dec 17 2024 at 21:54):

{ Cli.weave Args.to_os_raw <-
    alpha: Opt.u64 { short: "a", help: "Set the alpha level" },
    verbosity: Opt.count { short: "v", long: "verbose", help: "How loud we should be." },
    files: Param.strList { name: "files", help: "The files to process." },
}

Luke Boswell (Dec 17 2024 at 21:57):

Richard Feldman (Dec 17 2024 at 21:57):

Sam Mohr (Dec 17 2024 at 21:57):

Richard Feldman (Dec 17 2024 at 21:57):

Sam Mohr (Dec 17 2024 at 21:58):

Since static dispatch doesn't allow implementing methods outside of the defining module for a type, I presume that once Weaver has converter your args to WeaverArg, the user wouldn't be able to get a method on WeaverArg to convert it to a roc-lang/path data type.

Sam Mohr (Dec 17 2024 at 21:58):

Richard Feldman (Dec 17 2024 at 21:58):

Sam Mohr (Dec 17 2024 at 21:58):

In the long run, this kind of data type is in std or a parent lib, but we're way out from that, and this is still 90% as nice for the end user

Luke Boswell (Dec 17 2024 at 21:59):

Richard Feldman (Dec 17 2024 at 21:59):

Brendan Hansknecht (Dec 17 2024 at 21:59):

For args, why do we need to support utf16? All utf16 can be converted to utf-8. Why not just always losslessly convert to list u8?

Richard Feldman (Dec 17 2024 at 21:59):

Sam Mohr (Dec 17 2024 at 21:59):

Brendan Hansknecht (Dec 17 2024 at 21:59):

Brendan Hansknecht (Dec 17 2024 at 22:00):

Richard Feldman (Dec 17 2024 at 22:00):

yeah, the point is that if people are just going to be using Weaver anyway, we can handle those edge cases naturally

Richard Feldman (Dec 17 2024 at 22:00):

Luke Boswell (Dec 17 2024 at 22:00):

Richard Feldman (Dec 17 2024 at 22:00):

Luke Boswell (Dec 17 2024 at 22:01):

Brendan Hansknecht (Dec 17 2024 at 22:02):

Luke Boswell (Dec 17 2024 at 22:03):

It's ok, @Sam Mohr has volunteered to handle all the edge cases in Weaver :cross:

Sam Mohr (Dec 17 2024 at 22:03):

Brendan Hansknecht (Dec 17 2024 at 22:04):

So Weaver will implement utf16 so it can parse windows CLI args? And it will be able to parse both utf-8 and utf16. It also will be able to collect args that aren't valid utf-8/16 for use as paths and such

Brendan Hansknecht (Dec 17 2024 at 22:04):

Luke Boswell (Dec 17 2024 at 22:05):

Brendan Hansknecht (Dec 17 2024 at 22:06):

I guess my biggest comment is that I really like being able to use simple args and keep the lie (of everything just working as utf-8). I find it very convenient for "basic" CLI. I personally don't generally use tools like Weaver (but maybe I should).

Luke Boswell (Dec 17 2024 at 22:07):

Maybe we want something to keep things more sane (only use the others for non-utf8 things)

Arg.as_os : Arg -> [ Utf8 Str, Unix (List U8), Windows (List U16) ]

Sam Mohr (Dec 17 2024 at 22:08):

Instead of handling every arg as Str, it's gonna use WeaverArg. For each arg, it'll attempt to parse to the right type from WeaverArg, be that:

Richard Feldman (Dec 17 2024 at 22:08):

yeah I think we can offer a "quick and dirty" thing you can use e.g. if you're just making a quick script for yourself and don't want to build out a full set of args with help docs and all that

Richard Feldman (Dec 17 2024 at 22:09):

Sam Mohr (Dec 17 2024 at 22:09):

Sam Mohr (Dec 17 2024 at 22:10):

Richard Feldman (Dec 17 2024 at 22:10):

also possible, but if I'm just making a quick script for myself and don't want to bother with edge cases, I probably don't want to bother with dependencies either :big_smile:

Sam Mohr (Dec 17 2024 at 22:10):

Sam Mohr (Dec 17 2024 at 22:11):

Richard Feldman (Dec 17 2024 at 22:11):

Sam Mohr (Dec 17 2024 at 22:11):

Richard Feldman (Dec 17 2024 at 22:11):

Richard Feldman (Dec 17 2024 at 22:12):

Sam Mohr (Dec 17 2024 at 22:12):

I just don't know what "quick and dirty args" will look like without giving users a footgun that this whole discussion just attempted to avoid

Sam Mohr (Dec 17 2024 at 22:12):

Luke Boswell (Dec 17 2024 at 22:12):

So we have the "pit-of-success" but somewhat complicated default for main! : List Arg -> ..., and then also provde a quick and dirty Arg.list! : {} => List Str which might crash on invalid utf8

Sam Mohr (Dec 17 2024 at 22:13):

Brendan Hansknecht (Dec 17 2024 at 22:13):

If we make Weaver part of basic CLI, I would be happy with it exposing a method to just convert all args to utf8

Richard Feldman (Dec 17 2024 at 22:13):

it's the difference between people being unpleasantly surprised when things don't work in unusual circumstances vs being like "hey the equivalent tool written in Roc works fine in this scenario, how come yours crashes, Non Roc Program?"

Brendan Hansknecht (Dec 17 2024 at 22:13):

Luke Boswell (Dec 17 2024 at 22:13):

Sam Mohr (Dec 17 2024 at 22:14):

Richard Feldman (Dec 17 2024 at 22:14):

Sam Mohr (Dec 17 2024 at 22:14):

Richard Feldman (Dec 17 2024 at 22:15):

also this discussion (and Path) are good examples of why having a pit of success is so important

Richard Feldman (Dec 17 2024 at 22:16):

how many application authors are gonna go far enough down the OS rabbit hole to know about these things?

Sam Mohr (Dec 17 2024 at 22:17):

Richard Feldman (Dec 17 2024 at 22:17):

finding an API where if they use it and naturally end up with edge cases Just Working is awesome

Brendan Hansknecht (Dec 17 2024 at 22:18):

Sam Mohr (Dec 17 2024 at 22:18):

Luke Boswell (Dec 17 2024 at 22:19):

Brendan Hansknecht (Dec 17 2024 at 22:20):

Yeah, I like where this design is reaching overall. Essentially main gets OsString args. Weaver knows how to handle them well. Otherwise, users can opt into the naive thing and the name calls out it is naive.

Sam Mohr (Dec 17 2024 at 22:20):

Sam Mohr (Dec 17 2024 at 22:21):

So I'm busy for 1.5 hours at work, I'll add Arg := [Unix (List U8), Windows (List U16)] after that to the basic-cli PR

Sam Mohr (Dec 17 2024 at 22:21):

Luke Boswell (Dec 17 2024 at 22:22):

While you're at it, feel like adding Str.from_utf16 to the builtins? :sweat_smile:

Sam Mohr (Dec 17 2024 at 22:22):

Brendan Hansknecht (Dec 17 2024 at 22:23):

Sam Mohr (Dec 17 2024 at 22:26):

Anthony Bullard (Dec 17 2024 at 22:31):

Richard Feldman (Dec 17 2024 at 22:37):

Richard Feldman (Dec 17 2024 at 22:38):

although I think Java might use one of the custom encodings that disallows zeros, I forget

Richard Feldman (Dec 17 2024 at 22:39):

Sam Mohr (Dec 17 2024 at 22:39):

jan kili (Dec 17 2024 at 22:42):

Sam Mohr (Dec 17 2024 at 22:44):

Anton (Dec 21 2024 at 14:37):

@Kilian Vounckx we're still getting in some final changes, so should be next week

Kilian Vounckx (Dec 21 2024 at 16:10):

Anton (Dec 28 2024 at 17:42):

Kilian Vounckx (Dec 28 2024 at 18:58):

jan kili (Jan 01 2025 at 03:36):

Today I successfully manually merged that pre-release into my downstream "fork" roc-on - @Luke Boswell I loooooove the glue splitting, I refactored my codegen pipeline to populate a new crate roc_midi

Stream: contributing

Topic: basic-cli purity inference

Luke Boswell (Nov 09 2024 at 06:48):

Anton (Nov 09 2024 at 11:22):

Luke Boswell (Nov 09 2024 at 11:24):

Agus Zubiaga (Nov 09 2024 at 12:10):

Agus Zubiaga (Nov 09 2024 at 23:55):

Agus Zubiaga (Nov 09 2024 at 23:56):

Agus Zubiaga (Nov 09 2024 at 23:57):

Agus Zubiaga (Nov 09 2024 at 23:58):

Agus Zubiaga (Nov 09 2024 at 23:58):

Agus Zubiaga (Nov 09 2024 at 23:58):

Agus Zubiaga (Nov 09 2024 at 23:59):

Luke Boswell (Nov 10 2024 at 00:17):

Sam Mohr (Nov 10 2024 at 01:22):

Sam Mohr (Nov 10 2024 at 01:22):

Sam Mohr (Nov 10 2024 at 01:24):

Sam Mohr (Nov 10 2024 at 01:25):

Sam Mohr (Nov 10 2024 at 01:26):

Luke Boswell (Nov 22 2024 at 03:32):

Luke Boswell (Nov 23 2024 at 02:30):

Sam Mohr (Nov 23 2024 at 02:31):

Sam Mohr (Nov 23 2024 at 02:31):

Luke Boswell (Nov 23 2024 at 02:35):

Luke Boswell (Nov 23 2024 at 02:37):

works

broken

Sam Mohr (Nov 23 2024 at 03:55):

Sam Mohr (Nov 23 2024 at 04:53):

Sam Mohr (Nov 23 2024 at 04:56):

Sam Mohr (Nov 23 2024 at 04:56):

Sam Mohr (Nov 23 2024 at 04:56):

Sam Mohr (Nov 23 2024 at 05:12):

Brendan Hansknecht (Nov 23 2024 at 06:59):

Brendan Hansknecht (Nov 23 2024 at 06:59):

Luke Boswell (Nov 23 2024 at 07:00):

Brendan Hansknecht (Nov 23 2024 at 07:10):

Luke Boswell (Nov 24 2024 at 00:02):

Sam Mohr (Nov 24 2024 at 00:13):

Luke Boswell (Nov 24 2024 at 00:32):

Luke Boswell (Nov 24 2024 at 00:33):

Luke Boswell (Nov 24 2024 at 00:35):

Luke Boswell (Nov 24 2024 at 00:35):

Luke Boswell (Nov 24 2024 at 00:56):

Brendan Hansknecht (Nov 24 2024 at 01:51):

Brendan Hansknecht (Nov 24 2024 at 01:52):

Brendan Hansknecht (Nov 24 2024 at 01:52):

Brendan Hansknecht (Nov 24 2024 at 01:52):

Brendan Hansknecht (Nov 24 2024 at 01:53):

Brendan Hansknecht (Nov 24 2024 at 01:53):

Brendan Hansknecht (Nov 24 2024 at 01:53):

Brendan Hansknecht (Nov 24 2024 at 01:53):

Brendan Hansknecht (Nov 24 2024 at 01:53):

Luke Boswell (Nov 24 2024 at 02:17):

Luke Boswell (Nov 30 2024 at 00:09):

Brendan Hansknecht (Nov 30 2024 at 03:47):

Brendan Hansknecht (Nov 30 2024 at 18:15):

Brendan Hansknecht (Nov 30 2024 at 18:16):

Brendan Hansknecht (Nov 30 2024 at 18:16):

Brendan Hansknecht (Nov 30 2024 at 18:17):

Brendan Hansknecht (Nov 30 2024 at 18:19):

Luke Boswell (Nov 30 2024 at 18:47):

Brendan Hansknecht (Nov 30 2024 at 18:48):

Luke Boswell (Nov 30 2024 at 18:50):

Brendan Hansknecht (Nov 30 2024 at 18:51):

Luke Boswell (Nov 30 2024 at 18:51):

Brendan Hansknecht (Nov 30 2024 at 19:10):

Brendan Hansknecht (Nov 30 2024 at 19:11):

Luke Boswell (Nov 30 2024 at 19:12):

Brendan Hansknecht (Nov 30 2024 at 19:48):

Brendan Hansknecht (Nov 30 2024 at 19:49):

jan kili (Nov 30 2024 at 19:56):

Brendan Hansknecht (Nov 30 2024 at 20:02):

Brendan Hansknecht (Nov 30 2024 at 20:05):

Brendan Hansknecht (Nov 30 2024 at 20:05):

Brendan Hansknecht (Nov 30 2024 at 20:24):

Brendan Hansknecht (Nov 30 2024 at 20:24):

Brendan Hansknecht (Nov 30 2024 at 20:26):

jan kili (Nov 30 2024 at 21:55):

Luke Boswell (Nov 30 2024 at 22:01):