functoria: Constraint the start function to 'unit Lwt.t'

[Julow]

This allows to catch subtle bugs caused by partial applications or by returning 'unit Lwt.t Lwt.t' using the type-system.

[hannesm]

the CI reports:

File "applications/dhcp/mirage/main.ml", line 190, characters 8-10:
Error: This variant pattern is expected to have type
         (unit, Netif.error) result
       There is no constructor () within type result

And indeed, the toplevel "Mirage_net.listen" returns a "(unit, error) result Lwt.t", while now "unit Lwt.t" is demanded. The error message is unfortunately slightly confusing. Any chance we come up with a better one?

[hannesm]

Let me rephrase my comment: I'm in favour of merging this as is (and fix the mirage-skeleton dhcp example). It improves the current situation (and the struggle you had for nearly a day).

[Julow]

I've found a way to have a better error message. Let's see if that also works in the CI.

[hannesm]

The error message is now:

File "applications/dhcp/mirage/main.ml", line 190, characters 8-18:
Error: This pattern matches values of type unit
       but a pattern was expected which matches values of type
         (unit, Netif.error) result

[Julow]

Right! I had other type constraint in my local test that interfered with my testing. I'm out of ideas of how to locate the error into user code.
With the last commit, the error is no longer about the pattern, which feels clearer, but that requires a new type definition in the prelude. Happy to revert that if the previous message was good enough and changing the prelude is problematic.

[hannesm]

the latest error is:

File "applications/dhcp/mirage/main.ml", line 190, characters 3-67:
Error: This expression has type (unit, Netif.error) result start
       but an expression was expected of type unit start
       Type (unit, Netif.error) result is not compatible with type unit 

could you run a dune build @fmt --auto?

The start in type unit start looks slightly strange, maybe it should be type unit io for better readability?

[Julow]

I amended the last commit to use io instead. I propose to keep the two commits in case the io type have to be reverted later.

[hannesm]

I opened mirage/mirage-skeleton#393 to fix the mirage-skeleton issue

[hannesm]

I'm happy with this change, the mirage-skeleton has been fixed. I don't understand why the CI doesn't catch up (I clicked on rebuild, but it reused the old mirage-skeleton). Any help is appreciated, maybe a force-push will solve something?

[Julow]

The CI is still failing after a rebase and a force push. I've run all the steps locally in the latest mirage-skeleton and it's building fine.

[hannesm]

I think I remember, the CI system uses the dev branch of mirage-skeleton for the main branch of mirage. I pushed the change to dhcp onto the dev branch as well, so if you could trigger the CI another time here, we may see green checkmarks.

Thanks so much.

EDIT: no need to do anything, CI kicked in automatically :)

[Julow]

The checks are passing!