Changeset 95159 in webkit


Ignore:
Timestamp:
Sep 14, 2011 9:08:41 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

PrettyPatch should handle "delta" patch mechanism in git binary patches
https://bugs.webkit.org/show_bug.cgi?id=67628

Git patches are encoded using two mechanisms - "literal" and "delta".
For details of these mechanisms, see the function emit_binary_diff_body
in the git source file diff.c (https://github.com/git/git/blob/master/diff.c).

When determining if a binary file patch is an image or not we should accept
both literal and delta patch encodings.

When reconstructing the images from the patches, if we have a delta patch
we may download the previous revision from svn.webkit.org to get the image data.

Patch by Ben Wells <benwells@chromium.org> on 2011-09-14
Reviewed by Adam Roben.

  • PrettyPatch/PrettyPatch.rb:
  • PrettyPatch/PrettyPatch_test.rb:
Location:
trunk/Websites/bugs.webkit.org
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Websites/bugs.webkit.org/ChangeLog

    r94555 r95159  
     12011-09-14  Ben Wells  <benwells@chromium.org>
     2
     3        PrettyPatch should handle "delta" patch mechanism in git binary patches
     4        https://bugs.webkit.org/show_bug.cgi?id=67628
     5
     6        Git patches are encoded using two mechanisms - "literal" and "delta".
     7        For details of these mechanisms, see the function emit_binary_diff_body
     8        in the git source file diff.c (https://github.com/git/git/blob/master/diff.c).
     9
     10        When determining if a binary file patch is an image or not we should accept
     11        both literal and delta patch encodings.
     12
     13        When reconstructing the images from the patches, if we have a delta patch
     14        we may download the previous revision from svn.webkit.org to get the image data.
     15
     16        Reviewed by Adam Roben.
     17
     18        * PrettyPatch/PrettyPatch.rb:
     19        * PrettyPatch/PrettyPatch_test.rb:
     20
    1212011-09-06  Sheriff Bot  <webkit.review.bot@gmail.com>
    222
  • trunk/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb

    r94555 r95159  
    22require 'diff'
    33require 'open3'
     4require 'open-uri'
    45require 'pp'
    56require 'set'
     
    1415    def self.prettify(string)
    1516        $last_prettify_file_count = -1
    16         $last_prettify_part_count = { "remove" => 0, "add" => 0, "shared" => 0 }
     17        $last_prettify_part_count = { "remove" => 0, "add" => 0, "shared" => 0, "binary" => 0, "extract-error" => 0 }
    1718        string = normalize_line_ending(string)
    18         fileDiffs = FileDiff.parse(string)
    19 
    2019        str = HEADER + "\n"
    2120
    2221        # Just look at the first line to see if it is an SVN revision number as added
    2322        # by webkit-patch for git checkouts.
     23        $svn_revision = 0
    2424        string.each_line do |line|
    2525            match = /^Subversion\ Revision: (\d*)$/.match(line)
    2626            unless match.nil?
    27               str += "<span class='revision'>" + match[1] + "</span>\n"
     27                str += "<span class='revision'>" + match[1] + "</span>\n"
     28                $svn_revision = match[1].to_i;
    2829            end
    2930            break
    3031        end
     32
     33        fileDiffs = FileDiff.parse(string)
    3134
    3235        $last_prettify_file_count = fileDiffs.length
     
    6669    GIT_BINARY_FILE_MARKER_FORMAT = /^GIT binary patch$/
    6770
     71    GIT_BINARY_PATCH_FORMAT = /^(literal|delta) \d+$/
     72
    6873    GIT_LITERAL_FORMAT = /^literal \d+$/
     74
     75    GIT_DELTA_FORMAT = /^delta \d+$/
    6976
    7077    START_OF_BINARY_DATA_FORMAT = /^[0-9a-zA-Z\+\/=]{20,}/ # Assume 20 chars without a space is base64 binary data.
     
    509516                when GIT_BINARY_FILE_MARKER_FORMAT
    510517                    @binary = true
    511                     if (GIT_LITERAL_FORMAT.match(lines[i + 1]) and PrettyPatch.has_image_suffix(@filename)) then
     518                    if (GIT_BINARY_PATCH_FORMAT.match(lines[i + 1]) and PrettyPatch.has_image_suffix(@filename)) then
    512519                        @git_image = true
    513520                        startOfSections = i + 1
     
    535542                    raise "no binary chunks" unless chunks
    536543
    537                     binary_contents = chunks.zip(@git_indexes).collect do |chunk, git_index|
    538                         FileDiff.extract_contents_from_git_binary_chunk(chunk, git_index)
    539                     end
    540 
    541                     @image_urls = binary_contents.collect { |content| content ? "data:image/png;base64," + [content].pack("m") : nil }
     544                    from_filepath = FileDiff.extract_contents_of_from_revision(@filename, chunks[0], @git_indexes[0])
     545                    to_filepath = FileDiff.extract_contents_of_to_revision(@filename, chunks[1], @git_indexes[1], from_filepath, @git_indexes[0])
     546                    filepaths = from_filepath, to_filepath
     547
     548                    binary_contents = filepaths.collect { |filepath| File.exists?(filepath) ? File.read(filepath) : nil }
     549
     550                    @image_urls = binary_contents.collect { |content| (content and not content.empty?) ? "data:image/png;base64," + [content].pack("m") : nil }
    542551                    @image_checksums = binary_contents.collect { |content| FileDiff.read_checksum_from_png(content) }
    543552                rescue
     553                    $last_prettify_part_count["extract-error"] += 1
    544554                    @image_error = "Exception raised during decoding git binary patch:<pre>#{CGI.escapeHTML($!.to_s + "\n" + $!.backtrace.join("\n"))}</pre>"
     555                ensure
     556                    File.unlink(from_filepath) if (from_filepath and File.exists?(from_filepath))
     557                    File.unlink(to_filepath) if (to_filepath and File.exists?(to_filepath))
    545558                end
    546559            end
     
    586599                end
    587600            elsif @binary then
     601                $last_prettify_part_count["binary"] += 1
    588602                str += "<span class='text'>Binary file, nothing to see here</span>"
    589603            else
     
    632646        end
    633647
    634         def self.extract_contents_from_git_binary_chunk(encoded_chunk, git_index)
    635             # We use Tempfile we need a unique file among processes.
     648        def self.git_changed_file_binary_patch(to_filename, from_filename, encoded_chunk, to_git_index, from_git_index)
     649            return <<END
     650diff --git a/#{from_filename} b/#{to_filename}
     651copy from #{from_filename}
     652+++ b/#{to_filename}
     653index #{from_git_index}..#{to_git_index}
     654GIT binary patch
     655#{encoded_chunk.join("")}literal 0
     656HcmV?d00001
     657
     658END
     659        end
     660
     661        def self.get_svn_uri(repository_path)
     662            "http://svn.webkit.org/repository/webkit/!svn/bc/" + $svn_revision.to_s + "/trunk/" + (repository_path)
     663        end
     664
     665        def self.get_new_temp_filepath_and_name
    636666            tempfile = Tempfile.new("PrettyPatch")
    637             # We need a filename which doesn't exist to apply a patch
    638             # which creates a new file. Append a suffix so filename
    639             # doesn't exist.
    640667            filepath = tempfile.path + '.bin'
    641668            filename = File.basename(filepath)
    642 
    643             patch = FileDiff.git_new_file_binary_patch(filename, encoded_chunk, git_index)
    644 
     669            return filepath, filename
     670        end
     671
     672        def self.download_from_revision_from_svn(repository_path)
     673            filepath, filename = get_new_temp_filepath_and_name
     674            svn_uri = get_svn_uri(repository_path)
     675            open(filepath, 'wb') do |to_file|
     676                to_file << open(svn_uri) { |from_file| from_file.read }
     677            end
     678            return filepath
     679        end
     680
     681        def self.run_git_apply_on_patch(output_filepath, patch)
    645682            # Apply the git binary patch using git-apply.
    646             cmd = GIT_PATH + " apply --directory=" + File.dirname(filepath)
     683            cmd = GIT_PATH + " apply --directory=" + File.dirname(output_filepath)
    647684            stdin, stdout, stderr = *Open3.popen3(cmd)
    648685            begin
     
    651688
    652689                error = stderr.read
     690                if error != ""
     691                    error = "Error running " + cmd + "\n" + "with patch:\n" + patch[0..500] + "...\n" + error
     692                end
    653693                raise error if error != ""
    654 
    655                 contents = File.read(filepath)
    656694            ensure
    657695                stdin.close unless stdin.closed?
    658696                stdout.close
    659697                stderr.close
    660                 File.unlink(filename) if File.exists?(filename)
    661             end
    662 
    663             return nil if contents.empty?
    664             return contents
     698            end
     699        end
     700
     701        def self.extract_contents_from_git_binary_literal_chunk(encoded_chunk, git_index)
     702            filepath, filename = get_new_temp_filepath_and_name
     703            patch = FileDiff.git_new_file_binary_patch(filename, encoded_chunk, git_index)
     704            run_git_apply_on_patch(filepath, patch)
     705            return filepath
     706        end
     707
     708        def self.extract_contents_from_git_binary_delta_chunk(from_filepath, from_git_index, encoded_chunk, to_git_index)
     709            to_filepath, to_filename = get_new_temp_filepath_and_name
     710            from_filename = File.basename(from_filepath)
     711            patch = FileDiff.git_changed_file_binary_patch(to_filename, from_filename, encoded_chunk, to_git_index, from_git_index)
     712            run_git_apply_on_patch(to_filepath, patch)
     713            return to_filepath
     714        end
     715
     716        def self.extract_contents_of_from_revision(repository_path, encoded_chunk, git_index)
     717            # For literal encoded, simply reconstruct.
     718            if GIT_LITERAL_FORMAT.match(encoded_chunk[0])
     719                return extract_contents_from_git_binary_literal_chunk(encoded_chunk, git_index)
     720            end
     721            #  For delta encoded, download from svn.
     722            if GIT_DELTA_FORMAT.match(encoded_chunk[0])
     723                return download_from_revision_from_svn(repository_path)
     724            end
     725            raise "Error: unknown git patch encoding"
     726        end
     727
     728        def self.extract_contents_of_to_revision(repository_path, encoded_chunk, git_index, from_filepath, from_git_index)
     729            # For literal encoded, simply reconstruct.
     730            if GIT_LITERAL_FORMAT.match(encoded_chunk[0])
     731                return extract_contents_from_git_binary_literal_chunk(encoded_chunk, git_index)
     732            end
     733            # For delta encoded, reconstruct using delta and previously constructed 'from' revision.
     734            if GIT_DELTA_FORMAT.match(encoded_chunk[0])
     735                return extract_contents_from_git_binary_delta_chunk(from_filepath, from_git_index, encoded_chunk, git_index)
     736            end
     737            raise "Error: unknown git patch encoding"
    665738        end
    666739    end
     
    805878            str += "</div>\n"
    806879        end
    807        
     880
    808881        def self.parse(lines)
    809882            linesForSections = lines.inject([[]]) do |sections, line|
  • trunk/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch_test.rb

    r94555 r95159  
    2727        83127 => ["Only add stuff", 2, 2, 0, 3],
    2828        85071 => ["Adds and removes from a file plus git signature", 2, 5, 3, 9],
     29        106368 => ["Images with git delta binary patch", 69, 8, 23, 10],
    2930    }
    3031
     
    5859        assert_equal(info[Info::REMOVE], $last_prettify_part_count["remove"], "Wrong number of 'remove' parts in " + description)
    5960        assert_equal(info[Info::SHARED], $last_prettify_part_count["shared"], "Wrong number of 'shared' parts in " + description)
     61        assert_equal(0, $last_prettify_part_count["binary"], "Wrong number of 'binary' parts in " + description)
     62        assert_equal(0, $last_prettify_part_count["extract-error"], "Wrong number of 'extract-error' parts in " + description)
    6063    end
    6164
Note: See TracChangeset for help on using the changeset viewer.