Title: [103046] trunk/Tools
Revision
103046
Author
hara...@chromium.org
Date
2011-12-16 01:21:02 -0800 (Fri, 16 Dec 2011)

Log Message

[Refactoring] Remove all global variables from prepare-ChangeLog
https://bugs.webkit.org/show_bug.cgi?id=74681

Reviewed by Ryosuke Niwa.

We are planning to write unit-tests for prepare-ChangeLog in a run-leaks_unittest
manner. This bug is one of the incremental refactorings to remove all top-level
code and global variables from prepare-ChangeLog. In this patch,
we make the following global variables be used only through parameter passing.
This patch removes all global variables from prepare-ChangeLog.
    - $mergeBase
    - $gitCommit
    - $gitIndex

* Scripts/prepare-ChangeLog:
(generateFunctionLists):
(changeLogNameFromArgs):
(changeLogEmailAddressFromArgs):
(generateNewChangeLogs):
(printDiff):
(diffFromToString):
(diffCommand):
(statusCommand):
(createPatchCommand):
(generateFileList):
(isConflictStatus):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (103045 => 103046)


--- trunk/Tools/ChangeLog	2011-12-16 09:08:00 UTC (rev 103045)
+++ trunk/Tools/ChangeLog	2011-12-16 09:21:02 UTC (rev 103046)
@@ -1,3 +1,32 @@
+2011-12-16  Kentaro Hara  <hara...@chromium.org>
+
+        [Refactoring] Remove all global variables from prepare-ChangeLog
+        https://bugs.webkit.org/show_bug.cgi?id=74681
+
+        Reviewed by Ryosuke Niwa.
+
+        We are planning to write unit-tests for prepare-ChangeLog in a run-leaks_unittest
+        manner. This bug is one of the incremental refactorings to remove all top-level
+        code and global variables from prepare-ChangeLog. In this patch,
+        we make the following global variables be used only through parameter passing.
+        This patch removes all global variables from prepare-ChangeLog.
+            - $mergeBase
+            - $gitCommit
+            - $gitIndex
+
+        * Scripts/prepare-ChangeLog:
+        (generateFunctionLists):
+        (changeLogNameFromArgs):
+        (changeLogEmailAddressFromArgs):
+        (generateNewChangeLogs):
+        (printDiff):
+        (diffFromToString):
+        (diffCommand):
+        (statusCommand):
+        (createPatchCommand):
+        (generateFileList):
+        (isConflictStatus):
+
 2011-12-15  Philippe Normand  <pnorm...@igalia.com>
 
         [GTK] Rounding errors on 32-bit machines causes tests to fail

Modified: trunk/Tools/Scripts/prepare-ChangeLog (103045 => 103046)


--- trunk/Tools/Scripts/prepare-ChangeLog	2011-12-16 09:08:00 UTC (rev 103045)
+++ trunk/Tools/Scripts/prepare-ChangeLog	2011-12-16 09:21:02 UTC (rev 103046)
@@ -65,29 +65,29 @@
 use VCSUtils;
 
 sub changeLogDate($);
-sub changeLogEmailAddressFromArgs($);
-sub changeLogNameFromArgs($);
+sub changeLogEmailAddressFromArgs($$);
+sub changeLogNameFromArgs($$);
 sub fetchBugDescriptionFromURL($$);
 sub findChangeLogs($);
 sub getLatestChangeLogs($);
 sub resolveConflictedChangeLogs($);
-sub generateNewChangeLogs($$$$$$$$$$);
-sub printDiff($);
+sub generateNewChangeLogs($$$$$$$$$$$);
+sub printDiff($$$$);
 sub openChangeLogs($);
-sub diffFromToString();
-sub diffCommand(@);
-sub statusCommand(@);
-sub createPatchCommand($);
+sub diffFromToString($$$);
+sub diffCommand($$$$);
+sub statusCommand($$$$);
+sub createPatchCommand($$$$);
 sub diffHeaderFormat();
 sub findOriginalFileFromSvn($);
 sub determinePropertyChanges($$$);
 sub pluralizeAndList($$@);
-sub generateFileList(\%);
-sub generateFunctionLists($$);
+sub generateFileList(\%$$$);
+sub generateFunctionLists($$$$$);
 sub isUnmodifiedStatus($);
 sub isModifiedStatus($);
 sub isAddedStatus($);
-sub isConflictStatus($);
+sub isConflictStatus($$$);
 sub statusDescription($$$$);
 sub propertyChangeDescription($);
 sub extractLineRange($);
@@ -171,7 +171,7 @@
 my %paths = processPaths(@ARGV);
 
 # Find the list of modified files
-my ($changedFiles, $conflictFiles, $functionLists, $addedRegressionTests) = generateFileList(%paths);
+my ($changedFiles, $conflictFiles, $functionLists, $addedRegressionTests) = generateFileList(%paths, $gitCommit, $gitIndex, $mergeBase);
 
 if (!@$changedFiles && !@$conflictFiles && !keys %$functionLists) {
     print STDERR "  No changes found.\n";
@@ -184,11 +184,11 @@
     exit 1;
 }
 
