Undeclared, non-recursive dependencies are not recorded in INSTALL_RECEIPT.json #2173
Comments
|
Note, of course, that the consequence of the above is that the |
|
So is this basically that |
|
Indeed. |
|
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. |
|
That makes no sense. This is precisely why autoremove won't be remotely safe. |
|
Eliminating opportunistic linking makes no sense? |
(note: I'm fine with "we need to eliminate opportunistic linking before we can add an |
|
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. |
|
Yeh, that's fair. |
|
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): |
|
Will need to be in superenv; the failure case for that means that it'll go 💥 rather than being stripped, unfortunately 😭 |
|
Not sure I follow, as the build was successful, not a sandbox violation. |
|
Also, it seems we've bent over backwards to do exactly the opposite in superenv so I think opportunistic linkage isn't going anywhere: #845 |
|
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 |
This approach is commendable, but the patch is currently broken because library paths in superenv relies on 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
endBlocking access to |
|
huh? did you actually try it? |
|
I did. Try |
|
Not sure why you were able to build |
|
Yeah, it doesn't seem acceptable for a build system to require access to /usr/local/lib as a generic grab bag. |
|
It looks like it works if we change that bit to |
|
Okay, took another look and it is indeed pkg-config that enables success location of libraries when 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 enabled libmp3lame && require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lamewhere this
It's not the build system's problem. Without your patch, So, the obvious fix is to add |
Yep, that's what I'm saying too. |
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 |
|
By the way, should we also give |
|
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. |
|
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 |
|
|
|
We might be able to give it a faux opt prefix, and provide our own pkgconfig file in Library/Homebrew/os/mac/pkgconfig |
|
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! |
|
@MikeMcQuaid yes the sequence in the top post still produces the opportunistic linkage. |
|
Another one: 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 Homebrew/homebrew-core#8334 |
|
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" => { |
|
@alyssais Yeh, I think so. I'd put most logic in |
ghost
added
in progress
brew updateand retried your prior step?brew doctor, fixed as many issues as possible and retried your prior step?Bug reports:
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:
CC @alyssais @MikeMcQuaid
The text was updated successfully, but these errors were encountered: