From c669f1d625cd9795af397bc5a9fb78a77f45ac18 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Sun, 19 Mar 2023 23:44:44 +0800 Subject: [PATCH 1/2] pr-pull: add `--no-cherry-pick` flag Needed for Homebrew/homebrew-core#125556. Without this, `pr-pull` attempts to cherry-pick commits from the PR branch onto the PR branch, and then gets upset that nothing happened. See https://github.com/Homebrew/homebrew-core/actions/runs/4461335852/jobs/7835095294#step:10:40 --- Library/Homebrew/dev-cmd/pr-pull.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 9a1a1618b9..378ae6e2ff 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -22,6 +22,8 @@ module Homebrew description: "Download the bottles but don't upload them." switch "--no-commit", description: "Do not generate a new commit before uploading." + switch "--no-cherry-pick", + description: "Do not cherry-pick commits from the pull request branch." switch "-n", "--dry-run", description: "Print what would be done rather than doing it." switch "--clean", @@ -450,7 +452,7 @@ module Homebrew original_commit = ENV["GITHUB_SHA"].presence || tap.path.git_head unless args.no_commit? - cherry_pick_pr!(user, repo, pr, path: tap.path, args: args) + cherry_pick_pr!(user, repo, pr, path: tap.path, args: args) unless args.no_cherry_pick? if !args.no_autosquash? && !args.dry_run? autosquash!(original_commit, tap: tap, verbose: args.verbose?, resolve: args.resolve?, reason: args.message) @@ -458,7 +460,13 @@ module Homebrew signoff!(tap.path, pr: pr, dry_run: args.dry_run?) unless args.clean? end - unless formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args) + # TODO: Fix determination of `original_commit` above for the `--no-cherry-pick` flag. + # When we pull to the PR branch that contains the commits we want to merge, + # `#formulae_need_bottles?` mistakenly returns `true`, because it thinks + # that no formulae have changed. We probably want to use the commit identified + # by `git merge-base`. + if !formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args) && + !args.no_cherry_pick? ohai "Skipping artifacts for ##{pr} as the formulae don't need bottles" next end From e5b9e97b920f37429142756d95e8a009599ec58a Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Mon, 20 Mar 2023 00:55:20 +0800 Subject: [PATCH 2/2] Use `git merge-base` to determine `original_commit` --- Library/Homebrew/dev-cmd/pr-pull.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 378ae6e2ff..e8ec2195db 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -449,7 +449,13 @@ module Homebrew ohai "Fetching #{tap} pull request ##{pr}" Dir.mktmpdir pr do |dir| cd dir do - original_commit = ENV["GITHUB_SHA"].presence || tap.path.git_head + current_branch_head = ENV["GITHUB_SHA"] || tap.git_head + original_commit = if args.no_cherry_pick? + # 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 + else + current_branch_head + end unless args.no_commit? cherry_pick_pr!(user, repo, pr, path: tap.path, args: args) unless args.no_cherry_pick? @@ -460,13 +466,7 @@ module Homebrew signoff!(tap.path, pr: pr, dry_run: args.dry_run?) unless args.clean? end - # TODO: Fix determination of `original_commit` above for the `--no-cherry-pick` flag. - # When we pull to the PR branch that contains the commits we want to merge, - # `#formulae_need_bottles?` mistakenly returns `true`, because it thinks - # that no formulae have changed. We probably want to use the commit identified - # by `git merge-base`. - if !formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args) && - !args.no_cherry_pick? + unless formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args) ohai "Skipping artifacts for ##{pr} as the formulae don't need bottles" next end