From 69b590012d12d4e79a4696dac80b9f8733d02d6b Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Tue, 16 Jan 2018 17:37:59 -0500 Subject: [PATCH 01/28] Berkeley db cache optimization for `brew linkage` command. --- Library/Homebrew/dev-cmd/linkage.rb | 12 +- .../extend/os/mac/formula_cellar_checks.rb | 5 +- Library/Homebrew/os/mac/cache_store.rb | 207 ++++++++++++++++++ Library/Homebrew/os/mac/linkage_checker.rb | 123 ++++++++--- 4 files changed, 315 insertions(+), 32 deletions(-) create mode 100644 Library/Homebrew/os/mac/cache_store.rb diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 31e9bd1036..0ade2bc646 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -1,4 +1,4 @@ -#: * `linkage` [`--test`] [`--reverse`] : +#: * `linkage` [`--test`] [`--reverse`] [`--rebuild`] : #: Checks the library links of an installed formula. #: #: Only works on installed formulae. An error is raised if it is run on @@ -9,6 +9,9 @@ #: #: If `--reverse` is passed, print the dylib followed by the binaries #: which link to it for each library the keg references. +#: +#: If `--rebuild` is passed, flushes the `LinkageStore` cache for each +#: 'keg.name' and forces a check on the dylibs. require "os/mac/linkage_checker" @@ -18,7 +21,10 @@ module Homebrew def linkage ARGV.kegs.each do |keg| ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 - result = LinkageChecker.new(keg) + database_cache = DatabaseCache.new("linkage") + result = LinkageChecker.new(keg, database_cache) + result.flush_cache_and_check_dylibs if ARGV.include?("--rebuild") + if ARGV.include?("--test") result.display_test_output Homebrew.failed = true if result.broken_dylibs? @@ -27,6 +33,8 @@ module Homebrew else result.display_normal_output end + + database_cache.close end end end diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 901d8945fe..00b151b641 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -64,7 +64,10 @@ module FormulaCellarChecks def check_linkage return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - checker = LinkageChecker.new(keg, formula) + database_cache = DatabaseCache.new("linkage") + checker = LinkageChecker.new(keg, database_cache, formula) + checker.flush_cache_and_check_dylibs + database_cache.close return unless checker.broken_dylibs? output = <<~EOS diff --git a/Library/Homebrew/os/mac/cache_store.rb b/Library/Homebrew/os/mac/cache_store.rb new file mode 100644 index 0000000000..49b341dcc9 --- /dev/null +++ b/Library/Homebrew/os/mac/cache_store.rb @@ -0,0 +1,207 @@ +require "dbm" +require "json" + +# +# `DatabaseCache` is a class acting as an interface to a persistent storage +# mechanism residing in the `HOMEBREW_CACHE` +# +class DatabaseCache + # Name of the database cache file located at /.db + # + # @return [String] + attr_accessor :name + + # Instantiates new `DatabaseCache` object + # + # @param [String] name + # @return [nil] + def initialize(name) + @name = name + end + + # Memoized `DBM` database object with on-disk database located in the + # `HOMEBREW_CACHE` + # + # @return [DBM] db + def db + @db ||= DBM.open("#{HOMEBREW_CACHE}/#{name}", 0666, DBM::WRCREAT) + end + + # Close the `DBM` database object after usage + # + # @return [nil] + def close + db.close + end +end + +# +# `CacheStore` is an abstract base class which provides methods to mutate and +# fetch data from a persistent storage mechanism +# +# @abstract +# +class CacheStore + # Instantiates a new `CacheStore` class + # + # @param [DatabaseCache] database_cache + # @return [nil] + def initialize(database_cache) + @db = database_cache.db + end + + # Inserts new values or updates existing cached values to persistent storage + # mechanism + # + # @abstract + # @param [Any] + # @return [nil] + def update!(*) + raise NotImplementedError + end + + # Fetches cached values in persistent storage according to the type of data + # stored + # + # @abstract + # @param [Any] + # @return [Any] + def fetch(*) + raise NotImplementedError + end + + # Deletes data from the cache based on a condition defined in a concrete class + # + # @abstract + # @return [nil] + def flush_cache! + raise NotImplementedError + end + + protected + + # A class instance providing access to the `DBM` database object + # + # @return [DBM] + attr_reader :db +end + +# +# `LinkageStore` is a concrete class providing methods to fetch and mutate +# linkage-specific data used by the `brew linkage` command +# +# If the cache hasn't changed, don't do extra processing in `LinkageChecker`. +# Instead, just fetch the data stored in the cache +# +class LinkageStore < CacheStore + # Types of dylibs of the form (label -> array) + HASH_LINKAGE_TYPES = %w[brewed_dylibs reverse_links].freeze + + # The keg name for the `LinkageChecker` class + # + # @return [String] + attr_reader :key + + # Initializes new `LinkageStore` class + # + # @param [String] keg_name + # @param [DatabaseCache] database_cache + # @return [nil] + def initialize(keg_name, database_cache) + @key = keg_name + super(database_cache) + end + + # Inserts new values or updates existing cached values to persistent storage + # mechanism according to the type of data + # + # @param [Hash] path_values + # @param [Hash] hash_values + # @return [nil] + def update!( + path_values: { + "system_dylibs" => %w[], "variable_dylibs" => %w[], "broken_dylibs" => %w[], + "indirect_deps" => %w[], "undeclared_deps" => %w[], "unnecessary_deps" => %w[] + }, + hash_values: { + "brewed_dylibs" => {}, "reverse_links" => {} + } + ) + db[key] = { + "path_values" => format_path_values(path_values), + "hash_values" => format_hash_values(hash_values), + } + end + + # Fetches cached values in persistent storage according to the type of data + # stored + # + # @param [String] type + # @return [Any] + def fetch(type:) + if HASH_LINKAGE_TYPES.include?(type) + fetch_hash_values(type: type) + else + fetch_path_values(type: type) + end + end + + # A condition for where to flush the cache + # + # @return [String] + def flush_cache! + db.delete(key) + end + + private + + # Fetches a subset of paths where the name = `key` + # + # @param [String] type + # @return [Array[String]] + def fetch_path_values(type:) + return [] unless db.key?(key) && !db[key].nil? + string_to_hash(db[key])["path_values"][type] + end + + # Fetches a subset of paths and labels where the name = `key`. Formats said + # paths/labels into `key => [value]` syntax expected by `LinkageChecker` + # + # @param [String] type + # @return [Hash] + def fetch_hash_values(type:) + return {} unless db.key?(key) && !db[key].nil? + string_to_hash(db[key])["hash_values"][type] + end + + # Parses `DBM` stored `String` into ruby `Hash` + # + # @param [String] string + # @return [Hash] + def string_to_hash(string) + JSON.parse(string.gsub("=>", ":")) + end + + # Formats the linkage data for `path_values` into a kind which can be parsed + # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s. + # + # @param [Hash(String, Set(String))] hash + # @return [Hash(String, Array(String))] + def format_path_values(hash) + hash.each_with_object({}) { |(k, v), h| h[k] = v.to_a } + end + + # Formats the linkage data for `hash_values` into a kind which can be parsed + # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s, and + # converts ruby `Pathname`s to `String`s + # + # @param [Hash(String, Set(Pathname))] hash + # @return [Hash(String, Array(String))] + def format_hash_values(hash) + hash.each_with_object({}) do |(outer_key, outer_values), outer_hash| + outer_hash[outer_key] = outer_values.each_with_object({}) do |(k, v), h| + h[k] = v.to_a.map(&:to_s) + end + end + end +end diff --git a/Library/Homebrew/os/mac/linkage_checker.rb b/Library/Homebrew/os/mac/linkage_checker.rb index cf6c12f22c..c123d72905 100644 --- a/Library/Homebrew/os/mac/linkage_checker.rb +++ b/Library/Homebrew/os/mac/linkage_checker.rb @@ -1,27 +1,56 @@ require "set" require "keg" require "formula" +require "os/mac/cache_store" class LinkageChecker - attr_reader :keg, :formula - attr_reader :brewed_dylibs, :system_dylibs, :broken_dylibs, :variable_dylibs - attr_reader :undeclared_deps, :unnecessary_deps, :reverse_links + attr_reader :keg, :formula, :store - def initialize(keg, formula = nil) + def initialize(keg, db, formula = nil) @keg = keg @formula = formula || resolve_formula(keg) - @brewed_dylibs = Hash.new { |h, k| h[k] = Set.new } - @system_dylibs = Set.new - @broken_dylibs = Set.new - @variable_dylibs = Set.new - @indirect_deps = [] - @undeclared_deps = [] - @reverse_links = Hash.new { |h, k| h[k] = Set.new } - @unnecessary_deps = [] - check_dylibs + @store = LinkageStore.new(keg.name, db) end - def check_dylibs + # 'Hash-type' cache values + + def brewed_dylibs + @brewed_dylibs ||= store.fetch(type: "brewed_dylibs") + end + + def reverse_links + @reverse_links ||= store.fetch(type: "reverse_links") + end + + # 'Path-type' cached values + + def system_dylibs + @system_dylibs ||= store.fetch(type: "system_dylibs") + end + + def broken_dylibs + @broken_dylibs ||= store.fetch(type: "broken_dylibs") + end + + def variable_dylibs + @variable_dylibs ||= store.fetch(type: "variable_dylibs") + end + + def undeclared_deps + @undeclared_deps ||= store.fetch(type: "undeclared_deps") + end + + def indirect_deps + @indirect_deps ||= store.fetch(type: "indirect_deps") + end + + def unnecessary_deps + @unnecessary_deps ||= store.fetch(type: "unnecessary_deps") + end + + def flush_cache_and_check_dylibs + reset_dylibs! + @keg.find do |file| next if file.symlink? || file.directory? next unless file.dylib? || file.binary_executable? || file.mach_o_bundle? @@ -54,6 +83,7 @@ class LinkageChecker end @indirect_deps, @undeclared_deps, @unnecessary_deps = check_undeclared_deps if formula + store_dylibs! end def check_undeclared_deps @@ -99,18 +129,18 @@ class LinkageChecker end def display_normal_output - display_items "System libraries", @system_dylibs - display_items "Homebrew libraries", @brewed_dylibs - display_items "Indirect dependencies with linkage", @indirect_deps - display_items "Variable-referenced libraries", @variable_dylibs - display_items "Missing libraries", @broken_dylibs - display_items "Undeclared dependencies with linkage", @undeclared_deps - display_items "Dependencies with no linkage", @unnecessary_deps + display_items "System libraries", system_dylibs + display_items "Homebrew libraries", brewed_dylibs + display_items "Indirect dependencies with linkage", indirect_deps + display_items "Variable-referenced libraries", variable_dylibs + display_items "Missing libraries", broken_dylibs + display_items "Undeclared dependencies with linkage", undeclared_deps + display_items "Dependencies with no linkage", unnecessary_deps end def display_reverse_output - return if @reverse_links.empty? - sorted = @reverse_links.sort + return if reverse_links.empty? + sorted = reverse_links.sort sorted.each do |dylib, files| puts dylib files.each do |f| @@ -122,21 +152,21 @@ class LinkageChecker end def display_test_output - display_items "Missing libraries", @broken_dylibs - display_items "Possible unnecessary dependencies", @unnecessary_deps - puts "No broken dylib links" if @broken_dylibs.empty? + display_items "Missing libraries", broken_dylibs + display_items "Possible unnecessary dependencies", unnecessary_deps + puts "No broken dylib links" if broken_dylibs.empty? end def broken_dylibs? - !@broken_dylibs.empty? + !broken_dylibs.empty? end def undeclared_deps? - !@undeclared_deps.empty? + !undeclared_deps.empty? end def unnecessary_deps? - !@unnecessary_deps.empty? + !unnecessary_deps.empty? end private @@ -175,4 +205,39 @@ class LinkageChecker rescue FormulaUnavailableError opoo "Formula unavailable: #{keg.name}" end + + # Helper function to reset dylib values when building cache + # + # @return [nil] + def reset_dylibs! + store.flush_cache! + @system_dylibs = Set.new + @broken_dylibs = Set.new + @variable_dylibs = Set.new + @brewed_dylibs = Hash.new { |h, k| h[k] = Set.new } + @reverse_links = Hash.new { |h, k| h[k] = Set.new } + @indirect_deps = [] + @undeclared_deps = [] + @unnecessary_deps = [] + end + + # Updates data store with package path values + # + # @return [nil] + def store_dylibs! + store.update!( + path_values: { + "system_dylibs" => @system_dylibs, + "variable_dylibs" => @variable_dylibs, + "broken_dylibs" => @broken_dylibs, + "indirect_deps" => @indirect_deps, + "undeclared_deps" => @undeclared_deps, + "unnecessary_deps" => @unnecessary_deps, + }, + hash_values: { + "brewed_dylibs" => @brewed_dylibs, + "reverse_links" => @reverse_links, + }, + ) + end end From 4bc6459ed7e4ad2aaa564dfbb90b2871489e058c Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Sat, 24 Feb 2018 17:32:29 -0500 Subject: [PATCH 02/28] Removed redundant documentation, use database_cache as a block, and use symbolic keys over string keys in function calls. --- Library/Homebrew/dev-cmd/linkage.rb | 28 +-- .../extend/os/mac/formula_cellar_checks.rb | 9 +- Library/Homebrew/os/mac/cache_store.rb | 164 +++++------------- Library/Homebrew/os/mac/linkage_checker.rb | 36 ++-- 4 files changed, 82 insertions(+), 155 deletions(-) diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 0ade2bc646..c5d43a48dc 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -19,22 +19,22 @@ module Homebrew module_function def linkage - ARGV.kegs.each do |keg| - ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 - database_cache = DatabaseCache.new("linkage") - result = LinkageChecker.new(keg, database_cache) - result.flush_cache_and_check_dylibs if ARGV.include?("--rebuild") + DatabaseCache.new(:linkage) do |database_cache| + ARGV.kegs.each do |keg| + ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 - if ARGV.include?("--test") - result.display_test_output - Homebrew.failed = true if result.broken_dylibs? - elsif ARGV.include?("--reverse") - result.display_reverse_output - else - result.display_normal_output + result = LinkageChecker.new(keg, database_cache) + result.flush_cache_and_check_dylibs if ARGV.include?("--rebuild") + + if ARGV.include?("--test") + result.display_test_output + Homebrew.failed = true if result.broken_dylibs? + elsif ARGV.include?("--reverse") + result.display_reverse_output + else + result.display_normal_output + end end - - database_cache.close end end end diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 00b151b641..0386beb9a4 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -64,10 +64,11 @@ module FormulaCellarChecks def check_linkage return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - database_cache = DatabaseCache.new("linkage") - checker = LinkageChecker.new(keg, database_cache, formula) - checker.flush_cache_and_check_dylibs - database_cache.close + + DatabaseCache.new(:linkage) do |database_cache| + checker = LinkageChecker.new(keg, database_cache, formula) + checker.flush_cache_and_check_dylibs + end return unless checker.broken_dylibs? output = <<~EOS diff --git a/Library/Homebrew/os/mac/cache_store.rb b/Library/Homebrew/os/mac/cache_store.rb index 49b341dcc9..fa919d4b4c 100644 --- a/Library/Homebrew/os/mac/cache_store.rb +++ b/Library/Homebrew/os/mac/cache_store.rb @@ -2,143 +2,97 @@ require "dbm" require "json" # -# `DatabaseCache` is a class acting as an interface to a persistent storage -# mechanism residing in the `HOMEBREW_CACHE` +# `DatabaseCache` acts as an interface to a persistent storage mechanism +# residing in the `HOMEBREW_CACHE` # class DatabaseCache - # Name of the database cache file located at /.db - # - # @return [String] - attr_accessor :name + # Users have read and write, but not execute permissions + DATABASE_MODE = 0666 - # Instantiates new `DatabaseCache` object + # Opens and yields a database in read/write mode # - # @param [String] name - # @return [nil] + # DBM::WRCREAT: Creates the database if it does not already exist def initialize(name) - @name = name - end - - # Memoized `DBM` database object with on-disk database located in the - # `HOMEBREW_CACHE` - # - # @return [DBM] db - def db - @db ||= DBM.open("#{HOMEBREW_CACHE}/#{name}", 0666, DBM::WRCREAT) - end - - # Close the `DBM` database object after usage - # - # @return [nil] - def close - db.close + @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) + yield(@db) + @db.close end end # -# `CacheStore` is an abstract base class which provides methods to mutate and -# fetch data from a persistent storage mechanism -# -# @abstract +# `CacheStore` provides methods to mutate and fetch data from a persistent +# storage mechanism # class CacheStore - # Instantiates a new `CacheStore` class - # - # @param [DatabaseCache] database_cache - # @return [nil] def initialize(database_cache) - @db = database_cache.db + @database_cache = database_cache end # Inserts new values or updates existing cached values to persistent storage # mechanism - # - # @abstract - # @param [Any] - # @return [nil] def update!(*) raise NotImplementedError end # Fetches cached values in persistent storage according to the type of data # stored - # - # @abstract - # @param [Any] - # @return [Any] - def fetch(*) + def fetch_type(*) raise NotImplementedError end # Deletes data from the cache based on a condition defined in a concrete class - # - # @abstract - # @return [nil] def flush_cache! raise NotImplementedError end protected - # A class instance providing access to the `DBM` database object + attr_reader :database_cache + + # Parses `DBM` stored `String` into ruby `Hash` # - # @return [DBM] - attr_reader :db + # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data, + # to convert the ruby string back into a ruby `Hash`, the string is converted + # into a JSON compatible string, where it may be parsed by the JSON.parse + # function + def string_to_hash(string) + JSON.parse(string.gsub("=>", ":")) + end end # -# `LinkageStore` is a concrete class providing methods to fetch and mutate -# linkage-specific data used by the `brew linkage` command -# -# If the cache hasn't changed, don't do extra processing in `LinkageChecker`. -# Instead, just fetch the data stored in the cache +# `LinkageStore` provides methods to fetch and mutate linkage-specific data used +# by the `brew linkage` command # class LinkageStore < CacheStore - # Types of dylibs of the form (label -> array) - HASH_LINKAGE_TYPES = %w[brewed_dylibs reverse_links].freeze + HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze - # The keg name for the `LinkageChecker` class - # - # @return [String] - attr_reader :key - - # Initializes new `LinkageStore` class - # - # @param [String] keg_name - # @param [DatabaseCache] database_cache - # @return [nil] def initialize(keg_name, database_cache) - @key = keg_name + @keg_name = keg_name super(database_cache) end - # Inserts new values or updates existing cached values to persistent storage - # mechanism according to the type of data - # - # @param [Hash] path_values - # @param [Hash] hash_values - # @return [nil] def update!( path_values: { - "system_dylibs" => %w[], "variable_dylibs" => %w[], "broken_dylibs" => %w[], - "indirect_deps" => %w[], "undeclared_deps" => %w[], "unnecessary_deps" => %w[] + system_dylibs: %w[], + variable_dylibs: %w[], + broken_dylibs: %w[], + indirect_deps: %w[], + undeclared_deps: %w[], + unnecessary_deps: %w[], }, hash_values: { - "brewed_dylibs" => {}, "reverse_links" => {} + brewed_dylibs: {}, + reverse_links: {}, } ) - db[key] = { + database_cache[keg_name] = { "path_values" => format_path_values(path_values), "hash_values" => format_hash_values(hash_values), } end - # Fetches cached values in persistent storage according to the type of data - # stored - # - # @param [String] type - # @return [Any] - def fetch(type:) + def fetch_type(type) if HASH_LINKAGE_TYPES.include?(type) fetch_hash_values(type: type) else @@ -146,60 +100,36 @@ class LinkageStore < CacheStore end end - # A condition for where to flush the cache - # - # @return [String] def flush_cache! - db.delete(key) + database_cache.delete(keg_name) end private - # Fetches a subset of paths where the name = `key` - # - # @param [String] type - # @return [Array[String]] + attr_reader :keg_name + def fetch_path_values(type:) - return [] unless db.key?(key) && !db[key].nil? - string_to_hash(db[key])["path_values"][type] + return [] if !database_cache.key?(keg_name) || database_cache[keg_name].nil? + string_to_hash(database_cache[keg_name])["path_values"][type.to_s] end - # Fetches a subset of paths and labels where the name = `key`. Formats said - # paths/labels into `key => [value]` syntax expected by `LinkageChecker` - # - # @param [String] type - # @return [Hash] def fetch_hash_values(type:) - return {} unless db.key?(key) && !db[key].nil? - string_to_hash(db[key])["hash_values"][type] - end - - # Parses `DBM` stored `String` into ruby `Hash` - # - # @param [String] string - # @return [Hash] - def string_to_hash(string) - JSON.parse(string.gsub("=>", ":")) + return {} if !database_cache.key?(keg_name) || database_cache[keg_name].nil? + string_to_hash(database_cache[keg_name])["hash_values"][type.to_s] end # Formats the linkage data for `path_values` into a kind which can be parsed - # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s. - # - # @param [Hash(String, Set(String))] hash - # @return [Hash(String, Array(String))] + # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s def format_path_values(hash) - hash.each_with_object({}) { |(k, v), h| h[k] = v.to_a } + hash.each_with_object({}) { |(k, v), h| h[k.to_s] = v.to_a } end # Formats the linkage data for `hash_values` into a kind which can be parsed # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s, and # converts ruby `Pathname`s to `String`s - # - # @param [Hash(String, Set(Pathname))] hash - # @return [Hash(String, Array(String))] def format_hash_values(hash) hash.each_with_object({}) do |(outer_key, outer_values), outer_hash| - outer_hash[outer_key] = outer_values.each_with_object({}) do |(k, v), h| + outer_hash[outer_key.to_s] = outer_values.each_with_object({}) do |(k, v), h| h[k] = v.to_a.map(&:to_s) end end diff --git a/Library/Homebrew/os/mac/linkage_checker.rb b/Library/Homebrew/os/mac/linkage_checker.rb index c123d72905..650cea0e9f 100644 --- a/Library/Homebrew/os/mac/linkage_checker.rb +++ b/Library/Homebrew/os/mac/linkage_checker.rb @@ -15,37 +15,37 @@ class LinkageChecker # 'Hash-type' cache values def brewed_dylibs - @brewed_dylibs ||= store.fetch(type: "brewed_dylibs") + @brewed_dylibs ||= store.fetch_type(:brewed_dylibs) end def reverse_links - @reverse_links ||= store.fetch(type: "reverse_links") + @reverse_links ||= store.fetch_type(:reverse_links) end # 'Path-type' cached values def system_dylibs - @system_dylibs ||= store.fetch(type: "system_dylibs") + @system_dylibs ||= store.fetch_type(:system_dylibs) end def broken_dylibs - @broken_dylibs ||= store.fetch(type: "broken_dylibs") + @broken_dylibs ||= store.fetch_type(:broken_dylibs) end def variable_dylibs - @variable_dylibs ||= store.fetch(type: "variable_dylibs") + @variable_dylibs ||= store.fetch_type(:variable_dylibs) end def undeclared_deps - @undeclared_deps ||= store.fetch(type: "undeclared_deps") + @undeclared_deps ||= store.fetch_type(:undeclared_deps) end def indirect_deps - @indirect_deps ||= store.fetch(type: "indirect_deps") + @indirect_deps ||= store.fetch_type(:indirect_deps) end def unnecessary_deps - @unnecessary_deps ||= store.fetch(type: "unnecessary_deps") + @unnecessary_deps ||= store.fetch_type(:unnecessary_deps) end def flush_cache_and_check_dylibs @@ -207,8 +207,6 @@ class LinkageChecker end # Helper function to reset dylib values when building cache - # - # @return [nil] def reset_dylibs! store.flush_cache! @system_dylibs = Set.new @@ -222,21 +220,19 @@ class LinkageChecker end # Updates data store with package path values - # - # @return [nil] def store_dylibs! store.update!( path_values: { - "system_dylibs" => @system_dylibs, - "variable_dylibs" => @variable_dylibs, - "broken_dylibs" => @broken_dylibs, - "indirect_deps" => @indirect_deps, - "undeclared_deps" => @undeclared_deps, - "unnecessary_deps" => @unnecessary_deps, + system_dylibs: @system_dylibs, + variable_dylibs: @variable_dylibs, + broken_dylibs: @broken_dylibs, + indirect_deps: @indirect_deps, + undeclared_deps: @undeclared_deps, + unnecessary_deps: @unnecessary_deps, }, hash_values: { - "brewed_dylibs" => @brewed_dylibs, - "reverse_links" => @reverse_links, + brewed_dylibs: @brewed_dylibs, + reverse_links: @reverse_links, }, ) end From 14256faa47f30dec1d43134b415ecf0245b390fd Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Tue, 27 Feb 2018 13:05:19 -0500 Subject: [PATCH 03/28] Added flag to `flush_cache` in `LinkageChecker`. Format ruby hash as JSON string before storing in `dbm`. --- Library/Homebrew/dev-cmd/linkage.rb | 3 +- .../extend/os/mac/formula_cellar_checks.rb | 5 +- Library/Homebrew/os/mac/cache_store.rb | 67 +++++++++++-------- Library/Homebrew/os/mac/linkage_checker.rb | 5 +- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index c5d43a48dc..87a0d9143e 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -23,8 +23,7 @@ module Homebrew ARGV.kegs.each do |keg| ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 - result = LinkageChecker.new(keg, database_cache) - result.flush_cache_and_check_dylibs if ARGV.include?("--rebuild") + result = LinkageChecker.new(keg, database_cache, ARGV.include?("--rebuild")) if ARGV.include?("--test") result.display_test_output diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 0386beb9a4..2b62e6bca7 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -65,10 +65,7 @@ module FormulaCellarChecks return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - DatabaseCache.new(:linkage) do |database_cache| - checker = LinkageChecker.new(keg, database_cache, formula) - checker.flush_cache_and_check_dylibs - end + DatabaseCache.new(:linkage) { |database_cache| LinkageChecker.new(keg, database_cache, true, formula) } return unless checker.broken_dylibs? output = <<~EOS diff --git a/Library/Homebrew/os/mac/cache_store.rb b/Library/Homebrew/os/mac/cache_store.rb index fa919d4b4c..70868dce0c 100644 --- a/Library/Homebrew/os/mac/cache_store.rb +++ b/Library/Homebrew/os/mac/cache_store.rb @@ -6,13 +6,13 @@ require "json" # residing in the `HOMEBREW_CACHE` # class DatabaseCache - # Users have read and write, but not execute permissions - DATABASE_MODE = 0666 + # The mode of any created files will be 0664 (that is, readable and writable + # by the owner and the group, and readable by everyone else) + DATABASE_MODE = 0664 # Opens and yields a database in read/write mode - # - # DBM::WRCREAT: Creates the database if it does not already exist def initialize(name) + # DBM::WRCREAT: Creates the database if it does not already exist @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) yield(@db) @db.close @@ -47,16 +47,24 @@ class CacheStore protected + # @return [DBM] attr_reader :database_cache - # Parses `DBM` stored `String` into ruby `Hash` - # # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data, # to convert the ruby string back into a ruby `Hash`, the string is converted - # into a JSON compatible string, where it may be parsed by the JSON.parse - # function - def string_to_hash(string) - JSON.parse(string.gsub("=>", ":")) + # into a JSON compatible string in `ruby_hash_to_json_string`, where it may + # later be parsed by `JSON.parse` in the `json_string_to_ruby_hash` method + # + # @param [Hash] + # @return [String] + def ruby_hash_to_json_string(hash) + hash.to_json + end + + # @param [String] + # @return [Hash] + def json_string_to_ruby_hash(string) + JSON.parse(string) end end @@ -73,7 +81,7 @@ class LinkageStore < CacheStore end def update!( - path_values: { + array_values: { system_dylibs: %w[], variable_dylibs: %w[], broken_dylibs: %w[], @@ -86,17 +94,17 @@ class LinkageStore < CacheStore reverse_links: {}, } ) - database_cache[keg_name] = { - "path_values" => format_path_values(path_values), - "hash_values" => format_hash_values(hash_values), - } + database_cache[keg_name] = ruby_hash_to_json_string( + array_values: format_array_values(array_values), + hash_values: format_hash_values(hash_values), + ) end def fetch_type(type) if HASH_LINKAGE_TYPES.include?(type) fetch_hash_values(type: type) else - fetch_path_values(type: type) + fetch_array_values(type: type) end end @@ -108,28 +116,33 @@ class LinkageStore < CacheStore attr_reader :keg_name - def fetch_path_values(type:) - return [] if !database_cache.key?(keg_name) || database_cache[keg_name].nil? - string_to_hash(database_cache[keg_name])["path_values"][type.to_s] + def fetch_array_values(type:) + return [] unless database_cache.key?(keg_name) + json_string_to_ruby_hash(database_cache[keg_name])["array_values"][type.to_s] end def fetch_hash_values(type:) - return {} if !database_cache.key?(keg_name) || database_cache[keg_name].nil? - string_to_hash(database_cache[keg_name])["hash_values"][type.to_s] + return {} unless database_cache.key?(keg_name) + json_string_to_ruby_hash(database_cache[keg_name])["hash_values"][type.to_s] end - # Formats the linkage data for `path_values` into a kind which can be parsed - # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s - def format_path_values(hash) - hash.each_with_object({}) { |(k, v), h| h[k.to_s] = v.to_a } + # Formats the linkage data for `array_values` into a kind which can be parsed + # by the `json_string_to_ruby_hash` method. Internally converts ruby `Set`s to + # `Array`s + # + # @return [String] + def format_array_values(hash) + hash.each_with_object({}) { |(k, v), h| h[k] = v.to_a } end # Formats the linkage data for `hash_values` into a kind which can be parsed - # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s, and + # by the `json_string_to_ruby_hash` method. Converts ruby `Set`s to `Array`s, and # converts ruby `Pathname`s to `String`s + # + # @return [String] def format_hash_values(hash) hash.each_with_object({}) do |(outer_key, outer_values), outer_hash| - outer_hash[outer_key.to_s] = outer_values.each_with_object({}) do |(k, v), h| + outer_hash[outer_key] = outer_values.each_with_object({}) do |(k, v), h| h[k] = v.to_a.map(&:to_s) end end diff --git a/Library/Homebrew/os/mac/linkage_checker.rb b/Library/Homebrew/os/mac/linkage_checker.rb index 650cea0e9f..ed5eb8acd0 100644 --- a/Library/Homebrew/os/mac/linkage_checker.rb +++ b/Library/Homebrew/os/mac/linkage_checker.rb @@ -6,10 +6,11 @@ require "os/mac/cache_store" class LinkageChecker attr_reader :keg, :formula, :store - def initialize(keg, db, formula = nil) + def initialize(keg, db, rebuild_cache = false, formula = nil) @keg = keg @formula = formula || resolve_formula(keg) @store = LinkageStore.new(keg.name, db) + flush_cache_and_check_dylibs if rebuild_cache end # 'Hash-type' cache values @@ -222,7 +223,7 @@ class LinkageChecker # Updates data store with package path values def store_dylibs! store.update!( - path_values: { + array_values: { system_dylibs: @system_dylibs, variable_dylibs: @variable_dylibs, broken_dylibs: @broken_dylibs, From d7765dd22300eaeecd46064b20f4f66a2b7bf03c Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Wed, 28 Feb 2018 10:39:15 -0500 Subject: [PATCH 04/28] Separated `os/mac/cache_store.rb` into `cache_store.rb` and `os/mac/linkage_cache_store.rb`. --- Library/Homebrew/cache_store.rb | 80 ++++++++++ Library/Homebrew/dev-cmd/linkage.rb | 1 + .../extend/os/mac/formula_cellar_checks.rb | 1 + Library/Homebrew/os/mac/cache_store.rb | 150 ------------------ .../Homebrew/os/mac/linkage_cache_store.rb | 97 +++++++++++ Library/Homebrew/os/mac/linkage_checker.rb | 22 ++- 6 files changed, 188 insertions(+), 163 deletions(-) create mode 100644 Library/Homebrew/cache_store.rb delete mode 100644 Library/Homebrew/os/mac/cache_store.rb create mode 100644 Library/Homebrew/os/mac/linkage_cache_store.rb diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb new file mode 100644 index 0000000000..07cc88363e --- /dev/null +++ b/Library/Homebrew/cache_store.rb @@ -0,0 +1,80 @@ +require "dbm" +require "json" + +# +# `DatabaseCache` acts as an interface to a persistent storage mechanism +# residing in the `HOMEBREW_CACHE` +# +class DatabaseCache + # The mode of any created files will be 0664 (that is, readable and writable + # by the owner and the group, and readable by everyone else) + DATABASE_MODE = 0664 + + # Opens and yields a database in read/write mode. Closes the database after use + # + # @yield [DBM] + # @return [nil] + def initialize(name) + # DBM::WRCREAT: Creates the database if it does not already exist + @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) + yield(@db) + @db.close + end +end + +# +# `CacheStore` provides methods to mutate and fetch data from a persistent +# storage mechanism +# +class CacheStore + # @param [DBM] database_cache + # @return [nil] + def initialize(database_cache) + @database_cache = database_cache + end + + # Inserts new values or updates existing cached values to persistent storage + # mechanism + # + # @abstract + def update!(*) + raise NotImplementedError + end + + # Fetches cached values in persistent storage according to the type of data + # stored + # + # @abstract + def fetch_type(*) + raise NotImplementedError + end + + # Deletes data from the cache based on a condition defined in a concrete class + # + # @abstract + def flush_cache! + raise NotImplementedError + end + + protected + + # @return [DBM] + attr_reader :database_cache + + # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data, + # to convert the ruby string back into a ruby `Hash`, the string is converted + # into a JSON compatible string in `ruby_hash_to_json_string`, where it may + # later be parsed by `JSON.parse` in the `json_string_to_ruby_hash` method + # + # @param [Hash] + # @return [String] + def ruby_hash_to_json_string(hash) + hash.to_json + end + + # @param [String] + # @return [Hash] + def json_string_to_ruby_hash(string) + JSON.parse(string) + end +end diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 87a0d9143e..c3c87e30df 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -13,6 +13,7 @@ #: If `--rebuild` is passed, flushes the `LinkageStore` cache for each #: 'keg.name' and forces a check on the dylibs. +require "cache_store" require "os/mac/linkage_checker" module Homebrew diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 2b62e6bca7..d381aa7eaf 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -1,3 +1,4 @@ +require "cache_store" require "os/mac/linkage_checker" module FormulaCellarChecks diff --git a/Library/Homebrew/os/mac/cache_store.rb b/Library/Homebrew/os/mac/cache_store.rb deleted file mode 100644 index 70868dce0c..0000000000 --- a/Library/Homebrew/os/mac/cache_store.rb +++ /dev/null @@ -1,150 +0,0 @@ -require "dbm" -require "json" - -# -# `DatabaseCache` acts as an interface to a persistent storage mechanism -# residing in the `HOMEBREW_CACHE` -# -class DatabaseCache - # The mode of any created files will be 0664 (that is, readable and writable - # by the owner and the group, and readable by everyone else) - DATABASE_MODE = 0664 - - # Opens and yields a database in read/write mode - def initialize(name) - # DBM::WRCREAT: Creates the database if it does not already exist - @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) - yield(@db) - @db.close - end -end - -# -# `CacheStore` provides methods to mutate and fetch data from a persistent -# storage mechanism -# -class CacheStore - def initialize(database_cache) - @database_cache = database_cache - end - - # Inserts new values or updates existing cached values to persistent storage - # mechanism - def update!(*) - raise NotImplementedError - end - - # Fetches cached values in persistent storage according to the type of data - # stored - def fetch_type(*) - raise NotImplementedError - end - - # Deletes data from the cache based on a condition defined in a concrete class - def flush_cache! - raise NotImplementedError - end - - protected - - # @return [DBM] - attr_reader :database_cache - - # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data, - # to convert the ruby string back into a ruby `Hash`, the string is converted - # into a JSON compatible string in `ruby_hash_to_json_string`, where it may - # later be parsed by `JSON.parse` in the `json_string_to_ruby_hash` method - # - # @param [Hash] - # @return [String] - def ruby_hash_to_json_string(hash) - hash.to_json - end - - # @param [String] - # @return [Hash] - def json_string_to_ruby_hash(string) - JSON.parse(string) - end -end - -# -# `LinkageStore` provides methods to fetch and mutate linkage-specific data used -# by the `brew linkage` command -# -class LinkageStore < CacheStore - HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze - - def initialize(keg_name, database_cache) - @keg_name = keg_name - super(database_cache) - end - - def update!( - array_values: { - system_dylibs: %w[], - variable_dylibs: %w[], - broken_dylibs: %w[], - indirect_deps: %w[], - undeclared_deps: %w[], - unnecessary_deps: %w[], - }, - hash_values: { - brewed_dylibs: {}, - reverse_links: {}, - } - ) - database_cache[keg_name] = ruby_hash_to_json_string( - array_values: format_array_values(array_values), - hash_values: format_hash_values(hash_values), - ) - end - - def fetch_type(type) - if HASH_LINKAGE_TYPES.include?(type) - fetch_hash_values(type: type) - else - fetch_array_values(type: type) - end - end - - def flush_cache! - database_cache.delete(keg_name) - end - - private - - attr_reader :keg_name - - def fetch_array_values(type:) - return [] unless database_cache.key?(keg_name) - json_string_to_ruby_hash(database_cache[keg_name])["array_values"][type.to_s] - end - - def fetch_hash_values(type:) - return {} unless database_cache.key?(keg_name) - json_string_to_ruby_hash(database_cache[keg_name])["hash_values"][type.to_s] - end - - # Formats the linkage data for `array_values` into a kind which can be parsed - # by the `json_string_to_ruby_hash` method. Internally converts ruby `Set`s to - # `Array`s - # - # @return [String] - def format_array_values(hash) - hash.each_with_object({}) { |(k, v), h| h[k] = v.to_a } - end - - # Formats the linkage data for `hash_values` into a kind which can be parsed - # by the `json_string_to_ruby_hash` method. Converts ruby `Set`s to `Array`s, and - # converts ruby `Pathname`s to `String`s - # - # @return [String] - def format_hash_values(hash) - hash.each_with_object({}) do |(outer_key, outer_values), outer_hash| - outer_hash[outer_key] = outer_values.each_with_object({}) do |(k, v), h| - h[k] = v.to_a.map(&:to_s) - end - end - end -end diff --git a/Library/Homebrew/os/mac/linkage_cache_store.rb b/Library/Homebrew/os/mac/linkage_cache_store.rb new file mode 100644 index 0000000000..5c5cfc65ca --- /dev/null +++ b/Library/Homebrew/os/mac/linkage_cache_store.rb @@ -0,0 +1,97 @@ +require "cache_store" + +# +# `LinkageStore` provides methods to fetch and mutate linkage-specific data used +# by the `brew linkage` command +# +class LinkageStore < CacheStore + HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze + + # @param [String] keg_name + # @param [DBM] database_cache + # @return [nil] + def initialize(keg_name, database_cache) + @keg_name = keg_name + super(database_cache) + end + + # Inserts dylib-related information into the cache if it does not exist or + # updates data into the linkage cache if it does exist + # + # @param [Hash] array_values + # @param [Hash] hash_values + # @param [Array[Hash]] values + # @return [nil] + def update!(array_values: {}, hash_values: {}, **values) + values.each do |key, value| + if value.is_a? Hash + hash_values[key] = value + else + array_values[key] = value + end + end + + database_cache[keg_name] = ruby_hash_to_json_string( + array_values: format_array_values(array_values), + hash_values: format_hash_values(hash_values), + ) + end + + # @param [Symbol] type + # @return [Hash | Array] + def fetch_type(type) + if HASH_LINKAGE_TYPES.include?(type) + fetch_hash_values(type) + else + fetch_array_values(type) + end + end + + # @return [nil] + def flush_cache! + database_cache.delete(keg_name) + end + + private + + # @return [String] + attr_reader :keg_name + + # @param [Symbol] type + # @return [Array] + def fetch_array_values(type) + return [] unless database_cache.key?(keg_name) + json_string_to_ruby_hash(database_cache[keg_name])["array_values"][type.to_s] + end + + # @param [Symbol] type + # @return [Hash] + def fetch_hash_values(type) + return {} unless database_cache.key?(keg_name) + json_string_to_ruby_hash(database_cache[keg_name])["hash_values"][type.to_s] + end + + # Formats the linkage data for `array_values` into a kind which can be parsed + # by the `json_string_to_ruby_hash` method. Internally converts ruby `Set`s to + # `Array`s + # + # @param [Hash] + # @return [String] + def format_array_values(hash) + hash.each_with_object({}) { |(k, v), h| h[k] = v.to_a } + end + + # Formats the linkage data for `hash_values` into a kind which can be parsed + # by the `json_string_to_ruby_hash` method. Converts ruby `Set`s to `Array`s, + # and converts ruby `Pathname`s to `String`s + # + # @param [Hash] + # @return [String] + def format_hash_values(hash) + hash.each_with_object({}) do |(outer_key, outer_values), outer_hash| + outer_hash[outer_key] = outer_values.each_with_object({}) do |(k, v), h| + h[k] = v.to_a.map(&:to_s) + end + end + end +end diff --git a/Library/Homebrew/os/mac/linkage_checker.rb b/Library/Homebrew/os/mac/linkage_checker.rb index ed5eb8acd0..d2da4e5055 100644 --- a/Library/Homebrew/os/mac/linkage_checker.rb +++ b/Library/Homebrew/os/mac/linkage_checker.rb @@ -1,7 +1,7 @@ require "set" require "keg" require "formula" -require "os/mac/cache_store" +require "os/mac/linkage_cache_store" class LinkageChecker attr_reader :keg, :formula, :store @@ -223,18 +223,14 @@ class LinkageChecker # Updates data store with package path values def store_dylibs! store.update!( - array_values: { - system_dylibs: @system_dylibs, - variable_dylibs: @variable_dylibs, - broken_dylibs: @broken_dylibs, - indirect_deps: @indirect_deps, - undeclared_deps: @undeclared_deps, - unnecessary_deps: @unnecessary_deps, - }, - hash_values: { - brewed_dylibs: @brewed_dylibs, - reverse_links: @reverse_links, - }, + system_dylibs: @system_dylibs, + variable_dylibs: @variable_dylibs, + broken_dylibs: @broken_dylibs, + indirect_deps: @indirect_deps, + undeclared_deps: @undeclared_deps, + unnecessary_deps: @unnecessary_deps, + brewed_dylibs: @brewed_dylibs, + reverse_links: @reverse_links, ) end end From c5a6724c5cb2bbf9ae8967d8bbff434989c88da2 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Tue, 6 Mar 2018 11:42:45 -0500 Subject: [PATCH 05/28] WIP --- Library/Homebrew/cache_store.rb | 2 +- Library/Homebrew/os/mac/linkage_cache_store.rb | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 07cc88363e..57b319248f 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -12,7 +12,7 @@ class DatabaseCache # Opens and yields a database in read/write mode. Closes the database after use # - # @yield [DBM] + # @yield [DBM] db # @return [nil] def initialize(name) # DBM::WRCREAT: Creates the database if it does not already exist diff --git a/Library/Homebrew/os/mac/linkage_cache_store.rb b/Library/Homebrew/os/mac/linkage_cache_store.rb index 5c5cfc65ca..4e2cb3ece5 100644 --- a/Library/Homebrew/os/mac/linkage_cache_store.rb +++ b/Library/Homebrew/os/mac/linkage_cache_store.rb @@ -5,7 +5,9 @@ require "cache_store" # by the `brew linkage` command # class LinkageStore < CacheStore - HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze + ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs, + :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze + HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze # @param [String] keg_name # @param [DBM] database_cache @@ -21,13 +23,16 @@ class LinkageStore < CacheStore # @param [Hash] array_values # @param [Hash] hash_values # @param [Array[Hash]] values + # @raise [TypeError] # @return [nil] def update!(array_values: {}, hash_values: {}, **values) values.each do |key, value| if value.is_a? Hash hash_values[key] = value - else + elsif value.is_a? Array array_values[key] = value + else + raise TypeError, "Can't store types that are not `Array` or `Hash` in the linkage store." end end @@ -37,13 +42,16 @@ class LinkageStore < CacheStore ) end - # @param [Symbol] type + # @param [Symbol] the type to fetch from the `LinkageStore` + # @raise [TypeError] # @return [Hash | Array] def fetch_type(type) if HASH_LINKAGE_TYPES.include?(type) fetch_hash_values(type) - else + elsif ARRAY_LINKAGE_TYPES.include?(type) fetch_array_values(type) + else + raise TypeError, "Can't fetch types that are not defined for the linkage store." end end From 8bd38d08cb65be99eac0cbb9cda98c8231057f06 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Tue, 6 Mar 2018 12:07:57 -0500 Subject: [PATCH 06/28] Move `linkage_cache_store.rb` to `~/Library/Homebrew`. --- Library/Homebrew/{os/mac => }/linkage_cache_store.rb | 5 +++-- Library/Homebrew/linkage_checker.rb | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) rename Library/Homebrew/{os/mac => }/linkage_cache_store.rb (97%) diff --git a/Library/Homebrew/os/mac/linkage_cache_store.rb b/Library/Homebrew/linkage_cache_store.rb similarity index 97% rename from Library/Homebrew/os/mac/linkage_cache_store.rb rename to Library/Homebrew/linkage_cache_store.rb index 4e2cb3ece5..cb306b9089 100644 --- a/Library/Homebrew/os/mac/linkage_cache_store.rb +++ b/Library/Homebrew/linkage_cache_store.rb @@ -1,3 +1,4 @@ +require "set" require "cache_store" # @@ -27,9 +28,9 @@ class LinkageStore < CacheStore # @return [nil] def update!(array_values: {}, hash_values: {}, **values) values.each do |key, value| - if value.is_a? Hash + if value.is_a?(Hash) hash_values[key] = value - elsif value.is_a? Array + elsif value.is_a?(Array) || value.is_a?(Set) array_values[key] = value else raise TypeError, "Can't store types that are not `Array` or `Hash` in the linkage store." diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 9b04dbc79a..0cf05441ea 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -1,7 +1,7 @@ require "set" require "keg" require "formula" -require "os/mac/linkage_cache_store" +require "linkage_cache_store" class LinkageChecker attr_reader :keg, :formula, :store From 2c7ae2544bfb28ea5ae70d45ed53d5d1c31eb7fc Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Tue, 6 Mar 2018 13:39:34 -0500 Subject: [PATCH 07/28] Updated documentation for `cache_store`. --- Library/Homebrew/cache_store.rb | 8 +++++--- Library/Homebrew/linkage_cache_store.rb | 22 ++++++++++++++-------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 57b319248f..1a9cf3be68 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -7,7 +7,9 @@ require "json" # class DatabaseCache # The mode of any created files will be 0664 (that is, readable and writable - # by the owner and the group, and readable by everyone else) + # by the owner and the group, and readable by everyone else). Files created + # will also be modified by the process' umask value at the time of creation: + # https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html DATABASE_MODE = 0664 # Opens and yields a database in read/write mode. Closes the database after use @@ -66,13 +68,13 @@ class CacheStore # into a JSON compatible string in `ruby_hash_to_json_string`, where it may # later be parsed by `JSON.parse` in the `json_string_to_ruby_hash` method # - # @param [Hash] + # @param [Hash] ruby `Hash` to be converted to `JSON` string # @return [String] def ruby_hash_to_json_string(hash) hash.to_json end - # @param [String] + # @param [String] `JSON` string to be converted to ruby `Hash` # @return [Hash] def json_string_to_ruby_hash(string) JSON.parse(string) diff --git a/Library/Homebrew/linkage_cache_store.rb b/Library/Homebrew/linkage_cache_store.rb index cb306b9089..fce17ecc05 100644 --- a/Library/Homebrew/linkage_cache_store.rb +++ b/Library/Homebrew/linkage_cache_store.rb @@ -21,10 +21,10 @@ class LinkageStore < CacheStore # Inserts dylib-related information into the cache if it does not exist or # updates data into the linkage cache if it does exist # - # @param [Hash] array_values - # @param [Hash] hash_values + # @param [Hash] array_values: hash containing KVPs of { :type => Array | Set } + # @param [Hash] hash_values: hash containing KVPs of { :type => Hash } # @param [Array[Hash]] values - # @raise [TypeError] + # @raise [TypeError] error if the values are not `Arary`, `Set`, or `Hash` # @return [nil] def update!(array_values: {}, hash_values: {}, **values) values.each do |key, value| @@ -33,7 +33,10 @@ class LinkageStore < CacheStore elsif value.is_a?(Array) || value.is_a?(Set) array_values[key] = value else - raise TypeError, "Can't store types that are not `Array` or `Hash` in the linkage store." + raise TypeError, <<~EOS + Can't store types that are not `Array`, `Set` or `Hash` in the + linkage store. + EOS end end @@ -44,7 +47,8 @@ class LinkageStore < CacheStore end # @param [Symbol] the type to fetch from the `LinkageStore` - # @raise [TypeError] + # @raise [TypeError] error if the type is not in `HASH_LINKAGE_TYPES` or + # `ARRAY_LINKAGE_TYPES` # @return [Hash | Array] def fetch_type(type) if HASH_LINKAGE_TYPES.include?(type) @@ -52,7 +56,9 @@ class LinkageStore < CacheStore elsif ARRAY_LINKAGE_TYPES.include?(type) fetch_array_values(type) else - raise TypeError, "Can't fetch types that are not defined for the linkage store." + raise TypeError, <<~EOS + Can't fetch types that are not defined for the linkage store + EOS end end @@ -63,10 +69,10 @@ class LinkageStore < CacheStore private - # @return [String] + # @return [String] the key to lookup items in the `CacheStore` attr_reader :keg_name - # @param [Symbol] type + # @param [Symbol] the type to fetch from the `LinkageStore` # @return [Array] def fetch_array_values(type) return [] unless database_cache.key?(keg_name) From bc76a8afcb87448e979c5f34e2198f349f634548 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Wed, 14 Mar 2018 10:57:08 -0400 Subject: [PATCH 08/28] Changed default behavior of `brew linkage` command to build cache instead of using cached output. Cached output may be printed with `--cached` flag. --- Library/Homebrew/dev-cmd/linkage.rb | 9 +++++---- Library/Homebrew/extend/os/mac/formula_cellar_checks.rb | 2 +- Library/Homebrew/linkage_checker.rb | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 391a129365..fe733e6a32 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -1,4 +1,4 @@ -#: * `linkage` [`--test`] [`--reverse`] [`--rebuild`] : +#: * `linkage` [`--test`] [`--reverse`] [`--cached`] : #: Checks the library links of an installed formula. #: #: Only works on installed formulae. An error is raised if it is run on @@ -10,8 +10,8 @@ #: If `--reverse` is passed, print the dylib followed by the binaries #: which link to it for each library the keg references. #: -#: If `--rebuild` is passed, flushes the `LinkageStore` cache for each -#: 'keg.name' and forces a check on the dylibs. +#: If `--cached` is passed, print the cached linkage values stored in +#: HOMEBREW_CACHE, set from a previous `brew linkage` run require "cache_store" require "linkage_checker" @@ -24,7 +24,8 @@ module Homebrew ARGV.kegs.each do |keg| ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 - result = LinkageChecker.new(keg, database_cache, ARGV.include?("--rebuild")) + use_cache = ARGV.include?("--cached") || ENV["HOMEBREW_LINKAGE_CACHE"] + result = LinkageChecker.new(keg, database_cache, use_cache) if ARGV.include?("--test") result.display_test_output diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 14be2f3b7e..f7e3a16935 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -66,7 +66,7 @@ module FormulaCellarChecks return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - DatabaseCache.new(:linkage) { |database_cache| LinkageChecker.new(keg, database_cache, true, formula) } + DatabaseCache.new(:linkage) { |database_cache| LinkageChecker.new(keg, database_cache, false, formula) } return unless checker.broken_dylibs? output = <<~EOS diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 0cf05441ea..a21d3a7cd5 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -6,11 +6,11 @@ require "linkage_cache_store" class LinkageChecker attr_reader :keg, :formula, :store - def initialize(keg, db, rebuild_cache = false, formula = nil) + def initialize(keg, db, use_cache = false, formula = nil) @keg = keg @formula = formula || resolve_formula(keg) @store = LinkageStore.new(keg.name, db) - flush_cache_and_check_dylibs if rebuild_cache + flush_cache_and_check_dylibs unless use_cache end # 'Hash-type' cache values From d5795d816ad16775541d2123d4d5fbcb3b7e5a4c Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Mon, 9 Apr 2018 14:00:48 -0400 Subject: [PATCH 09/28] Added `broken_deps` to `LinkageStore`. --- Library/Homebrew/linkage_cache_store.rb | 2 +- Library/Homebrew/linkage_checker.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/linkage_cache_store.rb b/Library/Homebrew/linkage_cache_store.rb index fce17ecc05..2816b681df 100644 --- a/Library/Homebrew/linkage_cache_store.rb +++ b/Library/Homebrew/linkage_cache_store.rb @@ -8,7 +8,7 @@ require "cache_store" class LinkageStore < CacheStore ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs, :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze - HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze + HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links, :broken_deps].freeze # @param [String] keg_name # @param [DBM] database_cache diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index f601dd667e..bc67d0b815 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -22,6 +22,10 @@ class LinkageChecker @reverse_links ||= store.fetch_type(:reverse_links) end + def broken_deps + @broken_deps ||= store.fetch_type(:broken_deps) + end + # 'Path-type' cached values def system_dylibs @@ -229,6 +233,7 @@ class LinkageChecker @variable_dylibs = Set.new @brewed_dylibs = Hash.new { |h, k| h[k] = Set.new } @reverse_links = Hash.new { |h, k| h[k] = Set.new } + @broken_deps = Hash.new { |h, k| h[k] = [] } @indirect_deps = [] @undeclared_deps = [] @unnecessary_deps = [] @@ -241,6 +246,7 @@ class LinkageChecker variable_dylibs: variable_dylibs, broken_dylibs: broken_dylibs, indirect_deps: indirect_deps, + broken_deps: broken_deps, undeclared_deps: undeclared_deps, unnecessary_deps: unnecessary_deps, brewed_dylibs: brewed_dylibs, From e5eaf57856218b0a14c3d35d6a9b06737d2724c8 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Mon, 9 Apr 2018 14:19:07 -0400 Subject: [PATCH 10/28] Fixed broken test due to changing usage of `DatabaseCache` to block usage. --- .../extend/os/mac/formula_cellar_checks.rb | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index f7e3a16935..bf2a086d3b 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -66,22 +66,25 @@ module FormulaCellarChecks return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - DatabaseCache.new(:linkage) { |database_cache| LinkageChecker.new(keg, database_cache, false, formula) } + DatabaseCache.new(:linkage) do |database_cache| + checker = LinkageChecker.new(keg, database_cache, false, formula) - return unless checker.broken_dylibs? - output = <<~EOS - #{formula} has broken dynamic library links: - #{checker.broken_dylibs.to_a * "\n "} - EOS - tab = Tab.for_keg(keg) - if tab.poured_from_bottle - output += <<~EOS - Rebuild this from source with: - brew reinstall --build-from-source #{formula} - If that's successful, file an issue#{formula.tap ? " here:\n #{formula.tap.issues_url}" : "."} + return unless checker.broken_dylibs? + output = <<~EOS + #{formula} has broken dynamic library links: + #{checker.broken_dylibs.to_a * "\n "} EOS + + tab = Tab.for_keg(keg) + if tab.poured_from_bottle + output += <<~EOS + Rebuild this from source with: + brew reinstall --build-from-source #{formula} + If that's successful, file an issue#{formula.tap ? " here:\n #{formula.tap.issues_url}" : "."} + EOS + end + problem_if_output output end - problem_if_output output end def audit_installed From 461d7d72de02e130f371b272e020eadd206a5f63 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Wed, 25 Apr 2018 10:25:00 -0400 Subject: [PATCH 11/28] Fixed failing tests in `formula.rb` caused by merge. --- Library/Homebrew/formula.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 5a146d4bcc..478cf9b57f 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1,3 +1,4 @@ +require "cache_store" require "formula_support" require "lock_file" require "formula_pin" @@ -1527,8 +1528,10 @@ class Formula keg = opt_or_installed_prefix_keg return [] unless keg - linkage_checker = LinkageChecker.new(keg, self) - linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } + DatabaseCache.new(:linkage) do |database_cache| + linkage_checker = LinkageChecker.new(keg, database_cache, false, self) + break linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } + end end # Returns a list of formulae depended on by this formula that aren't From a756af3b3217c285bfdfc44628644354ac3786d2 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Wed, 25 Apr 2018 10:27:03 -0400 Subject: [PATCH 12/28] Fixed style issue offenses from `brew style`. --- Library/Homebrew/extend/os/mac/formula_cellar_checks.rb | 2 +- Library/Homebrew/linkage_checker.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 7a3c4ef724..303f272b41 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -69,7 +69,7 @@ module FormulaCellarChecks DatabaseCache.new(:linkage) do |database_cache| checker = LinkageChecker.new(keg, database_cache, false, formula) - return unless checker.broken_library_linkage? + next unless checker.broken_library_linkage? output = <<~EOS #{formula} has broken dynamic library links: #{checker.display_test_output} diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index f0318d82c8..ab709b530c 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -50,7 +50,6 @@ class LinkageChecker @unnecessary_deps ||= store.fetch_type(:unnecessary_deps) end - def display_normal_output display_items "System libraries", system_dylibs display_items "Homebrew libraries", brewed_dylibs From 7af82ba20f82d68f09f11c1904213c98ba19c839 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 2 May 2018 09:59:18 +0100 Subject: [PATCH 13/28] Add `brew linkage --cached` manpage output. --- docs/Manpage.md | 5 ++++- manpages/brew-cask.1 | 2 +- manpages/brew.1 | 7 +++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docs/Manpage.md b/docs/Manpage.md index e4d6ce3901..d68d9784c0 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -779,7 +779,7 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note If `--pry` is passed or HOMEBREW_PRY is set, pry will be used instead of irb. - * `linkage` [`--test`] [`--reverse`] `formula`: + * `linkage` [`--test`] [`--reverse`] [`--cached`] `formula`: Checks the library links of an installed formula. Only works on installed formulae. An error is raised if it is run on @@ -791,6 +791,9 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note If `--reverse` is passed, print the dylib followed by the binaries which link to it for each library the keg references. + If `--cached` is passed, print the cached linkage values stored in + HOMEBREW_CACHE, set from a previous `brew linkage` run + * `man` [`--fail-if-changed`]: Generate Homebrew's manpages. diff --git a/manpages/brew-cask.1 b/manpages/brew-cask.1 index 346c45d6a8..586d4ff1c9 100644 --- a/manpages/brew-cask.1 +++ b/manpages/brew-cask.1 @@ -1,7 +1,7 @@ .\" generated with Ronn/v0.7.3 .\" http://github.com/rtomayko/ronn/tree/0.7.3 . -.TH "BREW\-CASK" "1" "April 2018" "Homebrew" "brew-cask" +.TH "BREW\-CASK" "1" "May 2018" "Homebrew" "brew-cask" . .SH "NAME" \fBbrew\-cask\fR \- a friendly binary installer for macOS diff --git a/manpages/brew.1 b/manpages/brew.1 index 08fffd289a..a083aa134f 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -1,7 +1,7 @@ .\" generated with Ronn/v0.7.3 .\" http://github.com/rtomayko/ronn/tree/0.7.3 . -.TH "BREW" "1" "April 2018" "Homebrew" "brew" +.TH "BREW" "1" "May 2018" "Homebrew" "brew" . .SH "NAME" \fBbrew\fR \- The missing package manager for macOS @@ -795,7 +795,7 @@ Enter the interactive Homebrew Ruby shell\. If \fB\-\-examples\fR is passed, several examples will be shown\. If \fB\-\-pry\fR is passed or HOMEBREW_PRY is set, pry will be used instead of irb\. . .TP -\fBlinkage\fR [\fB\-\-test\fR] [\fB\-\-reverse\fR] \fIformula\fR +\fBlinkage\fR [\fB\-\-test\fR] [\fB\-\-reverse\fR] [\fB\-\-cached\fR] \fIformula\fR Checks the library links of an installed formula\. . .IP @@ -807,6 +807,9 @@ If \fB\-\-test\fR is passed, only display missing libraries and exit with a non\ .IP If \fB\-\-reverse\fR is passed, print the dylib followed by the binaries which link to it for each library the keg references\. . +.IP +If \fB\-\-cached\fR is passed, print the cached linkage values stored in HOMEBREW_CACHE, set from a previous \fBbrew linkage\fR run +. .TP \fBman\fR [\fB\-\-fail\-if\-changed\fR] Generate Homebrew\'s manpages\. From d3120c020605ca288e5cbfec306d7b39a33ad2fc Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Sun, 6 May 2018 15:55:33 -0400 Subject: [PATCH 14/28] Use cache if `HOMEBREW_LINKAGE_CACHE` exists in `formula_cellar_checks` and `formula`. Make functions private in `LinkageChecker`. --- .../extend/os/mac/formula_cellar_checks.rb | 3 +- Library/Homebrew/formula.rb | 3 +- Library/Homebrew/linkage_checker.rb | 80 +++++++++---------- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 303f272b41..4e09799cdd 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -67,7 +67,8 @@ module FormulaCellarChecks keg = Keg.new(formula.prefix) DatabaseCache.new(:linkage) do |database_cache| - checker = LinkageChecker.new(keg, database_cache, false, formula) + use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? + checker = LinkageChecker.new(keg, database_cache, use_cache, formula) next unless checker.broken_library_linkage? output = <<~EOS diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 85d0998e6f..744038fbaf 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1528,7 +1528,8 @@ class Formula return [] unless keg DatabaseCache.new(:linkage) do |database_cache| - linkage_checker = LinkageChecker.new(keg, database_cache, false, self) + use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? + linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self) break linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end end diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 623dcbaf99..541f95848d 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -10,46 +10,6 @@ class LinkageChecker flush_cache_and_check_dylibs unless use_cache end - # 'Hash-type' cache values - - def brewed_dylibs - @brewed_dylibs ||= store.fetch_type(:brewed_dylibs) - end - - def reverse_links - @reverse_links ||= store.fetch_type(:reverse_links) - end - - def broken_deps - @broken_deps ||= store.fetch_type(:broken_deps) - end - - # 'Path-type' cached values - - def system_dylibs - @system_dylibs ||= store.fetch_type(:system_dylibs) - end - - def broken_dylibs - @broken_dylibs ||= store.fetch_type(:broken_dylibs) - end - - def variable_dylibs - @variable_dylibs ||= store.fetch_type(:variable_dylibs) - end - - def undeclared_deps - @undeclared_deps ||= store.fetch_type(:undeclared_deps) - end - - def indirect_deps - @indirect_deps ||= store.fetch_type(:indirect_deps) - end - - def unnecessary_deps - @unnecessary_deps ||= store.fetch_type(:unnecessary_deps) - end - def display_normal_output display_items "System libraries", system_dylibs display_items "Homebrew libraries", brewed_dylibs @@ -84,10 +44,50 @@ class LinkageChecker !@broken_dylibs.empty? || !@broken_deps.empty? end + def undeclared_deps + @undeclared_deps ||= store.fetch_type(:undeclared_deps) + end + private attr_reader :keg, :formula, :store + # 'Hash-type' cache values + + def brewed_dylibs + @brewed_dylibs ||= store.fetch_type(:brewed_dylibs) + end + + def reverse_links + @reverse_links ||= store.fetch_type(:reverse_links) + end + + def broken_deps + @broken_deps ||= store.fetch_type(:broken_deps) + end + + # 'Path-type' cached values + + def system_dylibs + @system_dylibs ||= store.fetch_type(:system_dylibs) + end + + def broken_dylibs + @broken_dylibs ||= store.fetch_type(:broken_dylibs) + end + + def variable_dylibs + @variable_dylibs ||= store.fetch_type(:variable_dylibs) + end + + def indirect_deps + @indirect_deps ||= store.fetch_type(:indirect_deps) + end + + def unnecessary_deps + @unnecessary_deps ||= store.fetch_type(:unnecessary_deps) + end + def dylib_to_dep(dylib) dylib =~ %r{#{Regexp.escape(HOMEBREW_PREFIX)}/(opt|Cellar)/([\w+-.@]+)/} Regexp.last_match(2) From b0dd3f4c9964ddba09a308ee1853ccef2bb29a2e Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Tue, 15 May 2018 12:49:48 -0400 Subject: [PATCH 15/28] Update the invalidated cache after doing a formula install. --- Library/Homebrew/formula_installer.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 7ed473ee89..6514a919cf 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -12,6 +12,8 @@ require "debrew" require "sandbox" require "emoji" require "development_tools" +require "cache_store" +require "linkage_checker" class FormulaInstaller include FormulaCellarChecks @@ -608,6 +610,11 @@ class FormulaInstaller ohai "Summary" if verbose? || show_summary_heading? puts summary + # Updates the cache for a particular formula after doing an install + DatabaseCache.new(:linkage) do |database_cache| + LinkageChecker.new(keg, database_cache, false, formula) + end + # let's reset Utils.git_available? if we just installed git Utils.clear_git_available_cache if formula.name == "git" From 17f484b0ae8dba08a676ddb4ca04ed92200e47df Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Tue, 15 May 2018 13:15:30 -0400 Subject: [PATCH 16/28] Set the `return_value` from the `initialize` block in the `DatabaseCache`. --- Library/Homebrew/cache_store.rb | 5 ++++- Library/Homebrew/formula.rb | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 1a9cf3be68..a94e1e98e1 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -12,6 +12,9 @@ class DatabaseCache # https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html DATABASE_MODE = 0664 + # Returned value from `initialize` block + attr_reader :return_value + # Opens and yields a database in read/write mode. Closes the database after use # # @yield [DBM] db @@ -19,7 +22,7 @@ class DatabaseCache def initialize(name) # DBM::WRCREAT: Creates the database if it does not already exist @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) - yield(@db) + @return_value = yield(@db) @db.close end end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 744038fbaf..b130c508ec 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1527,11 +1527,13 @@ class Formula keg = opt_or_installed_prefix_keg return [] unless keg - DatabaseCache.new(:linkage) do |database_cache| + cache = DatabaseCache.new(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self) - break linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } + linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end + + cache.return_value end # Returns a list of formulae depended on by this formula that aren't From ba5692f7e552f3b32963df178fafcbf0cc56a7e1 Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Thu, 17 May 2018 13:07:56 -0400 Subject: [PATCH 17/28] Only update the cache for a formula if the cache is not `empty?`. --- Library/Homebrew/formula_installer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 6514a919cf..92d4b2da3e 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -612,7 +612,7 @@ class FormulaInstaller # Updates the cache for a particular formula after doing an install DatabaseCache.new(:linkage) do |database_cache| - LinkageChecker.new(keg, database_cache, false, formula) + LinkageChecker.new(keg, database_cache, false, formula) unless database_cache.empty? end # let's reset Utils.git_available? if we just installed git From 4fb14d5c862278ff3992e62d634695f3e381456b Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Thu, 17 May 2018 16:35:35 -0400 Subject: [PATCH 18/28] `break if database_cache.empty?` --- Library/Homebrew/formula_installer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 92d4b2da3e..aa347a7ba5 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -612,7 +612,8 @@ class FormulaInstaller # Updates the cache for a particular formula after doing an install DatabaseCache.new(:linkage) do |database_cache| - LinkageChecker.new(keg, database_cache, false, formula) unless database_cache.empty? + break if database_cache.empty? + LinkageChecker.new(keg, database_cache, false, formula) end # let's reset Utils.git_available? if we just installed git From ddd2ec05d82cd8ff175fddd60d847c7e8a232b6e Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Thu, 17 May 2018 16:56:59 -0400 Subject: [PATCH 19/28] Added static `use` method for `DatabaseCache`. --- Library/Homebrew/cache_store.rb | 6 ++++++ Library/Homebrew/formula.rb | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index a94e1e98e1..3328235329 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -25,6 +25,12 @@ class DatabaseCache @return_value = yield(@db) @db.close end + + def self.use(type) + return_value = nil + DatabaseCache.new(type) { |db| return_value = yield(db) } + return_value + end end # diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index b130c508ec..2f4fc73ae7 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1527,13 +1527,13 @@ class Formula keg = opt_or_installed_prefix_keg return [] unless keg - cache = DatabaseCache.new(:linkage) do |database_cache| + undeclared_deps = DatabaseCache.use(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self) linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end - cache.return_value + undeclared_deps end # Returns a list of formulae depended on by this formula that aren't From cd6f89ca764791ee7ae3c706dec2e9427cd01f60 Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Fri, 18 May 2018 10:06:30 -0400 Subject: [PATCH 20/28] Made `DatabaseCache.new` private, and changes instances in code that call it to use `DatabaseCache.use` instead. --- Library/Homebrew/cache_store.rb | 17 ++++++++--------- Library/Homebrew/dev-cmd/linkage.rb | 2 +- .../extend/os/mac/formula_cellar_checks.rb | 2 +- Library/Homebrew/formula_installer.rb | 2 +- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 3328235329..a9b28f704b 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -12,8 +12,13 @@ class DatabaseCache # https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html DATABASE_MODE = 0664 - # Returned value from `initialize` block - attr_reader :return_value + def self.use(type) + return_value = nil + DatabaseCache.new(type) { |db| return_value = yield(db) } + return_value + end + + private # Opens and yields a database in read/write mode. Closes the database after use # @@ -22,15 +27,9 @@ class DatabaseCache def initialize(name) # DBM::WRCREAT: Creates the database if it does not already exist @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) - @return_value = yield(@db) + yield(@db) @db.close end - - def self.use(type) - return_value = nil - DatabaseCache.new(type) { |db| return_value = yield(db) } - return_value - end end # diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 7f2fbe62c3..01bdfda57f 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -20,7 +20,7 @@ module Homebrew module_function def linkage - DatabaseCache.new(:linkage) do |database_cache| + DatabaseCache.use(:linkage) do |database_cache| ARGV.kegs.each do |keg| ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 4e09799cdd..bd78f0e30b 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -66,7 +66,7 @@ module FormulaCellarChecks return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - DatabaseCache.new(:linkage) do |database_cache| + DatabaseCache.use(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? checker = LinkageChecker.new(keg, database_cache, use_cache, formula) diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index aa347a7ba5..946a6fddb3 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -611,7 +611,7 @@ class FormulaInstaller puts summary # Updates the cache for a particular formula after doing an install - DatabaseCache.new(:linkage) do |database_cache| + DatabaseCache.use(:linkage) do |database_cache| break if database_cache.empty? LinkageChecker.new(keg, database_cache, false, formula) end From e93e8f3266a28fb211e49bb59138f5bd82b0f8af Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Fri, 18 May 2018 16:37:01 -0400 Subject: [PATCH 21/28] Lazily load db of type `DBM` instance variable for `DatabaseCache` so the corresponding database file isn't created in the `.use` block for a `DatabaseCache`. --- Library/Homebrew/cache_store.rb | 54 ++++++++++++++----- Library/Homebrew/dev-cmd/linkage.rb | 2 +- .../extend/os/mac/formula_cellar_checks.rb | 2 +- Library/Homebrew/formula.rb | 2 +- Library/Homebrew/formula_installer.rb | 2 +- Library/Homebrew/linkage_cache_store.rb | 18 +++---- 6 files changed, 55 insertions(+), 25 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index a9b28f704b..8a1c7c334d 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -14,21 +14,51 @@ class DatabaseCache def self.use(type) return_value = nil - DatabaseCache.new(type) { |db| return_value = yield(db) } + + DatabaseCache.new(type) do |database_cache| + return_value = yield(database_cache) + end + return_value end + # Lazily loaded database in read/write mode. If this method is called, a + # database file with be created in the `HOMEBREW_CACHE` with name + # corresponding to the `@type` instance variable + # + # @return [DBM] db + def db + # DBM::WRCREAT: Creates the database if it does not already exist + @db ||= DBM.open(cache_path, DATABASE_MODE, DBM::WRCREAT) + end + + # Returns `true` if the cache is empty for the given `@type` + # + # @return [Boolean] + def empty? + !File.exist?(cache_path) + end + private - # Opens and yields a database in read/write mode. Closes the database after use + # Opens and yields the cache. Closes the database after use if it has been + # loaded # - # @yield [DBM] db + # @param [Symbol] type + # @yield [DatabaseCache] self # @return [nil] - def initialize(name) - # DBM::WRCREAT: Creates the database if it does not already exist - @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT) - yield(@db) - @db.close + def initialize(type) + @type = type + yield(self) + @db&.close + end + + # The path where the database resides in the `HOMEBREW_CACHE` for the given + # `@type` + # + # @return [String] + def cache_path + File.join(HOMEBREW_CACHE, "#{@type}.db") end end @@ -37,10 +67,10 @@ end # storage mechanism # class CacheStore - # @param [DBM] database_cache + # @param [DBM] db # @return [nil] - def initialize(database_cache) - @database_cache = database_cache + def initialize(db) + @db = db end # Inserts new values or updates existing cached values to persistent storage @@ -69,7 +99,7 @@ class CacheStore protected # @return [DBM] - attr_reader :database_cache + attr_reader :db # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data, # to convert the ruby string back into a ruby `Hash`, the string is converted diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 01bdfda57f..d0370310f5 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -25,7 +25,7 @@ module Homebrew ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 use_cache = ARGV.include?("--cached") || ENV["HOMEBREW_LINKAGE_CACHE"] - result = LinkageChecker.new(keg, database_cache, use_cache) + result = LinkageChecker.new(keg, database_cache.db, use_cache) if ARGV.include?("--test") result.display_test_output diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index bd78f0e30b..012716b10b 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -68,7 +68,7 @@ module FormulaCellarChecks DatabaseCache.use(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? - checker = LinkageChecker.new(keg, database_cache, use_cache, formula) + checker = LinkageChecker.new(keg, database_cache.db, use_cache, formula) next unless checker.broken_library_linkage? output = <<~EOS diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 2f4fc73ae7..8ff186c8f4 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1529,7 +1529,7 @@ class Formula undeclared_deps = DatabaseCache.use(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? - linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self) + linkage_checker = LinkageChecker.new(keg, database_cache.db, use_cache, self) linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 946a6fddb3..8901639fa6 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -613,7 +613,7 @@ class FormulaInstaller # Updates the cache for a particular formula after doing an install DatabaseCache.use(:linkage) do |database_cache| break if database_cache.empty? - LinkageChecker.new(keg, database_cache, false, formula) + LinkageChecker.new(keg, database_cache.db, false, formula) end # let's reset Utils.git_available? if we just installed git diff --git a/Library/Homebrew/linkage_cache_store.rb b/Library/Homebrew/linkage_cache_store.rb index 2816b681df..d6ca753abe 100644 --- a/Library/Homebrew/linkage_cache_store.rb +++ b/Library/Homebrew/linkage_cache_store.rb @@ -11,11 +11,11 @@ class LinkageStore < CacheStore HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links, :broken_deps].freeze # @param [String] keg_name - # @param [DBM] database_cache + # @param [DBM] db # @return [nil] - def initialize(keg_name, database_cache) + def initialize(keg_name, db) @keg_name = keg_name - super(database_cache) + super(db) end # Inserts dylib-related information into the cache if it does not exist or @@ -40,7 +40,7 @@ class LinkageStore < CacheStore end end - database_cache[keg_name] = ruby_hash_to_json_string( + db[keg_name] = ruby_hash_to_json_string( array_values: format_array_values(array_values), hash_values: format_hash_values(hash_values), ) @@ -64,7 +64,7 @@ class LinkageStore < CacheStore # @return [nil] def flush_cache! - database_cache.delete(keg_name) + db.delete(keg_name) end private @@ -75,15 +75,15 @@ class LinkageStore < CacheStore # @param [Symbol] the type to fetch from the `LinkageStore` # @return [Array] def fetch_array_values(type) - return [] unless database_cache.key?(keg_name) - json_string_to_ruby_hash(database_cache[keg_name])["array_values"][type.to_s] + return [] unless db.key?(keg_name) + json_string_to_ruby_hash(db[keg_name])["array_values"][type.to_s] end # @param [Symbol] type # @return [Hash] def fetch_hash_values(type) - return {} unless database_cache.key?(keg_name) - json_string_to_ruby_hash(database_cache[keg_name])["hash_values"][type.to_s] + return {} unless db.key?(keg_name) + json_string_to_ruby_hash(db[keg_name])["hash_values"][type.to_s] end # Formats the linkage data for `array_values` into a kind which can be parsed From 360a30150350aff6841a4f530add2c67487278c1 Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Mon, 21 May 2018 12:13:03 -0400 Subject: [PATCH 22/28] Fix file path issue caused by dbm implicitly appending `.db` to end of file path provided. --- Library/Homebrew/cache_store.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 8a1c7c334d..f313ee4c06 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -29,7 +29,7 @@ class DatabaseCache # @return [DBM] db def db # DBM::WRCREAT: Creates the database if it does not already exist - @db ||= DBM.open(cache_path, DATABASE_MODE, DBM::WRCREAT) + @db ||= DBM.open(dbm_file_path, DATABASE_MODE, DBM::WRCREAT) end # Returns `true` if the cache is empty for the given `@type` @@ -53,12 +53,20 @@ class DatabaseCache @db&.close end + # `DBM` appends `.db` file extension to the path provided, which is why it's + # not included + # + # @return [String] + def dbm_file_path + File.join(HOMEBREW_CACHE, @type.to_s) + end + # The path where the database resides in the `HOMEBREW_CACHE` for the given # `@type` # # @return [String] def cache_path - File.join(HOMEBREW_CACHE, "#{@type}.db") + "#{dbm_file_path}.db" end end From 010207b9823ebe6781a30a9de48764d2447611a2 Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Mon, 21 May 2018 16:15:52 -0400 Subject: [PATCH 23/28] Changed cache usage behavior. 1. Running `brew linkage some_package` does not set the cache. 2. Running `brew linkage --cached some_package` when `DatabaseCache.empty?` returns `true` should build the cache. 3. Running `brew linkage --cached some_package` when `DatabaseCache.empty?` returns `false` should use the cache. --- Library/Homebrew/dev-cmd/linkage.rb | 2 +- .../extend/os/mac/formula_cellar_checks.rb | 2 +- Library/Homebrew/formula.rb | 2 +- Library/Homebrew/formula_installer.rb | 2 +- Library/Homebrew/linkage_checker.rb | 35 +++++++++++-------- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index d0370310f5..01bdfda57f 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -25,7 +25,7 @@ module Homebrew ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 use_cache = ARGV.include?("--cached") || ENV["HOMEBREW_LINKAGE_CACHE"] - result = LinkageChecker.new(keg, database_cache.db, use_cache) + result = LinkageChecker.new(keg, database_cache, use_cache) if ARGV.include?("--test") result.display_test_output diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 012716b10b..bd78f0e30b 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -68,7 +68,7 @@ module FormulaCellarChecks DatabaseCache.use(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? - checker = LinkageChecker.new(keg, database_cache.db, use_cache, formula) + checker = LinkageChecker.new(keg, database_cache, use_cache, formula) next unless checker.broken_library_linkage? output = <<~EOS diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 8ff186c8f4..2f4fc73ae7 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1529,7 +1529,7 @@ class Formula undeclared_deps = DatabaseCache.use(:linkage) do |database_cache| use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? - linkage_checker = LinkageChecker.new(keg, database_cache.db, use_cache, self) + linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self) linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 8901639fa6..946a6fddb3 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -613,7 +613,7 @@ class FormulaInstaller # Updates the cache for a particular formula after doing an install DatabaseCache.use(:linkage) do |database_cache| break if database_cache.empty? - LinkageChecker.new(keg, database_cache.db, false, formula) + LinkageChecker.new(keg, database_cache, false, formula) end # let's reset Utils.git_available? if we just installed git diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 541f95848d..4cf84e1836 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -3,11 +3,16 @@ require "formula" require "linkage_cache_store" class LinkageChecker - def initialize(keg, db, use_cache = false, formula = nil) + def initialize(keg, database_cache, use_cache = false, formula = nil) @keg = keg @formula = formula || resolve_formula(keg) - @store = LinkageStore.new(keg.name, db) - flush_cache_and_check_dylibs unless use_cache + + if use_cache + @store = LinkageStore.new(keg.name, database_cache.db) + flush_cache_and_check_dylibs unless database_cache.db.key?(keg.name) + else + flush_cache_and_check_dylibs + end end def display_normal_output @@ -45,7 +50,7 @@ class LinkageChecker end def undeclared_deps - @undeclared_deps ||= store.fetch_type(:undeclared_deps) + @undeclared_deps ||= store.nil? ? [] : store.fetch_type(:undeclared_deps) end private @@ -55,37 +60,37 @@ class LinkageChecker # 'Hash-type' cache values def brewed_dylibs - @brewed_dylibs ||= store.fetch_type(:brewed_dylibs) + @brewed_dylibs ||= store.nil? ? {} : store.fetch_type(:brewed_dylibs) end def reverse_links - @reverse_links ||= store.fetch_type(:reverse_links) + @reverse_links ||= store.nil? ? {} : store.fetch_type(:reverse_links) end def broken_deps - @broken_deps ||= store.fetch_type(:broken_deps) + @broken_deps ||= store.nil? ? {} : store.fetch_type(:broken_deps) end # 'Path-type' cached values def system_dylibs - @system_dylibs ||= store.fetch_type(:system_dylibs) + @system_dylibs ||= store.nil? ? [] : store.fetch_type(:system_dylibs) end def broken_dylibs - @broken_dylibs ||= store.fetch_type(:broken_dylibs) + @broken_dylibs ||= store.nil? ? [] : store.fetch_type(:broken_dylibs) end def variable_dylibs - @variable_dylibs ||= store.fetch_type(:variable_dylibs) + @variable_dylibs ||= store.nil? ? [] : store.fetch_type(:variable_dylibs) end def indirect_deps - @indirect_deps ||= store.fetch_type(:indirect_deps) + @indirect_deps ||= store.nil? ? [] : store.fetch_type(:indirect_deps) end def unnecessary_deps - @unnecessary_deps ||= store.fetch_type(:unnecessary_deps) + @unnecessary_deps ||= store.nil? ? [] : store.fetch_type(:unnecessary_deps) end def dylib_to_dep(dylib) @@ -235,9 +240,9 @@ class LinkageChecker opoo "Formula unavailable: #{keg.name}" end - # Helper function to reset dylib values when building cache + # Helper function to reset dylib values def reset_dylibs! - store.flush_cache! + store&.flush_cache! @system_dylibs = Set.new @broken_dylibs = Set.new @variable_dylibs = Set.new @@ -251,7 +256,7 @@ class LinkageChecker # Updates data store with package path values def store_dylibs! - store.update!( + store&.update!( system_dylibs: system_dylibs, variable_dylibs: variable_dylibs, broken_dylibs: broken_dylibs, From 44f5d3ec7982c5e6f82a0fc85703c5c48419fcd0 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 22 May 2018 14:46:14 +0100 Subject: [PATCH 24/28] Refactor cache store code. --- Library/Homebrew/cache_store.rb | 82 ++++++++++++------- Library/Homebrew/dev-cmd/linkage.rb | 4 +- .../extend/os/mac/formula_cellar_checks.rb | 8 +- Library/Homebrew/formula.rb | 6 +- Library/Homebrew/formula_installer.rb | 6 +- Library/Homebrew/linkage_cache_store.rb | 43 ++++++---- Library/Homebrew/linkage_checker.rb | 32 ++++---- 7 files changed, 107 insertions(+), 74 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index f313ee4c06..fc95550c09 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -2,26 +2,59 @@ require "dbm" require "json" # -# `DatabaseCache` acts as an interface to a persistent storage mechanism +# `CacheStoreDatabase` acts as an interface to a persistent storage mechanism # residing in the `HOMEBREW_CACHE` # -class DatabaseCache +class CacheStoreDatabase + # Yields the cache store database. + # Closes the database after use if it has been loaded. + # + # @param [Symbol] type + # @yield [CacheStoreDatabase] self + def self.use(type) + database = CacheStoreDatabase.new(type) + return_value = yield(database) + database.close_if_open! + return_value + end + + # Sets a value in the underlying database (and creates it if necessary). + def set(key, value) + db[key] = value + end + + # Gets a value from the underlying database (if it already exists). + def get(key) + return unless created? + db[key] + end + + # Gets a value from the underlying database (if it already exists). + def delete(key) + return unless created? + db.delete(key) + end + + # Closes the underlying database (if it created and open). + def close_if_open! + @db&.close + end + + # Returns `true` if the cache file has been created for the given `@type` + # + # @return [Boolean] + def created? + File.exist?(cache_path) + end + + private + # The mode of any created files will be 0664 (that is, readable and writable # by the owner and the group, and readable by everyone else). Files created # will also be modified by the process' umask value at the time of creation: # https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html DATABASE_MODE = 0664 - def self.use(type) - return_value = nil - - DatabaseCache.new(type) do |database_cache| - return_value = yield(database_cache) - end - - return_value - end - # Lazily loaded database in read/write mode. If this method is called, a # database file with be created in the `HOMEBREW_CACHE` with name # corresponding to the `@type` instance variable @@ -32,25 +65,12 @@ class DatabaseCache @db ||= DBM.open(dbm_file_path, DATABASE_MODE, DBM::WRCREAT) end - # Returns `true` if the cache is empty for the given `@type` - # - # @return [Boolean] - def empty? - !File.exist?(cache_path) - end - - private - - # Opens and yields the cache. Closes the database after use if it has been - # loaded + # Creates a CacheStoreDatabase # # @param [Symbol] type - # @yield [DatabaseCache] self # @return [nil] def initialize(type) @type = type - yield(self) - @db&.close end # `DBM` appends `.db` file extension to the path provided, which is why it's @@ -75,10 +95,10 @@ end # storage mechanism # class CacheStore - # @param [DBM] db + # @param [CacheStoreDatabase] database # @return [nil] - def initialize(db) - @db = db + def initialize(database) + @database = database end # Inserts new values or updates existing cached values to persistent storage @@ -106,8 +126,8 @@ class CacheStore protected - # @return [DBM] - attr_reader :db + # @return [CacheStoreDatabase] + attr_reader :database # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data, # to convert the ruby string back into a ruby `Hash`, the string is converted diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index 01bdfda57f..65f38e8502 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -20,12 +20,12 @@ module Homebrew module_function def linkage - DatabaseCache.use(:linkage) do |database_cache| + CacheStoreDatabase.use(:linkage) do |db| ARGV.kegs.each do |keg| ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1 use_cache = ARGV.include?("--cached") || ENV["HOMEBREW_LINKAGE_CACHE"] - result = LinkageChecker.new(keg, database_cache, use_cache) + result = LinkageChecker.new(keg, use_cache: use_cache, cache_db: db) if ARGV.include?("--test") result.display_test_output diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index bd78f0e30b..c09ed11a06 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -66,11 +66,11 @@ module FormulaCellarChecks return unless formula.prefix.directory? keg = Keg.new(formula.prefix) - DatabaseCache.use(:linkage) do |database_cache| - use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? - checker = LinkageChecker.new(keg, database_cache, use_cache, formula) - + CacheStoreDatabase.use(:linkage) do |db| + checker = LinkageChecker.new keg, formula, cache_db: db, + use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil? next unless checker.broken_library_linkage? + output = <<~EOS #{formula} has broken dynamic library links: #{checker.display_test_output} diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 2f4fc73ae7..0560e6cd16 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1527,9 +1527,9 @@ class Formula keg = opt_or_installed_prefix_keg return [] unless keg - undeclared_deps = DatabaseCache.use(:linkage) do |database_cache| - use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil? - linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self) + undeclared_deps = CacheStoreDatabase.use(:linkage) do |db| + linkage_checker = LinkageChecker.new keg, self, cache_db: db, + use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil? linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 946a6fddb3..3662be4f79 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -611,9 +611,9 @@ class FormulaInstaller puts summary # Updates the cache for a particular formula after doing an install - DatabaseCache.use(:linkage) do |database_cache| - break if database_cache.empty? - LinkageChecker.new(keg, database_cache, false, formula) + CacheStoreDatabase.use(:linkage) do |db| + break unless db.created? + LinkageChecker.new(keg, formula, cache_db: db) end # let's reset Utils.git_available? if we just installed git diff --git a/Library/Homebrew/linkage_cache_store.rb b/Library/Homebrew/linkage_cache_store.rb index d6ca753abe..a3d1625770 100644 --- a/Library/Homebrew/linkage_cache_store.rb +++ b/Library/Homebrew/linkage_cache_store.rb @@ -2,20 +2,23 @@ require "set" require "cache_store" # -# `LinkageStore` provides methods to fetch and mutate linkage-specific data used +# `LinkageCacheStore` provides methods to fetch and mutate linkage-specific data used # by the `brew linkage` command # -class LinkageStore < CacheStore - ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs, - :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze - HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links, :broken_deps].freeze - +class LinkageCacheStore < CacheStore # @param [String] keg_name - # @param [DBM] db + # @param [CacheStoreDatabase] database # @return [nil] - def initialize(keg_name, db) + def initialize(keg_name, database) @keg_name = keg_name - super(db) + super(database) + end + + # Returns `true` if the database has any value for the current `keg_name` + # + # @return [Boolean] + def has_keg_name? + !database.get(keg_name).nil? end # Inserts dylib-related information into the cache if it does not exist or @@ -40,13 +43,13 @@ class LinkageStore < CacheStore end end - db[keg_name] = ruby_hash_to_json_string( + database.set keg_name, ruby_hash_to_json_string( array_values: format_array_values(array_values), hash_values: format_hash_values(hash_values), ) end - # @param [Symbol] the type to fetch from the `LinkageStore` + # @param [Symbol] the type to fetch from the `LinkageCacheStore` # @raise [TypeError] error if the type is not in `HASH_LINKAGE_TYPES` or # `ARRAY_LINKAGE_TYPES` # @return [Hash | Array] @@ -64,26 +67,32 @@ class LinkageStore < CacheStore # @return [nil] def flush_cache! - db.delete(keg_name) + database.delete(keg_name) end private + ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs, + :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze + HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links, :broken_deps].freeze + # @return [String] the key to lookup items in the `CacheStore` attr_reader :keg_name - # @param [Symbol] the type to fetch from the `LinkageStore` + # @param [Symbol] the type to fetch from the `LinkageCacheStore` # @return [Array] def fetch_array_values(type) - return [] unless db.key?(keg_name) - json_string_to_ruby_hash(db[keg_name])["array_values"][type.to_s] + keg_cache = database.get(keg_name) + return [] unless keg_cache + json_string_to_ruby_hash(keg_cache)["array_values"][type.to_s] end # @param [Symbol] type # @return [Hash] def fetch_hash_values(type) - return {} unless db.key?(keg_name) - json_string_to_ruby_hash(db[keg_name])["hash_values"][type.to_s] + keg_cache = database.get(keg_name) + return {} unless keg_cache + json_string_to_ruby_hash(keg_cache)["hash_values"][type.to_s] end # Formats the linkage data for `array_values` into a kind which can be parsed diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 4cf84e1836..10d79a03f3 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -3,13 +3,13 @@ require "formula" require "linkage_cache_store" class LinkageChecker - def initialize(keg, database_cache, use_cache = false, formula = nil) + def initialize(keg, formula = nil, use_cache: false, cache_db:) @keg = keg @formula = formula || resolve_formula(keg) if use_cache - @store = LinkageStore.new(keg.name, database_cache.db) - flush_cache_and_check_dylibs unless database_cache.db.key?(keg.name) + @store = LinkageCacheStore.new(keg.name, cache_db) + flush_cache_and_check_dylibs unless @store.has_keg_name? else flush_cache_and_check_dylibs end @@ -50,7 +50,7 @@ class LinkageChecker end def undeclared_deps - @undeclared_deps ||= store.nil? ? [] : store.fetch_type(:undeclared_deps) + @undeclared_deps ||= store.fetch_type(:undeclared_deps) end private @@ -60,37 +60,37 @@ class LinkageChecker # 'Hash-type' cache values def brewed_dylibs - @brewed_dylibs ||= store.nil? ? {} : store.fetch_type(:brewed_dylibs) + @brewed_dylibs ||= store.fetch_type(:brewed_dylibs) end def reverse_links - @reverse_links ||= store.nil? ? {} : store.fetch_type(:reverse_links) + @reverse_links ||= store.fetch_type(:reverse_links) end def broken_deps - @broken_deps ||= store.nil? ? {} : store.fetch_type(:broken_deps) + @broken_deps ||= store.fetch_type(:broken_deps) end # 'Path-type' cached values def system_dylibs - @system_dylibs ||= store.nil? ? [] : store.fetch_type(:system_dylibs) + @system_dylibs ||= store.fetch_type(:system_dylibs) end def broken_dylibs - @broken_dylibs ||= store.nil? ? [] : store.fetch_type(:broken_dylibs) + @broken_dylibs ||= store.fetch_type(:broken_dylibs) end def variable_dylibs - @variable_dylibs ||= store.nil? ? [] : store.fetch_type(:variable_dylibs) + @variable_dylibs ||= store.fetch_type(:variable_dylibs) end def indirect_deps - @indirect_deps ||= store.nil? ? [] : store.fetch_type(:indirect_deps) + @indirect_deps ||= store.fetch_type(:indirect_deps) end def unnecessary_deps - @unnecessary_deps ||= store.nil? ? [] : store.fetch_type(:unnecessary_deps) + @unnecessary_deps ||= store.fetch_type(:unnecessary_deps) end def dylib_to_dep(dylib) @@ -108,7 +108,8 @@ class LinkageChecker # weakly loaded dylibs may not actually exist on disk, so skip them # when checking for broken linkage - file.dynamically_linked_libraries(except: :LC_LOAD_WEAK_DYLIB).each do |dylib| + file.dynamically_linked_libraries(except: :LC_LOAD_WEAK_DYLIB) + .each do |dylib| @reverse_links[dylib] << file next if checked_dylibs.include? dylib if dylib.start_with? "@" @@ -139,7 +140,10 @@ class LinkageChecker end end - @indirect_deps, @undeclared_deps, @unnecessary_deps = check_undeclared_deps if formula + if formula + @indirect_deps, @undeclared_deps, @unnecessary_deps = + check_undeclared_deps + end store_dylibs! end From ddb7f06e9f787f88d530302efdb748b0831e129f Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Tue, 22 May 2018 12:54:54 -0400 Subject: [PATCH 25/28] Fixed `rubocop` offenses from `brew style`. --- .../Homebrew/extend/os/mac/formula_cellar_checks.rb | 5 +++-- Library/Homebrew/formula.rb | 5 +++-- Library/Homebrew/linkage_cache_store.rb | 10 ++++++---- Library/Homebrew/linkage_checker.rb | 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index c09ed11a06..4a77eb486c 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -67,8 +67,9 @@ module FormulaCellarChecks keg = Keg.new(formula.prefix) CacheStoreDatabase.use(:linkage) do |db| - checker = LinkageChecker.new keg, formula, cache_db: db, - use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil? + checker = LinkageChecker.new( + keg, formula, cache_db: db, use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil? + ) next unless checker.broken_library_linkage? output = <<~EOS diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 0560e6cd16..6609fc7d83 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1528,8 +1528,9 @@ class Formula return [] unless keg undeclared_deps = CacheStoreDatabase.use(:linkage) do |db| - linkage_checker = LinkageChecker.new keg, self, cache_db: db, - use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil? + linkage_checker = LinkageChecker.new( + keg, self, cache_db: db, use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil? + ) linkage_checker.undeclared_deps.map { |n| Dependency.new(n) } end diff --git a/Library/Homebrew/linkage_cache_store.rb b/Library/Homebrew/linkage_cache_store.rb index a3d1625770..cb2df93c5b 100644 --- a/Library/Homebrew/linkage_cache_store.rb +++ b/Library/Homebrew/linkage_cache_store.rb @@ -17,7 +17,7 @@ class LinkageCacheStore < CacheStore # Returns `true` if the database has any value for the current `keg_name` # # @return [Boolean] - def has_keg_name? + def keg_exists? !database.get(keg_name).nil? end @@ -72,9 +72,11 @@ class LinkageCacheStore < CacheStore private - ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs, - :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze - HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links, :broken_deps].freeze + ARRAY_LINKAGE_TYPES = [ + :system_dylibs, :variable_dylibs, :broken_dylibs, :indirect_deps, + :undeclared_deps, :unnecessary_deps + ].freeze + HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links, :broken_deps].freeze # @return [String] the key to lookup items in the `CacheStore` attr_reader :keg_name diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 10d79a03f3..559e1d7fa3 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -9,7 +9,7 @@ class LinkageChecker if use_cache @store = LinkageCacheStore.new(keg.name, cache_db) - flush_cache_and_check_dylibs unless @store.has_keg_name? + flush_cache_and_check_dylibs unless @store.keg_exists? else flush_cache_and_check_dylibs end From 91f7a5eb76d303f135fad255cec9eababea240cc Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Tue, 22 May 2018 14:15:36 -0400 Subject: [PATCH 26/28] Added unit tests for `CacheStoreDatabase` and `LinkageCacheStore`. --- Library/Homebrew/test/cache_store_spec.rb | 146 ++++++++++++++++++ .../Homebrew/test/linkage_cache_store_spec.rb | 88 +++++++++++ 2 files changed, 234 insertions(+) create mode 100644 Library/Homebrew/test/cache_store_spec.rb create mode 100644 Library/Homebrew/test/linkage_cache_store_spec.rb diff --git a/Library/Homebrew/test/cache_store_spec.rb b/Library/Homebrew/test/cache_store_spec.rb new file mode 100644 index 0000000000..6b54f45854 --- /dev/null +++ b/Library/Homebrew/test/cache_store_spec.rb @@ -0,0 +1,146 @@ +require "cache_store" + +describe CacheStoreDatabase do + subject { CacheStoreDatabase.new(:sample) } + + describe "self.use" do + let(:type) { :test } + + it "creates a new `DatabaseCache` instance and yields it" do + cache_store = double("cache_store", close_if_open!: nil) + expect(CacheStoreDatabase).to receive(:new).with(type).and_return(cache_store) + expect(CacheStoreDatabase).to receive(:call).with(cache_store) + end + end + + describe "#set" do + let(:db) { double("db", :[]= => nil) } + + before(:each) do + allow(File).to receive(:write) + allow(subject).to receive(:created?).and_return(true) + expect(db).to receive(:has_key?).with(:foo).and_return(false) + subject.stub(:db).and_return(db) + end + + it "sets the value in the `CacheStoreDatabase`" do + expect(db).to_not have_key(:foo) + subject.set(:foo, "bar") + end + end + + describe "#get" do + context "database created" do + let(:db) { double("db", :[] => "bar") } + + before(:each) do + allow(subject).to receive(:created?).and_return(true) + expect(db).to receive(:has_key?).with(:foo).and_return(true) + subject.stub(:db).and_return(db) + end + + it "gets value in the `CacheStoreDatabase` corresponding to the key" do + expect(db).to have_key(:foo) + expect(subject.get(:foo)).to equal("bar") + end + end + + context "database not created" do + let(:db) { double("db", :[] => nil) } + + before(:each) do + allow(subject).to receive(:created?).and_return(false) + subject.stub(:db).and_return(db) + end + + it "does not get value in the `CacheStoreDatabase` corresponding to key" do + expect(subject.get(:foo)).to_not be("bar") + end + + it "does not call `db[]` if `CacheStoreDatabase.created?` is `false`" do + expect(db).not_to receive(:[]) + subject.get(:foo) + end + end + end + + describe "#delete" do + context "database created" do + let(:db) { double("db", :[] => { foo: "bar" }) } + + before(:each) do + allow(subject).to receive(:created?).and_return(true) + subject.stub(:db).and_return(db) + end + + it "deletes value in the `CacheStoreDatabase` corresponding to the key" do + expect(db).to receive(:delete).with(:foo) + subject.delete(:foo) + end + end + + context "database not created" do + let(:db) { double("db", delete: nil) } + + before(:each) do + allow(subject).to receive(:created?).and_return(false) + subject.stub(:db).and_return(db) + end + + it "does not call `db.delete` if `CacheStoreDatabase.created?` is `false`" do + expect(db).not_to receive(:delete) + subject.delete(:foo) + end + end + end + + describe "#close_if_open!" do + context "database open" do + before(:each) do + subject.instance_variable_set(:@db, instance_double("db", close: nil)) + end + + it "does not raise an error when `close` is called on the database" do + expect { subject.close_if_open! }.to_not raise_error(NoMethodError) + end + end + + context "database not open" do + before(:each) do + subject.instance_variable_set(:@db, nil) + end + + it "does not raise an error when `close` is called on the database" do + expect { subject.close_if_open! }.to_not raise_error(NoMethodError) + end + end + end + + describe "#created?" do + let(:cache_path) { "path/to/homebrew/cache/sample.db" } + + before(:each) do + subject.stub(:cache_path).and_return(cache_path) + end + + context "`File.exist?(cache_path)` returns `true`" do + before(:each) do + allow(File).to receive(:exist?).with(cache_path).and_return(true) + end + + it "returns `true`" do + expect(subject.created?).to be(true) + end + end + + context "`File.exist?(cache_path)` returns `false`" do + before(:each) do + allow(File).to receive(:exist?).with(cache_path).and_return(false) + end + + it "returns `false`" do + expect(subject.created?).to be(false) + end + end + end +end diff --git a/Library/Homebrew/test/linkage_cache_store_spec.rb b/Library/Homebrew/test/linkage_cache_store_spec.rb new file mode 100644 index 0000000000..cfd3cc7f93 --- /dev/null +++ b/Library/Homebrew/test/linkage_cache_store_spec.rb @@ -0,0 +1,88 @@ +require "test/support/fixtures/testball" +require "linkage_cache_store" + +describe LinkageCacheStore do + let(:keg_name) { "keg_name" } + let(:database) { double("database") } + + subject { LinkageCacheStore.new(keg_name, database) } + + describe "#keg_exists?" do + context "`keg_name` exists in cache" do + before(:each) do + expect(database).to receive(:get).with(keg_name).and_return("") + end + + it "returns `true`" do + expect(subject.keg_exists?).to be(true) + end + end + + context "`keg_name` does not exist in cache" do + before(:each) do + expect(database).to receive(:get).with(keg_name).and_return(nil) + end + + it "returns `false`" do + expect(subject.keg_exists?).to be(false) + end + end + end + + describe "#update!" do + context "a `value` is a `Hash`" do + it "sets the cache for the `keg_name`" do + expect(database).to receive(:set).with(keg_name, anything) + subject.update!(a_value: { key: ["value"] }) + end + end + + context "a `value` is an `Array`" do + it "sets the cache for the `keg_name`" do + expect(database).to receive(:set).with(keg_name, anything) + subject.update!(a_value: ["value"]) + end + end + + context "a `value` is not an `Array` or `Hash`" do + it "raises a `TypeError` if a `value` is not an `Array` or `Hash`" do + expect { subject.update!(key: 1) }.to raise_error(TypeError) + end + end + end + + describe "#flush_cache!" do + it "calls `delete` on the `database` with `keg_name` as parameter" do + expect(database).to receive(:delete).with(keg_name) + subject.flush_cache! + end + end + + describe "#fetch_type" do + context "`HASH_LINKAGE_TYPES.include?(type)`" do + before(:each) do + expect(database).to receive(:get).with(keg_name).and_return(nil) + end + + it "returns a `Hash` of values" do + expect(subject.fetch_type(:brewed_dylibs)).to be_an_instance_of(Hash) + end + end + + context "`ARRAY_LINKAGE_TYPES.include?(type)`" do + before(:each) do + expect(database).to receive(:get).with(keg_name).and_return(nil) + end + + it "returns an `Array` of values" do + expect(subject.fetch_type(:system_dylibs)).to be_an_instance_of(Array) + end + end + + context "`type` not in `HASH_LINKAGE_TYPES` or `ARRAY_LINKAGE_TYPES`" do + it "raises a `TypeError` if the `type` is not supported" do + expect { subject.fetch_type(:bad_type) }.to raise_error(TypeError) + end + end + end +end From 218a7362dc072c25dbefb7d1cafff1c36f870f3f Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Tue, 22 May 2018 15:31:58 -0400 Subject: [PATCH 27/28] Fixed failing tests. --- Library/Homebrew/test/cache_store_spec.rb | 7 ++++--- Library/Homebrew/test/linkage_cache_store_spec.rb | 1 - 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/test/cache_store_spec.rb b/Library/Homebrew/test/cache_store_spec.rb index 6b54f45854..87e16b8590 100644 --- a/Library/Homebrew/test/cache_store_spec.rb +++ b/Library/Homebrew/test/cache_store_spec.rb @@ -6,10 +6,11 @@ describe CacheStoreDatabase do describe "self.use" do let(:type) { :test } - it "creates a new `DatabaseCache` instance and yields it" do + it "creates a new `DatabaseCache` instance" do cache_store = double("cache_store", close_if_open!: nil) expect(CacheStoreDatabase).to receive(:new).with(type).and_return(cache_store) - expect(CacheStoreDatabase).to receive(:call).with(cache_store) + expect(cache_store).to receive(:close_if_open!) + CacheStoreDatabase.use(type) { |_db| } end end @@ -41,7 +42,7 @@ describe CacheStoreDatabase do it "gets value in the `CacheStoreDatabase` corresponding to the key" do expect(db).to have_key(:foo) - expect(subject.get(:foo)).to equal("bar") + expect(subject.get(:foo)).to eq("bar") end end diff --git a/Library/Homebrew/test/linkage_cache_store_spec.rb b/Library/Homebrew/test/linkage_cache_store_spec.rb index cfd3cc7f93..6605ab282a 100644 --- a/Library/Homebrew/test/linkage_cache_store_spec.rb +++ b/Library/Homebrew/test/linkage_cache_store_spec.rb @@ -1,4 +1,3 @@ -require "test/support/fixtures/testball" require "linkage_cache_store" describe LinkageCacheStore do From a9606b4a189214bf87181ca3c65127a6a9d0b3a8 Mon Sep 17 00:00:00 2001 From: "Andrew R. McBurney" Date: Wed, 23 May 2018 13:00:05 -0400 Subject: [PATCH 28/28] Added trivial integration tests for `brew linkage`. --- Library/Homebrew/test/dev-cmd/linkage_spec.rb | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Library/Homebrew/test/dev-cmd/linkage_spec.rb diff --git a/Library/Homebrew/test/dev-cmd/linkage_spec.rb b/Library/Homebrew/test/dev-cmd/linkage_spec.rb new file mode 100644 index 0000000000..990c017c56 --- /dev/null +++ b/Library/Homebrew/test/dev-cmd/linkage_spec.rb @@ -0,0 +1,35 @@ +describe "brew linkage", :integration_test do + before do + setup_test_formula "testball" + (HOMEBREW_CELLAR/"testball/0.0.1/foo").mkpath + end + + context "no cache" do + it "works when no arguments are provided" do + expect { brew "linkage" } + .to be_a_success + .and not_to_output.to_stdout + .and not_to_output.to_stderr + end + + it "works when one argument is provided" do + expect { brew "linkage", "testball" } + .to be_a_success + .and not_to_output.to_stderr + end + end + + context "cache" do + it "works when no arguments are provided" do + expect { brew "linkage", "--cached" } + .to be_a_success + .and not_to_output.to_stderr + end + + it "works when one argument is provided" do + expect { brew "linkage", "--cached", "testball" } + .to be_a_success + .and not_to_output.to_stderr + end + end +end