From 6471516c1a0bc8e6f6cb484ad46fd6a1bb61af42 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 23 Feb 2024 15:11:54 +0100 Subject: [PATCH] Add type signatures for `Tap`. --- Library/Homebrew/dev-cmd/pr-pull.rb | 42 ++++----- Library/Homebrew/diagnostic.rb | 2 +- Library/Homebrew/tap.rb | 89 ++++++++++++++----- Library/Homebrew/test/dev-cmd/pr-pull_spec.rb | 22 ++--- 4 files changed, 103 insertions(+), 52 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 0186df88e1373b..86b5e5fd5e9f35 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -87,12 +87,12 @@ def self.separate_commit_message(message) [subject, body, trailers] end - def self.signoff!(git_repo, pull_request: nil, dry_run: false) + def self.signoff!(tap, pull_request: nil, dry_run: false) + git_repo = tap.git_repo subject, body, trailers = separate_commit_message(git_repo.commit_message) if pull_request # This is a tap pull request and approving reviewers should also sign-off. - tap = Tap.from_path(git_repo.pathname) review_trailers = GitHub.approved_reviews(tap.user, tap.full_name.split("/").last, pull_request).map do |r| "Signed-off-by: #{r["name"]} <#{r["email"]}>" end @@ -130,26 +130,24 @@ def self.get_package(tap, subject_name, subject_path, content) end end - def self.determine_bump_subject(old_contents, new_contents, subject_path, reason: nil) + def self.determine_bump_subject(old_contents, new_contents, subject_path, tap:, reason: nil) subject_path = Pathname(subject_path) - tap = Tap.from_path(subject_path) subject_name = subject_path.basename.to_s.chomp(".rb") - is_cask = subject_path.to_s.start_with?("#{tap.cask_dir}/") - name = is_cask ? "cask" : "formula" - new_package = get_package(tap, subject_name, subject_path, new_contents) + type = "cask" if tap.cask_file?(subject_path) + type = "formula" if tap.formula_file?(subject_path) - return "#{subject_name}: delete #{reason}".strip if new_package.blank? + new_package = get_package(tap, subject_name, subject_path, new_contents) + return "#{subject_name}: delete#{reason&.prepend(" ")}" if new_package.blank? old_package = get_package(tap, subject_name, subject_path, old_contents) + return "#{subject_name} #{new_package.version} (new #{type})" if old_package.blank? - if old_package.blank? - "#{subject_name} #{new_package.version} (new #{name})" - elsif old_package.version != new_package.version + if old_package.version != new_package.version "#{subject_name} #{new_package.version}" - elsif !is_cask && old_package.revision != new_package.revision + elsif type == "formula" && old_package.revision != new_package.revision "#{subject_name}: revision #{reason}".strip - elsif is_cask && old_package.sha256 != new_package.sha256 + elsif type == "cask" && old_package.sha256 != new_package.sha256 "#{subject_name}: checksum update #{reason}".strip else "#{subject_name}: #{reason || "rebuild"}".strip @@ -158,7 +156,9 @@ def self.determine_bump_subject(old_contents, new_contents, subject_path, reason # Cherry picks a single commit that modifies a single file. # Potentially rewords this commit using {determine_bump_subject}. - def self.reword_package_commit(commit, file, git_repo:, reason: "", verbose: false, resolve: false) + def self.reword_package_commit(commit, file, tap:, reason: "", verbose: false, resolve: false) + git_repo = tap.git_repo + package_file = git_repo.pathname / file package_name = package_file.basename.to_s.chomp(".rb") @@ -168,7 +168,7 @@ def self.reword_package_commit(commit, file, git_repo:, reason: "", verbose: fal old_package = Utils::Git.file_at_commit(git_repo.to_s, file, "HEAD^") new_package = Utils::Git.file_at_commit(git_repo.to_s, file, "HEAD") - bump_subject = determine_bump_subject(old_package, new_package, package_file, reason: reason).strip + bump_subject = determine_bump_subject(old_package, new_package, package_file, reason: reason, tap: tap).strip subject, body, trailers = separate_commit_message(git_repo.commit_message) if subject != bump_subject && !subject.start_with?("#{package_name}:") @@ -183,9 +183,11 @@ def self.reword_package_commit(commit, file, git_repo:, reason: "", verbose: fal # Cherry picks multiple commits that each modify a single file. # Words the commit according to {determine_bump_subject} with the body # corresponding to all the original commit messages combined. - def self.squash_package_commits(commits, file, git_repo:, reason: "", verbose: false, resolve: false) + def self.squash_package_commits(commits, file, tap:, reason: "", verbose: false, resolve: false) odebug "Squashing #{file}: #{commits.join " "}" + git_repo = tap.git_repo + # Format commit messages into something similar to `git fmt-merge-message`. # * subject 1 # * subject 2 @@ -220,7 +222,7 @@ def self.squash_package_commits(commits, file, git_repo:, reason: "", verbose: f package_file = git_repo.pathname / file old_package = Utils::Git.file_at_commit(git_repo.pathname, file, "#{commits.first}^") new_package = package_file.read - bump_subject = determine_bump_subject(old_package, new_package, package_file, reason: reason) + bump_subject = determine_bump_subject(old_package, new_package, package_file, reason: reason, tap: tap) # Commit with the new subject, body, and trailers. safe_system("git", "-C", git_repo.pathname, "commit", "--quiet", @@ -273,14 +275,14 @@ def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: if files.length == 1 && files_to_commits[files.first].length == 1 # If there's a 1:1 mapping of commits to files, just cherry pick and (maybe) reword. reword_package_commit( - commit, files.first, git_repo: git_repo, reason: reason, verbose: verbose, resolve: resolve + commit, files.first, tap: tap, reason: reason, verbose: verbose, resolve: resolve ) processed_commits << commit elsif files.length == 1 && files_to_commits[files.first].length > 1 # If multiple commits modify a single file, squash them down into a single commit. file = files.first commits = files_to_commits[file] - squash_package_commits(commits, file, git_repo: git_repo, reason: reason, verbose: verbose, resolve: resolve) + squash_package_commits(commits, file, tap: tap, reason: reason, verbose: verbose, resolve: resolve) processed_commits += commits else # We can't split commits (yet) so just raise an error. @@ -464,7 +466,7 @@ def self.pr_pull autosquash!(original_commit, tap: tap, cherry_picked: !args.no_cherry_pick?, verbose: args.verbose?, resolve: args.resolve?, reason: args.message) end - signoff!(git_repo, pull_request: pr, dry_run: args.dry_run?) unless args.clean? + signoff!(tap, pull_request: pr, dry_run: args.dry_run?) unless args.clean? end unless formulae_need_bottles?(tap, original_commit, pr_labels, args: args) diff --git a/Library/Homebrew/diagnostic.rb b/Library/Homebrew/diagnostic.rb index 7ad243f70b604e..13acbfabcd5e83 100644 --- a/Library/Homebrew/diagnostic.rb +++ b/Library/Homebrew/diagnostic.rb @@ -537,7 +537,7 @@ def check_casktap_integrity core_cask_tap = CoreCaskTap.instance return unless core_cask_tap.installed? - broken_tap(core_cask_tap) || examine_git_origin(core_cask_tap.git_repo, core_cask_tap.remote) + broken_tap(core_cask_tap) || examine_git_origin(core_cask_tap.git_repo, T.must(core_cask_tap.remote)) end sig { returns(T.nilable(String)) } diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index a3434745210844..a760501da63c1c 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -39,7 +39,7 @@ class Tap #{HOMEBREW_TAP_STYLE_EXCEPTIONS_DIR}/*.json ].freeze - sig { params(user: String, repo: String).returns(Tap) } + sig { params(user: String, repo: String).returns(T.attached_class) } def self.fetch(user, repo = T.unsafe(nil)) user, repo = user.split("/", 2) if repo.nil? @@ -54,13 +54,16 @@ def self.fetch(user, repo = T.unsafe(nil)) user = user.capitalize if ["homebrew", "linuxbrew"].include?(user) repo = repo.sub(HOMEBREW_OFFICIAL_REPO_PREFIXES_REGEX, "") - return CoreTap.instance if ["Homebrew", "Linuxbrew"].include?(user) && ["core", "homebrew"].include?(repo) - return CoreCaskTap.instance if user == "Homebrew" && repo == "cask" + if ["Homebrew", "Linuxbrew"].include?(user) && ["core", "homebrew"].include?(repo) + return T.cast(CoreTap.instance, T.attached_class) + end + return T.cast(CoreCaskTap.instance, T.attached_class) if user == "Homebrew" && repo == "cask" cache_key = "#{user}/#{repo}".downcase cache.fetch(cache_key) { |key| cache[key] = new(user, repo) } end + sig { params(path: T.any(String, Pathname)).returns(T.nilable(T.attached_class)) } def self.from_path(path) match = File.expand_path(path).match(HOMEBREW_TAP_PATH_REGEX) @@ -72,7 +75,7 @@ def self.from_path(path) end # @private - sig { params(name: String).returns(T.nilable([Tap, String])) } + sig { params(name: String).returns(T.nilable([T.attached_class, String])) } def self.with_formula_name(name) return unless (match = name.match(HOMEBREW_TAP_FORMULA_REGEX)) @@ -88,7 +91,7 @@ def self.with_formula_name(name) end # @private - sig { params(token: String).returns(T.nilable([Tap, String])) } + sig { params(token: String).returns(T.nilable([T.attached_class, String])) } def self.with_cask_token(token) return unless (match = token.match(HOMEBREW_TAP_CASK_REGEX)) @@ -149,6 +152,7 @@ def self.install_default_cask_tap_if_necessary(force: false) private_class_method :new # @private + sig { params(user: String, repo: String).void } def initialize(user, repo) @user = user @repo = repo @@ -159,6 +163,7 @@ def initialize(user, repo) end # Clear internal cache. + sig { void } def clear_cache @remote = nil @repo_var_suffix = nil @@ -198,6 +203,7 @@ def ensure_installed! # The remote path to this {Tap}. # e.g. `https://github.com/user/homebrew-repo` + sig { returns(T.nilable(String)) } def remote return default_remote unless installed? @@ -231,11 +237,13 @@ def repo_var_suffix end # True if this {Tap} is a Git repository. + sig { returns(T::Boolean) } def git? git_repo.git_repo? end # Git branch for this {Tap}. + sig { returns(T.nilable(String)) } def git_branch raise TapUnavailableError, name unless installed? @@ -243,6 +251,7 @@ def git_branch end # Git HEAD for this {Tap}. + sig { returns(T.nilable(String)) } def git_head raise TapUnavailableError, name unless installed? @@ -250,6 +259,7 @@ def git_head end # Time since last git commit for this {Tap}. + sig { returns(T.nilable(String)) } def git_last_commit raise TapUnavailableError, name unless installed? @@ -265,11 +275,13 @@ def issues_url "#{default_remote}/issues" end + sig { returns(String) } def to_s name end # True if this {Tap} is an official Homebrew tap. + sig { returns(T::Boolean) } def official? user == "Homebrew" end @@ -316,6 +328,7 @@ def installed? end # True if this {Tap} is not a full clone. + sig { returns(T::Boolean) } def shallow? (path/".git/shallow").exist? end @@ -341,6 +354,16 @@ def core_cask_tap? # @param custom_remote [Boolean] If set, change the tap's remote if already installed. # @param verify [Boolean] If set, verify all the formula, casks and aliases in the tap are valid. # @param force [Boolean] If set, force core and cask taps to install even under API mode. + sig { + params( + quiet: T::Boolean, + clone_target: T.nilable(T.any(String, Pathname)), + force_auto_update: T.nilable(T::Boolean), + custom_remote: T::Boolean, + verify: T::Boolean, + force: T::Boolean, + ).void + } def install(quiet: false, clone_target: nil, force_auto_update: nil, custom_remote: false, verify: false, force: false) require "descriptions" @@ -358,7 +381,7 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil, requested_remote = clone_target || default_remote if installed? && !custom_remote - raise TapRemoteMismatchError.new(name, @remote, requested_remote) if clone_target && requested_remote != remote + raise TapRemoteMismatchError.new(name, remote, requested_remote) if clone_target && requested_remote != remote raise TapAlreadyTappedError, name if force_auto_update.nil? && !shallow? end @@ -466,6 +489,7 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil, EOS end + sig { void } def link_completions_and_manpages command = "brew tap --repair" Utils::Link.link_manpages(path, command) @@ -478,6 +502,7 @@ def link_completions_and_manpages end end + sig { params(requested_remote: T.nilable(T.any(String, Pathname)), quiet: T::Boolean).void } def fix_remote_configuration(requested_remote: nil, quiet: false) if requested_remote.present? path.cd do @@ -511,6 +536,7 @@ def fix_remote_configuration(requested_remote: nil, quiet: false) end # Uninstall this {Tap}. + sig { params(manual: T::Boolean).void } def uninstall(manual: false) require "descriptions" raise TapUnavailableError, name unless installed? @@ -550,8 +576,9 @@ def uninstall(manual: false) sig { returns(T::Boolean) } def custom_remote? return true unless (remote = self.remote) + return false if remote.casecmp(default_remote)&.zero? - !remote.casecmp(default_remote).zero? + true end # Path to the directory of all {Formula} files for this {Tap}. @@ -592,6 +619,8 @@ def relative_cask_path(token) .delete_prefix("#{path}/") end + # @private + sig { returns(T::Array[String]) } def contents contents = [] @@ -671,7 +700,7 @@ def ruby_file?(file) # returns true if given path would present a {Formula} file in this {Tap}. # accepts both absolute path and relative path (relative to this {Tap}'s path) - # @private + # @api private sig { params(file: T.any(String, Pathname)).returns(T::Boolean) } def formula_file?(file) file = Pathname.new(file) unless file.is_a? Pathname @@ -684,7 +713,7 @@ def formula_file?(file) # returns true if given path would present a {Cask} file in this {Tap}. # accepts both absolute path and relative path (relative to this {Tap}'s path) - # @private + # @api private sig { params(file: T.any(String, Pathname)).returns(T::Boolean) } def cask_file?(file) file = Pathname.new(file) unless file.is_a? Pathname @@ -922,6 +951,11 @@ def ==(other) other.is_a?(self.class) && name == other.name end + sig { + override.params( + block: T.nilable(T.proc.params(arg0: T.attached_class).returns(T.untyped)), + ).returns(T::Enumerable[T.attached_class]) + } def self.each(&block) return to_enum unless block @@ -976,6 +1010,7 @@ def alias_file_to_name(file) "#{name}/#{file.basename}" end + sig { params(list: Symbol, formula_or_cask: String, value: T.untyped).returns(T.untyped) } def audit_exception(list, formula_or_cask, value = nil) return false if audit_exceptions.blank? return false unless audit_exceptions.key? list @@ -987,7 +1022,7 @@ def audit_exception(list, formula_or_cask, value = nil) list.include? formula_or_cask when Hash return false unless list.include? formula_or_cask - return list[formula_or_cask] if value.blank? + return list[formula_or_cask] if value.nil? list[formula_or_cask] == value end @@ -1028,6 +1063,9 @@ class AbstractCoreTap < Tap abstract! private_class_method :fetch + private_class_method :from_path + private_class_method :with_formula_name + private_class_method :with_cask_token sig { returns(T.attached_class) } def self.instance @@ -1063,7 +1101,7 @@ class CoreTap < AbstractCoreTap # @private sig { void } def initialize - super "Homebrew", "core" + super("Homebrew", "core") end sig { override.void } @@ -1073,7 +1111,7 @@ def ensure_installed! super end - sig { returns(T.nilable(String)) } + sig { override.returns(T.nilable(String)) } def remote return super if Homebrew::EnvConfig.no_install_from_api? @@ -1081,6 +1119,16 @@ def remote end # CoreTap never allows shallow clones (on request from GitHub). + sig { + override.params( + quiet: T::Boolean, + clone_target: T.nilable(T.any(String, Pathname)), + force_auto_update: T.nilable(T::Boolean), + custom_remote: T::Boolean, + verify: T::Boolean, + force: T::Boolean, + ).void + } def install(quiet: false, clone_target: nil, force_auto_update: nil, custom_remote: false, verify: false, force: false) remote = Homebrew::EnvConfig.core_git_remote # set by HOMEBREW_CORE_GIT_REMOTE @@ -1098,7 +1146,7 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil, end # @private - sig { params(manual: T::Boolean).void } + sig { override.params(manual: T::Boolean).void } def uninstall(manual: false) raise "Tap#uninstall is not available for CoreTap" if Homebrew::EnvConfig.no_install_from_api? @@ -1106,7 +1154,7 @@ def uninstall(manual: false) end # @private - sig { returns(T::Boolean) } + sig { override.returns(T::Boolean) } def core_tap? true end @@ -1118,7 +1166,7 @@ def linuxbrew_core? end # @private - sig { returns(Pathname) } + sig { override.returns(Pathname) } def formula_dir @formula_dir ||= begin ensure_installed! @@ -1160,7 +1208,7 @@ def formula_renames end # @private - sig { returns(Hash) } + sig { override.returns(T::Hash[String, String]) } def tap_migrations @tap_migrations ||= if Homebrew::EnvConfig.no_install_from_api? ensure_installed! @@ -1170,7 +1218,7 @@ def tap_migrations else migrations, = Homebrew::API.fetch_json_api_file "formula_tap_migrations.jws.json", stale_seconds: TAP_MIGRATIONS_STALE_SECONDS - migrations + T.cast(migrations, T::Hash[String, String]) end end @@ -1298,7 +1346,7 @@ class CoreCaskTap < AbstractCoreTap # @private sig { void } def initialize - super "Homebrew", "cask" + super("Homebrew", "cask") end # @private @@ -1350,14 +1398,15 @@ def cask_renames end end - sig { override.returns(Hash) } + # @private + sig { override.returns(T::Hash[String, String]) } def tap_migrations @tap_migrations ||= if Homebrew::EnvConfig.no_install_from_api? super else migrations, = Homebrew::API.fetch_json_api_file "cask_tap_migrations.jws.json", stale_seconds: TAP_MIGRATIONS_STALE_SECONDS - migrations + T.cast(migrations, T::Hash[String, String]) end end end diff --git a/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb b/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb index 72550c42540c43..01a27dd2e62a59 100644 --- a/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb +++ b/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb @@ -128,7 +128,7 @@ class Foo < Formula safe_system Utils::Git.git, "add", formula_file safe_system Utils::Git.git, "commit", "-m", "foo 1.0 (new formula)" end - described_class.signoff!(tap.git_repo) + described_class.signoff!(tap) expect(tap.git_repo.commit_message).to include("Signed-off-by:") (path/"Casks").mkpath @@ -137,7 +137,7 @@ class Foo < Formula safe_system Utils::Git.git, "add", cask_file safe_system Utils::Git.git, "commit", "-m", "food 1.0 (new cask)" end - described_class.signoff!(tap.git_repo) + described_class.signoff!(tap) expect(tap.git_repo.commit_message).to include("Signed-off-by:") end end @@ -162,41 +162,41 @@ class Foo < Formula describe "#determine_bump_subject" do it "correctly bumps a new formula" do - expect(described_class.determine_bump_subject("", formula, formula_file)).to eq("foo 1.0 (new formula)") + expect(described_class.determine_bump_subject("", formula, formula_file, tap: tap)).to eq("foo 1.0 (new formula)") end it "correctly bumps a new cask" do - expect(described_class.determine_bump_subject("", cask, cask_file)).to eq("food 1.0 (new cask)") + expect(described_class.determine_bump_subject("", cask, cask_file, tap: tap)).to eq("food 1.0 (new cask)") end it "correctly bumps a formula version" do - expect(described_class.determine_bump_subject(formula, formula_version, formula_file)).to eq("foo 2.0") + expect(described_class.determine_bump_subject(formula, formula_version, formula_file, tap: tap)).to eq("foo 2.0") end it "correctly bumps a cask version" do - expect(described_class.determine_bump_subject(cask, cask_version, cask_file)).to eq("food 2.0") + expect(described_class.determine_bump_subject(cask, cask_version, cask_file, tap: tap)).to eq("food 2.0") end it "correctly bumps a cask checksum" do - expect(described_class.determine_bump_subject(cask, cask_checksum, cask_file)).to eq("food: checksum update") + expect(described_class.determine_bump_subject(cask, cask_checksum, cask_file, tap: tap)).to eq("food: checksum update") end it "correctly bumps a formula revision with reason" do expect(described_class.determine_bump_subject( - formula, formula_revision, formula_file, reason: "for fun" + formula, formula_revision, formula_file, tap: tap, reason: "for fun" )).to eq("foo: revision for fun") end it "correctly bumps a formula rebuild" do - expect(described_class.determine_bump_subject(formula, formula_rebuild, formula_file)).to eq("foo: rebuild") + expect(described_class.determine_bump_subject(formula, formula_rebuild, formula_file, tap: tap)).to eq("foo: rebuild") end it "correctly bumps a formula deletion" do - expect(described_class.determine_bump_subject(formula, "", formula_file)).to eq("foo: delete") + expect(described_class.determine_bump_subject(formula, "", formula_file, tap: tap)).to eq("foo: delete") end it "correctly bumps a cask deletion" do - expect(described_class.determine_bump_subject(cask, "", cask_file)).to eq("food: delete") + expect(described_class.determine_bump_subject(cask, "", cask_file, tap: tap)).to eq("food: delete") end end end