<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[204549] trunk/Tools</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/204549">204549</a></dd>
<dt>Author</dt> <dd>dbates@webkit.org</dd>
<dt>Date</dt> <dd>2016-08-16 19:30:27 -0700 (Tue, 16 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>prepare-ChangeLog: Extract logic from generateFunctionLists() into a function that takes a delegate object
https://bugs.webkit.org/show_bug.cgi?id=160924
Reviewed by Stephanie Lewis.
Towards adding unit tests for generateFunctionLists() we move its logic into actuallyGenerateFunctionLists()
and have actuallyGenerateFunctionLists() take a delegate object to use to query the file system and SCM.
We modify generateFunctionLists() to call actuallyGenerateFunctionLists(). This will make is possible to
test the generate function list machinery without requiring a SCM checkout by substituting a delegate
object that mocks out the file system and SCM operations.
* Scripts/VCSUtils.pm:
(parseDiffStartLine): Parses an SVN or Git start line and returns the path to the target file.
* Scripts/prepare-ChangeLog:
(generateFunctionLists): Move functionality to actually generate the function lists to actuallyGenerateFunctionLists(),
abstracting the logic to query the file system and SCM into functions on a delegate object that
we pass to it.
(actuallyGenerateFunctionLists): Extracted from generateFunctionLists().
(diffHeaderFormat): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptsVCSUtilspm">trunk/Tools/Scripts/VCSUtils.pm</a></li>
<li><a href="#trunkToolsScriptsprepareChangeLog">trunk/Tools/Scripts/prepare-ChangeLog</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (204548 => 204549)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2016-08-17 02:29:30 UTC (rev 204548)
+++ trunk/Tools/ChangeLog 2016-08-17 02:30:27 UTC (rev 204549)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2016-08-16 Daniel Bates <dabates@apple.com>
+
+ prepare-ChangeLog: Extract logic from generateFunctionLists() into a function that takes a delegate object
+ https://bugs.webkit.org/show_bug.cgi?id=160924
+
+ Reviewed by Stephanie Lewis.
+
+ Towards adding unit tests for generateFunctionLists() we move its logic into actuallyGenerateFunctionLists()
+ and have actuallyGenerateFunctionLists() take a delegate object to use to query the file system and SCM.
+ We modify generateFunctionLists() to call actuallyGenerateFunctionLists(). This will make is possible to
+ test the generate function list machinery without requiring a SCM checkout by substituting a delegate
+ object that mocks out the file system and SCM operations.
+
+ * Scripts/VCSUtils.pm:
+ (parseDiffStartLine): Parses an SVN or Git start line and returns the path to the target file.
+ * Scripts/prepare-ChangeLog:
+ (generateFunctionLists): Move functionality to actually generate the function lists to actuallyGenerateFunctionLists(),
+ abstracting the logic to query the file system and SCM into functions on a delegate object that
+ we pass to it.
+ (actuallyGenerateFunctionLists): Extracted from generateFunctionLists().
+ (diffHeaderFormat): Deleted.
+
</ins><span class="cx"> 2016-08-16 Alex Christensen <achristensen@webkit.org>
</span><span class="cx">
</span><span class="cx"> URLParser should parse URLs without credentials
</span></span></pre></div>
<a id="trunkToolsScriptsVCSUtilspm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/VCSUtils.pm (204548 => 204549)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/VCSUtils.pm 2016-08-17 02:29:30 UTC (rev 204548)
+++ trunk/Tools/Scripts/VCSUtils.pm 2016-08-17 02:30:27 UTC (rev 204549)
</span><span class="lines">@@ -75,6 +75,7 @@
</span><span class="cx"> &mergeChangeLogs
</span><span class="cx"> &normalizePath
</span><span class="cx"> &parseChunkRange
</span><ins>+ &parseDiffStartLine
</ins><span class="cx"> &parseFirstEOL
</span><span class="cx"> &parsePatch
</span><span class="cx"> &pathRelativeToSVNRepositoryRootForPath
</span><span class="lines">@@ -669,6 +670,19 @@
</span><span class="cx"> return $fileMode % 2;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+# Parses an SVN or Git diff header start line.
+#
+# Args:
+# $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/;
+}
+
</ins><span class="cx"> # Parse the Git diff header start line.
</span><span class="cx"> #
</span><span class="cx"> # Args:
</span></span></pre></div>
<a id="trunkToolsScriptsprepareChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/prepare-ChangeLog (204548 => 204549)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/prepare-ChangeLog 2016-08-17 02:29:30 UTC (rev 204548)
+++ trunk/Tools/Scripts/prepare-ChangeLog 2016-08-17 02:30:27 UTC (rev 204549)
</span><span class="lines">@@ -64,6 +64,7 @@
</span><span class="cx"> use POSIX qw(strftime);
</span><span class="cx"> use VCSUtils;
</span><span class="cx">
</span><ins>+sub actuallyGenerateFunctionLists($$$$$$);
</ins><span class="cx"> sub attributeCommand($$);
</span><span class="cx"> sub changeLogDate($);
</span><span class="cx"> sub changeLogEmailAddressFromArgs($$);
</span><span class="lines">@@ -73,7 +74,6 @@
</span><span class="cx"> sub determinePropertyChanges($$$);
</span><span class="cx"> sub diffCommand($$$$);
</span><span class="cx"> sub diffFromToString($$$);
</span><del>-sub diffHeaderFormat();
</del><span class="cx"> sub extractLineRangeAfterChange($);
</span><span class="cx"> sub extractLineRangeBeforeChange($);
</span><span class="cx"> sub fetchBugXMLData($$);
</span><span class="lines">@@ -281,7 +281,34 @@
</span><span class="cx"> sub generateFunctionLists($$$$$)
</span><span class="cx"> {
</span><span class="cx"> my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase) = @_;
</span><ins>+ my %delegateHash = (
+ openDiff => sub ($$$$) {
+ my ($changedFiles, $gitCommit, $gitIndex, $mergeBase) = @_;
+ return unless open(DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase));
+ return \*DIFF;
+ },
+ openFile => sub ($) {
+ my ($file) = @_;
+ return unless open(SOURCE, "<", $file);
+ return \*SOURCE;
+ },
+ openOriginalFile => sub ($) {
+ my ($file, $gitCommit, $gitIndex, $mergeBase) = @_;
+ return unless open(SOURCE, "-|", originalFile($file, $gitCommit, $gitIndex, $mergeBase));
+ return \*SOURCE;
+ },
+ normalizePath => sub ($) {
+ my ($path) = @_;
+ return normalizePath(makeFilePathRelative($path));
+ },
+ );
+ actuallyGenerateFunctionLists($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase, \%delegateHash);
+}
</ins><span class="cx">
</span><ins>+sub actuallyGenerateFunctionLists($$$$$$)
+{
+ my ($changedFiles, $functionLists, $gitCommit, $gitIndex, $mergeBase, $delegateHashRef) = @_;
+
</ins><span class="cx"> my %line_ranges_after_changed;
</span><span class="cx"> my %line_ranges_before_changed;
</span><span class="cx"> if (@$changedFiles) {
</span><span class="lines">@@ -289,9 +316,13 @@
</span><span class="cx"> # Use line numbers from the "after" side of each diff.
</span><span class="cx"> print STDERR " Reviewing diff to determine which lines changed.\n";
</span><span class="cx"> my $file;
</span><del>- open DIFF, "-|", diffCommand($changedFiles, $gitCommit, $gitIndex, $mergeBase) or die "The diff failed: $!.\n";
- while (<DIFF>) {
- $file = normalizePath(makeFilePathRelative($1)) if $_ =~ diffHeaderFormat();
</del><ins>+ my $diffFileHandle = $delegateHashRef->{openDiff}($changedFiles, $gitCommit, $gitIndex, $mergeBase);
+ if (!$diffFileHandle) {
+ die "The diff failed: $!.\n";
+ }
+ while (<$diffFileHandle>) {
+ my $filePath = parseDiffStartLine($_);
+ $file = $delegateHashRef->{normalizePath}($filePath) if $filePath;
</ins><span class="cx"> if (defined $file) {
</span><span class="cx"> my ($before_start, $before_end) = extractLineRangeBeforeChange($_);
</span><span class="cx"> if ($before_start >= 1 && $before_end >= 1) {
</span><span class="lines">@@ -307,7 +338,7 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>- close DIFF;
</del><ins>+ close($diffFileHandle);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> # For each source file, convert line range to function list.
</span><span class="lines">@@ -321,9 +352,10 @@
</span><span class="cx"> my %saw_function;
</span><span class="cx"> # Find all the functions in the file.
</span><span class="cx"> if ($line_ranges_after_changed{$file}) {
</span><del>- open(SOURCE, "<", $file) or next;
- my @function_ranges = get_function_line_ranges(\*SOURCE, $file);
- close SOURCE;
</del><ins>+ my $sourceFileHandle = $delegateHashRef->{openFile}($file);
+ next unless $sourceFileHandle;
+ my @function_ranges = get_function_line_ranges($sourceFileHandle, $file);
+ close($sourceFileHandle);
</ins><span class="cx">
</span><span class="cx"> my @change_ranges = (@{$line_ranges_after_changed{$file}}, []);
</span><span class="cx"> my @functions = generateFunctionListsByRanges($file, \@change_ranges, \@function_ranges, \%saw_function);
</span><span class="lines">@@ -335,10 +367,11 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> # Find the deleted functions in the original file.
</span><del>- if($line_ranges_before_changed{$file}) {
- open SOURCE, "-|", originalFile($file, $gitCommit, $gitIndex, $mergeBase) or next;
- my @deleted_function_ranges = get_function_line_ranges(\*SOURCE, $file);
- close SOURCE;
</del><ins>+ if ($line_ranges_before_changed{$file}) {
+ my $originalFileHandle = $delegateHashRef->{openOriginalFile}($file, $gitCommit, $gitIndex, $mergeBase);
+ next unless $originalFileHandle;
+ my @deleted_function_ranges = get_function_line_ranges($originalFileHandle, $file);
+ close($originalFileHandle);
</ins><span class="cx">
</span><span class="cx"> my @change_ranges = (@{$line_ranges_before_changed{$file}}, []);
</span><span class="cx"> my @functions = generateFunctionListsByRanges($file, \@change_ranges, \@deleted_function_ranges, \%saw_function);
</span><span class="lines">@@ -2050,12 +2083,6 @@
</span><span class="cx"> return $command;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-sub diffHeaderFormat()
-{
- return qr/^Index: (\S+)[\r\n]*$/ if isSVN();
- return qr/^diff --git a\/.+ b\/(.+)$/ if isGit();
-}
-
</del><span class="cx"> sub findOriginalFileFromSvn($)
</span><span class="cx"> {
</span><span class="cx"> my ($file) = @_;
</span></span></pre>
</div>
</div>
</body>
</html>