Analysis task should look for a git commit based on abridged hashes
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 May 2016 02:39:54 +0000 (02:39 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 May 2016 02:39:54 +0000 (02:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157877
<rdar://problem/26254374>

Reviewed by Chris Dumez.

Made /privileged-api/associate-commit look for commits using LIKE instead of an exact match.
Associate the commit when there is exactly one match.

* public/include/commit-log-fetcher.php:
(CommitLogFetcher::fetch_between):
* public/include/db.php:
(Database::escape_for_like): Extracted from CommitLogFetcher::fetch_between.
* public/privileged-api/associate-commit.php:
(main): Look for the commits using LIKE. Reject whenever there are multiple commits. We limit the number of
matches to two for performance when the user specifies something that almost thousands of commits: e.g. "1".
* public/v3/pages/analysis-task-page.js:
(AnalysisTaskPage.prototype._associateCommit): Added human friendly error messages for mismatching commits.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201123 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/public/include/db.php
Websites/perf.webkit.org/public/privileged-api/associate-commit.php
Websites/perf.webkit.org/public/v3/pages/analysis-task-page.js

index e45ee431c76010589a89ac64322e6c08974e6aba..096151238200176a76d1028f4aa5f44d79db0d7f 100644 (file)
@@ -1,3 +1,24 @@
+2016-05-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Analysis task should look for a git commit based on abridged hashes
+        https://bugs.webkit.org/show_bug.cgi?id=157877
+        <rdar://problem/26254374>
+
+        Reviewed by Chris Dumez.
+
+        Made /privileged-api/associate-commit look for commits using LIKE instead of an exact match.
+        Associate the commit when there is exactly one match.
+
+        * public/include/commit-log-fetcher.php:
+        (CommitLogFetcher::fetch_between):
+        * public/include/db.php:
+        (Database::escape_for_like): Extracted from CommitLogFetcher::fetch_between.
+        * public/privileged-api/associate-commit.php:
+        (main): Look for the commits using LIKE. Reject whenever there are multiple commits. We limit the number of
+        matches to two for performance when the user specifies something that almost thousands of commits: e.g. "1".
+        * public/v3/pages/analysis-task-page.js:
+        (AnalysisTaskPage.prototype._associateCommit): Added human friendly error messages for mismatching commits.
+
 2016-05-18  Ryosuke Niwa  <rniwa@webkit.org>
 
         Unreviewed build fix. Use --date-order so that every child commit appears after its parent.
index 3046accc58a147332d856c7ceebf4e8f421a4084..0bb73fb9d56fb6bd2cf183e666d306c4c1334a31 100644 (file)
@@ -65,7 +65,7 @@ class CommitLogFetcher {
         }
 
         if ($keyword) {
-            array_push($values, '%' . str_replace(array('\\', '_', '%'), array('\\\\', '\\_', '\\%'), $keyword) . '%');
+            array_push($values, '%' . Database::escape_for_like($keyword) . '%');
             $keyword_index = '$' . count($values);
             array_push($values, ltrim($keyword, 'r'));
             $revision_index = '$' . count($values);
index 8749be0c9270c31ae6a0085850f9c5ae670b7c20..1cc3278196126c7794dd201050d74e3660c9a760 100644 (file)
@@ -84,6 +84,10 @@ class Database
         return intval($timestamp_in_s * 1000);
     }
 
+    static function escape_for_like($string) {
+        return str_replace(array('\\', '_', '%'), array('\\\\', '\\_', '\\%'), $string);
+    }
+
     function connect() {
         $databaseConfig = config('database');
         $this->connection = @pg_connect('host=' . $databaseConfig['host'] . ' port=' . $databaseConfig['port']
index 07c1ce01ce9bb011ac9130c50c3f82ef4a783427..30ec1161fb36d94a6cf4e33d0c2d3319792c6019 100644 (file)
@@ -29,12 +29,17 @@ function main() {
         require_format('Kind', $kind, '/^(cause|fix)$/');
 
         $commit_info = array('repository' => $repository_id, 'revision' => $revision);
-        $commit_row = $db->select_first_row('commits', 'commit', $commit_info);
-        if (!$commit_row) {
+        $commit_rows = $db->query_and_fetch_all('SELECT commit_id FROM commits WHERE commit_repository = $1 AND commit_revision LIKE $2 LIMIT 2',
+            array($repository_id, '%' . Database::escape_for_like($revision) . '%'));
+        if (count($commit_rows) > 1) {
+            $db->rollback_transaction();
+            exit_with_error('AmbiguousRevision', $commit_info);            
+        } else if (!$commit_rows) {
             $db->rollback_transaction();
             exit_with_error('CommitNotFound', $commit_info);
         }
-        $commit_id = $commit_row['commit_id'];
+
+        $commit_id = $commit_rows[0]['commit_id'];
 
         $association = array('task' => $analysis_task_id, 'commit' => $commit_id, 'is_fix' => Database::to_database_boolean($kind == 'fix'));
         $commit_id = $db->update_or_insert_row('task_commits', 'taskcommit',
index e38dfe433dae82763db0902e5e60d5e5b23294a3..bf30b59f2524c1087862b2896127cbe017802c93 100644 (file)
@@ -496,7 +496,12 @@ class AnalysisTaskPage extends PageWithHeading {
         var render = this.render.bind(this);
         return this._task.associateCommit(kind, repository, revision).then(render, function (error) {
             render();
-            alert('Failed to associate the commit: ' + error);
+            if (error == 'AmbiguousRevision')
+                alert('There are multiple revisions that match the specified string: ' + revision);
+            else if (error == 'CommitNotFound')
+                alert('There are no revisions that match the specified string:' + revision);
+            else
+                alert('Failed to associate the commit: ' + error);
         });
     }