From 44f5d3ec7982c5e6f82a0fc85703c5c48419fcd0 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 22 May 2018 14:46:14 +0100 Subject: [PATCH] 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