[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

ddkilzer at apple.com ddkilzer at apple.com
Thu Apr 8 00:52:49 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 7308a5a78ec9531e0f0da596b315b4d8aff917c7
Author: ddkilzer at apple.com <ddkilzer at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Sun Jan 3 21:25:30 2010 +0000

    Refactored svn-apply and svn-unapply to use a common "patch"
    command method, and added unit tests for this new method.
    
    Patch by Chris Jerdonek <chris.jerdonek at gmail.com> on 2010-01-03
    Reviewed by David Kilzer.
    
    https://bugs.webkit.org/show_bug.cgi?id=33098
    
    * Scripts/VCSUtils.pm:
      - Added generateRunPatchCommand().
      - Added runPatchCommand().
      - Added exitStatus() from webkitdirs.pm to address FIXME.
    
    * Scripts/VCSUtils_unittest.pl:
      - Added 10 unit tests for generateRunPatchCommand().
      - Added 4 unit tests for runPatchCommand().
      - Added callSilently() method.
    
    * Scripts/svn-apply:
      - Refactored applyPatch().
      - Removed $pathScriptWasRunFrom global variable.
      - Addressed issue where "--force" option was getting added twice.
    
    * Scripts/svn-unapply:
      - Refactored applyPatch().
      - Removed $pathScriptWasRunFrom global variable.
      - Added support for --force option.
      - Enhanced to return meaningful exit status.
    
    * Scripts/webkitdirs.pm:
      - Moved exitStatus() implementation to VCSUtils.pm.
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52692 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 37bc427..2f646d2 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,36 @@
+2010-01-03  Chris Jerdonek  <chris.jerdonek at gmail.com>
+
+        Reviewed by David Kilzer.
+
+        Refactored svn-apply and svn-unapply to use a common "patch"
+        command method, and added unit tests for this new method.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33098
+
+        * Scripts/VCSUtils.pm:
+          - Added generateRunPatchCommand().
+          - Added runPatchCommand().
+          - Added exitStatus() from webkitdirs.pm to address FIXME.
+
+        * Scripts/VCSUtils_unittest.pl:
+          - Added 10 unit tests for generateRunPatchCommand().
+          - Added 4 unit tests for runPatchCommand().
+          - Added callSilently() method.
+
+        * Scripts/svn-apply:
+          - Refactored applyPatch().
+          - Removed $pathScriptWasRunFrom global variable.
+          - Addressed issue where "--force" option was getting added twice.
+
+        * Scripts/svn-unapply:
+          - Refactored applyPatch().
+          - Removed $pathScriptWasRunFrom global variable.
+          - Added support for --force option.
+          - Enhanced to return meaningful exit status.
+
+        * Scripts/webkitdirs.pm:
+          - Moved exitStatus() implementation to VCSUtils.pm.
+
 2009-12-31  Adam Barth  <abarth at webkit.org>
 
         Reviewed by Eric Seidel.
diff --git a/WebKitTools/Scripts/VCSUtils.pm b/WebKitTools/Scripts/VCSUtils.pm
index aee0827..c1db3c0 100644
--- a/WebKitTools/Scripts/VCSUtils.pm
+++ b/WebKitTools/Scripts/VCSUtils.pm
@@ -1,5 +1,5 @@
 # Copyright (C) 2007, 2008, 2009 Apple Inc.  All rights reserved.
-# Copyright (C) 2009 Chris Jerdonek (chris.jerdonek at gmail.com)
+# Copyright (C) 2009, 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -34,6 +34,7 @@ use warnings;
 use Cwd qw();  # "qw()" prevents warnings about redefining getcwd() with "use POSIX;"
 use File::Basename;
 use File::Spec;
+use POSIX;
 
 BEGIN {
     use Exporter   ();
@@ -48,6 +49,7 @@ BEGIN {
         &decodeGitBinaryPatch
         &determineSVNRoot
         &determineVCSRoot
+        &exitStatus
         &fixChangeLogPatch
         &gitBranch
         &gitdiff2svndiff
@@ -60,6 +62,7 @@ BEGIN {
         &makeFilePathRelative
         &normalizePath
         &pathRelativeToSVNRepositoryRootForPath
+        &runPatchCommand
         &svnRevisionForDirectory
         &svnStatus
     );
@@ -76,6 +79,20 @@ my $isGitBranchBuild;
 my $isSVN;
 my $svnVersion;
 
+# This method is for portability. Return the system-appropriate exit
+# status of a child process.
+#
+# Args: pass the child error status returned by the last pipe close,
+#       for example "$?".
+sub exitStatus($)
+{
+    my ($returnvalue) = @_;
+    if ($^O eq "MSWin32") {
+        return $returnvalue >> 8;
+    }
+    return WEXITSTATUS($returnvalue);
+}
+
 sub isGitDirectory($)
 {
     my ($dir) = @_;
@@ -94,7 +111,7 @@ sub gitBranch()
 {
     unless (defined $gitBranch) {
         chomp($gitBranch = `git symbolic-ref -q HEAD`);
-        $gitBranch = "" if main::exitStatus($?); # FIXME: exitStatus is defined in webkitdirs.pm
+        $gitBranch = "" if exitStatus($?);
         $gitBranch =~ s#^refs/heads/##;
         $gitBranch = "" if $gitBranch eq "master";
     }
@@ -496,6 +513,109 @@ sub fixChangeLogPatch($)
     return join($lineEnding, @lines) . "\n"; # patch(1) expects an extra trailing newline.
 }
 
+# This is a supporting method for runPatchCommand.
+#
+# Arg: the optional $args parameter passed to runPatchCommand (can be undefined).
+#
+# Returns ($patchCommand, $isForcing).
+#
+# This subroutine has unit tests in VCSUtils_unittest.pl.
+sub generateRunPatchCommand($)
+{
+    my ($passedArgsHashRef) = @_;
+
+    my $argsHashRef = { # Defaults
+        ensureForce => 0,
+        shouldReverse => 0,
+        options => []
+    };
+    
+    # Merges hash references. It's okay here if passed hash reference is undefined.
+    @{$argsHashRef}{keys %{$passedArgsHashRef}} = values %{$passedArgsHashRef};
+    
+    my $ensureForce = $argsHashRef->{ensureForce};
+    my $shouldReverse = $argsHashRef->{shouldReverse};
+    my $options = $argsHashRef->{options};
+
+    if (! $options) {
+        $options = [];
+    } else {
+        $options = [@{$options}]; # Copy to avoid side effects.
+    }
+
+    my $isForcing = 0;
+    if (grep /^--force$/, @{$options}) {
+        $isForcing = 1;
+    } elsif ($ensureForce) {
+        push @{$options}, "--force";
+        $isForcing = 1;
+    }
+
+    if ($shouldReverse) { # No check: --reverse should never be passed explicitly.
+        push @{$options}, "--reverse";
+    }
+
+    @{$options} = sort(@{$options}); # For easier testing.
+
+    my $patchCommand = join(" ", "patch -p0", @{$options});
+
+    return ($patchCommand, $isForcing);
+}
+
+# Apply the given patch using the patch(1) command.
+#
+# On success, return the resulting exit status. Otherwise, exit with the
+# exit status. If "--force" is passed as an option, however, then never
+# exit and always return the exit status.
+#
+# Args:
+#   $patch: a patch string.
+#   $repositoryRootPath: an absolute path to the repository root.
+#   $pathRelativeToRoot: the path of the file to be patched, relative to the
+#                        repository root. This should normally be the path
+#                        found in the patch's "Index:" line. It is passed
+#                        explicitly rather than reparsed from the patch
+#                        string for optimization purposes.
+#                            This is used only for error reporting. The
+#                        patch command gleans the actual file to patch
+#                        from the patch string.
+#   $args: a reference to a hash of optional arguments. The possible
+#          keys are --
+#            ensureForce: whether to ensure --force is passed (defaults to 0).
+#            shouldReverse: whether to pass --reverse (defaults to 0).
+#            options: a reference to an array of options to pass to the
+#                     patch command. The subroutine passes the -p0 option
+#                     no matter what. This should not include --reverse.
+#
+# This subroutine has unit tests in VCSUtils_unittest.pl.
+sub runPatchCommand($$$;$)
+{
+    my ($patch, $repositoryRootPath, $pathRelativeToRoot, $args) = @_;
+
+    my ($patchCommand, $isForcing) = generateRunPatchCommand($args);
+
+    # Temporarily change the working directory since the path found
+    # in the patch's "Index:" line is relative to the repository root
+    # (i.e. the same as $pathRelativeToRoot).
+    my $cwd = Cwd::getcwd();
+    chdir $repositoryRootPath;
+
+    open PATCH, "| $patchCommand" or die "Could not call \"$patchCommand\" for file \"$pathRelativeToRoot\": $!";
+    print PATCH $patch;
+    close PATCH;
+    my $exitStatus = exitStatus($?);
+
+    chdir $cwd;
+
+    if ($exitStatus && !$isForcing) {
+        print "Calling \"$patchCommand\" for file \"$pathRelativeToRoot\" returned " .
+              "status $exitStatus.  Pass --force to ignore patch failures.\n";
+        exit $exitStatus;
+    }
+
+    return $exitStatus;
+}
+
 sub gitConfig($)
 {
     return unless $isGit;
diff --git a/WebKitTools/Scripts/VCSUtils_unittest.pl b/WebKitTools/Scripts/VCSUtils_unittest.pl
index 5a65cb3..7881924 100755
--- a/WebKitTools/Scripts/VCSUtils_unittest.pl
+++ b/WebKitTools/Scripts/VCSUtils_unittest.pl
@@ -1,6 +1,6 @@
 #!/usr/bin/perl
 #
-# Copyright (C) 2009 Chris Jerdonek (chris.jerdonek at gmail.com)
+# Copyright (C) 2009, 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
@@ -30,10 +30,23 @@
 
 # Unit tests of VCSUtils.pm.
 
-use Test::Simple tests => 7;
+use Test::Simple tests => 21;
 
 use VCSUtils;
 
+# Call a function while suppressing STDERR.
+sub callSilently($@) {
+    my ($func, @args) = @_;
+
+    open(OLDERR, ">&STDERR");
+    close(STDERR);
+    my @returnValue = &$func(@args);
+    open(STDERR, ">&OLDERR");
+    close(OLDERR); # FIXME: Is this necessary?
+
+    return @returnValue;
+}
+
 # fixChangeLogPatch
 #
 # The source ChangeLog for these tests is the following:
@@ -292,3 +305,117 @@ END
 
 ok(fixChangeLogPatch($in) eq $out, $title);
 
+# Tests: generateRunPatchCommand
+
+# New test
+$title = "generateRunPatchCommand: Undefined optional arguments.";
+
+my $argsHashRef;
+my ($patchCommand, $isForcing) = VCSUtils::generateRunPatchCommand($argsHashRef);
+
+ok($patchCommand eq "patch -p0", $title);
+ok($isForcing == 0, $title);
+
+# New test
+$title = "generateRunPatchCommand: Undefined options.";
+
+my $options;
+$argsHashRef = {options => $options};
+($patchCommand, $isForcing) = VCSUtils::generateRunPatchCommand($argsHashRef);
+
+ok($patchCommand eq "patch -p0", $title);
+ok($isForcing == 0, $title);
+
+# New test
+$title = "generateRunPatchCommand: --force and no \"ensure force\".";
+
+$argsHashRef = {options => ["--force"]};
+($patchCommand, $isForcing) = VCSUtils::generateRunPatchCommand($argsHashRef);
+
+ok($patchCommand eq "patch -p0 --force", $title);
+ok($isForcing == 1, $title);
+
+# New test
+$title = "generateRunPatchCommand: no --force and \"ensure force\".";
+
+$argsHashRef = {ensureForce => 1};
+($patchCommand, $isForcing) = VCSUtils::generateRunPatchCommand($argsHashRef);
+
+ok($patchCommand eq "patch -p0 --force", $title);
+ok($isForcing == 1, $title);
+
+# New test
+$title = "generateRunPatchCommand: \"should reverse\".";
+
+$argsHashRef = {shouldReverse => 1};
+($patchCommand, $isForcing) = VCSUtils::generateRunPatchCommand($argsHashRef);
+
+ok($patchCommand eq "patch -p0 --reverse", $title);
+
+# New test
+$title = "generateRunPatchCommand: --fuzz=3, --force.";
+
+$argsHashRef = {options => ["--fuzz=3", "--force"]};
+($patchCommand, $isForcing) = VCSUtils::generateRunPatchCommand($argsHashRef);
+
+ok($patchCommand eq "patch -p0 --force --fuzz=3", $title);
+
+# Tests: runPatchCommand
+
+# New test
+$title = "runPatchCommand: Unsuccessful patch, forcing.";
+
+# Since $patch has no "Index:" path, passing this to runPatchCommand
+# should not affect any files.
+my $patch = <<'END';
+Garbage patch contents
+END
+
+# We call via callSilently() to avoid output like the following to STDERR:
+# patch: **** Only garbage was found in the patch input.
+$argsHashRef = {ensureForce => 1};
+$exitStatus = callSilently(\&runPatchCommand, $patch, ".", "file_to_patch.txt", $argsHashRef);
+
+ok($exitStatus != 0, $title);
+
+# New test
+$title = "runPatchCommand: New file, --dry-run.";
+
+# This file should not exist after the tests, but we take care with the
+# file name and contents just in case.
+my $fileToPatch = "temp_OK_TO_ERASE__README_FOR_MORE.txt";
+$patch = <<END;
+Index: $fileToPatch
+===================================================================
+--- $fileToPatch	(revision 0)
++++ $fileToPatch	(revision 0)
+@@ -0,0 +1,5 @@
++This is a test file for WebKitTools/Scripts/VCSUtils_unittest.pl.
++This file should not have gotten created on your system.
++If it did, some unit tests don't seem to be working quite right:
++It would be great if you could file a bug report. Thanks!
++---------------------------------------------------------------------
+END
+
+# --dry-run prevents creating any files.
+# --silent suppresses the success message to STDOUT.
+$argsHashRef = {options => ["--dry-run", "--silent"]};
+$exitStatus = runPatchCommand($patch, ".", $fileToPatch, $argsHashRef);
+
+ok($exitStatus == 0, $title);
+
+# New test
+$title = "runPatchCommand: New file: \"$fileToPatch\".";
+
+$argsHashRef = {options => ["--silent"]};
+$exitStatus = runPatchCommand($patch, ".", $fileToPatch, $argsHashRef);
+
+ok($exitStatus == 0, $title);
+
+# New test
+$title = "runPatchCommand: Reverse new file (clean up previous).";
+
+$argsHashRef = {shouldReverse => 1,
+                options => ["--silent", "--remove-empty-files"]}; # To clean up.
+$exitStatus = runPatchCommand($patch, ".", $fileToPatch, $argsHashRef);
+ok($exitStatus == 0, $title);
diff --git a/WebKitTools/Scripts/svn-apply b/WebKitTools/Scripts/svn-apply
index 0373aa5..dc00a05 100755
--- a/WebKitTools/Scripts/svn-apply
+++ b/WebKitTools/Scripts/svn-apply
@@ -2,6 +2,7 @@
 
 # Copyright (C) 2005, 2006, 2007 Apple Inc.  All rights reserved.
 # Copyright (C) 2009 Cameron McCormack <cam at mcc.id.au>
+# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -116,10 +117,9 @@ my %removeDirectoryIgnoreList = (
     '_svn' => 1,
 );
 
-my $globalExitCode = 0;
+my $globalExitStatus = 0;
 
-my $pathScriptWasRunFrom = Cwd::getcwd();
-my $pathForRepositoryRoot = determineVCSRoot();
+my $repositoryRootPath = determineVCSRoot();
 
 my %checkedDirectories;
 my %copiedFiles;
@@ -193,7 +193,7 @@ for $patch (@patches) {
 
 removeDirectoriesIfNeeded();
 
-exit $globalExitCode;
+exit $globalExitStatus;
 
 sub addDirectoriesIfNeeded($)
 {
@@ -224,25 +224,22 @@ sub addDirectoriesIfNeeded($)
     }
 }
 
+# Args:
+#   $patch: a patch string.
+#   $pathRelativeToRoot: the path of the file to be patched, relative to the
+#                        repository root. This should normally be the path
+#                        found in the patch's "Index:" line.
+#   $options: a reference to an array of options to pass to the patch command.
 sub applyPatch($$;$)
 {
-    my ($patch, $fullPath, $options) = @_;
-    chdir $pathForRepositoryRoot;
-    $options = [] if (! $options);
-    push @{$options}, "--force" if $force;
-    my $command = "patch " . join(" ", "-p0", @{$options});
-    open PATCH, "| $command" or die "Failed to patch $fullPath\n";
-    print PATCH $patch;
-    close PATCH;
-    chdir $pathScriptWasRunFrom;
-
-    my $exitCode = $? >> 8;
-    if ($exitCode) {
-        if (!$force) {
-            print "$command \"$fullPath\" returned $exitCode.  Pass --force to ignore patch failures.\n";
-            exit $exitCode;
-        }
-        $globalExitCode = $exitCode;
+    my ($patch, $pathRelativeToRoot, $options) = @_;
+
+    my $optionalArgs = {options => $options, ensureForce => $force};
+
+    my $exitStatus = runPatchCommand($patch, $repositoryRootPath, $pathRelativeToRoot, $optionalArgs);
+
+    if ($exitStatus) {
+        $globalExitStatus = $exitStatus;
     }
 }
 
diff --git a/WebKitTools/Scripts/svn-unapply b/WebKitTools/Scripts/svn-unapply
index c277a3e..5f145b0 100755
--- a/WebKitTools/Scripts/svn-unapply
+++ b/WebKitTools/Scripts/svn-unapply
@@ -2,6 +2,7 @@
 
 # Copyright (C) 2005, 2006, 2007 Apple Inc.  All rights reserved.
 # Copyright (C) 2009 Cameron McCormack <cam at mcc.id.au>
+# Copyright (C) 2010 Chris Jerdonek (chris.jerdonek at gmail.com)
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -76,14 +77,22 @@ sub revertDirectories();
 sub unapplyPatch($$;$);
 sub unsetChangeLogDate($$);
 
+my $force = 0;
 my $showHelp = 0;
-if (!GetOptions("help!" => \$showHelp) || $showHelp) {
-    print STDERR basename($0) . " [-h|--help] patch1 [patch2 ...]\n";
+
+my $optionParseSuccess = GetOptions(
+    "force!" => \$force,
+    "help!" => \$showHelp
+);
+
+if (!$optionParseSuccess || $showHelp) {
+    print STDERR basename($0) . " [-h|--help] [--force] patch1 [patch2 ...]\n";
     exit 1;
 }
 
-my $pathScriptWasRunFrom = Cwd::getcwd();
-my $pathForRepositoryRoot = determineVCSRoot();
+my $globalExitStatus = 0;
+
+my $repositoryRootPath = determineVCSRoot();
 
 my @copiedFiles;
 my %directoriesToCheck;
@@ -142,7 +151,7 @@ if (isSVN()) {
     revertDirectories();
 }
 
-exit 0;
+exit $globalExitStatus;
 
 sub checksum($)
 {
@@ -228,7 +237,7 @@ sub patch($)
 
 sub revertDirectories()
 {
-    chdir $pathForRepositoryRoot;
+    chdir $repositoryRootPath;
     my %checkedDirectories;
     foreach my $path (reverse sort keys %directoriesToCheck) {
         my @dirs = File::Spec->splitdir($path);
@@ -258,16 +267,24 @@ sub revertDirectories()
     }
 }
 
+# Args:
+#   $patch: a patch string.
+#   $pathRelativeToRoot: the path of the file to be patched, relative to the
+#                        repository root. This should normally be the path
+#                        found in the patch's "Index:" line.
+#   $options: a reference to an array of options to pass to the patch command.
+#             Do not include --reverse in this array.
 sub unapplyPatch($$;$)
 {
-    my ($patch, $fullPath, $options) = @_;
-    chdir $pathForRepositoryRoot;
-    $options = [] if (! $options);
-    my $command = "patch " . join(" ", "-p0", "-R", @{$options});
-    open PATCH, "| $command" or die "Failed to patch $fullPath: $!";
-    print PATCH $patch;
-    close PATCH;
-    chdir $pathScriptWasRunFrom;
+    my ($patch, $pathRelativeToRoot, $options) = @_;
+
+    my $optionalArgs = {options => $options, ensureForce => $force, shouldReverse => 1};
+
+    my $exitStatus = runPatchCommand($patch, $repositoryRootPath, $pathRelativeToRoot, $optionalArgs);
+
+    if ($exitStatus) {
+        $globalExitStatus = $exitStatus;
+    }
 }
 
 sub unsetChangeLogDate($$)
diff --git a/WebKitTools/Scripts/webkitdirs.pm b/WebKitTools/Scripts/webkitdirs.pm
index 75e8905..17a83f6 100644
--- a/WebKitTools/Scripts/webkitdirs.pm
+++ b/WebKitTools/Scripts/webkitdirs.pm
@@ -70,6 +70,9 @@ my $vcBuildPath;
 my $windowsTmpPath;
 my $windowsSourceDir;
 
+# Defined in VCSUtils.
+sub exitStatus($);
+
 sub determineSourceDir
 {
     return if $sourceDir;
@@ -1602,15 +1605,6 @@ sub setPathForRunningWebKitApp
     $env->{PATH} = join(':', productDir(), dirname(installedSafariPath()), appleApplicationSupportPath(), $env->{PATH} || "");
 }
 
-sub exitStatus($)
-{
-    my ($returnvalue) = @_;
-    if ($^O eq "MSWin32") {
-        return $returnvalue >> 8;
-    }
-    return WEXITSTATUS($returnvalue);
-}
-
 sub runSafari
 {
     my ($debugger) = @_;

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list