util-linux: fix `libtool.m4` for macOS 11+ #87103

FnControlOption · 2021-10-12T00:33:52Z

Have you followed the guidelines for contributing?
Have you ensured that your commits follow the commit style guide?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Upstream issue: util-linux/util-linux#1468

carlocab · 2021-10-12T01:03:12Z

Formula/util-linux.rb

+diff --git a/configure.orig b/configure
+index 57b830a..0d33a08 100755
+--- a/configure.orig
+++ b/configure
+@@ -12286,16 +12286,11 @@ $as_echo "$lt_cv_ld_force_load" >&6; }
+       _lt_dar_allow_undefined='$wl-undefined ${wl}suppress' ;;
+     darwin1.*)
+       _lt_dar_allow_undefined='$wl-flat_namespace $wl-undefined ${wl}suppress' ;;
+-    darwin*) # darwin 5.x on
+-      # if running on 10.5 or later, the deployment target defaults
+-      # to the OS version, if on x86, and 10.4, the deployment
+-      # target defaults to 10.4. Don't you love it?
+-      case ${MACOSX_DEPLOYMENT_TARGET-10.0},$host in
+-	10.0,*86*-darwin8*|10.0,*-darwin[91]*)
+-	  _lt_dar_allow_undefined='$wl-undefined ${wl}dynamic_lookup' ;;
+-	10.[012][,.]*)
+    darwin*)
+      case ${MACOSX_DEPLOYMENT_TARGET},$host in
+        10.[012],*|,*powerpc*)
+ 	  _lt_dar_allow_undefined='$wl-flat_namespace $wl-undefined ${wl}suppress' ;;
+-	10.*)
+	*)
+ 	  _lt_dar_allow_undefined='$wl-undefined ${wl}dynamic_lookup' ;;
+       esac
+     ;;


Is this the same patch used in libgcrypt and libgpg-error? If so, let's move this patch to formula-patches and switch libgcrypt and libgpg-error to use those too.

The patches are slightly different with util-linux having 10.[012][,.]* but libgcrypt and libgpg-error having 10.[012]*. Speaking of which, libgcrypt's patch might have a bug because 10.[[012]] probably should be 10.[012].

Yea, looks like it:

❯ brew audit --strict libgcrypt libgcrypt: * Libraries were compiled with a flat namespace. This can cause linker errors due to name collisions, and is often due to a bug in detecting the macOS version. /usr/local/Cellar/libgcrypt/1.9.4_1/lib/libgcrypt.20.dylib Error: 1 problem in 1 formula detected

Nice catch. Would you like to open a PR to fix it?

CC @aconchillo

Edit: Nope, false positive. Looks like I need to check the audit more carefully.

❯ otool -hV libgcrypt.20.dylib libgcrypt.20.dylib: Mach header magic cputype cpusubtype caps filetype ncmds sizeofcmds flags MH_MAGIC_64 X86_64 ALL 0x00 DYLIB 15 1792 NOUNDEFS DYLDLINK TWOLEVEL NO_REEXPORTED_DYLIBS

Apologies for the ping!

The difference between 10.[[012]] and 10.[012] is because m4 and configure differences. In m4 you use [[012]] which then gets converted to [012]... I didn't look into why though.

Bo98 · 2021-10-12T01:06:32Z

Formula/util-linux.rb

+  # util-linux's libtool.m4 doesn't properly support macOS >= 11.x (see
+  # libtool.rb formula). This causes the library to be linked with a flat
+  # namespace which might cause issues when dynamically loading the library.
+  # Issue ref: https://github.com/karelzak/util-linux/issues/1468


Is there any way we can test for this?

Mock a library with a symbol that collides with something a util-linux library exports and then try to link with both, I guess. Might be fragile though. And terrible to automate.

Can we just filter out [-Wl,]-flat_namespace in superenv? I don't think we ever want a flat namespace anyway.

Potentially. Ideally we'd have much more information about whether any current formulae use it or not. If anything requires it then it probably doesn't belong in the filter list.

Not sure of how to best approach that. Sounds like some sort of CI-only analytics gathering.

The output of otool -hV includes TWOLEVEL if the library was linked normally (without -flat_namespace). Maybe that could be useful. https://stackoverflow.com/q/2553530

If filtering out -flat_namespace, maybe also replace -undefined suppress with -undefined dynamic_lookup. Is superenv able to handle environment variables like LDFLAGS?

Here are libraries I have installed that don't have TWOLEVEL from otool -hV:

