diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 57eb2ab4c9..3031da7219 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -3,7 +3,6 @@ require 'formula_lock' require 'formula_pin' require 'hardware' require 'bottles' -require 'patches' require 'compilers' require 'build_environment' require 'build_options' @@ -224,16 +223,7 @@ class Formula # any e.g. configure options for this package def options; [] end - # patches are automatically applied after extracting the tarball - # return an array of strings, or if you need a patch level other than -p1 - # return a Hash eg. - # { - # :p0 => ['http://foo.com/patch1', 'http://foo.com/patch2'], - # :p1 => 'http://bar.com/patch2' - # } - # The final option is to return DATA, then put a diff after __END__. You - # can still return a Hash with DATA as the value for a patch level key. - def patches; end + def patches; {} end # rarely, you don't want your library symlinked into the main prefix # see gettext.rb for an example @@ -633,23 +623,15 @@ class Formula end def patch - patch_list = Patches.new(patches) - return if patch_list.empty? + active_spec.add_legacy_patches(patches) + return if patchlist.empty? - if patch_list.external_patches? - ohai "Downloading patches" - patch_list.download! + active_spec.patches.select(&:external?).each do |patch| + patch.verify_download_integrity(patch.fetch) end ohai "Patching" - patch_list.each do |p| - case p.compression - when :gzip then with_system_path { safe_system "gunzip", p.compressed_filename } - when :bzip2 then with_system_path { safe_system "bunzip2", p.compressed_filename } - end - # -f means don't prompt the user if there are errors; just exit with non-zero status - safe_system '/usr/bin/patch', '-g', '0', '-f', *(p.patch_args) - end + active_spec.patches.each(&:apply) end # Explicitly request changing C++ standard library compatibility check diff --git a/Library/Homebrew/patch.rb b/Library/Homebrew/patch.rb index a4616eee60..64325e78d9 100644 --- a/Library/Homebrew/patch.rb +++ b/Library/Homebrew/patch.rb @@ -1,5 +1,6 @@ require 'resource' require 'stringio' +require 'erb' class Patch def self.create(strip, io=nil, &block) @@ -22,6 +23,32 @@ class Patch end end + def self.normalize_legacy_patches(list) + patches = [] + + case list + when Hash + list + when Array, String, IO, StringIO + { :p1 => list } + else + {} + end.each_pair do |strip, urls| + urls = [urls] unless Array === urls + urls.each do |url| + case url + when IO, StringIO + patch = IOPatch.new(url, strip) + else + patch = LegacyPatch.new(strip, url) + end + patches << patch + end + end + + patches + end + attr_reader :whence def external? @@ -80,3 +107,31 @@ class ExternalPatch < Patch end end end + +# Legacy patches have no checksum and are not cached +class LegacyPatch < ExternalPatch + def initialize(strip, url) + super(strip) + resource.url = url + end + + def owner= owner + super + resource.name = "patch-#{ERB::Util.url_encode(resource.url)}" + end + + def fetch + resource.clear_cache + super + end + + def verify_download_integrity(fn) + # no-op + end + + def apply + super + ensure + resource.clear_cache + end +end diff --git a/Library/Homebrew/patches.rb b/Library/Homebrew/patches.rb deleted file mode 100644 index ccd677e7db..0000000000 --- a/Library/Homebrew/patches.rb +++ /dev/null @@ -1,143 +0,0 @@ -require 'stringio' - -class Patches - include Enumerable - - # The patches defined in a formula and the DATA from that file - def initialize patches - @patches = [] - return if patches.nil? - n = 0 - normalize_patches(patches).each do |patch_p, urls| - # Wrap the urls list in an array if it isn't already; - # DATA.each does each line, which doesn't work so great - urls = [urls] unless urls.kind_of? Array - urls.each do |url| - @patches << Patch.new(patch_p, '%03d-homebrew.diff' % n, url) - n += 1 - end - end - end - - def external_patches? - not external_curl_args.empty? - end - - def each(&blk) - @patches.each(&blk) - end - def empty? - @patches.empty? - end - - def download! - return unless external_patches? - - # downloading all at once is much more efficient, especially for FTP - curl(*external_curl_args) - - external_patches.each{|p| p.stage!} - end - - private - - def external_patches - @patches.select{|p| p.external?} - end - - # Collects the urls and output names of all external patches - def external_curl_args - external_patches.collect{|p| p.curl_args}.flatten - end - - def normalize_patches patches - if patches.kind_of? Hash - patches - else - { :p1 => patches } # We assume -p1 - end - end - -end - -class Patch - # Used by formula to unpack after downloading - attr_reader :compression - attr_reader :compressed_filename - # Used by audit - attr_reader :url - - def initialize patch_p, filename, url - @patch_p = patch_p - @patch_filename = filename - @compressed_filename = nil - @compression = nil - @url = nil - - if url.kind_of? IO or url.kind_of? StringIO - # File-like objects. Most common when DATA is passed. - write_data url - elsif looks_like_url(url) - @url = url # Save URL to mark this as an external patch - else - # it's a file on the local filesystem - # use URL as the filename for patch - @patch_filename = url - end - end - - # rename the downloaded file to take compression into account - def stage! - return unless external? - detect_compression! - case @compression - when :gzip - @compressed_filename = @patch_filename + '.gz' - FileUtils.mv @patch_filename, @compressed_filename - when :bzip2 - @compressed_filename = @patch_filename + '.bz2' - FileUtils.mv @patch_filename, @compressed_filename - end - end - - def external? - not @url.nil? - end - - def patch_args - ["-#{@patch_p}", '-i', @patch_filename] - end - - def curl_args - [@url, '-o', @patch_filename] - end - - private - - # Detect compression type from the downloaded patch. - def detect_compression! - # If nil we have not tried to detect yet - if @compression.nil? - path = Pathname.new(@patch_filename) - if path.exist? - @compression = path.compression_type - @compression ||= :none # If nil, convert to :none - end - end - end - - # Write the given file object (DATA) out to a local file for patch - def write_data f - pn = Pathname.new @patch_filename - pn.write(brew_var_substitution(f.read.to_s)) - end - - # Do any supported substitutions of HOMEBREW vars in a DATA patch - def brew_var_substitution s - s.gsub("HOMEBREW_PREFIX", HOMEBREW_PREFIX) - end - - def looks_like_url str - str =~ %r[^\w+\://] - end -end diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index 0349dd3dd2..a3a37902af 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -91,6 +91,12 @@ class SoftwareSpec def patch strip=:p1, io=nil, &block patches << Patch.create(strip, io, &block) end + + def add_legacy_patches(list) + list = Patch.normalize_legacy_patches(list) + list.each { |p| p.owner = self } + patches.concat(list) + end end class HeadSoftwareSpec < SoftwareSpec diff --git a/Library/Homebrew/test/test_patch.rb b/Library/Homebrew/test/test_patch.rb index 1cc1ac7af6..9056e8371f 100644 --- a/Library/Homebrew/test/test_patch.rb +++ b/Library/Homebrew/test/test_patch.rb @@ -50,3 +50,66 @@ class PatchTests < Test::Unit::TestCase assert_raises(ArgumentError) { Patch.create(Object.new) } end end + +class LegacyPatchTests < Test::Unit::TestCase + def test_patch_single_string + patches = Patch.normalize_legacy_patches("http://example.com/patch.diff") + assert_equal 1, patches.length + assert_equal :p1, patches.first.strip + end + + def test_patch_array + patches = Patch.normalize_legacy_patches( + %w{http://example.com/patch1.diff http://example.com/patch2.diff} + ) + + assert_equal 2, patches.length + assert_equal :p1, patches[0].strip + assert_equal :p1, patches[1].strip + end + + def test_p0_hash_to_string + patches = Patch.normalize_legacy_patches( + :p0 => "http://example.com/patch.diff" + ) + + assert_equal 1, patches.length + assert_equal :p0, patches.first.strip + end + + def test_p1_hash_to_string + patches = Patch.normalize_legacy_patches( + :p1 => "http://example.com/patch.diff" + ) + + assert_equal 1, patches.length + assert_equal :p1, patches.first.strip + end + + def test_mixed_hash_to_strings + patches = Patch.normalize_legacy_patches( + :p1 => "http://example.com/patch1.diff", + :p0 => "http://example.com/patch0.diff" + ) + assert_equal 2, patches.length + assert_equal 1, patches.select { |p| p.strip == :p0 }.length + assert_equal 1, patches.select { |p| p.strip == :p1 }.length + end + + def test_mixed_hash_to_arrays + patches = Patch.normalize_legacy_patches( + :p1 => ["http://example.com/patch10.diff", + "http://example.com/patch11.diff"], + :p0 => ["http://example.com/patch00.diff", + "http://example.com/patch01.diff"] + ) + + assert_equal 4, patches.length + assert_equal 2, patches.select { |p| p.strip == :p0 }.length + assert_equal 2, patches.select { |p| p.strip == :p1 }.length + end + + def test_nil + assert_empty Patch.normalize_legacy_patches(nil) + end +end diff --git a/Library/Homebrew/test/test_patches.rb b/Library/Homebrew/test/test_patches.rb deleted file mode 100644 index 668d4b395e..0000000000 --- a/Library/Homebrew/test/test_patches.rb +++ /dev/null @@ -1,88 +0,0 @@ -require 'testing_env' -require 'test/testball' -require 'set' - -# Expose some internals -class Patches - attr_reader :patches -end - -class Patch - attr_reader :patch_p - attr_reader :patch_filename -end - - -class PatchingTests < Test::Unit::TestCase - def test_patchSingleString - patches = Patches.new("http://example.com/patch.diff") - assert_equal 1, patches.patches.length - p = patches.patches[0] - assert_equal :p1, p.patch_p - end - - def test_patchArray - patches = Patches.new(["http://example.com/patch1.diff", "http://example.com/patch2.diff"]) - assert_equal 2, patches.patches.length - - p1 = patches.patches[0] - assert_equal :p1, p1.patch_p - - p2 = patches.patches[0] - assert_equal :p1, p2.patch_p - end - - def test_p0_hash_to_string - patches = Patches.new({ - :p0 => "http://example.com/patch.diff" - }) - assert_equal 1, patches.patches.length - - p = patches.patches[0] - assert_equal :p0, p.patch_p - end - - def test_p1_hash_to_string - patches = Patches.new({ - :p1 => "http://example.com/patch.diff" - }) - assert_equal 1, patches.patches.length - - p = patches.patches[0] - assert_equal :p1, p.patch_p - end - - def test_mixed_hash_to_strings - expected = { - :p1 => "http://example.com/patch1.diff", - :p0 => "http://example.com/patch0.diff" - } - patches = Patches.new(expected) - assert_equal 2, patches.patches.length - - # Make sure unique filenames were assigned - filenames = Set.new - patches.each do |p| - filenames << p.patch_filename - end - - assert_equal 2, filenames.size - end - - def test_mixed_hash_to_arrays - expected = { - :p1 => ["http://example.com/patch10.diff","http://example.com/patch11.diff"], - :p0 => ["http://example.com/patch00.diff","http://example.com/patch01.diff"] - } - patches = Patches.new(expected) - assert_equal 4, patches.patches.length - - # Make sure unique filenames were assigned - filenames = Set.new - patches.each do |p| - filenames << p.patch_filename - end - - assert_equal 4, filenames.size - end -end diff --git a/Library/Homebrew/test/test_patching.rb b/Library/Homebrew/test/test_patching.rb index 6642704c08..a82a8092a1 100644 --- a/Library/Homebrew/test/test_patching.rb +++ b/Library/Homebrew/test/test_patching.rb @@ -1,64 +1,153 @@ require 'testing_env' -require 'test/testball' +require 'formula' +require 'testball' class PatchingTests < Test::Unit::TestCase - def read_file path - File.open(path, 'r') { |f| f.read } + def formula(&block) + super do + url "file:///#{TEST_FOLDER}/tarballs/testball-0.1.tbz" + sha1 "482e737739d946b7c8cbaf127d9ee9c148b999f5" + class_eval(&block) + end + end + + def assert_patched(path) + s = File.read(path) + assert !s.include?("NOOP"), "File was unpatched." + assert s.include?("ABCD"), "File was not patched as expected." end def test_single_patch shutup do - Class.new(TestBall) do + formula do def patches "file:///#{TEST_FOLDER}/patches/noop-a.diff" end - end.new("test_single_patch").brew do - s = read_file 'libexec/NOOP' - assert !s.include?("NOOP"), "File was unpatched." - assert s.include?("ABCD"), "File was not patched as expected." + end.brew { assert_patched 'libexec/NOOP' } + end + end + + def test_single_patch_dsl + shutup do + formula do + patch do + url "file:///#{TEST_FOLDER}/patches/noop-a.diff" + sha1 "fa8af2e803892e523fdedc6b758117c45e5749a2" + end + end.brew { assert_patched 'libexec/NOOP' } + end + end + + def test_single_patch_dsl_with_strip + shutup do + formula do + patch :p1 do + url "file:///#{TEST_FOLDER}/patches/noop-a.diff" + sha1 "fa8af2e803892e523fdedc6b758117c45e5749a2" + end + end.brew { assert_patched 'libexec/NOOP' } + end + end + + def test_single_patch_dsl_with_incorrect_strip + assert_raises(ErrorDuringExecution) do + shutup do + formula do + patch :p0 do + url "file:///#{TEST_FOLDER}/patches/noop-a.diff" + sha1 "fa8af2e803892e523fdedc6b758117c45e5749a2" + end + end.brew { } end end end - def test_patch_array + def test_patch_p0_dsl shutup do - Class.new(TestBall) do - def patches - ["file:///#{TEST_FOLDER}/patches/noop-a.diff"] + formula do + patch :p0 do + url "file:///#{TEST_FOLDER}/patches/noop-b.diff" + sha1 "3b54bd576f998ef6d6623705ee023b55062b9504" end - end.new("test_patch_array").brew do - s = read_file 'libexec/NOOP' - assert !s.include?("NOOP"), "File was unpatched." - assert s.include?("ABCD"), "File was not patched as expected." - end + end.brew { assert_patched 'libexec/NOOP' } end end def test_patch_p0 shutup do - Class.new(TestBall) do + formula do def patches - "file:///#{TEST_FOLDER}/patches/noop-a.diff" + { :p0 => "file:///#{TEST_FOLDER}/patches/noop-b.diff" } end - end.new("test_patch_p0").brew do - s = read_file 'libexec/NOOP' - assert !s.include?("NOOP"), "File was unpatched." - assert s.include?("ABCD"), "File was not patched as expected." - end + end.brew { assert_patched 'libexec/NOOP' } end end - def test_patch_p1 + def test_patch_array shutup do - Class.new(TestBall) do + formula do + def patches + ["file:///#{TEST_FOLDER}/patches/noop-a.diff"] + end + end.brew { assert_patched 'libexec/noop' } + end + end + + def test_patch_hash + shutup do + formula do + def patches + { :p1 => "file:///#{TEST_FOLDER}/patches/noop-a.diff" } + end + end.brew { assert_patched 'libexec/noop' } + end + end + + def test_patch_hash_array + shutup do + formula do def patches { :p1 => ["file:///#{TEST_FOLDER}/patches/noop-a.diff"] } end - end.new("test_patch_p1").brew do - s = read_file 'libexec/NOOP' - assert !s.include?("NOOP"), "File was unpatched." - assert s.include?("ABCD"), "File was not patched as expected." - end + end.brew { assert_patched 'libexec/noop' } + end + end + + def test_patch_string + shutup do + formula do + patch %q{ +diff --git a/libexec/NOOP b/libexec/NOOP +index bfdda4c..e08d8f4 100755 +--- a/libexec/NOOP ++++ b/libexec/NOOP +@@ -1,2 +1,2 @@ + #!/bin/bash +-echo NOOP +\ No newline at end of file ++echo ABCD +\ No newline at end of file +} + end.brew { assert_patched 'libexec/noop' } + end + end + + def test_patch_string_with_strip + shutup do + formula do + patch :p0, %q{ +diff --git libexec/NOOP libexec/NOOP +index bfdda4c..e08d8f4 100755 +--- libexec/NOOP ++++ libexec/NOOP +@@ -1,2 +1,2 @@ + #!/bin/bash +-echo NOOP +\ No newline at end of file ++echo ABCD +\ No newline at end of file +} + end.brew { assert_patched 'libexec/noop' } end end end