Changeset 56472 in webkit

Timestamp:

Mar 24, 2010 4:50:23 PM (14 years ago)

Author:

ddkilzer@apple.com

Message:

<​http://webkit.org/b/36560> resolve-ChangeLogs: git-rebase fails when resolve-ChangeLogs can't merge

Reviewed by Eric Seidel.

When resolve-ChangeLogs fails to merge a patch while running as
a git merge driver, it deletes the original file, which causes
an internal failure and stops git mid-merge:

fatal: Failed to execute internal merge

The fix is to use the --force switch with patch so that it will
always attempt to apply the patch. (The change in
mergeChangeLogs() for the previous commit also fixed this, but
adding --force also prevents any potential user interaction that
patch may want to display.)

• Scripts/VCSUtils.pm:

(mergeChangeLogs): Added --force switch to patch command. Also
changed to use the exit status from the patch command to
determine the return value for this method.

• Scripts/webkitperl/VCSUtils_unittest/mergeChangeLogs.pl: Added

test to cover this bug.

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/VCSUtils.pm (modified) (2 diffs)
• Scripts/webkitperl/VCSUtils_unittest/mergeChangeLogs.pl (modified) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-03-24  David Kilzer  <ddkilzer@apple.com>
+
+        <http://webkit.org/b/36560> resolve-ChangeLogs: git-rebase fails when resolve-ChangeLogs can't merge
+
+        Reviewed by Eric Seidel.
+
+        When resolve-ChangeLogs fails to merge a patch while running as
+        a git merge driver, it deletes the original file, which causes
+        an internal failure and stops git mid-merge:
+
+            fatal: Failed to execute internal merge
+
+        The fix is to use the --force switch with patch so that it will
+        always attempt to apply the patch.  (The change in
+        mergeChangeLogs() for the previous commit also fixed this, but
+        adding --force also prevents any potential user interaction that
+        patch may want to display.)
+
+        * Scripts/VCSUtils.pm:
+        (mergeChangeLogs): Added --force switch to patch command.  Also
+        changed to use the exit status from the patch command to
+        determine the return value for this method.
+        * Scripts/webkitperl/VCSUtils_unittest/mergeChangeLogs.pl: Added
+        test to cover this bug.
+
 2010-03-24  David Kilzer  <ddkilzer@apple.com>
 

trunk/WebKitTools/Scripts/VCSUtils.pm

@@ -862 +862 @@
     unlink("${fileNewer}.rej");
 
-    open(PATCH, "| patch --fuzz=3 --binary $fileNewer > " . File::Spec->devnull()) or die $!;
+    open(PATCH, "| patch --force --fuzz=3 --binary $fileNewer > " . File::Spec->devnull()) or die $!;
     print PATCH ($traditionalReject ? $patch : fixChangeLogPatch($patch));
     close(PATCH);
 
-    my $result;
+    my $result = !exitStatus($?);
 
     # Refuse to merge the patch if it did not apply cleanly
@@ -875 +875 @@
             rename("${fileNewer}.orig", $fileNewer);
         }
-        $result = 0;
     } else {
         unlink("${fileNewer}.orig");
-        $result = 1;
     }
 

trunk/WebKitTools/Scripts/webkitperl/VCSUtils_unittest/mergeChangeLogs.pl

@@ -27 +27 @@
 use strict;
 
-use Test::Simple tests => 12;
+use Test::Simple tests => 16;
 use File::Temp qw(tempfile);
 use VCSUtils;
@@ -264 +264 @@
 # --------------------------------------------------------------------------------
 
+{
+    # New test
+    my $title = "mergeChangeLogs: patch fails";
+
+    my $fileMineContent = <<'EOF';
+2010-01-29  Mark Rowe  <mrowe@apple.com>
+
+        Fix the Mac build.
+
+        Disable ENABLE_INDEXED_DATABASE since it is "completely non-functional".
+
+2010-01-29  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        Rubber-stamped by Maciej Stachowiak.
+
+        Fix the ARM build.
+EOF
+    my $fileMine = writeTempFile("fileMine", "", $fileMineContent);
+
+    my $fileOlderContent = <<'EOF';
+2010-01-29  Mark Rowe  <mrowe@apple.com>
+
+        Fix the Mac build.
+
+        Disable ENABLE_INDEXED_DATABASE since it is "completely non-functional".
+
+2010-01-29  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Darin Adler.
+
+        JSC is failing to propagate anonymous slot count on some transitions
+
+2010-01-29  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        Rubber-stamped by Maciej Stachowiak.
+
+        Fix the ARM build.
+EOF
+    my $fileOlder = writeTempFile("fileOlder", "", $fileOlderContent);
+
+    my $fileNewerContent = <<'EOF';
+2010-01-29  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Darin Adler.
+
+        JSC is failing to propagate anonymous slot count on some transitions
+
+2010-01-29  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        Rubber-stamped by Maciej Stachowiak.
+
+        Fix the ARM build.
+EOF
+    my $fileNewer = writeTempFile("fileNewer", "", $fileNewerContent);
+
+    my $exitStatus = mergeChangeLogs($fileMine, $fileOlder, $fileNewer);
+
+    # mergeChangeLogs() should return a non-zero exit status since the patch failed.
+    ok($exitStatus == 0, "$title: return non-zero exit status for failure");
+
+    ok(readFile($fileMine) eq $fileMineContent, "$title: \$fileMine should be unchanged");
+    ok(readFile($fileOlder) eq $fileOlderContent, "$title: \$fileOlder should be unchanged");
+
+    # $fileNewer should still exist unchanged because the patch failed
+    ok(readFile($fileNewer) eq $fileNewerContent, "$title: \$fileNewer should be unchanged");
+
+    unlink($fileMine, $fileOlder, $fileNewer);
+}
+
+# --------------------------------------------------------------------------------
+