mirage / ocaml-solo5

Freestanding OCaml runtime
Other
101 stars 30 forks source link

Clean a bit the `Makefile` and delete some deprecated usage of the `./configure` #123

Open dinosaure opened 1 year ago

dinosaure commented 1 year ago

From a great discussion with @shindere, we invoke the ./configure in the wrong way: 1) we should not define our own OC_CFLAGS 2) we should call ./configure with envrionment variables like: ./configure LD=... instead of LD=... ./configure

From a certain perspective, this work is not really needed (because ocaml-solo5 already work for 4.14) but it can helps us to move forward about the OCaml 5 support.

shindere commented 1 year ago

Calascibetta Romain (2022/11/02 14:27 -0700):

From a great discussion with @shindere, we invoke the ./configure in the wrong way: 1) we should not define our own OC_CFLAGS

Indeed. Sorry for the lack of documentaiton about the conventions used in the compiler's build system but, roughly speaking, the build variable prefixed with OC_ are reserved for the build system itself. Overriding them may break the build by removingflags which are mendatory. Normally, it should anway not be necessary to override these variables because every OC_FOO* variable is supposed to have a FOO counterpart which is precisely here for the user to rpovide additional flags that will take precedence over the private ones. If you guys happen to encounter a situation where you are not able to override something you need to, please let me know so that I can see how to best fix the compiler's build system. This should lead to easyer-to-maintain code on your side and a more flexible build system on the compiler's side.

2) we should call ./configure with envrionment variables like: ./configure LD=... instead of LD=... ./configure

Yes, indeed. To phrase this in autoconf's terminology, in ./configure LD=... LD is a configuration variable and that's what should be used. In the LD=... ./configure example, LD is an environment variable and that is to be avoided.

See for instance https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Setting-Output-Variables.html

From a certain perspective, this work is not really needed (because ocaml-solo5 already work for 4.14) but it can helps us to move forward about the OCaml 5 support.

It does indeed not seem important to backport such changes, especially given that the modifications of the build system you would rely on have been done incrementally so by backporting too far you would fall in places where the interface I suggested to use was simply not there or was there but not yet working properly, which is exactly why you guys had to do all the stuff you are currently doing. For the future, though, one can hope that, by using the compiler's build system as suggested you will be able to reduce the complexity of the set of patches you have to maintain. Also, forom the compiler's perspective, if you guys manage to do what you need for your use-case just by invoking the build system and withouot having to do weird things, it will demonstrate that the adjustments done to make the build system more flexible work as expected and do correspond to some real needs.