WebKit Bugzilla
Attachment 348862 Details for
Bug 14590
: svn-create-patch fails when svn mv is used on directory trees
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch v5
bug-14590-20180904160117.patch (text/plain), 17.28 KB, created by
David Kilzer (:ddkilzer)
on 2018-09-04 16:01:18 PDT
(
hide
)
Description:
Patch v5
Filename:
MIME Type:
Creator:
David Kilzer (:ddkilzer)
Created:
2018-09-04 16:01:18 PDT
Size:
17.28 KB
patch
obsolete
>Index: Tools/ChangeLog >=================================================================== >--- Tools/ChangeLog (revision 235620) >+++ Tools/ChangeLog (working copy) >@@ -1,3 +1,38 @@ >+2018-09-04 David Kilzer <ddkilzer@apple.com> >+ >+ svn-create-patch fails when svn mv is used on directory trees >+ <https://webkit.org/b/14590> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * Scripts/VCSUtils.pm: Export parseSvnDiffStartLine() for >+ svn-create-patch. >+ (parseDiffStartLine): Use parseSvnDiffStartLine(). >+ (parseGitDiffStartLine): Document a prerequisite. Fix a bug >+ when parsing git patches using --no-prefix with CFLF or LF line >+ endings. Found by new tests written for >+ Scripts/webkitperl/VCSUtils_unittest/parseDiffStartLine.pl. >+ (parseSvnDiffStartLine): Add. Extract logic from >+ parseDiffStartLine() for use with svn-create-patch. >+ >+ * Scripts/svn-create-patch: Update copyright and license. >+ (generateDiff): Return early for moved directories since >+ individual files within the directory are handled separately. >+ (generateFileList): Keep track of moved directories in the >+ @additionWithHistoryDirectories array, then process this array >+ in reverse order to create delete/add patches for each file in >+ a moved directory. This also prevents duplicate patches. >+ (manufacturePatchForAdditionWithHistory): Fix a long-standing >+ bug where the path used to describe property changes contained >+ the original (moved-from) path instead of the new (moved-to) >+ path. This could cause svn-apply to fail mysteriously when >+ trying to apply an empty patch created by the property change >+ containing the moved-from path. >+ >+ * Scripts/webkitperl/VCSUtils_unittest/parseDiffStartLine.pl: Add. >+ Tests for parseDiffStartLine(), parseGitDiffStartLine() and >+ parseSvnDiffStartLine(). >+ > 2018-09-04 Jer Noble <jer.noble@apple.com> > > REGRESSION (r234081): TestWebKitAPI.VideoControlsManager.VideoControlsManagerAudioElementFollowingUserInteraction is a flaky timeout >Index: Tools/Scripts/VCSUtils.pm >=================================================================== >--- Tools/Scripts/VCSUtils.pm (revision 235584) >+++ Tools/Scripts/VCSUtils.pm (working copy) >@@ -1,4 +1,4 @@ >-# Copyright (C) 2007-2013, 2015 Apple Inc. All rights reserved. >+# Copyright (C) 2007-2018 Apple Inc. All rights reserved. > # Copyright (C) 2009, 2010 Chris Jerdonek (chris.jerdonek@gmail.com) > # Copyright (C) 2010, 2011 Research In Motion Limited. All rights reserved. > # Copyright (C) 2012 Daniel Bates (dbates@intudata.com) >@@ -82,6 +82,7 @@ BEGIN { > &parseDiffStartLine > &parseFirstEOL > &parsePatch >+ &parseSvnDiffStartLine > &pathRelativeToSVNRepositoryRootForPath > &possiblyColored > &prepareParsedPatch >@@ -723,14 +724,14 @@ sub isExecutable($) > # Parses an SVN or Git diff header start line. > # > # Args: >-# $line: "Index: " line or "diff --git" line >+# $line: "Index: " line or "diff --git" line. > # > # Returns the path of the target file or undef if the $line is unrecognized. > sub parseDiffStartLine($) > { > my ($line) = @_; >- return $1 if $line =~ /$svnDiffStartRegEx/; > return parseGitDiffStartLine($line) if $line =~ /$gitDiffStartRegEx/; >+ return parseSvnDiffStartLine($line); > } > > # Parse the Git diff header start line. >@@ -738,6 +739,9 @@ sub parseDiffStartLine($) > # Args: > # $line: "diff --git" line. > # >+# Prerequisites: >+# $line argument matches /$gitDiffStartRegEx/. >+# > # Returns the path of the target file. > sub parseGitDiffStartLine($) > { >@@ -752,13 +756,26 @@ sub parseGitDiffStartLine($) > 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."); > } > my $pathPrefix = $1; >- if (!/^diff --git \Q$pathPrefix\E.+ (\Q$pathPrefix\E.+)$/) { >+ if (!/^diff --git \Q$pathPrefix\E.+ (\Q$pathPrefix\E[^\r\n]+)/) { > # FIXME: Moving a file through sub directories of top directory is not supported (e.g diff --git A/B.txt C/B.txt). > 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."); > } > return $1; > } > >+# Parses an SVN diff header start line. >+# >+# Args: >+# $line: "Index: " line. >+# >+# Returns the path of the target file or undef if the $line is unrecognized. >+sub parseSvnDiffStartLine($) >+{ >+ my ($line) = @_; >+ return $1 if $line =~ /$svnDiffStartRegEx/; >+ return undef; >+} >+ > # Parse the next Git diff header from the given file handle, and advance > # the handle so the last line read is the first line after the header. > # >Index: Tools/Scripts/svn-create-patch >=================================================================== >--- Tools/Scripts/svn-create-patch (revision 235584) >+++ Tools/Scripts/svn-create-patch (working copy) >@@ -1,30 +1,26 @@ > #!/usr/bin/env perl > >-# Copyright (C) 2005, 2006 Apple Inc. All rights reserved. >+# Copyright (C) 2005-2018 Apple Inc. All rights reserved. > # > # Redistribution and use in source and binary forms, with or without > # modification, are permitted provided that the following conditions > # are met: >-# > # 1. Redistributions of source code must retain the above copyright >-# notice, this list of conditions and the following disclaimer. >+# notice, this list of conditions and the following disclaimer. > # 2. Redistributions in binary form must reproduce the above copyright > # notice, this list of conditions and the following disclaimer in the >-# documentation and/or other materials provided with the distribution. >-# 3. Neither the name of Apple Inc. ("Apple") nor the names of >-# its contributors may be used to endorse or promote products derived >-# from this software without specific prior written permission. >+# documentation and/or other materials provided with the distribution. > # >-# THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY >+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY > # EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED > # WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE >-# DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY >+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY > # DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES > # (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; >-# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND >-# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF >-# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON >+# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS >+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > # Extended "svn diff" script for WebKit Open Source Project, used to make patches. > >@@ -35,10 +31,7 @@ > # Handles binary files (encoded as a base64 chunk of text). > # Sorts the diffs alphabetically by text files, then binary files. > # Handles copied and moved files. >-# >-# Missing features: >-# >-# Handle copied and moved directories. >+# Handles copied and moved directories. > > use strict; > use warnings; >@@ -252,6 +245,8 @@ sub generateDiff($$) > my $patch = ""; > my $isAdditionWithHistory = $fileData->{modificationType} eq "additionWithHistory"; > if ($isAdditionWithHistory) { >+ # Nothing to do for a moved directory since each moved file is handled individually. >+ return if -d $fileData->{path}; > manufacturePatchForAdditionWithHistory($fileData, $prefix); > } > >@@ -283,6 +278,7 @@ sub generateFileList($\%) > my %testDirectories = map { $_ => 1 } qw(LayoutTests); > my $escapedStatPath = escapeSubversionPath($statPath); > my @deletedFiles; >+ my @additionWithHistoryDirectories; > > print STDERR "Performing \"svn stat '$escapedStatPath'\"\n" if $verbose; > >@@ -305,8 +301,6 @@ sub generateFileList($\%) > $path = substr($line, 7); > } > >- next if -d $path; >- > my $modificationType = findModificationType($stat); > > if ($modificationType eq "missing") { >@@ -343,10 +337,38 @@ sub generateFileList($\%) > my ($sourceFile, $sourceRevision) = findSourceFileAndRevision($path); > $diffFiles->{$path}->{sourceFile} = $sourceFile; > $diffFiles->{$path}->{sourceRevision} = $sourceRevision; >+ push(@additionWithHistoryDirectories, $path) if -d $path; > } > } > close STAT; > >+ # Handle these in reverse order so that the deepest directory moves are processed first. >+ # Shallow directory moves include changes for deeper directories, which causes double >+ # processing and causes the wrong original path to be used if the order is not reversed. >+ foreach my $directory (reverse @additionWithHistoryDirectories) { >+ my ($sourceDirectory, $sourceRevision) = findSourceFileAndRevision($directory); >+ # Gather a hierarchical list of files inside the moved directory. >+ my $diffOptions = diffOptionsForFile($sourceDirectory); >+ my $escapedDirectory = escapeSubversionPath($directory); >+ print STDERR "Performing \"svn diff --diff-cmd diff -x -$diffOptions '$escapedDirectory'\"\n" if $verbose; >+ open DIFF, "svn diff --diff-cmd diff -x -$diffOptions '$escapedDirectory' |" or die; >+ while (<DIFF>) { >+ my $movedFile = parseSvnDiffStartLine($_); >+ if ($movedFile) { >+ # Ignore other files added/moved into the moved directory. >+ next if exists $diffFiles->{$movedFile}; >+ $diffFiles->{$movedFile}->{path} = $movedFile; >+ $diffFiles->{$movedFile}->{modificationType} = "additionWithHistory"; >+ $diffFiles->{$movedFile}->{isBinary} = isBinaryMimeType($movedFile); >+ $diffFiles->{$movedFile}->{isTestFile} = exists $testDirectories{(File::Spec->splitdir($movedFile))[0]} ? 1 : 0; >+ my $relativePath = File::Spec->abs2rel("/" . $movedFile, "/" . $directory); >+ $diffFiles->{$movedFile}->{sourceFile} = File::Spec->catfile($sourceDirectory, $relativePath); >+ $diffFiles->{$movedFile}->{sourceRevision} = $sourceRevision; >+ } >+ } >+ close DIFF; >+ } >+ > foreach my $path (@deletedFiles) { > my $isInsideDeletedDirectory = 0; > foreach my $compare (@deletedFiles) { >@@ -408,6 +430,7 @@ sub manufacturePatchForAdditionWithHisto > while (<DIFF>) { > # Skip the diff header, since it was manufactured aboved. > next if ++$count < 5; >+ s/^(Property changes on:\s+)$sourceFile([\r\n]+)/$1$file$2/; > print $_; > } > close DIFF; >Index: Tools/Scripts/webkitperl/VCSUtils_unittest/parseDiffStartLine.pl >=================================================================== >--- Tools/Scripts/webkitperl/VCSUtils_unittest/parseDiffStartLine.pl (nonexistent) >+++ Tools/Scripts/webkitperl/VCSUtils_unittest/parseDiffStartLine.pl (working copy) >@@ -0,0 +1,145 @@ >+#!/usr/bin/env perl >+ >+# Copyright (C) 2018 Apple Inc. All rights reserved. >+# >+# Redistribution and use in source and binary forms, with or without >+# modification, are permitted provided that the following conditions >+# are met: >+# 1. Redistributions of source code must retain the above copyright >+# notice, this list of conditions and the following disclaimer. >+# 2. Redistributions in binary form must reproduce the above copyright >+# notice, this list of conditions and the following disclaimer in the >+# documentation and/or other materials provided with the distribution. >+# >+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY >+# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED >+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE >+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY >+# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES >+# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; >+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON >+# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS >+# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >+ >+# Unit tests for parseDiffStartLine(), parseGitDiffStartLine(), parseSvnDiffStartLine(). >+ >+use strict; >+use warnings; >+ >+use constant { >+ CR => "\r", >+ CRLF => "\r\n", >+ LF => "\n", >+}; >+ >+use Test::More; >+use VCSUtils; >+ >+my @testCases = ( >+{ >+ expected => "Source/Makefile.shared", >+ input => "diff --git a/Source/Makefile.shared b/Source/Makefile.shared", >+ isGit => 1, >+ isValid => 1, >+ testName => "Git", >+}, >+{ >+ expected => "Source/Makefile.shared", >+ input => "diff --git Source/Makefile.shared Source/Makefile.shared", >+ isGit => 1, >+ isValid => 1, >+ testName => "Git --no-prefix", >+}, >+{ >+ expected => "LayoutTests/http/tests/misc/resources/a success.html", >+ input => "diff --git a/LayoutTests/http/tests/misc/resources/a success.html b/LayoutTests/http/tests/misc/resources/a success.html", >+ isGit => 1, >+ isValid => 1, >+ testName => "Git with space in path", >+}, >+{ >+ expected => undef, >+ input => "===================================================================", >+ isGit => 1, >+ isValid => 0, >+ testName => "Git with invalid input", >+}, >+{ >+ expected => "Source/Makefile.shared", >+ input => "Index: Source/Makefile.shared", >+ isSvn => 1, >+ isValid => 1, >+ testName => "Svn", >+}, >+{ >+ expected => "LayoutTests/http/tests/misc/resources/a success.html", >+ input => "Index: LayoutTests/http/tests/misc/resources/a success.html", >+ isSvn => 1, >+ isValid => 1, >+ testName => "Svn with space in path", >+}, >+{ >+ expected => undef, >+ input => "===================================================================", >+ isSvn => 1, >+ isValid => 0, >+ testName => "Svn with invalid input", >+}, >+); >+ >+my $testCaseCount = scalar @testCases; >+plan(tests => 9 * $testCaseCount); >+ >+foreach my $testCase (@testCases) { >+ # Make sure future modifications or new test cases are valid. >+ ok(exists $testCase->{expected}, "'expected' key exists: $testCase->{testName}"); >+ ok(exists $testCase->{input}, "'input' key exists: $testCase->{testName}"); >+ ok(exists $testCase->{isGit} ^ exists $testCase->{isSvn}, "Exactly one of 'isGit' key or 'isSvn' key exists: $testCase->{testName}"); >+ ok(exists $testCase->{isValid}, "'isValid' key exists: $testCase->{testName}"); >+ ok(exists $testCase->{testName}, "'testName' key exists"); >+ >+ my $testCaseName = "parseDiffStartLine(): $testCase->{testName}"; >+ my $got = VCSUtils::parseDiffStartLine($testCase->{input}); >+ is($got, $testCase->{expected}, $testCaseName); >+ >+ if ($testCase->{isGit}) { >+ SKIP: { >+ skip "parseGitDiffStartLine() does not handle invalid input", 3 if !$testCase->{isValid}; >+ >+ $testCaseName = "parseGitDiffStartLine(): CR line ending: $testCase->{testName}"; >+ my $input = $testCase->{input} . CR; >+ $got = VCSUtils::parseGitDiffStartLine($input); >+ is($got, $testCase->{expected}, $testCaseName); >+ >+ $testCaseName = "parseGitDiffStartLine(): CRLF line ending: $testCase->{testName}"; >+ $input = $testCase->{input} . CRLF; >+ $got = VCSUtils::parseGitDiffStartLine($input); >+ is($got, $testCase->{expected}, $testCaseName); >+ >+ $testCaseName = "parseGitDiffStartLine(): LF line ending: $testCase->{testName}"; >+ $input = $testCase->{input} . LF; >+ $got = VCSUtils::parseGitDiffStartLine($input); >+ is($got, $testCase->{expected}, $testCaseName); >+ } >+ } elsif ($testCase->{isSvn}) { >+ $testCaseName = "parseSvnDiffStartLine(): CR line ending: $testCase->{testName}"; >+ my $input = $testCase->{input} . CR; >+ $got = VCSUtils::parseSvnDiffStartLine($input); >+ is($got, $testCase->{expected}, $testCaseName); >+ >+ $testCaseName = "parseSvnDiffStartLine(): CRLF line ending: $testCase->{testName}"; >+ $input = $testCase->{input} . CRLF; >+ $got = VCSUtils::parseSvnDiffStartLine($input); >+ is($got, $testCase->{expected}, $testCaseName); >+ >+ $testCaseName = "parseSvnDiffStartLine(): LF line ending: $testCase->{testName}"; >+ $input = $testCase->{input} . LF; >+ $got = VCSUtils::parseSvnDiffStartLine($input); >+ is($got, $testCase->{expected}, $testCaseName); >+ } else { >+ fail("CR line ending: isGit or isSvn is not set for $testCase->{testName}"); >+ fail("CFLF line ending: isGit or isSvn is not set for $testCase->{testName}"); >+ fail("FL line ending: isGit or isSvn is not set for $testCase->{testName}"); >+ } >+}
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 14590
:
348766
|
348782
|
348784
|
348853
|
348862
|
348961