avoid filesystem checks from Pathname presence tests

This commit is contained in:
Douglas Eichelberger
2026-07-06 18:31:19 -07:00
parent 988192e555
commit 22a514b56f
26 changed files with 127 additions and 39 deletions
+1 -1
View File
@@ -66,7 +66,7 @@ module Homebrew
sig { returns(Pathname) }
def self.gh_executable
@gh_executable ||= T.let(nil, T.nilable(Pathname))
return @gh_executable if @gh_executable.present?
return @gh_executable if @gh_executable
# NOTE: We set HOMEBREW_NO_VERIFY_ATTESTATIONS when installing `gh` itself,
# to prevent a cycle during bootstrapping. This can eventually be resolved
+1 -1
View File
@@ -77,7 +77,7 @@ begin
# External commands expect a normal PATH
ENV["PATH"] = homebrew_path.to_s
external_ruby_v2_cmd = Commands.external_ruby_v2_cmd_path(cmd).present?
external_ruby_v2_cmd = !Commands.external_ruby_v2_cmd_path(cmd).nil?
external_ruby_cmd_path = Commands.external_ruby_cmd_path(cmd) unless external_ruby_v2_cmd
external_cmd_path = Commands.external_cmd_path(cmd) if !external_ruby_v2_cmd && external_ruby_cmd_path.nil?
end
+1 -1
View File
@@ -238,7 +238,7 @@ class Build
elsif formula.prefix.directory?
formula.prefix
elsif (children = formula.rack.children.presence) && children.size == 1 &&
(first_child = children.first.presence) && first_child.directory?
(first_child = children.first) && first_child.directory?
first_child
else
raise
+1 -1
View File
@@ -150,7 +150,7 @@ module Homebrew
tab = Tab.for_name(name)
tabfile = tab.tabfile
next if tabfile.blank? || !tabfile.exist?
next unless tabfile&.exist?
next if tab.installed_on_request
next name if use_brew_tab
+1 -1
View File
@@ -169,7 +169,7 @@ module Homebrew
# For commands which aren't either absolute or relative
# Add the command directory to PATH, since it may get blown away by superenv
if command.exclude?("/") && (which_command = which(command)).present?
if command.exclude?("/") && (which_command = which(command))
ENV.prepend_path "PATH", which_command.dirname.to_s
end
+1 -1
View File
@@ -750,7 +750,7 @@ module Homebrew
db.each_key do |keg|
keg = T.cast(keg, String)
next if rack.present? && !keg.start_with?("#{rack}/")
next if rack && !keg.start_with?("#{rack}/")
next if File.directory?(keg)
LinkageCacheStore.new(
+1 -1
View File
@@ -581,7 +581,7 @@ module Homebrew
return cask.sourcefile_path.to_s if tap.blank? || tap.remote.blank?
sourcefile_path = cask.sourcefile_path
if sourcefile_path.blank? || sourcefile_path.extname != ".rb"
if !sourcefile_path || sourcefile_path.extname != ".rb"
return "#{tap.default_remote}/blob/HEAD/#{tap.relative_cask_path(cask.token)}"
end
+4 -4
View File
@@ -65,15 +65,15 @@ module Homebrew
cask = formula_or_cask
cask_tab = cask.tab
cask_tabfile = cask_tab.tabfile
if cask_tabfile.blank? || !cask_tabfile.exist?
[cask.token, Cask::Tab.create(cask), true]
else
if cask_tabfile&.exist?
[cask.token, cask_tab, false]
else
[cask.token, Cask::Tab.create(cask), true]
end
end
tabfile = tab.tabfile
if !created_tab && (tabfile.blank? || !tabfile.exist?)
if !created_tab && !tabfile&.exist?
raise ArgumentError,
"Tab file for #{name} does not exist."
end
+5 -5
View File
@@ -242,7 +242,7 @@ module Commands
return if command == "help"
path = self.path(command)
return if path.blank?
return unless path
if (cmd_parser = Homebrew::CLI::Parser.from_cmd_path(path))
processed_options = if subcommand.nil? && cmd_parser.subcommands.present?
@@ -275,7 +275,7 @@ module Commands
sig { params(command: String, short: T::Boolean).returns(T.nilable(String)) }
def self.command_description(command, short: false)
path = self.path(command)
return if path.blank?
return unless path
if (cmd_parser = Homebrew::CLI::Parser.from_cmd_path(path))
if short
@@ -305,7 +305,7 @@ module Commands
sig { params(command: String).returns(T::Array[Homebrew::CLI::Parser::Subcommand]) }
def self.command_subcommands(command)
path = self.path(command)
return [] if path.blank?
return [] unless path
cmd_parser = Homebrew::CLI::Parser.from_cmd_path(path)
return [] if cmd_parser.blank?
@@ -319,7 +319,7 @@ module Commands
}
def self.named_args_type(command, subcommand: nil)
path = self.path(command)
return if path.blank?
return unless path
cmd_parser = Homebrew::CLI::Parser.from_cmd_path(path)
return if cmd_parser.blank?
@@ -336,7 +336,7 @@ module Commands
sig { params(command: String, option: String).returns(T.nilable(T::Array[String])) }
def self.option_conflicts(command, option)
path = self.path(command)
return if path.blank?
return unless path
cmd_parser = Homebrew::CLI::Parser.from_cmd_path(path)
return if cmd_parser.blank?
+2 -2
View File
@@ -390,7 +390,7 @@ module Homebrew
sig { params(formula: Formula).void }
def bottle_formula(formula)
local_bottle_json = args.json? && formula.local_bottle_path.present?
local_bottle_json = args.json? && formula.local_bottle_path
unless local_bottle_json
unless formula.latest_version_installed?
@@ -458,7 +458,7 @@ module Homebrew
if local_bottle_json
bottle_path = formula.local_bottle_path
return if bottle_path.blank?
return unless bottle_path
local_filename = bottle_path.basename.to_s
+1 -1
View File
@@ -74,7 +74,7 @@ module Homebrew
"--no-document", "--no-wrappers", "--ignore-dependencies", "--force"
(HOMEBREW_LIBRARY_PATH/"vendor/gems").cd do
source = Pathname.glob("#{gem}-*/").first
next if source.blank?
next unless source
# We cannot use `#ln_sf` here because that has unintended consequences when
# the symlink we want to create exists and points to an existing directory.
+1 -1
View File
@@ -28,7 +28,7 @@ class DevelopmentTools
sig { returns(T::Boolean) }
def installed?
locate("clang").present? || locate("gcc").present?
!!(locate("clang") || locate("gcc"))
end
sig { returns(String) }
+1
View File
@@ -56,4 +56,5 @@ require "extend/blank/hash"
require "extend/blank/symbol"
require "extend/blank/string"
require "extend/blank/numeric"
require "extend/blank/pathname"
require "extend/blank/time"
+24
View File
@@ -0,0 +1,24 @@
# typed: strict
# frozen_string_literal: true
class Pathname
# A Pathname is blank if its path is empty. Unlike `Pathname#empty?`,
# this never touches the filesystem, so an existing-but-empty file or
# directory is still present.
#
# ```ruby
# Pathname.new("").blank? # => true
# Pathname.new(" ").blank? # => false
# Pathname.new("test").blank? # => false
# ```
#
# @see https://github.com/rails/rails/blob/main/activesupport/lib/active_support/core_ext/pathname/blank.rb
# `Pathname#blank?`
sig { returns(T::Boolean) }
def blank?
to_s.empty?
end
sig { returns(T::Boolean) }
def present? = !blank? # :nodoc:
end
+1 -3
View File
@@ -499,7 +499,7 @@ class Formula
return @unresolved_path if @unresolved_path.exist?
return local_bottle_path if local_bottle_path.presence&.exist?
return local_bottle_path if local_bottle_path&.exist?
alias_path || @unresolved_path
end
@@ -962,8 +962,6 @@ class Formula
def linked_keg
linked_keg = possible_names.map { |name| HOMEBREW_LINKED_KEGS/name }
.find(&:directory?)
# Truthiness, not `.present?`: `Pathname#present?` checks directory
# emptiness on disk, which is a wasted syscall here.
linked_keg || (HOMEBREW_LINKED_KEGS/name)
end
+1 -1
View File
@@ -666,7 +666,7 @@ module Homebrew
return unless @new_formula
# Using internal API here as using `Formulary.factory` is too slow
return if !@online && Homebrew::API::Internal.cached_packages_json_file_path.blank?
return if !@online && !Homebrew::API::Internal.cached_packages_json_file_path.exist?
formula_url = formula.stable&.url
return unless formula_url
+4 -4
View File
@@ -268,7 +268,7 @@ class FormulaInstaller
return false
end
return true if formula.local_bottle_path.present?
return true if formula.local_bottle_path
bottle = api_bottle || formula.bottle_for_tag(Utils::Bottles.tag)
return false if bottle.nil?
@@ -1380,7 +1380,7 @@ on_request: installed_on_request?, options:)
keg_formula_path = installed_prefix/".brew/#{formula.name}.rb"
return keg_formula_path if formula.loaded_from_api?
return keg_formula_path if formula.local_bottle_path.present?
return keg_formula_path if formula.local_bottle_path
return keg_formula_path if build_from_source?
return keg_formula_path unless tap_formula_path.exist?
@@ -1470,7 +1470,7 @@ on_request: installed_on_request?, options:)
sig { params(quiet: T::Boolean, enqueue: T::Boolean).void }
def fetch_bottle_tab(quiet: false, enqueue: false)
return if @fetch_bottle_tab
return if formula.local_bottle_path.present?
return if formula.local_bottle_path
if (bottle = api_bottle || formula.bottle) &&
(manifest_resource = bottle.github_packages_manifest_resource) &&
@@ -1500,7 +1500,7 @@ on_request: installed_on_request?, options:)
fetch_dependencies
return if only_deps?
return if formula.local_bottle_path.present?
return if formula.local_bottle_path
downloadable_object = downloadable
check_attestation = if pour_bottle?(output_warning: true)
+1 -1
View File
@@ -124,7 +124,7 @@ module OS
else
"https://docs.brew.sh/Troubleshooting"
end.freeze
PATH_OPEN = if wsl? && (wslview = which("wslview").presence)
PATH_OPEN = if wsl? && (wslview = which("wslview"))
wslview.to_s
else
"xdg-open"
+2 -2
View File
@@ -146,7 +146,7 @@ module Homebrew
install_service_file(service, file)
if file.blank? && verbose
if !file && verbose
ohai "Generated service file for #{service.formula.name}:"
puts " #{service.dest.read.gsub("\n", "\n ")}"
puts
@@ -412,7 +412,7 @@ module Homebrew
end
temp = Tempfile.new(service.service_name)
temp << if file.blank?
temp << if file.nil?
contents = service.service_file.read
if sudo_service_user && System.launchctl?
@@ -0,0 +1,50 @@
# typed: true
# frozen_string_literal: true
require "extend/blank"
# Modelled on ActiveSupport's `test/core_ext/pathname/blank_test.rb`.
RSpec.describe Pathname do
let(:blank) { [described_class.new("")] }
let(:present) { [described_class.new(" "), described_class.new("."), described_class.new("test")] }
describe "#blank?" do
it "is blank if and only if the path string is empty" do
blank.each { |path| expect(path.blank?).to be(true), "#{path.inspect} should be blank" }
present.each { |path| expect(path.blank?).to be(false), "#{path.inspect} should not be blank" }
end
end
describe "#present?" do
it "is present if and only if the path string is not empty" do
blank.each { |path| expect(path.present?).to be(false), "#{path.inspect} should not be present" }
present.each { |path| expect(path.present?).to be(true), "#{path.inspect} should be present" }
end
end
describe "#presence" do
it "returns the pathname when present, otherwise nil" do
blank.each { |path| expect(path.presence).to be_nil }
present.each { |path| expect(path.presence).to be path }
end
end
describe "filesystem independence" do
# Before `Pathname#blank?` was redefined it dispatched to the filesystem
# via `Pathname#empty?`, so the empty path string was present and the
# existing empty file and directory were blank. A nonexistent path was
# present under both implementations.
it "judges by the path string, not filesystem content" do
expect(described_class.new("").blank?).to be true
expect(mktmpdir.present?).to be true
expect((mktmpdir/"nonexistent").present?).to be true
end
it "treats an existing empty file as present" do
file = mktmpdir/"empty-file"
FileUtils.touch file
expect(file.present?).to be true
end
end
end
+16 -2
View File
@@ -1997,7 +1997,7 @@ RSpec.describe Homebrew::FormulaAuditor do
end
specify "it warns if new formula uses the same URL as already existing package" do
fa = formula_auditor "duplicate-foo", <<~RUBY, new_formula: true, core_tap: true
fa = formula_auditor "duplicate-foo", <<~RUBY, new_formula: true, core_tap: true, online: true
class DuplicateFoo < Formula
url "https://brew.sh/foo-1.0.tgz"
end
@@ -2010,7 +2010,7 @@ RSpec.describe Homebrew::FormulaAuditor do
end
specify "it does not warn about duplicates if formula is not new" do
fa = formula_auditor "duplicate-foo", <<~RUBY, new_formula: false, core_tap: true
fa = formula_auditor "duplicate-foo", <<~RUBY, new_formula: false, core_tap: true, online: true
class DuplicateFoo < Formula
url "https://brew.sh/foo-1.0.tgz"
end
@@ -2020,6 +2020,20 @@ RSpec.describe Homebrew::FormulaAuditor do
expect(fa.new_formula_problems).to be_empty
end
specify "it skips the duplicate check offline when no packages data is cached" do
fa = formula_auditor "duplicate-foo", <<~RUBY, new_formula: true, core_tap: true, online: false
class DuplicateFoo < Formula
url "https://brew.sh/foo-1.0.tgz"
end
RUBY
allow(Homebrew::API::Internal).to receive(:cached_packages_json_file_path)
.and_return(Pathname("/nonexistent/packages.jws.json"))
fa.audit_duplicate_formula
expect(fa.new_formula_problems).to be_empty
end
end
describe "#audit_conflicts" do
+1 -1
View File
@@ -17,7 +17,7 @@ RSpec.describe Utils::Tar do
describe ".available?" do
it "returns true if tar or gnu-tar is available" do
if described_class.executable.present?
if described_class.executable
expect(described_class).to be_available
else
expect(described_class).not_to be_available
+3 -2
View File
@@ -67,7 +67,8 @@ module Homebrew
# If we have a cached event payload, then we failed to get the artifact we wanted
# from `GITHUB_EVENT_PATH`, so use the cached payload to check for a SHA1.
event_payload = JSON.parse(T.must(cached_event_json).read) if cached_event_json.present?
cached_json = cached_event_json
event_payload = JSON.parse(cached_json.read) if cached_json
event_payload ||= payload
event_payload.fetch("before", nil)
@@ -236,7 +237,7 @@ module Homebrew
sig { params(formula: String, bottle_dir: Pathname).returns(T.nilable(T::Hash[String, T.untyped])) }
def local_bottle_hash(formula, bottle_dir:)
return if (local_bottle_json = bottle_glob(formula, bottle_dir, ".json").first).blank?
return unless (local_bottle_json = bottle_glob(formula, bottle_dir, ".json").first)
JSON.parse(local_bottle_json.read)
end
+1 -1
View File
@@ -213,7 +213,7 @@ module Utils
to: T.nilable(String)).returns(Integer)
}
def self.count_coauthors(repository_path, person, from:, to:)
return 0 if repository_path.blank?
return 0 unless repository_path
cmd = [git.to_s, "-C", repository_path.to_s, "log", "--oneline"]
cmd << "--format='%(trailers:key=Co-authored-by:)''"
+1 -1
View File
@@ -107,7 +107,7 @@ module GitHub
# the `GITHUB_WORKSPACE` directory or if no `file` is specified.
sig { returns(T::Boolean) }
def relevant?
return true if @file.blank?
return true unless @file
@file.descend.next.to_s != ".."
end
+1 -1
View File
@@ -15,7 +15,7 @@ module Utils
sig { returns(T::Boolean) }
def available?
executable.present?
!!executable
end
sig { returns(T.nilable(Pathname)) }