DSL method "apply" to specify patch files

The "apply" DSL method can be called from patch-do blocks to specify
the paths within an archive of the desired patch files, which will be
applied in the order in which they were supplied to the "apply" calls.
If "apply" isn't used, raise an error whenever the extracted directory
doesn't contain exactly one file.

The "apply" method can be called zero or more times within a patch-do
block with the following syntaxes supported:

    apply "single_apply"
    apply "multiple_apply_1", "multiple_apply_2"
    apply [array_of_apply]

If apply must be used, a single call using the second syntax above is
usually best practice. Each apply leaf should be the relative path to a
specific patch file in the extracted directory.

For example, if extracting this-v123-patches.tar.gz gives you

    this-123
    this-123/.DS_Store
    this-123/LICENSE.txt
    this-123/patches
    this-123/patches/A.diff
    this-123/patches/B.diff
    this-123/patches/C.diff
    this-123/README.txt

and you want to apply only B.diff and C.diff, then you need to use
"patches/B.diff" and "patches/C.diff" for the lowest-level apply leaves.

The code was provided by Xu Cheng. Any mistakes are mine.

Library/Homebrew/exceptions.rb

@@ -441,6 +441,9 @@ class DuplicateResourceError < ArgumentError
   end
 end
 
+# raised when a single patch file is not found and apply hasn't been specified
+class MissingApplyError < RuntimeError ; end
+
 class BottleVersionMismatchError < RuntimeError
   def initialize(bottle_file, bottle_version, formula, formula_version)
     super <<-EOS.undent

Library/Homebrew/patch.rb

@@ -112,7 +112,7 @@ class ExternalPatch
 
   def initialize(strip, &block)
     @strip    = strip
-    @resource = Resource.new("patch", &block)
+    @resource = Resource::Patch.new(&block)
   end
 
   def external?
@@ -127,9 +127,25 @@ class ExternalPatch
   def apply
     dir = Pathname.pwd
     resource.unpack do
-      # Assumption: the only file in the staging directory is the patch
-      patchfile = Pathname.pwd.children.first
-      dir.cd { safe_system "/usr/bin/patch", "-g", "0", "-f", "-#{strip}", "-i", patchfile }
+      patch_dir = Pathname.pwd
+      if patch_files.empty?
+        children = patch_dir.children
+        if (children.count == 1 && children.first.file?)
+          patch_files << children.first.basename
+        else
+          raise MissingApplyError, <<-EOS.undent
+            There should be exactly one patch file in the staging directory unless
+            the "apply" method was used one or more times in the patch-do block.
+          EOS
+        end
+      end
+      dir.cd do
+        patch_files.each do |patch_file|
+          ohai "Applying #{patch_file}"
+          patch_file = patch_dir/patch_file
+          safe_system "/usr/bin/patch", "-g", "0", "-f", "-#{strip}", "-i", patch_file
+        end
+      end
     end
   end
 
@@ -141,6 +157,10 @@ class ExternalPatch
     resource.fetch
   end
 
+  def patch_files
+    resource.patch_files
+  end
+
   def verify_download_integrity(fn)
     resource.verify_download_integrity(fn)
   end

Library/Homebrew/resource.rb

@@ -165,4 +165,19 @@ class Resource
       super(target/name)
     end
   end
+
+  class Patch < Resource
+    attr_reader :patch_files
+
+    def initialize(&block)
+      @patch_files = []
+      super "patch", &block
+    end
+
+    def apply(*paths)
+      paths.flatten!
+      @patch_files.concat(paths)
+      @patch_files.uniq!
+    end
+  end
 end

Library/Homebrew/test/lib/config.rb

@@ -21,3 +21,6 @@ HOMEBREW_LOGS          = HOMEBREW_PREFIX.parent+"logs"
 
 TESTBALL_SHA1 = "be478fd8a80fe7f29196d6400326ac91dad68c37"
 TESTBALL_SHA256 = "91e3f7930c98d7ccfb288e115ed52d06b0e5bc16fec7dce8bdda86530027067b"
