From 751be9122769bd547e6997268e81a5eea797a37c 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/test/dev-cmd/pr-pull_spec.rb | 26 +++++++----- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 7c07c711cae29..b56892119e55e 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:).strip + bump_subject = determine_bump_subject(old_package, new_package, package_file, reason:, 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:) + bump_subject = determine_bump_subject(old_package, new_package, package_file, tap:, reason:) # 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:, reason:, verbose:, resolve: + commit, files.first, tap:, reason:, verbose:, 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:, reason:, verbose:, resolve:) + squash_package_commits(commits, file, tap:, reason:, verbose:, 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:, 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:) diff --git a/Library/Homebrew/diagnostic.rb b/Library/Homebrew/diagnostic.rb index 5c6d81e605e0e..783a8cc919130 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/test/dev-cmd/pr-pull_spec.rb b/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb index 19ff337e3f46a..640310468ab34 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,45 @@ 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:)).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:)).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:)).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:)).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:)).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:, 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:)).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:)).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:)).to eq("food: delete") end end end