pr-pull: allow pulling casks

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

@@ -108,51 +108,53 @@ def signoff!(path, pr: nil, dry_run: false)
     end
   end
 
-  def determine_bump_subject(old_contents, new_contents, formula_path, reason: nil)
-    formula_path = Pathname(formula_path)
-    formula_name = formula_path.basename.to_s.chomp(".rb")
+  def determine_bump_subject(old_contents, new_contents, subject_path, reason: nil)
+    subject_path = Pathname(subject_path)
+    subject_name = subject_path.basename.to_s.chomp(".rb")
+    is_cask      = subject_path.to_s.include? "Casks"
 
-    new_formula = begin
-      Formulary.from_contents(formula_name, formula_path, new_contents, :stable)
+    new_package = begin
+      Formulary.from_contents(subject_name, subject_path, new_contents, :stable)
     rescue FormulaUnavailableError
-      nil
+      Cask::Cask.new(subject_name, sourcefile_path: subject_path, source: new_contents) if is_cask
     end
 
-    return "#{formula_name}: delete #{reason}".strip if new_formula.blank?
+    return "#{subject_name}: delete #{reason}".strip if new_package.blank?
 
-    old_formula = begin
-      Formulary.from_contents(formula_name, formula_path, old_contents, :stable)
+    old_package = begin
+      Formulary.from_contents(subject_name, subject_path, old_contents, :stable)
     rescue FormulaUnavailableError
-      nil
+      Cask::Cask.new(subject_name, sourcefile_path: subject_path, source: old_contents) if is_cask
     end
 
-    return "#{formula_name} #{new_formula.stable.version} (new formula)" if old_formula.blank?
+    return "#{subject_name} #{new_package.version} (new #{is_cask ? "cask" : "formula"})" if old_package.blank?
 
-    if old_formula.stable.version != new_formula.stable.version
-      "#{formula_name} #{new_formula.stable.version}"
-    elsif old_formula.revision != new_formula.revision
-      "#{formula_name}: revision #{reason}".strip
+    ohai old_package.version, new_package.version
+    if old_package.version != new_package.version
+      "#{subject_name} #{new_package.version}"
+    elsif old_package.respond_to?(:revision) && old_package.revision != new_package.revision
+      "#{subject_name}: revision #{reason}".strip
     else
-      "#{formula_name}: #{reason || "rebuild"}".strip
+      "#{subject_name}: #{reason || "rebuild"}".strip
     end
   end
 
   # Cherry picks a single commit that modifies a single file.
   # Potentially rewords this commit using {determine_bump_subject}.
-  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")
+  def reword_package_commit(commit, file, reason: "", verbose: false, resolve: false, path: ".")
+    package_file = Pathname.new(path) / file
+    package_name = package_file.basename.to_s.chomp(".rb")
 
-    odebug "Cherry-picking #{formula_file}: #{commit}"
+    odebug "Cherry-picking #{package_file}: #{commit}"
     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")
+    old_package = Utils::Git.file_at_commit(path, file, "HEAD^")
+    new_package = Utils::Git.file_at_commit(path, file, "HEAD")
 
-    bump_subject = determine_bump_subject(old_formula, new_formula, formula_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.git_commit_message)
 
-    if subject != bump_subject && !subject.start_with?("#{formula_name}:")
+    if subject != bump_subject && !subject.start_with?("#{package_name}:")
       safe_system("git", "-C", path, "commit", "--amend", "-q",
                   "-m", bump_subject, "-m", subject, "-m", body, "-m", trailers)
       ohai bump_subject
