Changeset 151300 in webkit


Ignore:
Timestamp:
Jun 6, 2013 4:37:24 PM (11 years ago)
Author:
commit-queue@webkit.org
Message:

svn-apply cannot apply patches which is generated by git to files that contain space characters in their path
​https://bugs.webkit.org/show_bug.cgi?id=111066

Patch by Yuki Sekiguchi <​yuki.sekiguchi@access-company.com> on 2013-06-06
Reviewed by Daniel Bates.

Fixes an issue where parseGitDiffHeader() would extract the wrong substring of the diff --git line as the target file path when the source file path contains a space character.

ParseGitDiffHeader() should support the path which line has space characters.
To support this, I changed parsing algorithm like the following:

  • When the diff have prefix, we consider next characters after "b/" as part of a file path.
  • When the diff have no prefix, we assume that both path have same directory prefix, and we split the diff line using the prefix.

We only support --src-prefix and --dst-prefix don't contain a non-word character (\W) and end with '/' because we cannot distinguish the prefix from directory path.

If the path has a tab, the patch(1) command thinks file path is characters before the tab.
I added a dummy tab and revision when we convert git diff to svn diff.

  • Scripts/VCSUtils.pm:

(parseGitDiffHeader):

  • Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl: Update expectations for dummy revision.
  • Scripts/webkitperl/VCSUtils_unittest/parseDiffWithMockFiles.pl: Ditto.
  • Scripts/webkitperl/VCSUtils_unittest/parseGitDiffHeader.pl: Ditto.
    • Added test case for files which have space in their path and --src-prefix and --dst-prefix option.