/usr/local/Cellar/berkeley-db/18.1.40/lib/libdb-18.1.dylib /usr/local/Cellar/berkeley-db/18.1.40/lib/libdb_cxx-18.1.dylib /usr/local/Cellar/berkeley-db/18.1.40/lib/libdb_stl-18.1.dylib /usr/local/Cellar/gdbm/1.21/lib/libgdbm.6.dylib /usr/local/Cellar/gdbm/1.21/lib/libgdbm_compat.4.dylib /usr/local/Cellar/gettext/0.21/lib/libasprintf.0.dylib /usr/local/Cellar/gmp/6.2.1_1/lib/libgmp.10.dylib /usr/local/Cellar/gmp/6.2.1_1/lib/libgmpxx.4.dylib /usr/local/Cellar/gpgme/1.16.0/lib/libgpgme.11.dylib /usr/local/Cellar/gsl/2.7/lib/libgsl.25.dylib /usr/local/Cellar/gsl/2.7/lib/libgslcblas.0.dylib /usr/local/Cellar/gts/0.7.6_2/lib/libgts-0.7.5.0.1.dylib /usr/local/Cellar/isl/0.24/lib/libisl.23.dylib /usr/local/Cellar/jq/1.6/lib/libjq.1.dylib /usr/local/Cellar/libassuan/2.5.5/lib/libassuan.0.dylib /usr/local/Cellar/libb2/0.98.1/lib/libb2.1.dylib /usr/local/Cellar/libev/4.33/lib/libev.4.dylib /usr/local/Cellar/libksba/1.6.0/lib/libksba.8.dylib /usr/local/Cellar/libmpc/1.2.1/lib/libmpc.3.dylib /usr/local/Cellar/mpfr/4.1.0/lib/libmpfr.6.dylib /usr/local/Cellar/npth/1.6/lib/libnpth.0.dylib /usr/local/Cellar/pcre/8.45/lib/libpcre.1.dylib /usr/local/Cellar/pcre/8.45/lib/libpcre16.0.dylib /usr/local/Cellar/pcre/8.45/lib/libpcre32.0.dylib /usr/local/Cellar/pcre/8.45/lib/libpcrecpp.0.dylib /usr/local/Cellar/pcre/8.45/lib/libpcreposix.0.dylib /usr/local/Cellar/pcre2/10.38/lib/libpcre2-16.0.dylib /usr/local/Cellar/pcre2/10.38/lib/libpcre2-32.0.dylib /usr/local/Cellar/pcre2/10.38/lib/libpcre2-8.0.dylib /usr/local/Cellar/pcre2/10.38/lib/libpcre2-posix.3.dylib

The output of otool -hV includes TWOLEVEL if the library was linked normally (without -flat_namespace). Maybe that could be useful. https://stackoverflow.com/q/2553530

