From 51c4c84a3f39a64d4694beebd15952d2cf1ae664 Mon Sep 17 00:00:00 2001 From: "Tim D. Smith" Date: Sun, 2 Apr 2017 08:17:07 -0700 Subject: [PATCH 1/5] Don't follow symlinks when hunting for strings When we're assessing whether a bottle is relocatable, we shouldn't have to descend into symlink paths we encounter. This is supposed to be the default behavior but it doesn't appear to be (perhaps because we pass a symlink to the keg on the command line?). All of the switches that control this behavior differ between BSD and GNU grep, so sniff the grep flavor first. --- Library/Homebrew/keg_relocate.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 834cda7684..00e941ce2f 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -97,7 +97,12 @@ class Keg end def each_unique_file_matching(string) - Utils.popen_read("/usr/bin/fgrep", "-lr", string, to_s) do |io| + bsd = `/usr/bin/fgrep -V`.include?("BSD grep") + grep_args = "-lr" + # Don't recurse into symlinks; the man page says this is the default, but + # it's wrong. + grep_args += "O" if bsd + Utils.popen_read("/usr/bin/fgrep", grep_args, string, to_s) do |io| hardlinks = Set.new until io.eof? From d0feae0632e29194d09a26adf961a05de760a22a Mon Sep 17 00:00:00 2001 From: "Tim D. Smith" Date: Sun, 2 Apr 2017 08:19:29 -0700 Subject: [PATCH 2/5] Unlink before rewriting link ln_sf does the right thing when `dest` is a symlink pointing to a file: the symlink gets overwritten with a link pointing to the new src. But when dest points to a directory, we create a new symlink inside the folder dest points to, which doesn't help us at all. --- Library/Homebrew/keg_relocate.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 00e941ce2f..0b1a1bc94d 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -17,7 +17,9 @@ class Keg # Don't fix relative symlinks next unless link.absolute? if link.to_s.start_with?(HOMEBREW_CELLAR.to_s) || link.to_s.start_with?(HOMEBREW_PREFIX.to_s) - FileUtils.ln_sf(link.relative_path_from(file.parent), file) + new_src = link.relative_path_from(file.parent) + file.unlink + FileUtils.ln_s(new_src, file) end end end From 2f4eaf26a035727f8541a432c8e48b75f817ac0a Mon Sep 17 00:00:00 2001 From: "Tim D. Smith" Date: Sun, 2 Apr 2017 09:04:49 -0700 Subject: [PATCH 3/5] Use extend/OS mechanism for grep args --- Library/Homebrew/extend/os/mac/keg_relocate.rb | 6 ++++++ Library/Homebrew/keg_relocate.rb | 13 +++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/keg_relocate.rb b/Library/Homebrew/extend/os/mac/keg_relocate.rb index 476e5da4a4..7b5bb11f97 100644 --- a/Library/Homebrew/extend/os/mac/keg_relocate.rb +++ b/Library/Homebrew/extend/os/mac/keg_relocate.rb @@ -131,6 +131,12 @@ class Keg mach_o_files end + def recursive_fgrep_args + # Don't recurse into symlinks; the man page says this is the default, but + # it's wrong. -O is a BSD-grep-only option. + "-lrO" + end + def self.file_linked_libraries(file, string) # Check dynamic library linkage. Importantly, do not perform for static # libraries, which will falsely report "linkage" to themselves. diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 0b1a1bc94d..9a99025841 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -98,13 +98,14 @@ class Keg [] end + def recursive_fgrep_args + # for GNU grep; overridden for BSD grep on OS X + "-lr" + end + alias generic_recursive_fgrep_args recursive_fgrep_args + def each_unique_file_matching(string) - bsd = `/usr/bin/fgrep -V`.include?("BSD grep") - grep_args = "-lr" - # Don't recurse into symlinks; the man page says this is the default, but - # it's wrong. - grep_args += "O" if bsd - Utils.popen_read("/usr/bin/fgrep", grep_args, string, to_s) do |io| + Utils.popen_read("/usr/bin/fgrep", recursive_fgrep_args, string, to_s) do |io| hardlinks = Set.new until io.eof? From 3b75075727786eb4f02b25a49d178f3b4a23708b Mon Sep 17 00:00:00 2001 From: "Tim D. Smith" Date: Sun, 2 Apr 2017 10:26:05 -0700 Subject: [PATCH 4/5] rubocop --- Library/Homebrew/keg_relocate.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 9a99025841..4ec9136429 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -16,11 +16,10 @@ class Keg link = file.readlink # Don't fix relative symlinks next unless link.absolute? - if link.to_s.start_with?(HOMEBREW_CELLAR.to_s) || link.to_s.start_with?(HOMEBREW_PREFIX.to_s) - new_src = link.relative_path_from(file.parent) - file.unlink - FileUtils.ln_s(new_src, file) - end + next unless link.to_s.start_with?(HOMEBREW_CELLAR.to_s) || link.to_s.start_with?(HOMEBREW_PREFIX.to_s) + new_src = link.relative_path_from(file.parent) + file.unlink + FileUtils.ln_s(new_src, file) end end alias generic_fix_dynamic_linkage fix_dynamic_linkage From 2144c35539318a8ec88ba3ebcca473a917eee71b Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 5 Apr 2017 09:00:11 +0100 Subject: [PATCH 5/5] keg_relocate: slim long lines. --- Library/Homebrew/keg_relocate.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 4ec9136429..6044426ee4 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -16,7 +16,9 @@ class Keg link = file.readlink # Don't fix relative symlinks next unless link.absolute? - next unless link.to_s.start_with?(HOMEBREW_CELLAR.to_s) || link.to_s.start_with?(HOMEBREW_PREFIX.to_s) + link_starts_cellar = link.to_s.start_with?(HOMEBREW_CELLAR.to_s) + link_starts_prefix = link.to_s.start_with?(HOMEBREW_PREFIX.to_s) + next if !link_starts_cellar && !link_starts_prefix new_src = link.relative_path_from(file.parent) file.unlink FileUtils.ln_s(new_src, file)