Skip to content

Commit

Permalink
Merge pull request #12944 from Bo98/pr-pull-formula-dir
Browse files Browse the repository at this point in the history
dev-cmd/pr-pull: consider alternative tap formula directories
  • Loading branch information
Bo98 committed Mar 1, 2022
2 parents 5bce84e + e4159a7 commit 0afd0d7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 23 deletions.
23 changes: 11 additions & 12 deletions Library/Homebrew/dev-cmd/pr-pull.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,22 +209,21 @@ def squash_formula_commits(commits, file, reason: "", verbose: false, resolve: f
ohai bump_subject
end

def autosquash!(original_commit, path: ".", reason: "", verbose: false, resolve: false)
path = Pathname(path).extend(GitRepositoryExtension)
original_head = path.git_head
def autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: false)
original_head = tap.path.git_head

commits = Utils.safe_popen_read("git", "-C", path, "rev-list",
commits = Utils.safe_popen_read("git", "-C", tap.path, "rev-list",
"--reverse", "#{original_commit}..HEAD").lines.map(&:strip)

# Generate a bidirectional mapping of commits <=> formula files.
files_to_commits = {}
commits_to_files = commits.to_h do |commit|
files = Utils.safe_popen_read("git", "-C", path, "diff-tree", "--diff-filter=AMD",
files = Utils.safe_popen_read("git", "-C", tap.path, "diff-tree", "--diff-filter=AMD",
"-r", "--name-only", "#{commit}^", commit).lines.map(&:strip)
files.each do |file|
files_to_commits[file] ||= []
files_to_commits[file] << commit
next if %r{^Formula/.*\.rb$}.match?(file)
next if (tap.path/file).dirname == tap.formula_dir && File.extname(file) == ".rb"

odie <<~EOS
Autosquash can't squash commits that modify non-formula files.
Expand All @@ -236,7 +235,7 @@ def autosquash!(original_commit, path: ".", reason: "", verbose: false, resolve:
end

# Reset to state before cherry-picking.
safe_system "git", "-C", path, "reset", "--hard", original_commit
safe_system "git", "-C", tap.path, "reset", "--hard", original_commit

# Iterate over every commit in the pull request series, but if we have to squash
# multiple commits into one, ensure that we skip over commits we've already squashed.
Expand All @@ -247,13 +246,13 @@ def autosquash!(original_commit, path: ".", reason: "", verbose: false, resolve:
files = commits_to_files[commit]
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_formula_commit(commit, files.first, path: path, reason: reason, verbose: verbose, resolve: resolve)
reword_formula_commit(commit, files.first, path: tap.path, 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_formula_commits(commits, file, path: path, reason: reason, verbose: verbose, resolve: resolve)
squash_formula_commits(commits, file, path: tap.path, reason: reason, verbose: verbose, resolve: resolve)
processed_commits += commits
else
# We can't split commits (yet) so just raise an error.
Expand All @@ -266,8 +265,8 @@ def autosquash!(original_commit, path: ".", reason: "", verbose: false, resolve:
end
rescue
opoo "Autosquash encountered an error; resetting to original cherry-picked state at #{original_head}"
system "git", "-C", path, "reset", "--hard", original_head
system "git", "-C", path, "cherry-pick", "--abort"
system "git", "-C", tap.path, "reset", "--hard", original_head
system "git", "-C", tap.path, "cherry-pick", "--abort"
raise
end

Expand Down Expand Up @@ -367,7 +366,7 @@ def pr_pull
unless args.no_commit?
cherry_pick_pr!(user, repo, pr, path: tap.path, args: args)
if !args.no_autosquash? && !args.dry_run?
autosquash!(original_commit, path: tap.path,
autosquash!(original_commit, tap: tap,
verbose: args.verbose?, resolve: args.resolve?, reason: args.message)
end
signoff!(tap.path, pr: pr, dry_run: args.dry_run?) unless args.clean?
Expand Down
22 changes: 11 additions & 11 deletions Library/Homebrew/test/dev-cmd/pr-pull_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ class Foo < Formula
end
EOS
end
let(:formula_file) { path/"Formula/foo.rb" }
let(:path) { (Tap::TAP_DIRECTORY/"homebrew/homebrew-foo").extend(GitRepositoryExtension) }
let(:tap) { Tap.fetch("Homebrew", "foo") }
let(:formula_file) { tap.path/"Formula/foo.rb" }

describe "#autosquash!" do
it "squashes a formula correctly" do
secondary_author = "Someone Else <me@example.com>"
(path/"Formula").mkpath
(tap.path/"Formula").mkpath
formula_file.write(formula)
cd path do
cd tap.path do
safe_system Utils::Git.git, "init"
safe_system Utils::Git.git, "add", formula_file
safe_system Utils::Git.git, "commit", "-m", "foo 1.0 (new formula)"
Expand All @@ -57,24 +57,24 @@ class Foo < Formula
safe_system Utils::Git.git, "commit", formula_file, "-m", "revision"
File.write(formula_file, formula_version)
safe_system Utils::Git.git, "commit", formula_file, "-m", "version", "--author=#{secondary_author}"
described_class.autosquash!(original_hash, path: path)
expect(path.git_commit_message).to include("foo 2.0")
expect(path.git_commit_message).to include("Co-authored-by: #{secondary_author}")
described_class.autosquash!(original_hash, tap: tap)
expect(tap.path.git_commit_message).to include("foo 2.0")
expect(tap.path.git_commit_message).to include("Co-authored-by: #{secondary_author}")
end
end
end

describe "#signoff!" do
it "signs off a formula" do
(path/"Formula").mkpath
(tap.path/"Formula").mkpath
formula_file.write(formula)
cd path do
cd tap.path do
safe_system Utils::Git.git, "init"
safe_system Utils::Git.git, "add", formula_file
safe_system Utils::Git.git, "commit", "-m", "foo 1.0 (new formula)"
end
described_class.signoff!(path)
expect(path.git_commit_message).to include("Signed-off-by:")
described_class.signoff!(tap.path)
expect(tap.path.git_commit_message).to include("Signed-off-by:")
end
end

Expand Down

0 comments on commit 0afd0d7

Please sign in to comment.