Undeclared, non-recursive dependencies are not recorded in INSTALL_RECEIPT.json

[ilovezfs]

• [x] Ran brew update and retried your prior step?
• [x] Ran brew doctor, fixed as many issues as possible and retried your prior step?
• [x] Confirmed this is problem with Homebrew/brew and not specific formulae? If it's a formulae-specific problem please file this issue at https://github.com/Homebrew/homebrew-core/issues/new

Bug reports:

iMac-TMP:~ joe$ brew doctor
Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry and just ignore them. Thanks!

Warning: The filesystem on / appears to be case-sensitive.
The default macOS filesystem is case-insensitive. Please report any apparent problems.

iMac-TMP:~ joe$ brew config
HOMEBREW_VERSION: 1.1.10-295-g93e2cb3
ORIGIN: https://github.com/Homebrew/brew
HEAD: 93e2cb31afa1671c36023d119d9a539373b4be43
Last commit: 61 minutes ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: d88f2d6d27258b31c8f61d91df9aa6b3cb175c2c
Core tap last commit: 65 minutes ago
HOMEBREW_PREFIX: /usr/local
HOMEBREW_REPOSITORY: /usr/local/Homebrew
HOMEBREW_CELLAR: /usr/local/Cellar
HOMEBREW_BOTTLE_DOMAIN: https://homebrew.bintray.com
CPU: octa-core 64-bit skylake
Homebrew Ruby: 2.0.0-p648 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.0.0-p648/bin/ruby
Clang: 8.0 build 800
Git: 2.10.1 => /Applications/Xcode.app/Contents/Developer/usr/bin/git
Perl: /usr/bin/perl
Python: /usr/local/bin/python => /usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/bin/python2.7
Ruby: /usr/local/bin/ruby => /usr/local/Cellar/ruby/2.4.0/bin/ruby
Java: 1.8.0_92, 1.8.0_77, 1.7.0_80, 1.6.0_65-b14-468
macOS: 10.11.6-x86_64
Xcode: 8.2.1
CLT: 8.2.0.0.1.1480973914
X11: 2.7.11 => /opt/X11

Unless an undeclared dependency is declared indirectly (i.e., recursively by one of the declared dependencies), it is not recorded in the tab. This means that at uninstall-time, the dependency will not be protected from removal when HOMEBREW_DEVELOPER is unset, or will not trigger a warning if HOMEBREW_DEVELOPER is set.

Example:

iMac-TMP:~ joe$ brew deps --include-build plplot
cairo
cmake
fontconfig
freetype
gettext
git
glib
gobject-introspection
harfbuzz
icu4c
libffi
libpng
libtool
pango
pcre
pixman
pkg-config
sphinx-doc
iMac-TMP:~ joe$ brew install homebrew/science/qhull
==> Installing qhull from homebrew/science
==> Downloading https://homebrew.bintray.com/bottles-science/qhull-2015.2.el_capitan.bottle.tar.gz
Already downloaded: /Users/joe/Library/Caches/Homebrew/qhull-2015.2.el_capitan.bottle.tar.gz
==> Pouring qhull-2015.2.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/qhull/2015.2: 131 files, 5.1M
iMac-TMP:~ joe$ brew install -s plplot
==> Using the sandbox
==> Downloading https://downloads.sourceforge.net/project/plplot/plplot/5.12.0%20Source/plplot-5.12.0
Already downloaded: /Users/joe/Library/Caches/Homebrew/plplot-5.12.0.tar.gz
==> cmake .. -DCMAKE_C_FLAGS_RELEASE=-DNDEBUG -DCMAKE_CXX_FLAGS_RELEASE=-DNDEBUG -DCMAKE_INSTALL_PREF
==> make
==> make install
🍺  /usr/local/Cellar/plplot/5.12.0: 186 files, 2.7M, built in 15 seconds
iMac-TMP:~ joe$ cat /usr/local/Cellar/plplot/5.12.0/INSTALL_RECEIPT.json 
{"homebrew_version":"1.1.10-295-g93e2cb3","used_options":[],"unused_options":["--with-x11","--with-fortran","--with-java"],"built_as_bottle":false,"poured_from_bottle":false,"installed_as_dependency":false,"installed_on_request":true,"changed_files":null,"time":1488008662,"source_modified_time":1485654706,"HEAD":"93e2cb31afa1671c36023d119d9a539373b4be43","stdlib":"libcxx","compiler":"clang","runtime_dependencies":[{"full_name":"libpng","version":"1.6.28"},{"full_name":"freetype","version":"2.7.1"},{"full_name":"fontconfig","version":"2.12.1"},{"full_name":"pixman","version":"0.34.0"},{"full_name":"gettext","version":"0.19.8.1"},{"full_name":"libffi","version":"3.0.13"},{"full_name":"pcre","version":"8.39"},{"full_name":"glib","version":"2.50.3"},{"full_name":"cairo","version":"1.14.8"},{"full_name":"harfbuzz","version":"1.4.2"},{"full_name":"pkg-config","version":"0.29.1"},{"full_name":"gobject-introspection","version":"1.50.0"},{"full_name":"pango","version":"1.40.3"},{"full_name":"libtool","version":"2.4.6"}],"source":{"path":"/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/plplot.rb","tap":"homebrew/core","spec":"stable","versions":{"stable":"5.12.0","devel":null,"head":null,"version_scheme":0}}}iMac-TMP:~ joe$ brew linkage plplot
System libraries:
  /usr/lib/libSystem.B.dylib
  /usr/lib/libc++.1.dylib