Location:
trunk/Tools
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r151294 r151300  
     12013-06-06  Yuki Sekiguchi  <yuki.sekiguchi@access-company.com>
     2
     3        svn-apply cannot apply patches which is generated by git to files that contain space characters in their path
     4        https://bugs.webkit.org/show_bug.cgi?id=111066
     5
     6        Reviewed by Daniel Bates.
     7
     8        Fixes an issue where parseGitDiffHeader() would extract the wrong substring of the diff --git line as the target file path when the source file path contains a space character.
     9
     10        ParseGitDiffHeader() should support the path which line has space characters.
     11        To support this, I changed parsing algorithm like the following:
     12        - When the diff have prefix, we consider next characters after "b/" as part of a file path.
     13        - When the diff have no prefix, we assume that both path have same directory prefix, and we split the diff line using the prefix.
     14
     15        We only support --src-prefix and --dst-prefix don't contain a non-word character (\W) and end with '/' because we cannot distinguish the prefix from directory path.
     16
     17        If the path has a tab, the patch(1) command thinks file path is characters before the tab.
     18        I added a dummy tab and revision when we convert git diff to svn diff.
     19
     20        * Scripts/VCSUtils.pm:
     21        (parseGitDiffHeader):
     22        * Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl: Update expectations for dummy revision.
     23        * Scripts/webkitperl/VCSUtils_unittest/parseDiffWithMockFiles.pl: Ditto.
     24        * Scripts/webkitperl/VCSUtils_unittest/parseGitDiffHeader.pl: Ditto.
     25          - Added test case for files which have space in their path and --src-prefix and --dst-prefix option.
     26
    1272013-06-06  Simon Fraser  <simon.fraser@apple.com>
    228

  • trunk/Tools/Scripts/VCSUtils.pm

    r146249 r151300  
    108108my $changeLogTimeZone = "PST8PDT";
    109109
    110 my $gitDiffStartRegEx = qr#^diff --git (\w/)?(.+) (\w/)?([^\r\n]+)#;
     110my $gitDiffStartRegEx = qr#^diff --git [^\r\n]+#;
     111my $gitDiffStartWithPrefixRegEx = qr#^diff --git \w/(.+) \w/([^\r\n]+)#; # We suppose that --src-prefix and --dst-prefix don't contain a non-word character (\W) and end with '/'.
     112my $gitDiffStartWithoutPrefixNoSpaceRegEx = qr#^diff --git (\S+) (\S+)$#;
    111113my $svnDiffStartRegEx = qr#^Index: ([^\r\n]+)#;
     114my $gitDiffStartWithoutPrefixSourceDirectoryPrefixRegExp = qr#^diff --git ([^/]+/)#;
    112115my $svnPropertiesStartRegEx = qr#^Property changes on: ([^\r\n]+)#; # $1 is normally the same as the index path.
    113116my $svnPropertyStartRegEx = qr#^(Modified|Name|Added|Deleted): ([^\r\n]+)#; # $2 is the name of the property.
    … …  
    614617
    615618    return $fileMode % 2;
     619}
     620
     621# Parse the Git diff header start line.
     622#
     623# Args:
     624#   $line: "diff --git" line.
     625#
     626# Returns the path of the target file.
     627sub parseGitDiffStartLine($)
     628{
     629    my $line = shift;
     630    $_ = $line;
     631    if (/$gitDiffStartWithPrefixRegEx/ || /$gitDiffStartWithoutPrefixNoSpaceRegEx/) {
     632        return $2;
     633    }
     634    # Assume the diff was generated with --no-prefix (e.g. git diff --no-prefix).
     635    if (!/$gitDiffStartWithoutPrefixSourceDirectoryPrefixRegExp/) {
     636        # FIXME: Moving top directory file is not supported (e.g diff --git A.txt B.txt).
     637        die("Could not find '/' in \"diff --git\" line: \"$line\"; only non-prefixed git diffs (i.e. not generated with --no-prefix) that move a top-level directory file are supported.");
     638    }
     639    my $pathPrefix = $1;
     640    if (!/^diff --git \Q$pathPrefix\E.+ (\Q$pathPrefix\E.+)$/) {
     641        # FIXME: Moving a file through sub directories of top directory is not supported (e.g diff --git A/B.txt C/B.txt).
     642        die("Could not find '/' in \"diff --git\" line: \"$line\"; only non-prefixed git diffs (i.e. not generated with --no-prefix) that move a file between top-level directories are supported.");
     643    }
     644    return $1;
    616645}
    617646
    … …  
    657686    my $indexPath;
    658687    if (/$gitDiffStartRegEx/) {
     688        # Use $POSTMATCH to preserve the end-of-line character.
     689        my $eol = $POSTMATCH;
     690
    659691        # The first and second paths can differ in the case of copies
    660692        # and renames.  We use the second file path because it is the
    661693        # destination path.
    662         $indexPath = adjustPathForRecentRenamings($4);
    663         # Use $POSTMATCH to preserve the end-of-line character.
    664         $_ = "Index: $indexPath$POSTMATCH"; # Convert to SVN format.
     694        $indexPath = adjustPathForRecentRenamings(parseGitDiffStartLine($_));
     695
     696        $_ = "Index: $indexPath$eol"; # Convert to SVN format.
    665697    } else {
    666698        die("Could not parse leading \"diff --git\" line: \"$line\".");
    … …  
    691723        } elsif (/^similarity index (\d+)%/) {
    692724            $similarityIndex = $1;
    693         } elsif (/^copy from (\S+)/) {
     725        } elsif (/^copy from ([^\t\r\n]+)/) {
    694726            $copiedFromPath = $1;
    695         } elsif (/^rename from (\S+)/) {
     727        } elsif (/^rename from ([^\t\r\n]+)/) {
    696728            # FIXME: Record this as a move rather than as a copy-and-delete.
    697729            #        This will simplify adding rename support to svn-unapply.
    … …  
    703735            $shouldDeleteSource = 1;
    704736        } elsif (/^--- \S+/) {
    705             $_ = "--- $indexPath"; # Convert to SVN format.
     737            # Convert to SVN format.
     738            # We emit the suffix "\t(revision 0)" to handle $indexPath which contains a space character.
     739            # The patch(1) command thinks a file path is characters before a tab.
     740            # This suffix make our diff more closely match the SVN diff format.
     741            $_ = "--- $indexPath\t(revision 0)";
    706742        } elsif (/^\+\+\+ \S+/) {
    707             $_ = "+++ $indexPath"; # Convert to SVN format.
     743            # Convert to SVN format.
     744            # We emit the suffix "\t(working copy)" to handle $indexPath which contains a space character.
     745            # The patch(1) command thinks a file path is characters before a tab.
     746            # This suffix make our diff more closely match the SVN diff format.
     747            $_ = "+++ $indexPath\t(working copy)";
    708748            $foundHeaderEnding = 1;
    709749        } elsif (/^GIT binary patch$/ ) {

  • trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseDiff.pl

    r87641 r151300  
    981981    expectedReturn => [
    982982[{
    983     svnConvertedText =>  <<'END',
     983    svnConvertedText =>  <<"END",
    984984Index: Makefile
    985985index f5d5e74..3b6aa92 100644
    986 --- Makefile
    987 +++ Makefile
     986--- Makefile\t(revision 0)
     987+++ Makefile\t(working copy)
    988988@@ -1,1 +1,1 @@ public:
    989989END
    … …  
    10091009    expectedReturn => [
    10101010[{
    1011     svnConvertedText =>  <<'END',
     1011    svnConvertedText =>  <<"END",
    10121012Index: foo
    10131013index 863339f..db418b2 100644
    1014 --- foo
    1015 +++ foo
     1014--- foo\t(revision 0)
     1015+++ foo\t(working copy)
    10161016@@ -1 +1,2 @@
    10171017 Passed
    … …  
    10401040    expectedReturn => [
    10411041[{
    1042     svnConvertedText => <<'END',
     1042    svnConvertedText => <<"END",
    10431043Index: foo.h
    10441044new file mode 100644
    10451045index 0000000..3c9f114
    1046 --- foo.h
    1047 +++ foo.h
     1046--- foo.h\t(revision 0)
     1047+++ foo.h\t(working copy)
    10481048@@ -0,0 +1,34 @@
    10491049+<html>
    … …  
    10721072    expectedReturn => [
    10731073[{
    1074     svnConvertedText => <<'END',
     1074    svnConvertedText => <<"END",
    10751075Index: foo
    10761076deleted file mode 100644
    10771077index 1e50d1d..0000000
    1078 --- foo
    1079 +++ foo
     1078--- foo\t(revision 0)
     1079+++ foo\t(working copy)
    10801080@@ -1,1 +0,0 @@
    10811081-line1
    … …  
    11041104    expectedReturn => [
    11051105[{
    1106     svnConvertedText =>  <<'END',
     1106    svnConvertedText =>  <<"END",
    11071107Index: Makefile
    11081108index f5d5e74..3b6aa92 100644
    1109 --- Makefile
    1110 +++ Makefile
     1109--- Makefile\t(revision 0)
     1110+++ Makefile\t(working copy)
    11111111@@ -1,1 +1,1 @@ public:
    11121112Index: Makefile_new
    … …  
    12001200    isGit => 1,
    12011201    numTextChunks => 1,
    1202     svnConvertedText => <<'END',
     1202    svnConvertedText => <<"END",
    12031203Index: foo_new
    12041204similarity index 99%
    … …  
    12061206rename to foo_new
    12071207index 1e50d1d..1459d21 100644
    1208 --- foo_new
    1209 +++ foo_new
     1208--- foo_new\t(revision 0)
     1209+++ foo_new\t(working copy)
    12101210@@ -15,3 +15,4 @@ release r deployment dep deploy:
    12111211 line1

  • trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseDiffWithMockFiles.pl

    r146249 r151300  
    7272EOF
    7373
    74 my $svnConvertedGitDiffHeader = <<EOF;
     74my $svnConvertedGitDiffHeader = <<"EOF";
    7575Index: Makefile
    7676index 756e864..04d2ae1 100644
    77 --- Makefile
    78 +++ Makefile
     77--- Makefile\t(revision 0)
     78+++ Makefile\t(working copy)
    7979@@ -1,3 +1,4 @@
    8080EOF
    8181
    82 my $svnConvertedGitDiffHeaderForNewFile = <<EOF;
     82my $svnConvertedGitDiffHeaderForNewFile = <<"EOF";
    8383Index: Makefile
    8484new file mode 100644
    8585index 0000000..756e864
    86 --- Makefile
    87 +++ Makefile
     86--- Makefile\t(revision 0)
     87+++ Makefile\t(working copy)
    8888@@ -0,0 +1,17 @@
    8989EOF
    … …  
    355355Index: MakefileWithWindowsEOL
    356356index e7e8475..ae16fc3 100644
    357 --- MakefileWithWindowsEOL
    358 +++ MakefileWithWindowsEOL
     357--- MakefileWithWindowsEOL\t(revision 0)
     358+++ MakefileWithWindowsEOL\t(working copy)
    359359@@ -1,3 +1,4 @@\r
    360360 MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKitTools\r
    … …  
    387387[{
    388388    # Same as input text
    389     svnConvertedText => q(Index: MakefileWithMacEOL
     389    svnConvertedText => qq(Index: MakefileWithMacEOL
    390390index e7e8475..ae16fc3 100644
    391 --- MakefileWithMacEOL
    392 +++ MakefileWithMacEOL
     391--- MakefileWithMacEOL\t(revision 0)
     392+++ MakefileWithMacEOL\t(working copy)
    393393@@ -1,3 +1,4 @@\r MODULES = JavaScriptCore JavaScriptGlue WebCore WebKit WebKitTools\r \r-all:
    394394\\ No newline at end of file

  • trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseGitDiffHeader.pl

    r59048 r151300  
    4646    expectedReturn => [
    4747{
    48     svnConvertedText => <<'END',
     48    svnConvertedText => <<"END",
    4949Index: foo.h
    5050index f5d5e74..3b6aa92 100644
    51 --- foo.h
    52 +++ foo.h
     51--- foo.h\t(revision 0)
     52+++ foo.h\t(working copy)
    5353END
    5454    indexPath => "foo.h",
     55},
     56"@@ -1 +1 @@\n"],
     57    expectedNextLine => "-file contents\n",
     58},
     59{
     60    diffName => "Modified file using --src-prefix and --dst-prefix option",
     61    inputText => <<'END',
     62diff --git s/foo.h d/foo.h
     63index f5d5e74..3b6aa92 100644
     64--- s/foo.h
     65+++ d/foo.h
     66@@ -1 +1 @@
     67-file contents
     68+new file contents
     69END
     70    expectedReturn => [
     71{
     72    svnConvertedText => <<"END",
     73Index: foo.h
     74index f5d5e74..3b6aa92 100644
     75--- foo.h\t(revision 0)
     76+++ foo.h\t(working copy)
     77END
     78    indexPath => "foo.h",
     79},
     80"@@ -1 +1 @@\n"],
     81    expectedNextLine => "-file contents\n",
     82},
     83{   # New test
     84    diffName => "Modified file which have space characters in path",
     85    inputText => <<'END',
     86diff --git a/foo bar.h b/foo bar.h
     87index f5d5e74..3b6aa92 100644
     88--- a/foo bar.h
     89+++ b/foo bar.h
     90@@ -1 +1 @@
     91-file contents
     92+new file contents
     93END
     94    expectedReturn => [
     95{
     96    svnConvertedText => <<"END",
     97Index: foo bar.h
     98index f5d5e74..3b6aa92 100644
     99--- foo bar.h\t(revision 0)
     100+++ foo bar.h\t(working copy)
     101END
     102    indexPath => "foo bar.h",
     103},
     104"@@ -1 +1 @@\n"],
     105    expectedNextLine => "-file contents\n",
     106},
     107{   # New test
     108    diffName => "Modified file which have space characters in path using --no-prefix",
     109    inputText => <<'END',
     110diff --git directory/foo bar.h directory/foo bar.h
     111index f5d5e74..3b6aa92 100644
     112--- directory/foo bar.h
     113+++ directory/foo bar.h
     114@@ -1 +1 @@
     115-file contents
     116+new file contents
     117END
     118    expectedReturn => [
     119{
     120    svnConvertedText => <<"END",
     121Index: directory/foo bar.h
     122index f5d5e74..3b6aa92 100644
     123--- directory/foo bar.h\t(revision 0)
     124+++ directory/foo bar.h\t(working copy)
     125END
     126    indexPath => "directory/foo bar.h",
    55127},
    56128"@@ -1 +1 @@\n"],
    … …  
    70142    expectedReturn => [
    71143{
    72     svnConvertedText => <<'END',
     144    svnConvertedText => <<"END",
    73145Index: foo.h
    74146new file mode 100644
    75147index 0000000..3c9f114
    76 --- foo.h
    77 +++ foo.h
     148--- foo.h\t(revision 0)
     149+++ foo.h\t(working copy)
    78150END
    79151    indexPath => "foo.h",
    … …  
    98170    expectedReturn => [
    99171{
    100     svnConvertedText => <<'END',
     172    svnConvertedText => <<"END",
    101173Index: foo
    102174deleted file mode 100644
    103175index 1e50d1d..0000000
    104 --- foo
    105 +++ foo
     176--- foo\t(revision 0)
     177+++ foo\t(working copy)
    106178END
    107179    indexPath => "foo",
     180    isDeletion => 1,
     181},
     182"@@ -1,1 +0,0 @@\n"],
     183    expectedNextLine => "-line1\n",
     184},
     185{
     186    diffName => "delete file which have space characters in path using --no-prefix",
     187    inputText => <<'END',
     188diff --git directory/foo bar.h directory/foo bar.h
     189deleted file mode 100644
     190index 1e50d1d..0000000
     191--- directory/foo bar.h
     192+++ /dev/null
     193@@ -1,1 +0,0 @@
     194-line1
     195diff --git a/configure.ac b/configure.ac
     196index d45dd40..3494526 100644
     197END
     198    expectedReturn => [
     199{
     200    svnConvertedText => <<"END",
     201Index: directory/foo bar.h
     202deleted file mode 100644
     203index 1e50d1d..0000000
     204--- directory/foo bar.h\t(revision 0)
     205+++ directory/foo bar.h\t(working copy)
     206END
     207    indexPath => "directory/foo bar.h",
    108208    isDeletion => 1,
    109209},
    … …  
    123223    expectedReturn => [
    124224{
    125     svnConvertedText => <<'END',
     225    svnConvertedText => <<"END",
    126226Index: foo.h
    127227index c925780..9e65c43 100644
    128 --- foo.h
    129 +++ foo.h
     228--- foo.h\t(revision 0)
     229+++ foo.h\t(working copy)
    130230END
    131231    indexPath => "foo.h",
    … …  
    157257    copiedFromPath => "foo",
    158258    indexPath => "foo_new",
     259},
     260"diff --git a/bar b/bar\n"],
     261    expectedNextLine => "index d45dd40..3494526 100644\n",
     262},
     263{
     264    diffName => "copy file which have space characters in path using --no-prefix (with similarity index 100%)",
     265    inputText => <<'END',
     266diff --git directory/foo bar.h directory/foo baz.h
     267similarity index 100%
     268copy from directory/foo bar.h
     269copy to directory/foo baz.h
     270diff --git a/bar b/bar
     271index d45dd40..3494526 100644
     272END
     273    expectedReturn => [
     274{
     275    svnConvertedText => <<'END',
     276Index: directory/foo baz.h
     277similarity index 100%
     278copy from directory/foo bar.h
     279copy to directory/foo baz.h
     280END
     281    copiedFromPath => "directory/foo bar.h",
     282    indexPath => "directory/foo baz.h",
    159283},
    160284"diff --git a/bar b/bar\n"],
    … …  
    206330    copiedFromPath => "foo",
    207331    indexPath => "foo_new",
     332    shouldDeleteSource => 1,
     333},
     334"diff --git a/bar b/bar\n"],
     335    expectedNextLine => "index d45dd40..3494526 100644\n",
     336},
     337{
     338    diffName => "rename file which have space characters in path using --no-prefix (with similarity index 100%)",
     339    inputText => <<'END',
     340diff --git directory/foo bar.h directory/foo baz.h
     341similarity index 100%
     342rename from directory/foo bar.h
     343rename to directory/foo baz.h
     344diff --git a/bar b/bar
     345index d45dd40..3494526 100644
     346END
     347    expectedReturn => [
     348{
     349    svnConvertedText => <<'END',
     350Index: directory/foo baz.h
     351similarity index 100%
     352rename from directory/foo bar.h
     353rename to directory/foo baz.h
     354END
     355    copiedFromPath => "directory/foo bar.h",
     356    indexPath => "directory/foo baz.h",
    208357    shouldDeleteSource => 1,
    209358},
    … …  
    231380    expectedReturn => [
    232381{
    233     svnConvertedText => <<'END',
     382    svnConvertedText => <<"END",
    234383Index: foo_new
    235384similarity index 99%
    … …  
    237386rename to foo_new
    238387index 1e50d1d..1459d21 100644
    239 --- foo_new
    240 +++ foo_new
     388--- foo_new\t(revision 0)
     389+++ foo_new\t(working copy)
    241390END
    242391    copiedFromPath => "foo",
    … …  
    360509    expectedReturn => [
    361510{
    362     svnConvertedText => <<'END',
     511    svnConvertedText => <<"END",
    363512Index: foo
    364513index d03e242..435ad3a 100755
    365 --- foo
    366 +++ foo
     514--- foo\t(revision 0)
     515+++ foo\t(working copy)
    367516END
    368517    indexPath => "foo",
    … …  
    430579    expectedReturn => [
    431580{
    432     svnConvertedText => <<'END',
     581    svnConvertedText => <<"END",
    433582Index: foo
    434583new file mode 100755
    435584index 0000000..d03e242
    436 --- foo
    437 +++ foo
     585--- foo\t(revision 0)
     586+++ foo\t(working copy)
    438587END
    439588    executableBitDelta => 1,
    … …  
    459608    expectedReturn => [
    460609{
    461     svnConvertedText => <<'END',
     610    svnConvertedText => <<"END",
    462611Index: foo
    463612deleted file mode 100755
    464613index d03e242..0000000
    465 --- foo
    466 +++ foo
     614--- foo\t(revision 0)
     615+++ foo\t(working copy)
    467616END
    468617    executableBitDelta => -1,
    … …  
    473622    expectedNextLine => "-file contents\n",
    474623},
     624{
     625    # svn-apply rejected https://bug-111042-attachments.webkit.org/attachment.cgi?id=190663
     626    diffName => "Modified file which have space characters in path. svn-apply rejected attachment #190663.",
     627    inputText => <<'END',
     628diff --git a/WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme b/WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme
     629index 72d60effb9bbba0520e520ec3c1aa43f348c6997..b7924b96d5978c1ad1053dca7e554023235d9a16 100644
     630--- a/WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme
     631+++ b/WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme
     632@@ -161,7 +161,7 @@
     633       <EnvironmentVariables>
     634          <EnvironmentVariable
     635             key = "DYLD_INSERT_LIBRARIES"
     636-            value = "$(BUILT_PRODUCTS_DIR)/WebProcessShim.dylib"
     637+            value = "$(BUILT_PRODUCTS_DIR)/SecItemShim.dylib"
     638             isEnabled = "YES">
     639          </EnvironmentVariable>
     640       </EnvironmentVariables>
     641END
     642    expectedReturn => [
     643{
     644    svnConvertedText => <<"END",
     645Index: WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme
     646index 72d60effb9bbba0520e520ec3c1aa43f348c6997..b7924b96d5978c1ad1053dca7e554023235d9a16 100644
     647--- WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme\t(revision 0)
     648+++ WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme\t(working copy)
     649END
     650    indexPath => "WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme",
     651},
     652"@@ -161,7 +161,7 @@\n"],
     653    expectedNextLine => "       <EnvironmentVariables>\n",
     654},
    475655);
    476656
Note: See TracChangeset for help on using the changeset viewer.