diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index bb8ac10e9d04eed6f2c61b45a35482f7de5bf6b5..88cd9532793b9b268d320edcfe478eb8f1cf0b5f 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -150,17 +150,17 @@ module Homebrew # Cherry picks a single commit that modifies a single file. # Potentially rewords this commit using `determine_bump_subject`. - def reword_formula_commit(commit, file, args:, path: ".") + def reword_formula_commit(commit, file, reason: "", verbose: false, resolve: false, path: ".") formula_file = Pathname.new(path) / file formula_name = formula_file.basename.to_s.chomp(".rb") odebug "Cherry-picking #{formula_file}: #{commit}" - Utils::Git.cherry_pick!(path, commit, verbose: args.verbose?, resolve: args.resolve?) + Utils::Git.cherry_pick!(path, commit, verbose: verbose, resolve: resolve) old_formula = Utils::Git.file_at_commit(path, file, "HEAD^") new_formula = Utils::Git.file_at_commit(path, file, "HEAD") - bump_subject = determine_bump_subject(old_formula, new_formula, formula_file, reason: args.message) + bump_subject = determine_bump_subject(old_formula, new_formula, formula_file, reason: reason).strip subject, body, trailers = separate_commit_message(Utils::Git.commit_message(path)) if subject != bump_subject && !subject.start_with?("#{formula_name}:") @@ -175,7 +175,7 @@ module Homebrew # 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 squash_formula_commits(commits, file, args:, path: ".") + def squash_formula_commits(commits, file, reason: "", verbose: false, resolve: false, path: ".") odebug "Squashing #{file}: #{commits.join " "}" # Format commit messages into something similar to `git fmt-merge-message`. @@ -205,7 +205,7 @@ module Homebrew trailers = [trailers + co_author_trailers].flatten.uniq.compact # Apply the patch series but don't commit anything yet. - Utils::Git.cherry_pick!(path, "--no-commit", *commits, verbose: args.verbose?, resolve: args.resolve?) + Utils::Git.cherry_pick!(path, "--no-commit", *commits, verbose: verbose, resolve: resolve) # Determine the bump subject by comparing the original state of the tree to its current state. formula_file = Pathname.new(path) / file @@ -220,11 +220,7 @@ module Homebrew ohai bump_subject end - def autosquash!(original_commit, path: ".", args: nil) - # Autosquash assumes we've already modified the current state of the git repository, - # so just exit early if we're in a dry run. - return if args.dry_run? - + def autosquash!(original_commit, path: ".", reason: "", verbose: false, resolve: false) original_head = Utils.safe_popen_read("git", "-C", path, "rev-parse", "HEAD").strip commits = Utils.safe_popen_read("git", "-C", path, "rev-list", @@ -261,13 +257,13 @@ module Homebrew 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, args: args) + reword_formula_commit(commit, files.first, path: 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, args: args) + squash_formula_commits(commits, file, path: path, reason: reason, verbose: verbose, resolve: resolve) processed_commits += commits else # We can't split commits (yet) so just raise an error. @@ -397,7 +393,10 @@ module Homebrew cd dir do original_commit = Utils.popen_read("git", "-C", tap.path, "rev-parse", "HEAD").chomp cherry_pick_pr!(user, repo, pr, path: tap.path, args: args) - autosquash!(original_commit, path: tap.path, args: args) if args.autosquash? + if args.autosquash? && !args.dry_run? + autosquash!(original_commit, path: tap.path, + verbose: args.verbose?, resolve: args.resolve?, reason: args.message) + end signoff!(tap.path, pr: pr, dry_run: args.dry_run?) unless args.clean? unless args.no_upload? diff --git a/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb b/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb index 7fb6578bd706a1838e05cfd73767c9bdad32ae46..f11823c52c4d33738f9249590a3dedc308f865e4 100644 --- a/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb +++ b/Library/Homebrew/test/dev-cmd/pr-pull_spec.rb @@ -43,6 +43,27 @@ describe Homebrew do it_behaves_like "parseable arguments" end + describe "#autosquash!" do + it "squashes a formula correctly" do + secondary_author = "Someone Else <me@example.com>" + (path/"Formula").mkpath + formula_file.write(formula) + cd 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)" + original_hash = `git rev-parse HEAD`.chomp + File.open(formula_file, "w") { |f| f.write(formula_revision) } + safe_system Utils::Git.git, "commit", formula_file, "-m", "revision" + File.open(formula_file, "w") { |f| f.write(formula_version) } + safe_system Utils::Git.git, "commit", formula_file, "-m", "version", "--author=#{secondary_author}" + described_class.autosquash!(original_hash, path: ".") + expect(Utils::Git.commit_message(path)).to include("foo 2.0") + expect(Utils::Git.commit_message(path)).to include("Co-authored-by: #{secondary_author}") + end + end + end + describe "#signoff!" do it "signs off a formula" do (path/"Formula").mkpath