diff --git a/src/applications/repository/engine/__tests__/data/CHC.svn.tgz b/src/applications/repository/engine/__tests__/data/CHC.svn.tgz new file mode 100644 index 0000000000..93da53515b Binary files /dev/null and b/src/applications/repository/engine/__tests__/data/CHC.svn.tgz differ diff --git a/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php b/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php index 913fa4678f..1194c8922b 100644 --- a/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php +++ b/src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php @@ -393,6 +393,199 @@ final class PhabricatorChangeParserTestCase )); } + public function testSubversionParser() { + $repository = $this->buildDiscoveredRepository('CHC'); + $viewer = PhabricatorUser::getOmnipotentUser(); + + $commits = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withRepositoryIDs(array($repository->getID())) + ->execute(); + + $this->expectChanges( + $repository, + $commits, + array( + '7' => array( + array( + '/', + null, + null, + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 7, + ), + array( + '/file_moved', + null, + null, + DifferentialChangeType::TYPE_CHANGE, + DifferentialChangeType::FILE_NORMAL, + 1, + 7, + ), + ), + + '6' => array( + array( + '/', + null, + null, + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 6, + ), + array( + '/file_link', + null, + null, + DifferentialChangeType::TYPE_ADD, + // TODO: This is not correct, and should be FILE_SYMLINK. + DifferentialChangeType::FILE_NORMAL, + 1, + 6, + ), + ), + + '5' => array( + array( + '/', + null, + null, + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 5, + ), + array( + '/dir', + null, + null, + DifferentialChangeType::TYPE_ADD, + DifferentialChangeType::FILE_DIRECTORY, + 1, + 5, + ), + array( + '/dir/subfile', + null, + null, + DifferentialChangeType::TYPE_ADD, + DifferentialChangeType::FILE_NORMAL, + 1, + 5, + ), + ), + + '4' => array( + array( + '/', + null, + null, + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 4, + ), + array( + '/file', + null, + null, + DifferentialChangeType::TYPE_MOVE_AWAY, + DifferentialChangeType::FILE_NORMAL, + 1, + 4, + ), + array( + '/file_moved', + '/file', + '2', + DifferentialChangeType::TYPE_MOVE_HERE, + DifferentialChangeType::FILE_NORMAL, + 1, + 4, + ), + ), + + '3' => array( + array( + '/', + null, + null, + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 3, + ), + array( + '/file', + null, + null, + DifferentialChangeType::TYPE_COPY_AWAY, + DifferentialChangeType::FILE_NORMAL, + 0, + 3, + ), + array( + '/file_copy', + '/file', + '2', + DifferentialChangeType::TYPE_COPY_HERE, + DifferentialChangeType::FILE_NORMAL, + 1, + 3, + ), + ), + + '2' => array( + array( + '/', + null, + null, + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 2, + ), + array( + '/file', + null, + null, + DifferentialChangeType::TYPE_CHANGE, + DifferentialChangeType::FILE_NORMAL, + 1, + 2, + ), + ), + + '1' => array( + array( + '/', + null, + null, + // The Git and Svn parsers don't recognize the first commit as + // creating "/", while the Mercurial parser does. All the parsers + // should probably behave like the Mercurial parser. + DifferentialChangeType::TYPE_CHILD, + DifferentialChangeType::FILE_DIRECTORY, + 0, + 1, + ), + array( + '/file', + null, + null, + DifferentialChangeType::TYPE_ADD, + DifferentialChangeType::FILE_NORMAL, + 1, + 1, + ), + ), + )); + } + private function expectChanges( PhabricatorRepository $repository, array $commits, @@ -405,6 +598,9 @@ final class PhabricatorChangeParserTestCase case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: $parser = 'PhabricatorRepositoryMercurialCommitChangeParserWorker'; break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $parser = 'PhabricatorRepositorySvnCommitChangeParserWorker'; + break; default: throw new Exception(pht('No support yet.')); } @@ -448,7 +644,7 @@ final class PhabricatorChangeParserTestCase $dicts[$key] = array( $path_map[(int)$change->getPathID()], $target_path, - $target_commit, + $target_commit ? (string)$target_commit : null, (int)$change->getChangeType(), (int)$change->getFileType(), (int)$change->getIsDirect(), diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php index b09eefdda6..2aaaf17c33 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php @@ -29,7 +29,7 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker // Pull the top-level path changes out of "svn log". This is pretty // straightforward; just parse the XML log. - $log = $this->getSVNLogXMLObject($uri, $svn_commit); + $log = $this->getSVNLogXMLObject($repository, $uri, $svn_commit); $entry = $log->logentry[0]; @@ -357,58 +357,50 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker $path_map = $this->lookupOrCreatePaths($lookup_paths); $commit_map = $this->lookupSvnCommits($repository, $lookup_commits); - $this->writeChanges($repository, $commit, $effects, $path_map, $commit_map); $this->writeBrowse($repository, $commit, $effects, $path_map); - return array(); + return $this->buildChanges( + $repository, + $commit, + $effects, + $path_map, + $commit_map); } - private function writeChanges( + private function buildChanges( PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit, array $effects, array $path_map, array $commit_map) { - $conn_w = $repository->establishConnection('w'); - - $sql = array(); + $results = array(); foreach ($effects as $effect) { - $sql[] = qsprintf( - $conn_w, - '(%d, %d, %d, %nd, %nd, %d, %d, %d, %d)', - $repository->getID(), - $path_map[$effect['rawPath']], - $commit->getID(), - $effect['rawTargetPath'] - ? $path_map[$effect['rawTargetPath']] - : null, - $effect['rawTargetCommit'] - ? $commit_map[$effect['rawTargetCommit']] - : null, - $effect['changeType'], - $effect['fileType'], - $effect['rawDirect'] - ? 1 - : 0, - $commit->getCommitIdentifier()); + $path_id = $path_map[$effect['rawPath']]; + + $target_path_id = null; + if ($effect['rawTargetPath']) { + $target_path_id = $path_map[$effect['rawTargetPath']]; + } + + $target_commit_id = null; + if ($effect['rawTargetCommit']) { + $target_commit_id = $commit_map[$effect['rawTargetCommit']]; + } + + $result = id(new PhabricatorRepositoryParsedChange()) + ->setPathID($path_id) + ->setTargetPathID($target_path_id) + ->setTargetCommitID($target_commit_id) + ->setChangeType($effect['changeType']) + ->setFileType($effect['fileType']) + ->setIsDirect($effect['rawDirect']) + ->setCommitSequence($commit->getCommitIdentifier()); + + $results[] = $result; } - queryfx( - $conn_w, - 'DELETE FROM %T WHERE commitID = %d', - PhabricatorRepository::TABLE_PATHCHANGE, - $commit->getID()); - foreach (array_chunk($sql, 512) as $sql_chunk) { - queryfx( - $conn_w, - 'INSERT INTO %T - (repositoryID, pathID, commitID, targetPathID, targetCommitID, - changeType, fileType, isDirect, commitSequence) - VALUES %Q', - PhabricatorRepository::TABLE_PATHCHANGE, - implode(', ', $sql_chunk)); - } + return $results; } private function writeBrowse( @@ -771,13 +763,12 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker return $parents; } - /** - * This method is kind of awkward here but both the SVN message and - * change parsers use it. - */ - private function getSVNLogXMLObject($uri, $revision) { - list($xml) = $this->repository->execxRemoteCommand( - "log --xml --verbose --limit 1 %s@%d", + private function getSVNLogXMLObject( + PhabricatorRepository $repository, + $uri, + $revision) { + list($xml) = $repository->execxRemoteCommand( + 'log --xml --verbose --limit 1 %s@%d', $uri, $revision);