Skip to content

Commit

Permalink
Add type signatures for Tap.
Browse files Browse the repository at this point in the history
  • Loading branch information
reitermarkus committed Mar 19, 2024
1 parent e3797d3 commit 751be91
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 32 deletions.
42 changes: 22 additions & 20 deletions Library/Homebrew/dev-cmd/pr-pull.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Check warning on line 160 in Library/Homebrew/dev-cmd/pr-pull.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/dev-cmd/pr-pull.rb#L160

Added line #L160 was not covered by tests

package_file = git_repo.pathname / file
package_name = package_file.basename.to_s.chomp(".rb")

Expand All @@ -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

Check warning on line 171 in Library/Homebrew/dev-cmd/pr-pull.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/dev-cmd/pr-pull.rb#L171

Added line #L171 was not covered by tests
subject, body, trailers = separate_commit_message(git_repo.commit_message)

if subject != bump_subject && !subject.start_with?("#{package_name}:")
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:)
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/diagnostic.rb
Expand Up @@ -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))

Check warning on line 540 in Library/Homebrew/diagnostic.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/diagnostic.rb#L540

Added line #L540 was not covered by tests
end

sig { returns(T.nilable(String)) }
Expand Down
26 changes: 15 additions & 11 deletions Library/Homebrew/test/dev-cmd/pr-pull_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 751be91

Please sign in to comment.