Merge pull request #3714 from ilovezfs/resource-patches

resource: allow patches to be applied

Library/Homebrew/cmd/fetch.rb

@@ -63,7 +63,12 @@ module Homebrew
 
       next if fetched_bottle
       fetch_formula(f)
-      f.resources.each { |r| fetch_resource(r) }
+
+      f.resources.each do |r|
+        fetch_resource(r)
+        r.patches.each { |p| fetch_patch(p) if p.external? }
+      end
+
       f.patchlist.each { |p| fetch_patch(p) if p.external? }
     end
   end

Library/Homebrew/patch.rb

@@ -119,7 +119,7 @@ class ExternalPatch
 
   def initialize(strip, &block)
     @strip    = strip
-    @resource = Resource::Patch.new(&block)
+    @resource = Resource::PatchResource.new(&block)
   end
 
   def external?

Library/Homebrew/resource.rb

@@ -8,13 +8,13 @@ require "version"
 class Resource
   include FileUtils
 
-  attr_reader :mirrors, :specs, :using, :source_modified_time
+  attr_reader :mirrors, :specs, :using, :source_modified_time, :patches, :owner
   attr_writer :version
   attr_accessor :download_strategy, :checksum
 
   # Formula name must be set after the DSL, as we have no access to the
   # formula name before initialization of the formula
-  attr_accessor :name, :owner
+  attr_accessor :name
 
   class Download
     def initialize(resource)
@@ -46,9 +46,15 @@ class Resource
     @specs = {}
     @checksum = nil
     @using = nil
+    @patches = []
     instance_eval(&block) if block_given?
   end
 
+  def owner=(owner)
+    @owner = owner
+    patches.each { |p| p.owner = owner }
+  end
+
   def downloader
     download_strategy.new(download_name, Download.new(self))
   end
@@ -82,9 +88,24 @@ class Resource
     end
 
     verify_download_integrity(fetch)
+    prepare_patches
     unpack(target, &block)
   end
 
+  def prepare_patches
+    patches.grep(DATAPatch) { |p| p.path = owner.owner.path }
+
+    patches.each do |patch|
+      patch.verify_download_integrity(patch.fetch) if patch.external?
+    end
+  end
+
+  def apply_patches
+    return if patches.empty?
+    ohai "Patching #{name}"
+    patches.each(&:apply)
+  end
+
   # If a target is given, unpack there; else unpack to a temp folder.
   # If block is given, yield to that block with |stage|, where stage
   # is a ResourceStagingContext.
@@ -93,6 +114,7 @@ class Resource
     mktemp(download_name) do |staging|
       downloader.stage
       @source_modified_time = downloader.source_modified_time
+      apply_patches
       if block_given?
         yield ResourceStageContext.new(self, staging)
       elsif target
@@ -154,6 +176,11 @@ class Resource
     mirrors << val
   end
 
+  def patch(strip = :p1, src = nil, &block)
+    p = Patch.create(strip, src, &block)
+    patches << p
+  end
+
   private
 
   def detect_version(val)
@@ -174,7 +201,7 @@ class Resource
     end
   end
 
-  class Patch < Resource
+  class PatchResource < Resource
     attr_reader :patch_files
 
     def initialize(&block)

Library/Homebrew/test/patch_spec.rb

@@ -48,7 +48,7 @@ describe Patch do
     subject { described_class.create(:p2, nil) }
 
     context "empty patch" do
-      its(:resource) { is_expected.to be_kind_of Resource::Patch }
+      its(:resource) { is_expected.to be_kind_of Resource::PatchResource }
       its(:patch_files) { is_expected.to eq(subject.resource.patch_files) }
       its(:patch_files) { is_expected.to eq([]) }
     end

Library/Homebrew/test/patching_spec.rb

@@ -30,6 +30,17 @@ describe "patching" do
     end
   end
 
+  matcher :have_its_resource_patched do
+    match do |formula|
+      formula.brew do
+        formula.resources.first.stage Pathname.pwd/"resource_dir"
+        s = File.read("resource_dir/libexec/NOOP")
+        expect(s).not_to include("NOOP"), "libexec/NOOP was not patched as expected"
+        expect(s).to include("ABCD"), "libexec/NOOP was not patched as expected"
+      end
+    end
+  end
+
   matcher :be_sequentially_patched do
     match do |formula|
       formula.brew do
@@ -73,6 +84,22 @@ describe "patching" do
     ).to be_patched
   end
 
+  specify "single_patch_dsl_for_resource" do
+    expect(
+      formula do
+        resource "some_resource" do
+          url TESTBALL_URL
+          sha256 TESTBALL_SHA256
+
+          patch do
+            url PATCH_URL_A
+            sha256 PATCH_A_SHA256
+          end
+        end
+      end,
+    ).to have_its_resource_patched
+  end
+
   specify "single_patch_dsl_with_apply" do
     expect(
       formula do

Library/Homebrew/test/resource_spec.rb

@@ -119,6 +119,31 @@ describe Resource do
     end
   end
 
+  describe "#owner" do
+    it "sets the owner" do
+      owner = Object.new
+      subject.owner = owner
+      expect(subject.owner).to eq(owner)
+    end
+
+    it "sets its owner to be the patches' owner" do
+      subject.patch(:p1) { url "file:///my.patch" }
+      owner = Object.new
+      subject.owner = owner
+      subject.patches.each do |p|
+        expect(p.resource.owner).to eq(owner)
+      end
+    end
+  end
+
+  describe "#patch" do
+    it "adds a patch" do
+      subject.patch(:p1, :DATA)
+      expect(subject.patches.count).to eq(1)
+      expect(subject.patches.first.strip).to eq(:p1)
+    end
+  end
+
   specify "#verify_download_integrity_missing" do
     fn = Pathname.new("test")