Homebrew libraries:
  /usr/local/opt/cairo/lib/libcairo.2.dylib (cairo)
  /usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
  /usr/local/opt/glib/lib/libglib-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgobject-2.0.0.dylib (glib)
  /usr/local/opt/qhull/lib/libqhull.7.dylib (homebrew/science/qhull)
  /usr/local/opt/libtool/lib/libltdl.7.dylib (libtool)
  /usr/local/opt/pango/lib/libpango-1.0.0.dylib (pango)
  /usr/local/opt/pango/lib/libpangocairo-1.0.0.dylib (pango)
  /usr/local/Cellar/plplot/5.12.0/lib/libcsirocsa.0.dylib (plplot)
  /usr/local/Cellar/plplot/5.12.0/lib/libcsironn.0.dylib (plplot)
  /usr/local/Cellar/plplot/5.12.0/lib/libqsastime.0.dylib (plplot)
Variable-referenced libraries:
  @rpath/libplplot.14.dylib
Possible undeclared dependencies:
  gettext
  glib
  homebrew/science/qhull
iMac-TMP:~ joe$ unset HOMEBREW_DEVELOPER
iMac-TMP:~ joe$ brew uninstall qhull
Uninstalling /usr/local/Cellar/qhull/2015.2... (131 files, 5.1M)
iMac-TMP:~ joe$

CC @alyssais @MikeMcQuaid

[ilovezfs]

Note, of course, that the consequence of the above is that the plplotinstallation is hosed:

iMac-TMP:~ joe$ brew test plplot
Testing plplot
==> Using the sandbox
==> /usr/bin/clang test.c -o test -I/usr/local/Cellar/plplot/5.12.0/include/plplot -L/usr/local/Cella
==> ./test
Last 15 lines from /Users/joe/Library/Logs/Homebrew/plplot/test.02.test:
2017-02-24 23:47:18 -0800

./test

dyld: Library not loaded: /usr/local/opt/qhull/lib/libqhull.7.dylib
  Referenced from: /usr/local/Cellar/plplot/5.12.0/lib/libcsironn.0.dylib
  Reason: image not found
