mirage / mirage-solo5

Solo5 core platform libraries for MirageOS
ISC License
20 stars 21 forks source link

Port to dune #44

Closed TheLortex closed 5 years ago

TheLortex commented 5 years ago

In the global effort of switching mirage to dune: mirage/mirage#969 Fixes https://github.com/mirage/mirage-solo5/issues/41

This is a work in progress, I'm trying to keep track of every change that need to be made!

mato commented 5 years ago

On Saturday, 25.05.2019 at 16:48, Anil Madhavapeddy wrote:

avsm commented on this pull request.

@@ -0,0 +1,18 @@ +(library

  • (name oS)
  • (public_name mirage-solo5.internals)
  • (modules lifecycle main mM oS solo5 time)
  • (c_names alloc_pages_stubs atomic_stubs barrier_stubs checksum_stubs clock_stubs cstruct_stubs main mm_stubs solo5_block_stubs solo5_console_stubs solo5_net_stubs)
  • (c_flags (:include cflags) -O2 -std=c99 -Wall -Werror)

I realise that the -Werror is existing behaviour, but it makes it really painful when doing production builds with a newer compiler that introduces some random warning.

Isn't that consistent with Dune behaviour for OCaml code, i.e. treating all warnings as errors by default? Why deal with C code differently?

avsm commented 5 years ago

Almost is, but not quite. Dune has profiles, and in the default dev mode it treats warnings as errors. But if dune build --profile=release or dune build -p <package is specified its in release mode, and they are just warnings. We could do something similar by creating a (profile) in the dune-project that adds -Werror to CFLAGS only in dev builds

emillon commented 5 years ago

if configurator is used, it's possible to pass it the selected profile as argument, and the configure binary can output -Werror or not based on this piece of info.

TheLortex commented 5 years ago

@hannesm We can make a first PR that put solo5-specific modules in an Os_solo5 module and update mirage-(block|net|*)-solo5 accordingly. And then we can make Os an implementation of mirage-os-shim. What do you think ?