dbuenzli / topkg

The transitory OCaml software packager
http://erratique.ch/software/topkg
ISC License
69 stars 25 forks source link

Topkg.clib tries to install `.so` even if the compiler has no shared library support #136

Closed anmonteiro closed 4 years ago

anmonteiro commented 4 years ago

I stumbled upon this when trying to build mtime and ptime in a static musl-libc environment without shared library support. Error attached below.

It seems like topkg is trying to generate a rule for building + installing the shared objects unconditionally (relevant snippet). It sounds like OCamlbuild might support checking this via supports_shared_libraries (based on this).

The error is as follows:

Solver failed:
  Ocamlbuild knows of no rules that apply to a target named src-os/dllmtime_clock_stubs.so. This can happen if you ask Ocamlbuild to build a target with the wrong extension (e.g. .opt instead of .native) or if the source files live in directories that have not been specified as include directories.
Backtrace:
  - Failed to build the target src-os/dllmtime_clock_stubs.so
      - Building src-os/dllmtime_clock_stubs.so
pkg.ml: [ERROR] cmd ['ocamlbuild' '-use-ocamlfind' '-classic-display' '-tag' 'debug'
     '-build-dir' '_build' 'opam' 'pkg/META' 'CHANGES.md' 'LICENSE.md'
     'README.md' 'src/mtime.a' 'src/mtime.cmxa' 'src/mtime.cma'
     'src/mtime.cmx' 'src/mtime.cmi' 'src/mtime.mli' 'src/mtime_top.a'
     'src/mtime_top.cmxa' 'src/mtime_top.cma' 'src/mtime_top.cmx'
     'src/mtime_top_init.ml' 'doc/index.mld' 'src-os/mtime_clock.a'
     'src-os/mtime_clock.cmxa' 'src-os/mtime_clock.cma'
     'src-os/mtime_clock.cmx' 'src-os/mtime_clock.cmi'
     'src-os/mtime_clock.mli' 'src-os/dllmtime_clock_stubs.so'
     'src-os/libmtime_clock_stubs.a' 'test-os/min_os.ml']: exited with 6
dbuenzli commented 4 years ago

There's already logic to drop the cmxs, see here so we could likely use it for .so aswell. However detection relies on whether the native dynlink library is available. So using that will be wrong on a configure with --disable-native-compiler on a platform that does support dynlinking. Isn't configurability great ?

Better would be to use the newer supports_shared_libraries of ocamlc -config but that only went into 4.08.0 (topkg still supports 4.03.0).

dbuenzli commented 4 years ago

I think I'll just fallback to test for the existence of either lib{asm,caml}run_shared).so .

dbuenzli commented 4 years ago

I added something in dfdd31983faceef7bc642d30. Could you please do an opam pin topkg --dev to confirm it works and indicate me on which OCaml version you are actually testing this.

anmonteiro commented 4 years ago

Thanks for jumping on this so quickly.

indicate me on which OCaml version you are actually testing this.

I'm on OCaml 4.10.

I tested the change you made and I still got the same error. I dug into the code and found that it works with the patch in #137.

dbuenzli commented 4 years ago

Excellent thanks for digging into the stuff. I merged a slightly amended version of the patch. Once you confirm everything is right on your side we can proceed to release.

anmonteiro commented 4 years ago

Thanks for your help with this! I can confirm mtime and ptime now build in this environment.

Releasing is up to you, don't bother if it's time consuming. I can depend on the development version for now if you want to batch other fixes eventually before getting it out.

dbuenzli commented 4 years ago

The software is rather stable and in maintenance mode so that could be a long wait. Here you go https://github.com/ocaml/opam-repository/pull/16915 . Thanks again for pointing at the right places, it helped in doing that swiftly.