From 2b2641192a28d931236fdcdd2a76997e3eadd24e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 6 May 2013 12:33:37 -0700 Subject: [PATCH] Parse VCS revert langauge in commit messages Summary: Ref T1751. When commit messages include language like "reverts X", parse it. This change doesn't do anything with the commits yet. I attempted to cover all "natural" VCS messages and all reasonable human variations of these messages. Test Plan: Added unit tests. Added `var_dump()` and used `scripts/repository/reparse.php --message X` to reparse some commit messages, with expected results. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1751 Differential Revision: https://secure.phabricator.com/D5840 --- src/__phutil_library_map__.php | 4 ++ ...DifferentialFreeformFieldSpecification.php | 52 ++++++++++++++++++- .../DifferentialFreeformFieldTestCase.php | 31 +++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 src/applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index db3c0f7613..ba191b19a2 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -334,6 +334,7 @@ phutil_register_library_map(array( 'DifferentialFieldSpecificationIncompleteException' => 'applications/differential/field/exception/DifferentialFieldSpecificationIncompleteException.php', 'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php', 'DifferentialFreeformFieldSpecification' => 'applications/differential/field/specification/DifferentialFreeformFieldSpecification.php', + 'DifferentialFreeformFieldTestCase' => 'applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php', 'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php', 'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php', 'DifferentialHovercardEventListener' => 'applications/differential/events/DifferentialHovercardEventListener.php', @@ -1221,6 +1222,7 @@ phutil_register_library_map(array( 'PhabricatorPhabricatorOAuthConfigOptions' => 'applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php', 'PhabricatorPhameConfigOptions' => 'applications/phame/config/PhabricatorPhameConfigOptions.php', 'PhabricatorPholioConfigOptions' => 'applications/pholio/config/PhabricatorPholioConfigOptions.php', + 'PhabricatorPholioMockTestDataGenerator' => 'applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php', 'PhabricatorPhortuneConfigOptions' => 'applications/phortune/option/PhabricatorPhortuneConfigOptions.php', 'PhabricatorPhrequentConfigOptions' => 'applications/phrequent/config/PhabricatorPhrequentConfigOptions.php', 'PhabricatorPhrictionConfigOptions' => 'applications/phriction/config/PhabricatorPhrictionConfigOptions.php', @@ -2099,6 +2101,7 @@ phutil_register_library_map(array( 'DifferentialFieldSpecificationIncompleteException' => 'Exception', 'DifferentialFieldValidationException' => 'Exception', 'DifferentialFreeformFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialFreeformFieldTestCase' => 'PhabricatorTestCase', 'DifferentialGitSVNIDFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialHovercardEventListener' => 'PhutilEventListener', @@ -2935,6 +2938,7 @@ phutil_register_library_map(array( 'PhabricatorPhabricatorOAuthConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPhameConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPholioConfigOptions' => 'PhabricatorApplicationConfigOptions', + 'PhabricatorPholioMockTestDataGenerator' => 'PhabricatorTestDataGenerator', 'PhabricatorPhortuneConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPhrequentConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPhrictionConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php b/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php index 5e663c5bef..cbb7d657af 100644 --- a/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php @@ -98,6 +98,53 @@ abstract class DifferentialFreeformFieldSpecification return $dependents; } + public static function findRevertedCommits($message) { + $reverts = array(); + $matches = null; + + // NOTE: Git language is "This reverts commit X." + // NOTE: Mercurial language is "Backed out changeset Y". + + $prefixes = array( + 'revert' => true, + 'reverts' => true, + 'back\s*out' => true, + 'backs\s*out' => true, + 'backed\s*out' => true, + 'undo' => true, + 'undoes' => true, + ); + + $optional = array( + 'commit' => true, + 'changeset' => true, + 'rev' => true, + 'revision' => true, + 'change' => true, + 'diff' => true, + ); + + $pre_re = implode('|', array_keys($prefixes)); + $opt_re = implode('|', array_keys($optional)); + + $matches = null; + preg_match_all( + '/\b(?i:'.$pre_re.')(?:\s+(?i:'.$opt_re.'))?([rA-Z0-9a-f,\s]+)\b/', + $message, + $matches); + + $result = array(); + foreach ($matches[1] as $commits) { + $commits = preg_split('/[,\s]+/', $commits); + $commits = array_filter($commits); + foreach ($commits as $commit) { + $result[$commit] = $commit; + } + } + + return $result; + } + public function didWriteRevision(DifferentialRevisionEditor $editor) { $message = $this->renderValueForCommitMessage(false); @@ -151,6 +198,10 @@ abstract class DifferentialFreeformFieldSpecification PhabricatorRepositoryCommit $commit, PhabricatorRepositoryCommitData $data) { + $message = $this->renderValueForCommitMessage($is_edit = false); + + $commits = self::findRevertedCommits($message); + $user = id(new PhabricatorUser())->loadOneWhere( 'phid = %s', $data->getCommitDetail('authorPHID')); @@ -158,7 +209,6 @@ abstract class DifferentialFreeformFieldSpecification return; } - $message = $this->renderValueForCommitMessage($is_edit = false); $tasks_statuses = $this->findMentionedTasks($message); if (!$tasks_statuses) { return; diff --git a/src/applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php b/src/applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php new file mode 100644 index 0000000000..20b304aa40 --- /dev/null +++ b/src/applications/differential/field/specification/__tests__/DifferentialFreeformFieldTestCase.php @@ -0,0 +1,31 @@ + array('123'), + "Reverts r123" => array('r123'), + "Reverts ac382f2" => array('ac382f2'), + "Reverts r22, r23" => array('r22', 'r23'), + "Reverts D99" => array('D99'), + "Backs out commit\n99\n100" => array('99', '100'), + "undo change f9f9f8f8" => array('f9f9f8f8'), + "Backedout Changeset rX1234" => array('rX1234'), + "This doesn't revert anything" => array(), + 'nonrevert of r11' => array(), + "fixed a bug" => array(), + ); + + foreach ($map as $input => $expect) { + $actual = array_values( + DifferentialFreeformFieldSpecification::findRevertedCommits($input)); + + $this->assertEqual( + $expect, + $actual, + "Reverted commits in: {$input}"); + } + } + +}