mirage / functoria

A DSL to invoke otherworldly functors
ISC License
63 stars 21 forks source link

fix regression in `mirage configure` #176

Closed samoht closed 4 years ago

samoht commented 5 years ago

Simplify the code and structure a bit.

Now functoria configure will generate the following files:

So compared to master:

samoht commented 5 years ago

Ok I've pushed a fix where I run the tests manually instead of running them inside dune.

Using dune runtest for functoria-runtime will fail as dune runtest is generating dune files inside the _build tree which depends on local libraries defined in the source tree. This doesn't seem to be really well supported by dune, so instead use it directly like the mirage tool will do anyway.

samoht commented 5 years ago

(I've also updated the description of the issue to be more accurate)

samoht commented 5 years ago

All green!

I now need to make a PR on mirage/mirage to overwrite dune.build with the correct build runes...

hannesm commented 5 years ago

sounds and looks good to me (tests pass! :)); does this now need changes to mirage?

dinosaure commented 4 years ago

What is the status of this PR?

samoht commented 4 years ago

Ready to be merged once the counterpart in mirage (generating dune.build instead of dune) is done :-)

hannesm commented 4 years ago

@samoht ok, is this PR (+ #177 likely) compatible with the released mirage 3.x versions?

samoht commented 4 years ago

@hannesm hum, that needs to be tested (I'll try to do this later this week) but yes that should probably work.

What this PR is doing is generating a dune, dune.config and (an empty) dune.build to build config.ml. So if the mirage tool still generates a Makefile and myocamlbuild I guess it should "just work". .. Interesting side-effect :-)

samoht commented 4 years ago

that needs to be tested

I've just done this and it's slightly broken. Working on a fix now.

samoht commented 4 years ago

So with my last commit functoria.dev + mirage 3.6.0 seems to work on mirage-skeleton 🎉

I'll clean up the patches next week and I'll try to document the updated process.

samoht commented 4 years ago

Some weird errors:

Fatal error: Invalid character in package name "\027[01;04mfunctoria-runtime\027[0m"

run ['opam' 'config' 'subst' 'info_gen.ml']: exited with 99run ['dune' 'exec' '--root'
     '/Users/thomas/git/mirage-skeleton/tutorial/app_info' '--'
     './config.exe' 'build']: exited with 1
hannesm commented 4 years ago

@samoht you'll need to adapt #177 to avoid ansi escape sequences in the output of opam list used by app_info

samoht commented 4 years ago

With #177 functoria.dev + mirage.3.6.0 works great (I've tested, make, make clean). And the description of that PR accurately describes the new build process for config.ml.

hannesm commented 4 years ago

that's great @samoht -- just for understanding: did you test any non-unix target? a combined changes between 2.2.x to 3.y/master would be welcome in CHANGES (it is mainly: use dune and two stages instead of dynlink()ing config.ml?) -- I suspect there are various dune files (generated / for configuration) and .some directories being generated (custom_build_)!?

samoht commented 4 years ago

CHANGELOG updated

samoht commented 4 years ago

I suspect there are various dune files (generated / for configuration) and .some directories being generated (custombuild)!?

It's just generating dune, dune.config and dune.build. custom_build_ is just used in the tests to to the --build-dir <dir> option.

samoht commented 4 years ago

Version number fixed and I confirm that I can build an spt and hvt hello world with pinning functoria to that branch.