admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

cleanup

Browse Source
This commit is contained in:
Douglas Eichelberger 2023-04-15 18:13:38 -07:00
parent 8307255ce8
commit 1ffb77f821
4 changed files with 48 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
Library/Homebrew/dev-cmd/pr-pull.rb
View File

@ -85,12 +85,12 @@ module Homebrew
[subject, body, trailers] [subject, body, trailers]
end end
def self.signoff!(path, pull_request: nil, dry_run: false) def self.signoff!(git_repo, pull_request: nil, dry_run: false)
subject, body, trailers = separate_commit_message(path.commit_message) subject, body, trailers = separate_commit_message(git_repo.commit_message)
if pull_request if pull_request
# This is a tap pull request and approving reviewers should also sign-off. # This is a tap pull request and approving reviewers should also sign-off.
tap = Tap.from_path(path) tap = Tap.from_path(git_repo.pathname)
review_trailers = GitHub.approved_reviews(tap.user, tap.full_name.split("/").last, pull_request).map do |r| review_trailers = GitHub.approved_reviews(tap.user, tap.full_name.split("/").last, pull_request).map do |r|
"Signed-off-by: #{r["name"]} <#{r["email"]}>" "Signed-off-by: #{r["name"]} <#{r["email"]}>"
end end
@ -101,7 +101,7 @@ module Homebrew
body += "\n\n#{close_message}" unless body.include? close_message body += "\n\n#{close_message}" unless body.include? close_message
end end
git_args = Utils::Git.git, "-C", path, "commit", "--amend", "--signoff", "--allow-empty", "--quiet", git_args = Utils::Git.git, "-C", git_repo.pathname, "commit", "--amend", "--signoff", "--allow-empty", "--quiet",
"--message", subject, "--message", body, "--message", trailers "--message", subject, "--message", body, "--message", trailers
if dry_run if dry_run
@ -156,21 +156,21 @@ module Homebrew
# Cherry picks a single commit that modifies a single file. # Cherry picks a single commit that modifies a single file.
# Potentially rewords this commit using {determine_bump_subject}. # Potentially rewords this commit using {determine_bump_subject}.
def self.reword_package_commit(commit, file, reason: "", verbose: false, resolve: false, path: ".") def self.reword_package_commit(commit, file, reason: "", verbose: false, resolve: false, git_repo:)
package_file = Pathname.new(path) / file package_file = git_repo.pathname / file
package_name = package_file.basename.to_s.chomp(".rb") package_name = package_file.basename.to_s.chomp(".rb")
odebug "Cherry-picking #{package_file}: #{commit}" odebug "Cherry-picking #{package_file}: #{commit}"
Utils::Git.cherry_pick!(path, commit, verbose: verbose, resolve: resolve) Utils::Git.cherry_pick!(git_repo, commit, verbose: verbose, resolve: resolve)
old_package = Utils::Git.file_at_commit(path, file, "HEAD^") old_package = Utils::Git.file_at_commit(git_repo, file, "HEAD^")
new_package = Utils::Git.file_at_commit(path, file, "HEAD") new_package = Utils::Git.file_at_commit(git_repo, 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).strip
subject, body, trailers = separate_commit_message(path.commit_message) subject, body, trailers = separate_commit_message(git_repo.commit_message)
if subject != bump_subject && !subject.start_with?("#{package_name}:") if subject != bump_subject && !subject.start_with?("#{package_name}:")
safe_system("git", "-C", path, "commit", "--amend", "-q", safe_system("git", "-C", git_repo.pathname, "commit", "--amend", "-q",
"-m", bump_subject, "-m", subject, "-m", body, "-m", trailers) "-m", bump_subject, "-m", subject, "-m", body, "-m", trailers)
ohai bump_subject ohai bump_subject
else else
@ -181,7 +181,7 @@ module Homebrew
# Cherry picks multiple commits that each modify a single file. # Cherry picks multiple commits that each modify a single file.
# Words the commit according to {determine_bump_subject} with the body # Words the commit according to {determine_bump_subject} with the body
# corresponding to all the original commit messages combined. # corresponding to all the original commit messages combined.
def self.squash_package_commits(commits, file, reason: "", verbose: false, resolve: false, path: ".") def self.squash_package_commits(commits, file, reason: "", verbose: false, resolve: false, git_repo:)
odebug "Squashing #{file}: #{commits.join " "}" odebug "Squashing #{file}: #{commits.join " "}"
# Format commit messages into something similar to `git fmt-merge-message`. # Format commit messages into something similar to `git fmt-merge-message`.
@ -192,35 +192,35 @@ module Homebrew
messages = [] messages = []
trailers = [] trailers = []
commits.each do |commit| commits.each do |commit|
subject, body, trailer = separate_commit_message(path.commit_message(commit)) subject, body, trailer = separate_commit_message(git_repo.commit_message(commit))
body = body.lines.map { |line| " #{line.strip}" }.join("\n") body = body.lines.map { |line| " #{line.strip}" }.join("\n")
messages << "* #{subject}\n#{body}".strip messages << "* #{subject}\n#{body}".strip
trailers << trailer trailers << trailer
end end
# Get the set of authors in this series. # Get the set of authors in this series.
authors = Utils.safe_popen_read("git", "-C", path, "show", authors = Utils.safe_popen_read("git", "-C", git_repo.pathname, "show",
"--no-patch", "--pretty=%an <%ae>", *commits).lines.map(&:strip).uniq.compact "--no-patch", "--pretty=%an <%ae>", *commits).lines.map(&:strip).uniq.compact
# Get the author and date of the first commit of this series, which we use for the squashed commit. # Get the author and date of the first commit of this series, which we use for the squashed commit.
original_author = authors.shift original_author = authors.shift
original_date = Utils.safe_popen_read "git", "-C", path, "show", "--no-patch", "--pretty=%ad", commits.first original_date = Utils.safe_popen_read "git", "-C", git_repo.pathname, "show", "--no-patch", "--pretty=%ad", commits.first
# Generate trailers for coauthors and combine them with the existing trailers. # Generate trailers for coauthors and combine them with the existing trailers.
co_author_trailers = authors.map { |au| "Co-authored-by: #{au}" } co_author_trailers = authors.map { |au| "Co-authored-by: #{au}" }
trailers = [trailers + co_author_trailers].flatten.uniq.compact trailers = [trailers + co_author_trailers].flatten.uniq.compact
# Apply the patch series but don't commit anything yet. # Apply the patch series but don't commit anything yet.
Utils::Git.cherry_pick!(path, "--no-commit", *commits, verbose: verbose, resolve: resolve) Utils::Git.cherry_pick!(git_repo, "--no-commit", *commits, verbose: verbose, resolve: resolve)
# Determine the bump subject by comparing the original state of the tree to its current state. # Determine the bump subject by comparing the original state of the tree to its current state.
package_file = Pathname.new(path) / file package_file = git_repo.pathname / file
old_package = Utils::Git.file_at_commit(path, file, "#{commits.first}^") old_package = Utils::Git.file_at_commit(git_repo, file, "#{commits.first}^")
new_package = package_file.read 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)
# Commit with the new subject, body, and trailers. # Commit with the new subject, body, and trailers.
safe_system("git", "-C", path, "commit", "--quiet", safe_system("git", "-C", git_repo.pathname, "commit", "--quiet",
"-m", bump_subject, "-m", messages.join("\n"), "-m", trailers.join("\n"), "-m", bump_subject, "-m", messages.join("\n"), "-m", trailers.join("\n"),
"--author", original_author, "--date", original_date, "--", file) "--author", original_author, "--date", original_date, "--", file)
ohai bump_subject ohai bump_subject
@ -228,7 +228,7 @@ module Homebrew
# TODO: fix test in `test/dev-cmd/pr-pull_spec.rb` and assume `cherry_picked: false`. # TODO: fix test in `test/dev-cmd/pr-pull_spec.rb` and assume `cherry_picked: false`.
def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: false, cherry_picked: true) def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: false, cherry_picked: true)
original_head = tap.path.head_ref original_head = tap.git_repo.head_ref
commits = Utils.safe_popen_read("git", "-C", tap.path, "rev-list", commits = Utils.safe_popen_read("git", "-C", tap.path, "rev-list",
"--reverse", "#{original_commit}..HEAD").lines.map(&:strip) "--reverse", "#{original_commit}..HEAD").lines.map(&:strip)
@ -268,13 +268,13 @@ module Homebrew
files = commits_to_files[commit] files = commits_to_files[commit]
if files.length == 1 && files_to_commits[files.first].length == 1 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. # If there's a 1:1 mapping of commits to files, just cherry pick and (maybe) reword.
reword_package_commit(commit, files.first, path: tap.path, reason: reason, verbose: verbose, resolve: resolve) reword_package_commit(commit, files.first, git_repo: tap.git_repo, reason: reason, verbose: verbose, resolve: resolve)
processed_commits << commit processed_commits << commit
elsif files.length == 1 && files_to_commits[files.first].length > 1 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. # If multiple commits modify a single file, squash them down into a single commit.
file = files.first file = files.first
commits = files_to_commits[file] commits = files_to_commits[file]
squash_package_commits(commits, file, path: tap.path, reason: reason, verbose: verbose, resolve: resolve) squash_package_commits(commits, file, git_repo: tap.git_repo, reason: reason, verbose: verbose, resolve: resolve)
processed_commits += commits processed_commits += commits
else else
# We can't split commits (yet) so just raise an error. # We can't split commits (yet) so just raise an error.
@ -450,8 +450,9 @@ module Homebrew
_, user, repo, pr = *url_match _, user, repo, pr = *url_match
odie "Not a GitHub pull request: #{arg}" unless pr odie "Not a GitHub pull request: #{arg}" unless pr
if !tap.path.default_origin_branch? || args.branch_okay? || args.clean? git_repo = tap.git_repo
opoo "Current branch is #{tap.path.branch_name}: do you need to pull inside #{tap.path.origin_branch_name}?" if !git_repo.default_origin_branch? || args.branch_okay? || args.clean?
opoo "Current branch is #{git_repo.branch_name}: do you need to pull inside #{git_repo.origin_branch_name}?"
end end
pr_labels = GitHub.pull_request_labels(user, repo, pr) pr_labels = GitHub.pull_request_labels(user, repo, pr)
@ -464,7 +465,7 @@ module Homebrew
ohai "Fetching #{tap} pull request ##{pr}" ohai "Fetching #{tap} pull request ##{pr}"
Dir.mktmpdir pr do |dir| Dir.mktmpdir pr do |dir|
cd dir do cd dir do
current_branch_head = ENV["GITHUB_SHA"] || tap.head_ref current_branch_head = ENV["GITHUB_SHA"] || tap.git_head
original_commit = if args.no_cherry_pick? original_commit = if args.no_cherry_pick?
# TODO: Handle the case where `merge-base` returns multiple commits. # TODO: Handle the case where `merge-base` returns multiple commits.
Utils.safe_popen_read("git", "-C", tap.path, "merge-base", "origin/HEAD", current_branch_head).strip Utils.safe_popen_read("git", "-C", tap.path, "merge-base", "origin/HEAD", current_branch_head).strip

7
Library/Homebrew/extend/git_repo_path.rbi
View File

@ -7,11 +7,4 @@ class GitRepoPath < SimpleDelegator
# @see https://github.com/sorbet/sorbet/issues/2378#issuecomment-569474238 # @see https://github.com/sorbet/sorbet/issues/2378#issuecomment-569474238
sig { returns(Pathname) } sig { returns(Pathname) }
def __getobj__; end def __getobj__; end
def /(_arg0); end
def parent; end
def abv; end
def rmtree; end
def cd; end
def directory?; end
end end

29
Library/Homebrew/tap.rb
View File

@ -93,7 +93,6 @@ class Tap
# The local path to this {Tap}. # The local path to this {Tap}.
# e.g. `/usr/local/Library/Taps/user/homebrew-repo` # e.g. `/usr/local/Library/Taps/user/homebrew-repo`
sig { returns(GitRepoPath) }
attr_reader :path attr_reader :path
alias git_repo path alias git_repo path
@ -138,7 +137,7 @@ class Tap
def remote def remote
return default_remote unless installed? return default_remote unless installed?
@remote ||= path.origin_url @remote ||= git_repo.origin_url
end end
# The remote repository name of this {Tap}. # The remote repository name of this {Tap}.
@ -173,21 +172,21 @@ class Tap
def git_branch def git_branch
raise TapUnavailableError, name unless installed? raise TapUnavailableError, name unless installed?
path.branch_name git_repo.branch_name
end end
# git HEAD for this {Tap}. # git HEAD for this {Tap}.
def git_head def git_head
raise TapUnavailableError, name unless installed? raise TapUnavailableError, name unless installed?
@git_head ||= path.head_ref @git_head ||= git_repo.head_ref
end end
# Time since last git commit for this {Tap}. # Time since last git commit for this {Tap}.
def git_last_commit def git_last_commit
raise TapUnavailableError, name unless installed? raise TapUnavailableError, name unless installed?
path.last_committed git_repo.last_committed
end end
# The issues URL of this {Tap}. # The issues URL of this {Tap}.
@ -317,7 +316,7 @@ class Tap
ignore_interrupts do ignore_interrupts do
# wait for git to possibly cleanup the top directory when interrupt happens. # wait for git to possibly cleanup the top directory when interrupt happens.
sleep 0.1 sleep 0.1
FileUtils.rm_rf git_repo.pathname FileUtils.rm_rf path
path.parent.rmdir_if_possible path.parent.rmdir_if_possible
end end
raise raise
@ -388,20 +387,20 @@ class Tap
$stderr.ohai "#{name}: changed remote from #{remote} to #{requested_remote}" unless quiet $stderr.ohai "#{name}: changed remote from #{remote} to #{requested_remote}" unless quiet
end end
current_upstream_head = T.must(path.origin_branch_name) current_upstream_head = git_repo.origin_branch_name
return if requested_remote.blank? && path.origin_has_branch?(current_upstream_head) return if requested_remote.blank? && git_repo.origin_has_branch?(current_upstream_head)
args = %w[fetch] args = %w[fetch]
args << "--quiet" if quiet args << "--quiet" if quiet
args << "origin" args << "origin"
safe_system "git", "-C", path, *args safe_system "git", "-C", path, *args
path.set_head_origin_auto git_repo.set_head_origin_auto
new_upstream_head = T.must(path.origin_branch_name) new_upstream_head = git_repo.origin_branch_name
return if new_upstream_head == current_upstream_head return if new_upstream_head == current_upstream_head
path.rename_branch old: current_upstream_head, new: new_upstream_head git_repo.rename_branch old: current_upstream_head, new: new_upstream_head
path.set_upstream_branch local: new_upstream_head, origin: new_upstream_head git_repo.set_upstream_branch local: new_upstream_head, origin: new_upstream_head
return if quiet return if quiet
@ -464,7 +463,7 @@ class Tap
sig { returns(T::Array[Pathname]) } sig { returns(T::Array[Pathname]) }
def potential_formula_dirs def potential_formula_dirs
@potential_formula_dirs ||= [path/"Formula", path/"HomebrewFormula", git_repo.pathname].freeze @potential_formula_dirs ||= [path/"Formula", path/"HomebrewFormula", path].freeze
end end
# Path to the directory of all {Cask} files for this {Tap}. # Path to the directory of all {Cask} files for this {Tap}.
@ -569,7 +568,7 @@ class Tap
sig { params(file: T.any(String, Pathname)).returns(T::Boolean) } sig { params(file: T.any(String, Pathname)).returns(T::Boolean) }
def formula_file?(file) def formula_file?(file)
file = Pathname.new(file) unless file.is_a? Pathname file = Pathname.new(file) unless file.is_a? Pathname
file = file.expand_path(git_repo.pathname) file = file.expand_path(path)
return false unless ruby_file?(file) return false unless ruby_file?(file)
return false if cask_file?(file) return false if cask_file?(file)
@ -582,7 +581,7 @@ class Tap
sig { params(file: T.any(String, Pathname)).returns(T::Boolean) } sig { params(file: T.any(String, Pathname)).returns(T::Boolean) }
def cask_file?(file) def cask_file?(file)
file = Pathname.new(file) unless file.is_a? Pathname file = Pathname.new(file) unless file.is_a? Pathname
file = file.expand_path(git_repo.pathname) file = file.expand_path(path)
return false unless ruby_file?(file) return false unless ruby_file?(file)
file.to_s.start_with?("#{cask_dir}/") file.to_s.start_with?("#{cask_dir}/")

