- Revision
- 117715
- Author
- [email protected]
- Date
- 2012-05-20 16:32:43 -0700 (Sun, 20 May 2012)
Log Message
svn-apply cannot apply patches to files that contain space
characters in their path
https://bugs.webkit.org/show_bug.cgi?id=85742
Reviewed by Eric Seidel.
Fixes an issue where svn-apply cannot apply a patch to a file
if there is a space in its file path.
The regular _expression_ we were using to fix up +++/--- lines
was too strict; it only matched file paths that contained non-
whitespace characters. Instead, it's sufficient to match file
paths whose characters aren't in the set {\t, \n, \r}.
* Scripts/VCSUtils.pm:
(parseSvnDiffHeader):
(runCommand): Added.
* Scripts/svn-apply:
(scmWillDeleteFile): Modified to call runCommand() so as to
handle querying git for a file whose path may contain a space
character.
* Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:
- Added test case "new file with spaces in its name".
* Scripts/webkitperl/VCSUtils_unittest/runCommand.pl: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/Tools/ChangeLog (117714 => 117715)
--- trunk/Tools/ChangeLog 2012-05-20 23:29:58 UTC (rev 117714)
+++ trunk/Tools/ChangeLog 2012-05-20 23:32:43 UTC (rev 117715)
@@ -1,5 +1,32 @@
2012-05-20 Daniel Bates <[email protected]>
+ svn-apply cannot apply patches to files that contain space
+ characters in their path
+ https://bugs.webkit.org/show_bug.cgi?id=85742
+
+ Reviewed by Eric Seidel.
+
+ Fixes an issue where svn-apply cannot apply a patch to a file
+ if there is a space in its file path.
+
+ The regular _expression_ we were using to fix up +++/--- lines
+ was too strict; it only matched file paths that contained non-
+ whitespace characters. Instead, it's sufficient to match file
+ paths whose characters aren't in the set {\t, \n, \r}.
+
+ * Scripts/VCSUtils.pm:
+ (parseSvnDiffHeader):
+ (runCommand): Added.
+ * Scripts/svn-apply:
+ (scmWillDeleteFile): Modified to call runCommand() so as to
+ handle querying git for a file whose path may contain a space
+ character.
+ * Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl:
+ - Added test case "new file with spaces in its name".
+ * Scripts/webkitperl/VCSUtils_unittest/runCommand.pl: Added.
+
+2012-05-20 Daniel Bates <[email protected]>
+
svn-apply fails to apply a patch that moves files from directory A to A/B
https://bugs.webkit.org/show_bug.cgi?id=86973
Modified: trunk/Tools/Scripts/VCSUtils.pm (117714 => 117715)
--- trunk/Tools/Scripts/VCSUtils.pm 2012-05-20 23:29:58 UTC (rev 117714)
+++ trunk/Tools/Scripts/VCSUtils.pm 2012-05-20 23:32:43 UTC (rev 117715)
@@ -78,6 +78,7 @@
&possiblyColored
&prepareParsedPatch
&removeEOL
+ &runCommand
&runPatchCommand
&scmMoveOrRenameFile
&scmToggleExecutableBit
@@ -770,9 +771,9 @@
s/([\n\r]+)$//;
my $eol = $1;
- # Fix paths on ""---" and "+++" lines to match the leading
+ # Fix paths on "---" and "+++" lines to match the leading
# index line.
- if (s/^--- \S+/--- $indexPath/) {
+ if (s/^--- [^\t\n\r]+/--- $indexPath/) {
# ---
if (/^--- .+\(revision (\d+)\)/) {
$sourceRevision = $1;
@@ -785,7 +786,7 @@
"source revision number \"$sourceRevision\".") if ($2 != $sourceRevision);
}
}
- } elsif (s/^\+\+\+ \S+/+++ $indexPath/) {
+ } elsif (s/^\+\+\+ [^\t\n\r]+/+++ $indexPath/) {
$foundHeaderEnding = 1;
} elsif (/^Cannot display: file marked as a binary type.$/) {
$isBinary = 1;
@@ -2017,4 +2018,28 @@
return $path;
}
+sub runCommand(@)
+{
+ my @args = @_;
+ my $pid = open(CHILD, "-|");
+ if (!defined($pid)) {
+ die "Failed to fork(): $!";
+ }
+ if ($pid) {
+ # Parent process
+ my $childStdout;
+ while (<CHILD>) {
+ $childStdout .= $_;
+ }
+ close(CHILD);
+ my %childOutput;
+ $childOutput{exitStatus} = exitStatus($?);
+ $childOutput{stdout} = $childStdout if $childStdout;
+ return \%childOutput;
+ }
+ # Child process
+ # FIXME: Consider further hardening of this function, including sanitizing the environment.
+ exec { $args[0] } @args or die "Failed to exec(): $!";
+}
+
1;
Modified: trunk/Tools/Scripts/svn-apply (117714 => 117715)
--- trunk/Tools/Scripts/svn-apply 2012-05-20 23:29:58 UTC (rev 117714)
+++ trunk/Tools/Scripts/svn-apply 2012-05-20 23:32:43 UTC (rev 117715)
@@ -386,8 +386,8 @@
my $svnOutput = svnStatus($path);
return 1 if $svnOutput && substr($svnOutput, 0, 1) eq "D";
} elsif (isGit()) {
- my $gitOutput = `git diff-index --name-status HEAD -- $path`;
- return 1 if $gitOutput && substr($gitOutput, 0, 1) eq "D";
+ my $command = runCommand("git", "diff-index", "--name-status", "HEAD", "--", $path);
+ return 1 if $command->{stdout} && substr($command->{stdout}, 0, 1) eq "D";
}
return 0;
}
Modified: trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl (117714 => 117715)
--- trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl 2012-05-20 23:29:58 UTC (rev 117714)
+++ trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/parseSvnDiffHeader.pl 2012-05-20 23:32:43 UTC (rev 117715)
@@ -90,6 +90,31 @@
},
{
# New test
+ diffName => "new file with spaces in its name",
+ inputText => <<'END',
+Index: WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme
+===================================================================
+--- WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme (revision 0)
++++ WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme (revision 0)
+@@ -0,0 +1,8 @@
++<?xml version="1.0" encoding="UTF-8"?>
+END
+ expectedReturn => [
+{
+ svnConvertedText => <<'END',
+Index: WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme
+===================================================================
+--- WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme (revision 0)
++++ WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme (revision 0)
+END
+ indexPath => "WebKit.xcworkspace/xcshareddata/xcschemes/All Source (target WebProcess).xcscheme",
+ isNew => 1,
+},
+"@@ -0,0 +1,8 @@\n"],
+ expectedNextLine => "+<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n",
+},
+{
+ # New test
diffName => "copied file",
inputText => <<'END',
Index: index_path.py
Added: trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/runCommand.pl (0 => 117715)
--- trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/runCommand.pl (rev 0)
+++ trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/runCommand.pl 2012-05-20 23:32:43 UTC (rev 117715)
@@ -0,0 +1,75 @@
+#!/usr/bin/perl -w
+#
+# Copyright (C) 2012 Daniel Bates ([email protected]). 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 of VCSUtils::runCommand().
+
+use strict;
+use warnings;
+
+use Test::More;
+use VCSUtils;
+
+use constant ENOENT => 2; # See <errno.h>
+
+# The array of test cases.
+my @testCaseHashRefs = (
+{
+ # New test
+ testName => "Simple",
+ inputArgs => ["echo", "hello"],
+ expectedReturn => {
+ exitStatus => 0,
+ stdout => "hello\n"
+ }
+},
+{
+ # New test
+ testName => "Multiple commands",
+ inputArgs => ["echo", "first-command;echo second-command"],
+ expectedReturn => {
+ exitStatus => 0,
+ stdout => "first-command;echo second-command\n"
+ }
+},
+{
+ # New test
+ testName => "Non-existent command",
+ inputArgs => ["/usr/bin/non-existent-command"],
+ expectedReturn => {
+ exitStatus => ENOENT
+ }
+}
+);
+
+my $testCasesCount = @testCaseHashRefs;
+plan(tests => $testCasesCount); # Total number of assertions.
+
+foreach my $testCase (@testCaseHashRefs) {
+ my $testNameStart = "runCommand(): $testCase->{testName}: comparing";
+
+ my $got = VCSUtils::runCommand(@{$testCase->{inputArgs}});
+ my $expectedReturn = $testCase->{expectedReturn};
+
+ is_deeply($got, $expectedReturn, "$testNameStart return value.");
+}