This is really useful, and makes this easy to check with an audit. ~~Not sure checking for a two-level namespace is implemented in ruby-macho (maybe I just don't understand the API), but doing so seems to just be a matter of implementing some getters.~~ (This is false; I found it.)

In the Catalina bottle:

❯ otool -hV gmp/6.2.1_1/lib/libgmp.10.dylib gmp/6.2.1_1/lib/libgmp.10.dylib: Mach header magic cputype cpusubtype caps filetype ncmds sizeofcmds flags MH_MAGIC_64 X86_64 ALL 0x00 DYLIB 14 1632 NOUNDEFS DYLDLINK TWOLEVEL NO_REEXPORTED_DYLIBS

In the Big Sur bottle:

❯ otool -hV gmp/6.2.1_1/lib/libgmp.10.dylib gmp/6.2.1_1/lib/libgmp.10.dylib: Mach header magic cputype cpusubtype caps filetype ncmds sizeofcmds flags MH_MAGIC_64 X86_64 ALL 0x00 DYLIB 14 1624 DYLDLINK NO_REEXPORTED_DYLIBS

Looks like this bug is pretty common.

Implementing an audit. Will open a PR after lunch.

Homebrew/brew#12225

There are at least five instances where a formula has libraries compiled with `-flat_namespace` due to a bug in detecting the macOS version (cf. Homebrew/homebrew-core#87103, Homebrew/homebrew-core#85974, Homebrew/homebrew-core#85973). I think it makes sense to check for this more generally. It is sometimes intentional, so I've added a check for an allowlist for those instances. Running this on the current `util-linux` bottle produces ❯ brew audit --strict util-linux util-linux: * Libraries were compiled with a flat namespace. This can cause linker errors due to name collisions, and is often due to a bug in detecting the macOS version. /usr/local/Cellar/util-linux/2.37.2/lib/libblkid.1.dylib /usr/local/Cellar/util-linux/2.37.2/lib/libfdisk.1.dylib /usr/local/Cellar/util-linux/2.37.2/lib/libsmartcols.1.dylib /usr/local/Cellar/util-linux/2.37.2/lib/libuuid.1.dylib Error: 1 problem in 1 formula detected Some things that still need to be done here: - fix this check for universal binaries - check if we want to restrict this audit check to newer versions of macOS - fix false positives (try `brew audit --strict llvm` and compare the output of `otool -hV` on the identified files) While we're here, let's fix the formatting of the output of these other audits (cf. Homebrew#12217).

SMillerDev · 2021-10-12T07:17:08Z

Has this patch been upstreamed yet?

aconchillo · 2021-10-12T16:19:45Z

Has this patch been upstreamed yet?

If you refer to libgcrypt and libgpg-error, then yes. However, there hasn't been any release yet.

https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=commit;h=c9cebf3d1824d6ec90fd864a744bb81c97ac7d31
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgpg-error.git;a=commit;h=a3987e44970505a5540f9702c1e41292c22b69cf

The libgcrypt maintainers are waiting to see if there's any incompatibility. If no issues occur they are planning to port these changes to other libraries (e.g. libassuan).

FnControlOption · 2021-10-12T17:56:01Z

For util-linux, I opened util-linux/util-linux#1468 instead of a pull request because m4/libtool.m4 is not included in the Git repository. Presumably, they run ./autogen.sh as part of the release process, so they would need a version of libtool that includes this patch. Although, it's anyone's best guess when that will get merged and subsequently included in a new libtool release.

SMillerDev · 2021-10-12T18:53:23Z

Can't we just run autogen ourselves then? That seems to be easier than maintaining a bunch of patches.

carlocab · 2021-10-13T01:33:07Z

Can't we just run autogen ourselves then? That seems to be easier than maintaining a bunch of patches.

It also means carrying around a dependency that's likely to stick around long after they're needed. A patch is less likely to stick around when unnecessary (usually because it fails to apply).

If we do this we should add lots of comments saying why we need the extra deps and when they can be removed.

Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>

carlocab

Switched this to use a generic patch. Thanks, @FnControlOption!

carlocab · 2021-10-26T14:48:03Z

Linux errors are unrelated. Merging.

BrewTestBot · 2021-10-26T14:49:10Z

@carlocab has triggered a merge.

carlocab reviewed Oct 12, 2021

View reviewed changes

Bo98 reviewed Oct 12, 2021

View reviewed changes

carlocab mentioned this pull request Oct 12, 2021

mac/formula_cellar_checks: check for flat namespace libraries Homebrew/brew#12225

Merged

7 tasks

carlocab added the CI-long-timeout Use longer GitHub Actions CI timeout. label Oct 12, 2021

This was referenced Oct 19, 2021

strongswan 5.9.4 #87564

Closed

pypy & pypy3 7.3.6 #87517

Closed

jonchang mentioned this pull request Oct 21, 2021

jellyfish 2.3.0 (new formula) #87662

Closed

6 tasks

This was referenced Oct 23, 2021

configure script misdetects new versions of macOS strongswan/strongswan#683

Open

Monterey bottling #87708

Closed

util-linux: fix libtool.m4 for macOS 11+

56f5358

Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>

carlocab added CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Oct 25, 2021

carlocab force-pushed the util-linux branch from 4f522fe to 56f5358 Compare October 25, 2021 17:56

carlocab approved these changes Oct 25, 2021

View reviewed changes

BrewTestBot closed this in 69fe504 Oct 26, 2021

github-actions bot added the outdated PR was locked due to age label Nov 26, 2021

github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

util-linux: fix `libtool.m4` for macOS 11+ #87103

util-linux: fix `libtool.m4` for macOS 11+ #87103

FnControlOption commented Oct 12, 2021

carlocab Oct 12, 2021

FnControlOption Oct 12, 2021

carlocab Oct 12, 2021 •

edited

aconchillo Oct 12, 2021

Bo98 Oct 12, 2021

carlocab Oct 12, 2021 •

edited

carlocab Oct 12, 2021

Bo98 Oct 12, 2021

FnControlOption Oct 12, 2021 •

edited

carlocab Oct 12, 2021

carlocab Oct 12, 2021 •

edited

carlocab Oct 12, 2021

carlocab Oct 12, 2021

carlocab Oct 12, 2021

SMillerDev commented Oct 12, 2021

aconchillo commented Oct 12, 2021

FnControlOption commented Oct 12, 2021

SMillerDev commented Oct 12, 2021

carlocab commented Oct 13, 2021

carlocab left a comment

carlocab commented Oct 26, 2021

BrewTestBot commented Oct 26, 2021

util-linux: fix libtool.m4 for macOS 11+ #87103

util-linux: fix libtool.m4 for macOS 11+ #87103

Conversation

FnControlOption commented Oct 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlocab Oct 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlocab Oct 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FnControlOption Oct 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlocab Oct 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SMillerDev commented Oct 12, 2021

aconchillo commented Oct 12, 2021

FnControlOption commented Oct 12, 2021

SMillerDev commented Oct 12, 2021

carlocab commented Oct 13, 2021

carlocab left a comment

Choose a reason for hiding this comment

carlocab commented Oct 26, 2021

BrewTestBot commented Oct 26, 2021

util-linux: fix `libtool.m4` for macOS 11+ #87103

util-linux: fix `libtool.m4` for macOS 11+ #87103

carlocab Oct 12, 2021 •

edited

carlocab Oct 12, 2021 •

edited

FnControlOption Oct 12, 2021 •

edited

carlocab Oct 12, 2021 •

edited