flavorjones / mini_portile

mini_portile and mini_portile2 - Simple autoconf and cmake builder for developers
MIT License
114 stars 51 forks source link

explore alternatives to setting `LIBRARY_PATH` #118

Open flavorjones opened 1 year ago

flavorjones commented 1 year ago

Description

Currently, mini_portile sets the environment variable LIBRARY_PATH in #activate. This behavior has been present since the first commit by Luis in 2011 (c06b4ab).

This is OK the vast majority of the time, however an interesting and unfortunate edge case has shown up when the target system is using Fedora's alternative pkgconf (more reading).

The problem is this: on Fedora, pkg-config --libs will not report any directories that are present in LIBRARY_PATH, which means that any extconf.rb that relies on a vendored library's .pc file to set paths correctly will fail to find the vendored library.

See https://github.com/sparklemotion/sqlite3-ruby/issues/354 for an example of where this has generated a bug report, and https://gitlab.kitware.com/cmake/cmake/-/issues/22148#note_980802 for a related discussion in a non-Ruby context.

Possible solutions

  1. mini_portile could automatically append_ldflags(lib_path)
  2. mini_portile could stop setting LIBRARY_PATH
  3. mini_portile could do both 1 and 2

I think any of these might be a reasonable solution, but only thorough testing against downstream projects is going to reveal whether we'll be introducing a subtle breaking change.

Next steps

Reproduce this issue in CI:

kwilczynski commented 1 year ago

@flavorjones, thank you for looking into this!

@eregon, does this impact Nokogiri on Red Hat too?

flavorjones commented 1 year ago

Nokogiri isn't impacted because its extconf.rb does not read the .pc file for any of the vendored libraries.

Ruby-magic likely is affected (and I would say the same for rcee/packaged_tarball) but I haven't yet attempted to reproduce it in CI to confirm.

flavorjones commented 1 year ago

(And I've updated the description to be clearer that this only affect projects that are relying on a vendored .pc file to set paths correctly.)

flavorjones commented 1 year ago

See https://github.com/flavorjones/mini_portile/pull/131 where I'm working on this.

flavorjones commented 1 year ago

Note for y'all who are following this issue: the approach I'm taking in #131 is for a new method, MiniPortile#mkmf_config which can a pkg-config file and sets $LDFLAGS and $CFLAGS.

flavorjones commented 1 year ago

Going to create a release candidate with #131 and #133 in it.

flavorjones commented 12 months ago

Reopening until I've made a final release that addresses this.

flavorjones commented 11 months ago

See

which are both green, but currently there is a high risk of pulling in system shared libraries inadvertently. I've got a few options, but need to find the time to finish this work.

flavorjones commented 10 months ago

Note: the re2 gem worked around shared linking in https://github.com/mudge/re2/pull/93, there are probably lessons in there we can upstream.

flavorjones commented 6 months ago

Picking this back up again this week after a related problem was raised in https://github.com/sparklemotion/nokogiri/issues/3133

flavorjones commented 6 months ago

Note that the mkmf_config(static:) call that's in mini_portile 2.8.5 seems to fix the issue with accidentally dynamically linking. Need to fully test it across a few gems but I think it's mostly there.