Add a .mllib -> .cmxs rule

[dbuenzli]

This rule should be put after the .mldylib -> .cmxs rule so that it doesn't override a possible (but unlikely) different explicit way of building the shared library for a library.

See also discussion there dbuenzli/uuseg#4

[gasche]

We should have a testcase to make sure that the ordering is as we expect. Armael, this should go in testsuite/internal.ml, to be run from testsuite/ as follows: OCAMLBUILD=$(pwd)/../ocamlbuild.byte ocaml internal.ml.

P.S.: Thanks a lot!

[gasche]

Closed by merging #132. Thanks @dbuenzli and @Armael

[gasche]

I'm suddenly confused: in 0.9.3 there was already a rule defined as follows

rule "ocaml: cmxa & a -> cmxs & so"
  ~prods:["%.cmxs"; x_dll]
  ~deps:["%.cmxa"; x_a]
  ~doc:"This rule allows to build a .cmxs from a .cmxa, to avoid having \
        to duplicate a .mllib file into a .mldylib."
  (Ocaml_compiler.native_shared_library_link ~tags:["linkall"] "%.cmxa" "%.cmxs");;

which is documented in the manual.

Why was a new rule necessary if this rule was present? Is there something that actually prevents it from working? Is there a conflict between this old rule and the new rule?

I am in the work of preparing a new release, and found out about this when giving finishing touches to the manual. I am considering reverting #132 from the release in order to give ourselves (that is, @Armael :-^ ) more time to think about this. Or it could still go in the release but be amended/removed if found redundant with the existing approach -- once this existing approach is fixed.

[Armael]

I don't have any particular opinion about whether the rule added in #132 is actually needed or not. I also discovered the existence of the cmxa -> cmxs rule while implementing it, but thought for some reason a mllib -> cmxs rule was also needed, because of this issue.

[gasche]

The following test case seems to work under 0.9.3:

let () = test "CmxsFromMllib"
  ~description:"Check that a .cmxs file can be built from a .mllib file"
  ~options:[`no_ocamlfind; `no_plugin]
  ~tree:[
    T.f "a.ml" ~content:"let a = 1\n";
    T.f "b.ml" ~content:"let b = true\n";
    T.f "foo.mllib" ~content:"A\nB\n";
  ]
  ~targets:("foo.cmxs", []) ();;

[gasche]

So it seems to me that what is happening is as follows: the "legacy" way to build a .cmxs file from a .mllib is registered with a weaker priority than the .cmx -> .cmxs rule (this is why the test above succeeeds in 0.9.3 while MllibOverridesCmx fails), so it only works when none of the included modules has the same name as the library that is being built.

This suggests that it should be possible to simply change the relative priority of the two rules, to make sure that when a .mllib and a .ml of the same name are present, the .mllib -> .cmxa -> .cmxs path has priority over the .ml -> .cmx -> .cmxs path.

However, this also highlights the fact that this change is in fact changing pre-existing behavior in ways that might in theory break things among our users. Indeed, someone might rely on the current ordering. I see why the new ordering is better, though: if both a .ml and a .mllib are present, getting the .cmx -> .cmxs semantics only require a one-line .mldylib file, while getting the .cmxa -> .cxms semantics requires a full copy of the .mllib file. It is better if the costly action is the default, so that users don't have to do it.

[gasche]

Also: apologies, Armael, for not noticing the issue earlier :-)

Right now I'm thinking of reverting the .mllib -> .cmxs rule, but keeping the various tests in place to have a clear picture of the next-release behavior. Before doing this, I would like to check that it is indeed possible to recover the current (master) behavior by changing the ordering of pre-existing rules. If that works, I'm not opposed to switching to the better (current master) semantics, and announcing the changes to users, but I would like to get a not-compatibility-breaking release out first -- would that mean that we will soon have to use 0.10?

[dbuenzli]

TBH I wonder who and what the use case would be for someone relying on the current behaviour. Also IIRC oasis does generate .mldylib files so while this would change the semantics it seems to be a low risk to me.

[gasche]

Yeah, maybe it is ok handle this as a low-key change -- just like we did so far.

[dbuenzli]

(I actually think it will rather fix the cmxs of packages rather than break them...)

[gasche]

I now think that I could investigate right now, try the reversed rule order, and if that works have the "simpler approach" (revert the new rule and change the order) in the release. I don't really want to play the revert+unrevert game if I can avoid it.

[gasche]

For the record: the legacy (0.9.3 and before) rule order hasn't been changed since Nicolas implemented .cmxs support in 8f2181f. It was already the case at the time that libraries with no inner module of the same name would behave fairly differently from libraries with an inner module of the same name.

[gasche]

I understand now why this order was initially chosen (sorry for using this issue as a talking pillow, but it's helpful): if you put the .cmxa -> .cmxs rule before the .cmx -> .cmxs rule, then the path .cmx -> .cmxa -> .cmxs will always be chosen, which gives the "all dependencies" semantics instead of the "only the module" semantics for the .cmxs construction.

I am tempted to just drop the .cmx -> .cmxs rule with the "only the module" semantics, and ask users to always write a .mldylib in this case.

While we are at it, it would be nice if the change also fixed bug #77 -- I'll have to test against their repro case. (Edit: no, #77 is the stuff of nightmares, it is not directly related and looks super-hard to fix.)

[gasche]

So the minimalistic route of #135 did not go as well as planned: this immediately broke the build of some packages so the release was reverted from OPAM -- see ocaml/opam-repository#8155.

I think that we need to go back to the drawing board on this .cmxs stuff, and study the breakage more in detail.

I would like to make another release with the safe stuff in any case, hopefully in the next few days.

[dbuenzli]

So just to trim this down a bit, with a pin on 0e247d4 the .cmxs stuff seems to work for pure ocaml modules (e.g. uucp).

mtime does fail but it has c stubs and I can't exclude that I'm not doing anything silly in its myocamlbuild.ml.

[dbuenzli]

So tsdl which uses @pqwy's https://github.com/pqwy/ocb-stubblr also fails:

+ ocamlfind ocamlopt -shared -linkall -thread src/tsdl.cmxa -o src/tsdl.cmxs
# ld: library not found for -ltsdl
# clang: error: linker command failed with exit code 1 (use -v to see invocation)
# File "caml_startup", line 1:
# Error: Error during linking
# Command exited with code 2.

If I go back to ocamlbuild 0.9.3 src/tsdl.cmxs is compiled as follows:

ocamlfind ocamlopt -shared -thread src/tsdl.cmx -o src/tsdl.cmxs

So it seems that -linkall flag trips things over.

[gasche]

Thanks for the reports. This also looks fairly similar to the mtime failure:

# + ocamlfind ocamlopt -shared -linkall src-os/libmtime_stubs.a src-os/mtime.cmxa -o src-os/mtime.cmxs
# /usr/bin/ld: cannot find -lmtime_stubs
# collect2: ld returned 1 exit status
# File "caml_startup", line 1:
# Error: Error during linking
# Command exited with code 2.

[dbuenzli]

Yes basically the -linkall flag puts the Extra C object files: line of the mentioned .cmxa on the cli.

[dbuenzli]

Ah no I didn't realize that in tsdl it was building from the cmx. In fact the problem persist whether -linkall is here or not:

ocamlfind ocamlopt -shared src-os/libmtime_stubs.a src-os/mtime.cmxa -o src-os/mtime.cmxs
ld: library not found for -lmtime_stubs
clang: error: linker command failed with exit code 1 (use -v to see invocation)
File "caml_startup", line 1:
Error: Error during linking

[dbuenzli]

(but I think for sure it gets that -lmtime_stub option from the mtime.cmxa).

[dbuenzli]

So passing -noautolink to ocamlopt seems to do it.

ocamlopt -verbose -shared -noautolink src-os/libmtime_stubs.a src-os/mtime.cmxa -o src-os/mtime.cmxs
+ clang -arch x86_64 -c -o 'src-os/mtime.cmxs.startup.o' '/var/folders/g4/nl6g_ytj63bbn9d45_jhggxc0000gn/T/camlstartupb81161.s'
+ gcc -bundle -flat_namespace -undefined suppress                    -Wl,-no_compact_unwind -o 'src-os/mtime.cmxs'   '-L/Users/dbuenzli/.opam/4.03.0/lib/ocaml'  'src-os/mtime.cmxs.startup.o' 'src-os/mtime.a' 'src-os/libmtime_stubs.a' 

[gasche]

Thanks a lot. It is interesting that @diml did not encounter the need for this option in #77 -- he has his own simplified .cmxa -> .cmxs rule there.

I can put a commit adding -noautolink in the .cmxa -> .cmxs rules in master, if you would be willing to test with your other packages.

[dbuenzli]

But we need to be careful here. Basically are -l flags embedded into cmxs aswell ? Otherwise we may miss some other libs e.g. say -lz. Here the problem is simply that it doesn't find the stub library since it is not installed but provided on the cli, but the other flags of the Extra C object files: of the cmxa may be important for the cmxs no ?

[gasche]

So a noautolink approach is proposed in #139. I don't have the answer to your questions for now; what we would like, ideally, is for the .cmxa -> .cmxs to just preserve whatever flags the archive had. (And if we don't find a simple way to do this, it may be a good argument for going back the .mllib -> .cmxs route instead?)

P.S.: I have to work on something else that is also urgent, so I may not be as reactive as you yourself proved to be -- thanks again.

[gasche]

(Assuming that there is no direct way to get the preserve-linker-arguments behavior from the compiler,) One thing we could do is call ocamlobjinfo to get the command-line arguments of the archive (or a small helper program that knows how to do it, or teach ocamlbuild), and then pass them again with -cclib. Another thing we could do is to change the rule such that the exact same tags are passed to the .cmxa -> .cmxs steo that were passed to the .cmxa build -- ensuring that the same command-line is computed and stored. The latter does not work if someone comes up with a .cmxa in their source file (but who would do that?), and also sounds more fragile in the face of non-rock-solid rule writing (and it may be a bit conceptually ugly).

[dbuenzli]

Just to make sure I understand. In the x.cmxs for an OCaml library x that binds to C we want:

1. All the .o files of the OCaml compilation unit of the library (those are in the x.a C archive that accompanies the library's x.cmxa)
2. All the .o files of the stubs, those are (usually) in a libx_stub.a archive.
3. The link flags needed for the dynamic C libraries used by the stub library libx_stub.a. Those are (usually) in the .cmxa but there come along with the unwanted -lx_stub flag.

[dbuenzli]

In fact it seems to me that we could eschew 2. and rely on the dynlinkable stubs that are to be installed in $(ocamlc -where)/stublibs.

[dbuenzli]

Also rather than -noautolink we could simply instruct the linker to lookup for libx_stub.a by itself using -L rather than provide it on the cli. For example this works:

> ocamlopt -verbose -shared -ccopt -Lsrc-os  src-os/mtime.cmxa -o src-os/mtime.cmxs
+ clang -arch x86_64 -c -o 'src-os/mtime.cmxs.startup.o' '/var/folders/g4/nl6g_ytj63bbn9d45_jhggxc0000gn/T/camlstartup20a640.s'
+ gcc -bundle -flat_namespace -undefined suppress                    -Wl,-no_compact_unwind -o 'src-os/mtime.cmxs'   '-L/Users/dbuenzli/.opam/4.03.0/lib/ocaml' -Lsrc-os 'src-os/mtime.cmxs.startup.o' 'src-os/mtime.a' '-lmtime_stubs' 

[ghost]

Instead of -ccopt -L<path> you can just use -I <path>. For the record, this is the command we use in js-build-tools and in jbuilder:

$ ocamlopt -shared -linkall -I <path where libblah_stubs.a lives> -o foo.cmxs foo.cmxa

All the -lXX from the .cmxa are passed to the linker, which will usually decide to link the stubs statically and the rest dynamically.

We use a variant of this in our jenga rules as we handle the linking of stubs by hand.

[ghost]

BTW, relying on the dynlinkable stubs in stublibs would be a bit tedious as you would need to properly setup LD_LIBRARY_PATH or whatever your system uses at runtime. Moreover you might need to name files libfoo.so instead of dllfoo.so so that the dynamic linker can find them, but I didn't check.

[whitequark]

@diml This was addressed in the way you suggest in #159.