compiler-libs.native-toplevel fix

[nojb]

Fixes #4248

Since Native-toplevel is not a valid module name, I had to change Module.of_string to Module.of_string_allow_invalid, but perhaps there is a better way?

[ghost]

This value seems to be generated in meta.ml (in the main_modules function). Maybe we could generate a different value there?

BTW, I'm confused as to what this code in meta.ml does. For the builtin meta, we are supposed to generate the same thing as the META files distributed by findlib. @rgrinberg do you know what's going on there?

[rgrinberg]

BTW, I'm confused as to what this code in meta.ml does. For the builtin meta, we are supposed to generate the same thing as the META files distributed by findlib. @rgrinberg do you know what's going on there?

It's to support root_module for libraries that aren't define with dune. We "guess" the list of toplevel modules from the list of cmi's for such libraries. This is useful for libraries such as Unix for example.

Since Native-toplevel is not a valid module name, I had to change Module.of_string to Module.of_string_allow_invalid, but perhaps there is a better way?

I suppose we could just hard code it to the empty list for some libs.

[ghost]

What I'm thinking is the following: when findlib is installed, we don't use our builtin META files. Instead we use the ones installed by findlib and these don't have a main_modules field. What happens in this case?

[ghost]

We hit this issue while testing at Jane Street as well. I added a String.map ~f:(function '-' -> '_' | c -> c_) where we generated the offending main_modules field instead of allowing invalid module names.

I'm still curious as to why our builtin meta files are not the same as the one embed in findlib. This feels like a recipe for troubles.

[nojb]

We hit this issue while testing at Jane Street as well. I added a String.map ~f:(function '-' -> '_' | c -> c_) where we generated the offending main_modules field instead of allowing invalid module names.

Thanks!