Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

get libz from BinaryBuilder system #387

Closed
wants to merge 1 commit into from
Closed

get libz from BinaryBuilder system #387

wants to merge 1 commit into from

Conversation

RalphAS
Copy link

@RalphAS RalphAS commented Sep 14, 2018

This is an attempt to make Gtk.jl a better citizen in the Julia ecosystem.

The problem addressed here is that the boat-load of DLLs pulled in by Gtk includes libz.so, which is provided in an outdated form by many version of Linux (RHEL, Centos, Ubuntu, Debian...). When some other package (e.g. ImageMagick) tries to load a DLL which needs a newer version of libz, it currently sees the old one and fails to load. Note that libz jumps through hoops to be backwards compatible, so it is entirely innocent here.

The proposed solution is to have Gtk.jl load a current version of libz.so, as provided by BinaryBuilder, at an early stage. The Gtk DLLs which need libz should simply use the loaded one.

AFAICT this works on Debian and Centos; I hope reviewers will check elsewhere.

A simple test (assuming relevant packages are in the environment) is

julia -e 'using Gtk; import ImageMagick`

Refs: JuliaImages/ImageView.jl#156 JuliaIO/ImageMagick.jl#130

@tknopp
Copy link
Collaborator

tknopp commented Sep 14, 2018

This is clearly not the fault of Gtk.jl. Gtk.jl uses the system libraries which provide a coherent state (thats the point of a package manager).

The root cause is that people are migrating to BinaryBuilder without having a look at the ecosystem as a whole. Just have a look at JuliaGraphics/Cairo.jl#229 where again it was tried to make a solution for Cairo only which would have broken Gtk.jl and all depending packages.

Note: I am not in principle against BinaryBuilder (especially on Windows and Mac) but I don't like that there is no clear agenda.

@timholy
Copy link
Member

timholy commented Sep 14, 2018

One could argue that there is an agenda---switch to BinaryBuilder---and this is the first step. I would characterize these as "the growing pains of what seems like a step in the right direction" rather than "people aren't thinking this through carefully." With ImageMagick we've suffered mightily by relying on system libraries (different bugs on different platforms), so it is a relief to have everyone on the same binary version.

@RalphAS, I tested this on Ubuntu 16.04. I indeed verify that the released version of Gtk fails your test, and consequently ImageView isn't usable if you import the packages in the "wrong" order. This version fixes the problem. 👍

@tknopp
Copy link
Collaborator

tknopp commented Sep 14, 2018

This is a pretty vague agenda and there has not been a julep like document or at least post on discourse where the agenda has been announced. The philosophy of binary builder is interesting but its not obvious to me that this will work out in the end. For instance its not possible to build HDF5 with BB.

Concerning Gtk I would favor if we

  • first migrate Cairo and Gtk to BB on a branch
  • check that everything is working on all platforms
  • release that

But I am not a fan of mixing BB with BinDeps as has been done in this PR.

Would it be possible to use BinDeps for ImageMagick on linux until all this is shaked out?

@RalphAS
Copy link
Author

RalphAS commented Sep 14, 2018

@tknopp I'm sorry I implied that Gtk.jl is "at fault". There is a weakness in the way the dynamic linker is generally used on Linux, which leads to fragile load-order-dependent behavior. I'm afraid the coherent state mentioned above is disrupted as soon as we install new things (like, say, Julia), and we are driven to find some work-around that makes as many pieces cooperate as possible. This PR tries to do that by mimicing the action of loading one of the other packages first. Since Zlib is backward compatible by design, I think this is pretty robust.

Migrating Gtk.jl to BB in toto will be difficult because of the long chain of OS-associated linkages from libgtk.

@tknopp
Copy link
Collaborator

tknopp commented Sep 14, 2018

@RalphAS: Sorry, I did not want to sound negative or harsh. The point I wanted to make is that:

  • package A and B work in coexistance
  • package A decides that in wants to use BB
  • package C uses A and B and now has a problem
  • Now its requested to "fix" B in a way that C can handle A and B

And what I propose is:

  • discussions across(!!) packages which technology to be used
  • Hard work and implement BB for all packages. This might need a long time
  • Release everything together.

Wouldn't that be the better way?

@tknopp
Copy link
Collaborator

tknopp commented Sep 14, 2018

also there was a better workaround proposed here: https://discourse.julialang.org/t/dll-problems-episode-2/12405/12

@staticfloat
Copy link
Contributor

I would say that most of the benefit from the new approach to binaries that I'm trying to push forward comes from using BinaryProvider instead of other packages that might provide binaries (e.g. Homebrew, BinDeps, etc....)

From that perspective, where a binary comes from doesn't really matter. BP doesn't need whatever you tell it to download to come from BinaryBuilder, you could, for instance, package up HDF5 by manually building it on all systems, and use that with BinaryProvider. So if you have difficulties using BB but have access to binaries that could work, I would suggest trying to juts package those binaries up into tarballs and serve them with BP as a temporary solution (temporary in that BB provides much wider arch/OS support than most other systems, including FreeBSD, Alpine Linux, ARM architectures, etc..., so as we expand the reach of Julia we plan to be providing binaries on all of those platforms as well)

@tknopp
Copy link
Collaborator

tknopp commented Sep 14, 2018

but using BP with custom binaries will not solve the issue that we are having here. Apparently, all binaries provided by BP have to be compatible with each other. And this needs some form for coordination across packages. The plan for that is not clear for me after reading through https://discourse.julialang.org/t/dll-problems-episode-2/12405.

@RalphAS
Copy link
Author

RalphAS commented Sep 16, 2018

BinaryProvider is not an essential part of the problem I am trying to address here; it happens to be a convenient tool for a solution. Other packages want to use some third-party library (today libpng16) which needs a recent libz. The former library might equally well have come from BinDeps or a Python subsystem. libz is somewhat unusual in using versioned symbols instead of changing the SO-name, so extra collision-avoidance is needed. Julia itself once linked to libz, and I advocated for something similar at that time.

FWIW, I agree with @tknopp that more public coordination is wanted for the BinaryProvider project, and maybe the isolation suggested in the proposal he mentioned above would be wise (perhaps backed by something like tailored SO-names), but that seems tangential here.

@timholy
Copy link
Member

timholy commented Sep 20, 2018

I restarted the Travis build and it passes. Perhaps the appveyor build could be restarted? I'd vote for getting this in sooner rather than later, since currently the Images ecosystem is a bit busted.

@tknopp
Copy link
Collaborator

tknopp commented Sep 20, 2018

A solution has been outlined by @stevengj in https://discourse.julialang.org/t/dll-problems-episode-2/12405/12. The present patch puts the burden of maintaining "workaround code" on Gtk.jl, while the issue is within ImageMagick. I can understand the point but that the Image ecosystem is a bit busted is because of ImageMagick.jl and not because of Gtk.jl.

I feel that this is not a decision we two should fight out but that the people behind BB/BP should comment on the best strategy here. (ping @Keno @staticfloat but I don't know who is also behind the new binary strategy)

@tknopp
Copy link
Collaborator

tknopp commented Nov 29, 2019

superseded by #447

@tknopp tknopp closed this Nov 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants