Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to dune #44

Closed
wants to merge 3 commits into from
Closed

Conversation

TheLortex
Copy link
Member

In the global effort of switching mirage to dune: mirage/mirage#969
Fixes #41

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

lib/main.ml Outdated Show resolved Hide resolved
lib/time.ml Outdated Show resolved Hide resolved
lib/time.ml Outdated Show resolved Hide resolved
(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)
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I share your concerns, but IMHO such a change (removing -Werror) deserves its own PR rather than being included (hidden) in a build-system-changing PR. Out of curiousity, is it possible in dune to specify specific development vs release c_flags (so there could be -Werror in development mode)?

@mato
Copy link
Contributor

mato commented May 27, 2019 via email

@avsm
Copy link
Member

avsm commented May 27, 2019

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
Copy link

emillon commented May 28, 2019

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
Copy link
Member Author

@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 ?

@TheLortex TheLortex closed this Jul 9, 2019
@TheLortex TheLortex deleted the dune+shim-virtual branch July 9, 2019 11:49
kit-ty-kate pushed a commit to kit-ty-kate/mirage-solo5 that referenced this pull request Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port Mirage/Solo5 packages to Dune
5 participants