@@ -164,7 +166,7 @@ def reword_formula_commit(commit, file, reason: "", verbose: false, resolve: 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 squash_formula_commits(commits, file, reason: "", verbose: false, resolve: false, path: ".")
+  def squash_package_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`.
@@ -197,10 +199,10 @@ def squash_formula_commits(commits, file, reason: "", verbose: false, resolve: f
     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
-    old_formula = Utils::Git.file_at_commit(path, file, "#{commits.first}^")
-    new_formula = File.read(formula_file)
-    bump_subject = determine_bump_subject(old_formula, new_formula, formula_file, reason: reason)
+    package_file = Pathname.new(path) / file
+    old_package = Utils::Git.file_at_commit(path, file, "#{commits.first}^")
+    new_package = File.read(package_file)
+    bump_subject = determine_bump_subject(old_package, new_package, package_file, reason: reason)
 
     # Commit with the new subject, body, and trailers.
     safe_system("git", "-C", path, "commit", "--quiet",
@@ -216,18 +218,18 @@ def autosquash!(original_commit, path: ".", reason: "", verbose: false, resolve:
     commits = Utils.safe_popen_read("git", "-C", path, "rev-list",
                                     "--reverse", "#{original_commit}..HEAD").lines.map(&:strip)
 
-    # Generate a bidirectional mapping of commits <=> formula files.
+    # Generate a bidirectional mapping of commits <=> package 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",
                                     "-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 %r{^Formula|Casks/.*\.rb$}.match?(file)
 
         odie <<~EOS
-          Autosquash can't squash commits that modify non-formula files.
+          Autosquash can't squash commits that modify non-package files.
             File:   #{file}
             Commit: #{commit}
         EOS
@@ -247,13 +249,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_package_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, reason: reason, verbose: verbose, resolve: resolve)
+        squash_package_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.
@@ -287,34 +289,49 @@ def cherry_pick_pr!(user, repo, pr, args:, path: ".")
     Utils::Git.cherry_pick!(path, "--ff", "--allow-empty", *commits, verbose: args.verbose?, resolve: args.resolve?)
   end
 
-  def formulae_need_bottles?(tap, original_commit, user, repo, pr, args:)
+  def packages_need_bottles?(tap, original_commit, user, repo, pr, args:)
     return if args.dry_run?
 
     labels = GitHub.pull_request_labels(user, repo, pr)
 
     return false if labels.include?("CI-syntax-only") || labels.include?("CI-no-bottles")
 
-    changed_formulae(tap, original_commit).any? do |f|
-      !f.bottle_unneeded? && !f.bottle_disabled?
+    changed_packages(tap, original_commit).any? do |f|
+      !f.instance_of?(Cask::Cask) && !f.bottle_unneeded? && !f.bottle_disabled?
     end
   end
 
-  def changed_formulae(tap, original_commit)
+  def changed_packages(tap, original_commit)
     if Homebrew::EnvConfig.disable_load_formula?
       opoo "Can't check if updated bottles are necessary as HOMEBREW_DISABLE_LOAD_FORMULA is set!"
       return
     end
 
-    Utils.popen_read("git", "-C", tap.path, "diff-tree",
-                     "-r", "--name-only", "--diff-filter=AM",
-                     original_commit, "HEAD", "--", tap.formula_dir)
-         .lines
-         .map do |line|
+    formulae = Utils.popen_read("git", "-C", tap.path, "diff-tree",
+                                "-r", "--name-only", "--diff-filter=AM",
+                                original_commit, "HEAD", "--", tap.formula_dir)
+                    .lines
+                    .map do |line|
       next unless line.end_with? ".rb\n"
 
       name = "#{tap.name}/#{File.basename(line.chomp, ".rb")}"
-      Formula[name]
+      begin
+        Formulary.resolve(name)
+      rescue FormulaUnavailableError
+        nil
+      end
+    end.compact
+    casks = Utils.popen_read("git", "-C", tap.path, "diff-tree",
+                             "-r", "--name-only", "--diff-filter=AM",
+                             original_commit, "HEAD", "--", tap.cask_dir)
+                 .lines
+                 .map do |line|
+      next unless line.end_with? ".rb\n"
+
+      name = "#{tap.name}/#{File.basename(line.chomp, ".rb")}"
+      Cask::CaskLoader.load(name)
     end.compact
+    formulae + casks
   end
 
   def download_artifact(url, dir, pr)
@@ -373,8 +390,8 @@ def pr_pull
             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)
-            ohai "Skipping artifacts for ##{pr} as the formulae don't need bottles"
+          unless packages_need_bottles?(tap, original_commit, user, repo, pr, args: args)
+            ohai "Skipping artifacts for ##{pr} as the packages don't need bottles"
             next
           end
 

Library/Homebrew/test/dev-cmd/pr-pull_spec.rb

@@ -40,11 +40,34 @@ class Foo < Formula
         end
       EOS
     end
+    let(:cask_rebuild) do
+      <<~EOS
+        cask "food" do
+          desc "Helpful description"
+          url "https://brew.sh/food-1.0.tgz"
+        end
+      EOS
+    end
+    let(:cask_version) do
+      <<~EOS
+        cask "food" do
+          url "https://brew.sh/food-2.0.tgz"
+        end
+      EOS
+    end
+    let(:cask) do
+      <<~EOS
+        cask "food" do
+          url "https://brew.sh/food-1.0.tgz"
+        end
+      EOS
+    end
     let(:formula_file) { path/"Formula/foo.rb" }
+    let(:cask_file) { path/"Casks/food.rb" }
     let(:path) { (Tap::TAP_DIRECTORY/"homebrew/homebrew-foo").extend(GitRepositoryExtension) }
 
     describe "#autosquash!" do
-      it "squashes a formula correctly" do
+      it "squashes a formula and cask correctly" do
         secondary_author = "Someone Else <me@example.com>"
         (path/"Formula").mkpath
         formula_file.write(formula)
@@ -61,11 +84,24 @@ class Foo < Formula
           expect(path.git_commit_message).to include("foo 2.0")
           expect(path.git_commit_message).to include("Co-authored-by: #{secondary_author}")
         end
+
+        (path/"Casks").mkpath
+        cask_file.write(cask)
+        cd path do
+          safe_system Utils::Git.git, "add", cask_file
+          safe_system Utils::Git.git, "commit", "-m", "food 1.0 (new cask)"
+          original_hash = `git rev-parse HEAD`.chomp
+          File.write(cask_file, cask_version)
+          safe_system Utils::Git.git, "commit", cask_file, "-m", "version", "--author=#{secondary_author}"
+          described_class.autosquash!(original_hash, path: path)
+          expect(path.git_commit_message).to include("food 2.0")
+          expect(path.git_commit_message).to include("Co-authored-by: #{secondary_author}")
+        end
       end
     end
 
     describe "#signoff!" do
-      it "signs off a formula" do
+      it "signs off a formula and cask" do
         (path/"Formula").mkpath
         formula_file.write(formula)
         cd path do
@@ -75,6 +111,15 @@ class Foo < Formula
         end
         described_class.signoff!(path)
         expect(path.git_commit_message).to include("Signed-off-by:")
+
+        (path/"Casks").mkpath
+        cask_file.write(cask)
+        cd path do
+          safe_system Utils::Git.git, "add", cask_file
+          safe_system Utils::Git.git, "commit", "-m", "food 1.0 (new cask)"
+        end
+        described_class.signoff!(path)
+        expect(path.git_commit_message).to include("Signed-off-by:")
       end
     end
 
@@ -83,10 +128,18 @@ class Foo < Formula
         expect(described_class.determine_bump_subject("", formula, formula_file)).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)")
+      end
+
       it "correctly bumps a formula version" do
         expect(described_class.determine_bump_subject(formula, formula_version, formula_file)).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")
+      end
+
       it "correctly bumps a formula revision with reason" do
         expect(described_class.determine_bump_subject(
                  formula, formula_revision, formula_file, reason: "for fun"
@@ -100,6 +153,10 @@ class Foo < Formula
       it "correctly bumps a formula deletion" do
         expect(described_class.determine_bump_subject(formula, "", formula_file)).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")
+      end
     end
   end
 end