-generateFunctionLists($changedFiles, $functionLists);
+generateFunctionLists($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase);
 
 # Get some parameters for the ChangeLog we are about to write.
-$name = changeLogNameFromArgs($name);
-$emailAddress = changeLogEmailAddressFromArgs($emailAddress);
+$name = changeLogNameFromArgs($name, $gitCommit);
+$emailAddress = changeLogEmailAddressFromArgs($emailAddress, $gitCommit);
 
 print STDERR "  Change author: $name <$emailAddress>.\n";
 
@@ -213,7 +213,7 @@
     resolveConflictedChangeLogs($changeLogs);
 }
 
-generateNewChangeLogs($prefixes, $filesInChangeLog, $addedRegressionTests, $functionLists, $bugURL, $bugDescription, $name, $emailAddress, $gitReviewer, $writeChangeLogs);
+generateNewChangeLogs($prefixes, $filesInChangeLog, $addedRegressionTests, $functionLists, $bugURL, $bugDescription, $name, $emailAddress, $gitReviewer, $gitCommit, $writeChangeLogs);
 
 if ($writeChangeLogs) {
     print STDERR "-- Please remember to include a detailed description in your ChangeLog entry. --\n-- See <http://webkit.org/coding/contributing.html> for more info --\n";
@@ -221,7 +221,7 @@
 
 # Write out another diff.
 if ($spewDiff && @$changedFiles) {
-    printDiff($changedFiles);
+    printDiff($changedFiles, $gitCommit, $gitIndex, $mergeBase);
 }
 
 # Open ChangeLogs.
@@ -233,9 +233,9 @@
 exit;
 
 
-sub generateFunctionLists($$)
+sub generateFunctionLists($$$$$)
 {
-    my ($changedFiles, $functionLists) = @_;
+    my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase) = @_;
 
     my %changed_line_ranges;
     if (@$changedFiles) {
@@ -243,7 +243,7 @@
         # Use line numbers from the "after" side of each diff.
         print STDERR "  Reviewing diff to determine which lines changed.\n";
         my $file;
-        open DIFF, "-|", diffCommand(@$changedFiles) or die "The diff failed: $!.\n";
+        open DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase) or die "The diff failed: $!.\n";
         while (<DIFF>) {
             $file = makeFilePathRelative($1) if $_ =~ diffHeaderFormat();
             if (defined $file) {
@@ -318,18 +318,18 @@
     return $date;
 }
 
-sub changeLogNameFromArgs($)
+sub changeLogNameFromArgs($$)
 {
-    my ($nameFromArgs) = @_;
+    my ($nameFromArgs, $gitCommit) = @_;
     # Silently allow --git-commit to win, we could warn if $nameFromArgs is defined.
     return `$GIT log --max-count=1 --pretty=\"format:%an\" \"$gitCommit\"` if $gitCommit;
 
     return $nameFromArgs || changeLogName();
 }
 
-sub changeLogEmailAddressFromArgs($)
+sub changeLogEmailAddressFromArgs($$)
 {
-    my ($emailAddressFromArgs) = @_;
+    my ($emailAddressFromArgs, $gitCommit) = @_;
     # Silently allow --git-commit to win, we could warn if $emailAddressFromArgs is defined.
     return `$GIT log --max-count=1 --pretty=\"format:%ae\" \"$gitCommit\"` if $gitCommit;
 
@@ -452,9 +452,9 @@
     close RESOLVE;
 }
 
-sub generateNewChangeLogs($$$$$$$$$$)
+sub generateNewChangeLogs($$$$$$$$$$$)
 {
-    my ($prefixes, $filesInChangeLog, $addedRegressionTests, $functionLists, $bugURL, $bugDescription, $name, $emailAddress, $gitReviewer, $writeChangeLogs) = @_;
+    my ($prefixes, $filesInChangeLog, $addedRegressionTests, $functionLists, $bugURL, $bugDescription, $name, $emailAddress, $gitReviewer, $gitCommit, $writeChangeLogs) = @_;
 
     # Generate new ChangeLog entries and (optionally) write out new ChangeLog files.
     foreach my $prefix (@$prefixes) {
@@ -518,13 +518,14 @@
     }
 }
 
-sub printDiff($)
+sub printDiff($$$$)
 {
-    my ($changedFiles) = @_;
+    my ($changedFiles, $gitCommit, $gitIndex, $mergeBase) = @_;
 
     print STDERR "  Running diff to help you write the ChangeLog entries.\n";
     local $/ = undef; # local slurp mode
-    open DIFF, "-|", createPatchCommand("'" . join ("' '", @$changedFiles) . "'") or die "The diff failed: $!.\n";
+    my $changedFilesString = "'" . join("' '", @$changedFiles) . "'";
+    open DIFF, "-|", createPatchCommand($changedFilesString, $gitCommit, $gitIndex, $mergeBase) or die "The diff failed: $!.\n";
     print <DIFF>;
     close DIFF;
 }
@@ -1432,8 +1433,10 @@
     return %result;
 }
 
-sub diffFromToString()
+sub diffFromToString($$$)
 {
+    my ($gitCommit, $gitIndex, $mergeBase) = @_;
+
     return "" if isSVN();
     return $gitCommit if $gitCommit =~ m/.+\.\..+/;
     return "\"$gitCommit^\" \"$gitCommit\"" if $gitCommit;
@@ -1442,51 +1445,51 @@
     return "HEAD" if isGit();
 }
 
-sub diffCommand(@)
+sub diffCommand($$$$)
 {
-    my @paths = @_;
+    my ($paths, $gitCommit, $gitIndex, $mergeBase) = @_;
 
     my $command;
     if (isSVN()) {
-        my @escapedPaths = map(escapeSubversionPath($_), @paths);
+        my @escapedPaths = map(escapeSubversionPath($_), @$paths);
         my $escapedPathsString = "'" . join("' '", @escapedPaths) . "'";
         $command = "$SVN diff --diff-cmd diff -x -N $escapedPathsString";
     } elsif (isGit()) {
-        my $pathsString = "'" . join("' '", @paths) . "'"; 
-        $command = "$GIT diff --no-ext-diff -U0 " . diffFromToString();
+        my $pathsString = "'" . join("' '", @$paths) . "'"; 
+        $command = "$GIT diff --no-ext-diff -U0 " . diffFromToString($gitCommit, $gitIndex, $mergeBase);
         $command .= " -- $pathsString" unless $gitCommit or $mergeBase;
     }
 
     return $command;
 }
 
-sub statusCommand(@)
+sub statusCommand($$$$)
 {
-    my @files = @_;
+    my ($paths, $gitCommit, $gitIndex, $mergeBase) = @_;
 
     my $command;
     if (isSVN()) {
-        my @escapedFiles = map(escapeSubversionPath($_), @files);
+        my @escapedFiles = map(escapeSubversionPath($_), keys %$paths);
         my $escapedFilesString = "'" . join("' '", @escapedFiles) . "'";
         $command = "$SVN stat $escapedFilesString";
     } elsif (isGit()) {
-        my $filesString = "\"" . join ("\" \"", @files) . "\"";
-        $command = "$GIT diff -r --name-status -M -C " . diffFromToString();
+        my $filesString = '"' . join('" "', keys %$paths) . '"';
+        $command = "$GIT diff -r --name-status -M -C " . diffFromToString($gitCommit, $gitIndex, $mergeBase);
         $command .= " -- $filesString" unless $gitCommit;
     }
 
     return "$command 2>&1";
 }
 
-sub createPatchCommand($)
+sub createPatchCommand($$$$)
 {
-    my ($changedFilesString) = @_;
+    my ($changedFilesString, $gitCommit, $gitIndex, $mergeBase) = @_;
 
     my $command;
     if (isSVN()) {
         $command = "'$FindBin::Bin/svn-create-patch' $changedFilesString";
     } elsif (isGit()) {
-        $command = "$GIT diff -M -C " . diffFromToString();
+        $command = "$GIT diff -M -C " . diffFromToString($gitCommit, $gitIndex, $mergeBase);
         $command .= " -- $changedFilesString" unless $gitCommit;
     }
 
@@ -1594,16 +1597,16 @@
     return "$plural " . join(", ", @items[0 .. $#items - 1]) . " and " . $items[-1];
 }
 
-sub generateFileList(\%)
+sub generateFileList(\%$$$)
 {
-    my ($paths) = @_;
+    my ($paths, $gitCommit, $gitIndex, $mergeBase) = @_;
 
     my @changedFiles;
     my @conflictFiles;
     my %functionLists;
     my @addedRegressionTests;
     print STDERR "  Running status to find changed, added, or removed files.\n";
-    open STAT, "-|", statusCommand(keys %$paths) or die "The status failed: $!.\n";
+    open STAT, "-|", statusCommand($paths, $gitCommit, $gitIndex, $mergeBase) or die "The status failed: $!.\n";
     while (<STAT>) {
         my $status;
         my $propertyStatus;
@@ -1662,7 +1665,7 @@
                        && !scalar(grep(/^script-tests$/i, @components));
             }
             push @changedFiles, $file if $components[$#components] ne "ChangeLog";
-        } elsif (isConflictStatus($status) || isConflictStatus($propertyStatus)) {
+        } elsif (isConflictStatus($status, $gitCommit, $gitIndex) || isConflictStatus($propertyStatus, $gitCommit, $gitIndex)) {
             push @conflictFiles, $file;
         }
         if (basename($file) ne "ChangeLog") {
@@ -1709,9 +1712,9 @@
     return $statusCodes{$status};
 }
 
-sub isConflictStatus($)
+sub isConflictStatus($$$)
 {
-    my ($status) = @_;
+    my ($status, $gitCommit, $gitIndex) = @_;
 
     my %svn = (
         "C" => 1,
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to