15
Library/Homebrew/test/dev-cmd/pr-pull_spec.rb
View File

@ -81,7 +81,7 @@ describe "brew pr-pull" do
let(:tap) { Tap.fetch("Homebrew", "foo") } let(:tap) { Tap.fetch("Homebrew", "foo") }
let(:formula_file) { tap.path/"Formula/foo.rb" } let(:formula_file) { tap.path/"Formula/foo.rb" }
let(:cask_file) { tap.cask_dir/"food.rb" } let(:cask_file) { tap.cask_dir/"food.rb" }
let(:path) { GitRepoPath.new(Tap::TAP_DIRECTORY/"homebrew/homebrew-foo") } let(:path) { Pathname(Tap::TAP_DIRECTORY/"homebrew/homebrew-foo") }
describe "#autosquash!" do describe "#autosquash!" do
it "squashes a formula or cask correctly" do it "squashes a formula or cask correctly" do
@ -98,8 +98,8 @@ describe "brew pr-pull" do
File.write(formula_file, formula_version) File.write(formula_file, formula_version)
safe_system Utils::Git.git, "commit", formula_file, "-m", "version", "--author=#{secondary_author}" safe_system Utils::Git.git, "commit", formula_file, "-m", "version", "--author=#{secondary_author}"
described_class.autosquash!(original_hash, tap: tap) described_class.autosquash!(original_hash, tap: tap)
expect(tap.path.commit_message).to include("foo 2.0") expect(tap.git_repo.commit_message).to include("foo 2.0")
expect(tap.path.commit_message).to include("Co-authored-by: #{secondary_author}") expect(tap.git_repo.commit_message).to include("Co-authored-by: #{secondary_author}")
end end
(path/"Casks").mkpath (path/"Casks").mkpath
@ -113,8 +113,9 @@ describe "brew pr-pull" do
File.write(cask_file, cask_version) File.write(cask_file, cask_version)
safe_system Utils::Git.git, "commit", cask_file, "-m", "version", "--author=#{secondary_author}" safe_system Utils::Git.git, "commit", cask_file, "-m", "version", "--author=#{secondary_author}"
described_class.autosquash!(original_hash, tap: tap) described_class.autosquash!(original_hash, tap: tap)
expect(path.commit_message).to include("food 2.0") git_repo = GitRepoPath.new(path)
expect(path.commit_message).to include("Co-authored-by: #{secondary_author}") expect(git_repo.commit_message).to include("food 2.0")
expect(git_repo.commit_message).to include("Co-authored-by: #{secondary_author}")
end end
end end
end end
@ -129,7 +130,7 @@ describe "brew pr-pull" do
safe_system Utils::Git.git, "commit", "-m", "foo 1.0 (new formula)" safe_system Utils::Git.git, "commit", "-m", "foo 1.0 (new formula)"
end end
described_class.signoff!(tap.path) described_class.signoff!(tap.path)
expect(tap.path.commit_message).to include("Signed-off-by:") expect(tap.git_repo.commit_message).to include("Signed-off-by:")
(path/"Casks").mkpath (path/"Casks").mkpath
cask_file.write(cask) cask_file.write(cask)
@ -138,7 +139,7 @@ describe "brew pr-pull" do
safe_system Utils::Git.git, "commit", "-m", "food 1.0 (new cask)" safe_system Utils::Git.git, "commit", "-m", "food 1.0 (new cask)"
end end
described_class.signoff!(tap.path) described_class.signoff!(tap.path)
expect(tap.path.commit_message).to include("Signed-off-by:") expect(tap.git_repo.commit_message).to include("Signed-off-by:")
end end
end end