+TESTBALL_PATCHES_SHA256 = "799c2d551ac5c3a5759bea7796631a7906a6a24435b52261a317133a0bfb34d9"
+PATCH_A_SHA256 = "83404f4936d3257e65f176c4ffb5a5b8d6edd644a21c8d8dcc73e22a6d28fcfa"
+PATCH_B_SHA256 = "57958271bb802a59452d0816e0670d16c8b70bdf6530bcf6f78726489ad89b90"

Library/Homebrew/test/patches/noop-c.diff

@@ -0,0 +1,9 @@
+diff --git a/libexec/NOOP b/libexec/NOOP
+index e08d8f4..3ebfb9d 100755
+--- a/libexec/NOOP
++++ b/libexec/NOOP
+@@ -1,2 +1,2 @@
+ #!/bin/bash
+-echo ABCD
+\ No newline at end of file
++echo 1234

Library/Homebrew/test/test_patch.rb

@@ -128,3 +128,28 @@ class ExternalPatchTests < Homebrew::TestCase
     assert_equal "/tmp/foo.tar.gz", @p.cached_download
   end
 end
+
+class ApplyPatchTests < Homebrew::TestCase
+  def test_empty_patch_files
+    patch = Patch.create(:p2, nil)
+    resource = patch.resource
+    patch_files = patch.patch_files
+    assert_kind_of Resource::Patch, resource
+    assert_equal patch_files, resource.patch_files
+    assert_equal patch_files, []
+  end
+
+  def test_resource_patch_apply_method
+    patch = Patch.create(:p2, nil)
+    resource = patch.resource
+    patch_files = patch.patch_files
+    resource.apply("patch1.diff")
+    assert_equal patch_files, ["patch1.diff"]
+    resource.apply("patch2.diff", "patch3.diff")
+    assert_equal patch_files, ["patch1.diff", "patch2.diff", "patch3.diff"]
+    resource.apply(["patch4.diff", "patch5.diff"])
+    assert_equal patch_files.count, 5
+    resource.apply("patch4.diff", ["patch5.diff", "patch6.diff"], "patch7.diff")
+    assert_equal patch_files.count, 7
+  end
+end

Library/Homebrew/test/test_patching.rb

@@ -2,15 +2,20 @@ require "testing_env"
 require "formula"
 
 class PatchingTests < Homebrew::TestCase
+  TESTBALL_URL = "file://#{TEST_DIRECTORY}/tarballs/testball-0.1.tbz"
+  TESTBALL_PATCHES_URL = "file://#{TEST_DIRECTORY}/tarballs/testball-0.1-patches.tgz"
   PATCH_URL_A = "file://#{TEST_DIRECTORY}/patches/noop-a.diff"
   PATCH_URL_B = "file://#{TEST_DIRECTORY}/patches/noop-b.diff"
   PATCH_A_CONTENTS = File.read "#{TEST_DIRECTORY}/patches/noop-a.diff"
   PATCH_B_CONTENTS = File.read "#{TEST_DIRECTORY}/patches/noop-b.diff"
+  APPLY_A = "noop-a.diff"
+  APPLY_B = "noop-b.diff"
+  APPLY_C = "noop-c.diff"
 
   def formula(*args, &block)
     super do
-      url "file://#{TEST_DIRECTORY}/tarballs/testball-0.1.tbz"
-      sha1 TESTBALL_SHA1
+      url TESTBALL_URL
+      sha256 TESTBALL_SHA256
       class_eval(&block)
     end
   end
@@ -31,6 +36,28 @@ class PatchingTests < Homebrew::TestCase
     end
   end
 