Error: plplot: failed
Failed executing: ./test 
/usr/local/Homebrew/Library/Homebrew/formula.rb:1834:in `block in system'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1772:in `open'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1772:in `system'
/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/plplot.rb:69:in `block in <class:Plplot>'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1671:in `block (2 levels) in run_test'
/usr/local/Homebrew/Library/Homebrew/formula.rb:884:in `with_logging'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1670:in `block in run_test'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:14:in `block in mktemp'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:74:in `block in run'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:74:in `chdir'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:74:in `run'
/usr/local/Homebrew/Library/Homebrew/extend/fileutils.rb:13:in `mktemp'
/usr/local/Homebrew/Library/Homebrew/formula.rb:1664:in `run_test'
/usr/local/Homebrew/Library/Homebrew/test.rb:28:in `block in <main>'
/usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.0.0-p648/lib/ruby/2.0.0/timeout.rb:66:in `timeout'
/usr/local/Homebrew/Library/Homebrew/test.rb:27:in `<main>'

[MikeMcQuaid]

So is this basically that brew linkage isn't used to detect opportunistic dependencies here?

[ilovezfs]

Indeed.

[MikeMcQuaid]

Personally I'd consider this a WONTFIX for now given that opportunistic linking is itself a bug that we should probably 🔥 instead of working around it.

[ilovezfs]

That makes no sense. This is precisely why autoremove won't be remotely safe.

[MikeMcQuaid]

Eliminating opportunistic linking makes no sense?

[MikeMcQuaid]

Eliminating opportunistic linking makes no sense?

(note: I'm fine with "we need to eliminate opportunistic linking before we can add an autoremove command")

[ilovezfs]

Preventing opportunistic linkage is a fine goal to have, but the opportunistic linkage information is available at install time already, so making this safety feature contingent on eliminating opportunistic linkage is wholly artificial, and will unnecessarily delay getting a safe autoremove command out the door.

[MikeMcQuaid]

Yeh, that's fair.

[ilovezfs]

In terms of preventing opportunistic linkage, this is a bit of a sledgehammer, but I suspect it covers most cases (it does fix the one above):

diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb
index 9597daf..b0d6059 100644
--- a/Library/Homebrew/sandbox.rb
+++ b/Library/Homebrew/sandbox.rb
@@ -165,6 +165,9 @@ class Sandbox
           (regex #"^/dev/ttys?[0-9]*$")
           )
       (deny file-write*) ; deny non-whitelist file write operations
+      (deny file-read*
+          (literal "/usr/local/lib")
+      )
       (allow process-exec
           (literal "/bin/ps")
           (with no-sandbox)

[MikeMcQuaid]

Will need to be in superenv; the failure case for that means that it'll go 💥 rather than being stripped, unfortunately 😭

[ilovezfs]

Not sure I follow, as the build was successful, not a sandbox violation.

[ilovezfs]

Also, it seems we've bent over backwards to do exactly the opposite in superenv so I think opportunistic linkage isn't going anywhere: https://github.com/Homebrew/brew/pull/845

[MikeMcQuaid]

Oh, sorry, missed that you tested it. Neat. Worth a try on CI, I think? I was thinking about the problem being that the read case in that call generally errors but if configure and cmake handle those errors as "that library doesn't exist" then it seems like a reasonable solution. I guess it may even be worth experimenting moving more of this type of code from superenv to the sandbox instead.

[zmwangx]

diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb
index 9597daf..b0d6059 100644
--- a/Library/Homebrew/sandbox.rb
+++ b/Library/Homebrew/sandbox.rb
@@ -165,6 +165,9 @@ class Sandbox
           (regex #"^/dev/ttys?[0-9]*$")
           )
       (deny file-write*) ; deny non-whitelist file write operations
+      (deny file-read*
+          (literal "/usr/local/lib")
+      )
       (allow process-exec
           (literal "/bin/ps")
           (with no-sandbox)

This approach is commendable, but the patch is currently broken because library paths in superenv relies on #{HOMEBREW_PREFIX}/lib:

  def determine_library_paths
    paths = keg_only_deps.map { |d| d.opt_lib.to_s }
    paths << "#{HOMEBREW_PREFIX}/lib"
    paths += homebrew_extra_library_paths
    puts paths
    puts paths.to_path_s
    paths.to_path_s
  end

Blocking access to /usr/local/lib blocks libraries of all non-keg-only dependencies.

[ilovezfs]

huh? did you actually try it?

[zmwangx]

I did. Try ffmpeg for instance.

[zmwangx]

Not sure why you were able to build plplot, but I guess that's because it was able to locate stuff with pkg-config. FFmpeg doesn't use pkg-config.

[ilovezfs]

Yeah, it doesn't seem acceptable for a build system to require access to /usr/local/lib as a generic grab bag.

[ilovezfs]

It looks like it works if we change that bit to

paths = deps.map { |d| d.opt_lib.to_s }

[zmwangx]

Okay, took another look and it is indeed pkg-config that enables success location of libraries when /usr/local/lib is blocked. For instance, there's no problem with x264, because from FFmpeg's configure:

enabled libx264           && { use_pkg_config x264 "stdint.h x264.h" x264_encoder_encode ||
                               { require libx264 x264.h x264_encoder_encode -lx264 &&
                                 warn "using libx264 without pkg-config"; } } &&
                             { check_cpp_condition x264.h "X264_BUILD >= 118" ||
                               die "ERROR: libx264 must be installed and version must be >= 0.118."; } &&
                             { check_cpp_condition x264.h "X264_MPEG2" &&
                               enable libx262; }

The use_pkg_config invokes pkg-config. However, lame can't be found because it doesn't have a .pc file, and configure checks its existence as such:

enabled libmp3lame        && require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame

where this require function does a linking check. So any non-keg-only dependencies not using pkg-config are blocked.

Yeah, it doesn't seem acceptable for a build system to require access to /usr/local/lib as a generic grab bag.

It's not the build system's problem. Without your patch, /usr/local/lib goes into HOMEBREW_LIBRARY_PATHS (see determine_library_paths which I quoted above), which is then processed by our shim clang to add -L/usr/local/lib. With your path /usr/local/lib is stripped from HOMEBREW_LIBRARY_PATHS.

So, the obvious fix is to add opt_lib of all dependencies to HOMEBREW_LIBRARY_PATHS. Other methods in extend/ENV/super.rb might need similar treatment too.

[zmwangx]

It looks like it works if we change that bit to

paths = deps.map { |d| d.opt_lib.to_s }

Yep, that's what I'm saying too.

[ilovezfs]

Yep, that's what I'm saying too.

does that work with non-pkg-config crap too?

[zmwangx]

does that work with non-pkg-config crap too?

It should work with reasonable build systems. FFmpeg builds fine, for instance.

Not sure about crap though (e.g. build scripts hardcoded to look into /usr/local/lib, /opt/local/lib, /sw/lib, etc). Maybe this is a good way to uncover crappy projects.

[zmwangx]

By the way, should we also give $HOMEBREW_PREFIX/include the same treatment to prevent opportunistic inclusion of header only libraries? (Not remotely as harmful for sure.)

[ilovezfs]

Yeah, I've been thinking about that. I think we should, but it might be wise to leave it for a second stage of tightening.

[ilovezfs]

That said, it might not hurt to start out with it in there as well, and see what happens.

Also, I think we need a similar fix to the paths = deps.map { |d| d.opt_lib.to_s } for osxfuse (and possibly others).

[zmwangx]

osxfuse may present a unique challenge, in that its pkgconfig file points to /usr/local/lib:

$ cat /usr/local/lib/pkgconfig/osxfuse.pc
prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: fuse
Description: OSXFUSE
Version: 2.7.3
Libs: -L${libdir} -losxfuse -pthread  -liconv
Cflags: -I${includedir}/osxfuse/fuse -D_FILE_OFFSET_BITS=64 -D_DARWIN_USE_64_BIT_INODE

[ilovezfs]

We might be able to give it a faux opt prefix, and provide our own pkgconfig file in Library/Homebrew/os/mac/pkgconfig

[MikeMcQuaid]

Out of interest are we still seeing opportunistic linkage with the relatively recent superenv changes? If so, do you have a reproducible sequence of installations so I (or someone) can take a look at this? Ta!

[ilovezfs]

@MikeMcQuaid yes the sequence in the top post still produces the opportunistic linkage.

brew install homebrew/science/qhull
brew install -s plplot
brew linkage plplot

[ilovezfs]

Another one:

brew install gdal
brew install -s open-scene-graph
brew linkage open-scene-graph

See https://bot.brew.sh/job/Homebrew%20Core%20Pull%20Requests/498/ where it's causing harfbuzz CI failure.

It looks like gdal got opportunistically linked as part of https://github.com/Homebrew/homebrew-core/pull/8334/

[alyssais]

Does something like this look about right in terms of logic (not code quality/location)?

diff --git a/Library/Homebrew/tab.rb b/Library/Homebrew/tab.rb
index aa0208d51..fa3ffce61 100644
--- a/Library/Homebrew/tab.rb
+++ b/Library/Homebrew/tab.rb
@@ -16,6 +16,16 @@ class Tab < OpenStruct

   # Instantiates a Tab for a new installation of a formula.
   def self.create(formula, compiler, stdlib)
+    keg                   = Keg.new(formula.opt_prefix)
+    linkage_checker       = LinkageChecker.new(keg, formula)
+    dylib_formula_names   = linkage_checker.brewed_dylibs.keys
+    linked_formulae_names = dylib_formula_names - [formula.name]
+    linked_formulae       = linked_formulae_names.map { |n| Formulary.factory(n) }
+
+    depended_on_formulae  = formula.runtime_dependencies.map(&:to_formula)
+
+    runtime_dependencies  = linked_formulae | depended_on_formulae
+
     build = formula.build
     attributes = {
       "homebrew_version" => HOMEBREW_VERSION,
@@ -32,8 +42,7 @@ class Tab < OpenStruct
       "compiler" => compiler,
       "stdlib" => stdlib,
       "aliases" => formula.aliases,
-      "runtime_dependencies" => formula.runtime_dependencies.map do |dep|
-        f = dep.to_formula
+      "runtime_dependencies" => runtime_dependencies.map do |f|
         { "full_name" => f.full_name, "version" => f.version.to_s }
       end,
       "source" => {

[MikeMcQuaid]

@alyssais Yeh, I think so. I'd put most logic in Formula and Keg.