opam-repository icon indicating copy to clipboard operation
opam-repository copied to clipboard

Only set CAML_LD_LIBRARY_PATH for system switches

Open dra27 opened this issue 5 years ago • 9 comments

Continuing a suggestion from https://github.com/ocaml/ocaml/issues/6532#issuecomment-626613570.

At present, the ocaml package sets CAML_LD_LIBRARY_PATH. As noted in https://github.com/ocaml/ocaml/issues/6532#issuecomment-626605401, the setting is redundant for the ocaml-base-compiler and ocaml-variants package, as the setting will exactly mirror what is put in ld.conf by the compiler's build system.

I propose moving the setting of CAML_LD_LIBRARY from the ocaml package to the ocaml-system package. I also propose that it should only use += to append (or possibly prepend?!) the opam stub library directory.

Notes on this:

  • CAML_LD_LIBRARY path only ever adds directories to the search path (i.e. it does not override ld.conf), so it should not create an issue if opam ends up setting CAML_LD_LIBRARY_PATH to be empty.
  • This reduces problems with CAML_LD_LIBRARY_PATH so that they can only ever affect users who use system compilers.
  • There is work in the pipeline to allow the compiler to be relocated (i.e. copied to a new location). Part of this work also mitigates ocamlrun loading incorrect stub libraries (this would improve the error message for the cases here). That same work should also result in a reduction in opam users using system compilers (their principal benefit is of course speed of switch creation), which will further mitigate this problem.

dra27 avatar May 11 '20 11:05 dra27

If I understand correctly, the bug initially opened in the compiler (ocaml/ocaml#6532) was triggered because CAML_LD_LIBRARY_PATH was only set for system switches, so when you switch back to a non-system switch it stays set to the same value and everything breaks. So setting the variable to a value that is redundant is better than not setting it. Setting it to be empty would also work, of course. Appending to CAML_LD_LIBRARY_PATH is dangerous for the same reason, as the previous path may contain shared objects with the wrong version.

lthls avatar May 11 '20 12:05 lthls

That may have been true for opam 1.2.2 (@AltGr?), but opam 2.0 should be reverting any environment changes when you go between switches - so changing to a switch which doesn't set CAML_LD_LIBRARY_PATH should still cause the system switch changes to be reverted.

dra27 avatar May 11 '20 12:05 dra27

There is another even more conservative possibility: even in system switches, CAML_LD_LIBRARY_PATH only needs to be set if a package has installed stublibs. It is also possible that we could (mostly automatically) introduce a conf-ldconf package which sets CAML_LD_LIBRARY_PATH and is only pulled in if you have dependencies which need it. That said, in practice, I imagine most user's typical switches will include something that builds stubs of its own.

dra27 avatar May 11 '20 12:05 dra27

Thanks for the clarification. The only remaining issues are when some other program already defined CAML_LD_LIBRARY_PATH, in which case the problem persists (the libraries there are likely compatible with only one switch). But this should not be very common.

lthls avatar May 11 '20 13:05 lthls

I got bitten again by this problem yesterday, running bibtex2html as installed by Ubuntu in /usr/bin and failing because it tried to dynamically load stub code from CAML_LD_LIBRARY_PATH as set by OPAM.

So, let me voice my full support for @dra27 's proposal. CAML_LD_LIBRARY_PATH does not need to be set and should not be set for non-system switches.

xavierleroy avatar Oct 28 '20 15:10 xavierleroy

I still support @dra27's proposal and still think that the way OPAM uses CAML_LD_LIBRARY_PATH currently is a mistake.

xavierleroy avatar Jan 26 '21 18:01 xavierleroy

An update, as the stale bot politely gives its 90 day reminder:

  • This issue gets mitigated by compiler changes for relocating the compiler and runtime installation. These patches do now exist, and I intend to have them finished and PRs opened at least by the 4.13 freeze, if not in time to be reviewed as part of it.
  • opam 2.1 is days away from release candidate. The priority feature for opam 2.2, given the pending closure of opam-repository-mingw, is migrating the native Windows compilers into opam-repository, and I intend to address this issue as part of the considerable changes needed to the compiler package layout for that.

So hopefully this finally gets addressed properly by late summer. What makes me nervous to do it immediately is that a lot of testing is required. In particular, I'm worried about going from an opam-system switch which does set CAML_LD_LIBRARY_PATH to an ocaml-base-compiler switch which doesn't with a stale environment (i.e. at the moment, opam's setting of CAML_LD_LIBRARY_PATH breaks system-installed packages - I'm nervous to ensure that the change doesn't start breaking some opam-installed packages too)

dra27 avatar Apr 27 '21 07:04 dra27

This issue has been open 90 days with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. If you come across this issue in the future, you may also find it helpful to visit our forum at https://discuss.ocaml.org where queries related to OCaml package management are very welcome.

github-actions[bot] avatar Oct 24 '21 09:10 github-actions[bot]

This is oozing towards being permanently resolved.

dra27 avatar Oct 24 '21 12:10 dra27