+  def assert_sequentially_patched(formula)
+    shutup do
+      formula.brew do
+        formula.patch
+        s = File.read("libexec/NOOP")
+        refute_includes s, "NOOP", "libexec/NOOP was not patched as expected"
+        refute_includes s, "ABCD", "libexec/NOOP was not patched as expected"
+        assert_includes s, "1234", "libexec/NOOP was not patched as expected"
+      end
+    end
+  end
+
+  def assert_missing_apply_fail(formula)
+    assert_raises(MissingApplyError) do
+      shutup do
+        formula.brew do
+          formula.patch
+        end
+      end
+    end
+  end
+
   def test_single_patch
     assert_patched formula {
       def patches
@@ -43,7 +70,27 @@ class PatchingTests < Homebrew::TestCase
     assert_patched formula {
       patch do
         url PATCH_URL_A
-        sha1 "fa8af2e803892e523fdedc6b758117c45e5749a2"
+        sha256 PATCH_A_SHA256
+      end
+    }
+  end
+
+  def test_single_patch_dsl_with_apply
+    assert_patched formula {
+      patch do
+        url TESTBALL_PATCHES_URL
+        sha256 TESTBALL_PATCHES_SHA256
+        apply APPLY_A
+      end
+    }
+  end
+
+  def test_single_patch_dsl_with_sequential_apply
+    assert_sequentially_patched formula {
+      patch do
+        url TESTBALL_PATCHES_URL
+        sha256 TESTBALL_PATCHES_SHA256
+        apply APPLY_A, APPLY_C
       end
     }
   end
@@ -52,7 +99,17 @@ class PatchingTests < Homebrew::TestCase
     assert_patched formula {
       patch :p1 do
         url PATCH_URL_A
-        sha1 "fa8af2e803892e523fdedc6b758117c45e5749a2"
+        sha256 PATCH_A_SHA256
+      end
+    }
+  end
+
+  def test_single_patch_dsl_with_strip_with_apply
+    assert_patched formula {
+      patch :p1 do
+        url TESTBALL_PATCHES_URL
+        sha256 TESTBALL_PATCHES_SHA256
+        apply APPLY_A
       end
     }
   end
@@ -63,7 +120,21 @@ class PatchingTests < Homebrew::TestCase
         formula do
           patch :p0 do
             url PATCH_URL_A
-            sha1 "fa8af2e803892e523fdedc6b758117c45e5749a2"
+            sha256 PATCH_A_SHA256
+          end
+        end.brew(&:patch)
+      end
+    end
+  end
+
+  def test_single_patch_dsl_with_incorrect_strip_with_apply
+    assert_raises(ErrorDuringExecution) do
+      shutup do
+        formula do
+          patch :p0 do
+            url TESTBALL_PATCHES_URL
+            sha256 TESTBALL_PATCHES_SHA256
+            apply APPLY_A
           end
         end.brew(&:patch)
       end
@@ -74,7 +145,17 @@ class PatchingTests < Homebrew::TestCase
     assert_patched formula {
       patch :p0 do
         url PATCH_URL_B
-        sha1 "3b54bd576f998ef6d6623705ee023b55062b9504"
+        sha256 PATCH_B_SHA256
+      end
+    }
+  end
+
+  def test_patch_p0_dsl_with_apply
+    assert_patched formula {
+      patch :p0 do
+        url TESTBALL_PATCHES_URL
+        sha256 TESTBALL_PATCHES_SHA256
+        apply APPLY_B
       end
    }
   end
@@ -126,6 +207,37 @@ class PatchingTests < Homebrew::TestCase
       end
     }
   end
+
+  def test_single_patch_missing_apply_fail
+    assert_missing_apply_fail formula {
+      def patches
+        TESTBALL_PATCHES_URL
+      end
+    }
+  end
+
+  def test_single_patch_dsl_missing_apply_fail
+    assert_missing_apply_fail formula {
+      patch do
+        url TESTBALL_PATCHES_URL
+        sha256 TESTBALL_PATCHES_SHA256
+      end
+    }
+  end
+
+  def test_single_patch_dsl_with_apply_enoent_fail
+    assert_raises(ErrorDuringExecution) do
+      shutup do
+        formula do
+          patch do
+            url TESTBALL_PATCHES_URL
+            sha256 TESTBALL_PATCHES_SHA256
+            apply "patches/#{APPLY_A}"
+          end
+        end.brew(&:patch)
+      end
+    end
+  end
 end
 
 __END__