Fix various parsing bugs in Differential.
This commit is contained in:
		
							
								
								
									
										5
									
								
								resources/sql/patches/008.repoopt.sql
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										5
									
								
								resources/sql/patches/008.repoopt.sql
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,5 @@
 | 
			
		||||
ALTER TABLE phabricator_repository.repository_filesystem DROP PRIMARY KEY;
 | 
			
		||||
ALTER TABLE phabricator_repository.repository_filesystem
 | 
			
		||||
  DROP KEY repositoryID_2;
 | 
			
		||||
ALTER TABLE phabricator_repository.repository_filesystem
 | 
			
		||||
  ADD PRIMARY KEY (repositoryID, parentID, pathID, svnCommit);
 | 
			
		||||
@@ -35,6 +35,16 @@ class DiffusionBrowseController extends DiffusionController {
 | 
			
		||||
 | 
			
		||||
    if (!$results) {
 | 
			
		||||
 | 
			
		||||
      // TODO: Format all these commits into nice VCS-agnostic links, and
 | 
			
		||||
      // below.
 | 
			
		||||
      $commit = $drequest->getCommit();
 | 
			
		||||
      $callsign = $drequest->getRepository()->getCallsign();
 | 
			
		||||
      if ($commit) {
 | 
			
		||||
        $commit = "r{$callsign}{$commit}";
 | 
			
		||||
      } else {
 | 
			
		||||
        $commit = 'HEAD';
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
      switch ($browse_query->getReasonForEmptyResultSet()) {
 | 
			
		||||
        case DiffusionBrowseQuery::REASON_IS_NONEXISTENT:
 | 
			
		||||
          $title = 'Path Does Not Exist';
 | 
			
		||||
@@ -43,19 +53,15 @@ class DiffusionBrowseController extends DiffusionController {
 | 
			
		||||
          $body  = "This path does not exist anywhere.";
 | 
			
		||||
          $severity = AphrontErrorView::SEVERITY_ERROR;
 | 
			
		||||
          break;
 | 
			
		||||
        case DiffusionBrowseQuery::REASON_IS_EMPTY:
 | 
			
		||||
          $title = 'Empty Directory';
 | 
			
		||||
          $body = "This path was an empty directory at {$commit}.\n";
 | 
			
		||||
          $severity = AphrontErrorView::SEVERITY_NOTICE;
 | 
			
		||||
          break;
 | 
			
		||||
        case DiffusionBrowseQuery::REASON_IS_DELETED:
 | 
			
		||||
          // TODO: Format all these commits into nice VCS-agnostic links.
 | 
			
		||||
          $commit = $drequest->getCommit();
 | 
			
		||||
          $deleted = $browse_query->getDeletedAtCommit();
 | 
			
		||||
          $existed = $browse_query->getExistedAtCommit();
 | 
			
		||||
 | 
			
		||||
          $callsign = $drequest->getRepository()->getCallsign();
 | 
			
		||||
 | 
			
		||||
          if ($commit) {
 | 
			
		||||
            $commit = "r{$callsign}{$commit}";
 | 
			
		||||
          } else {
 | 
			
		||||
            $commit = 'HEAD';
 | 
			
		||||
          }
 | 
			
		||||
          $deleted = "r{$callsign}{$deleted}";
 | 
			
		||||
          $existed = "r{$callsign}{$existed}";
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -66,8 +66,12 @@ class DiffusionCommitController extends DiffusionController {
 | 
			
		||||
    $change_table->setDiffusionRequest($drequest);
 | 
			
		||||
    $change_table->setPathChanges($changes);
 | 
			
		||||
 | 
			
		||||
    // TODO: Large number of modified files check.
 | 
			
		||||
 | 
			
		||||
    $count = number_format(count($changes));
 | 
			
		||||
 | 
			
		||||
    $change_panel = new AphrontPanelView();
 | 
			
		||||
    $change_panel->setHeader('Changes');
 | 
			
		||||
    $change_panel->setHeader("Changes ({$count})");
 | 
			
		||||
    $change_panel->appendChild($change_table);
 | 
			
		||||
 | 
			
		||||
    $content[] = $change_panel;
 | 
			
		||||
 
 | 
			
		||||
@@ -28,6 +28,7 @@ abstract class DiffusionBrowseQuery {
 | 
			
		||||
  const REASON_IS_DELETED         = 'is-deleted';
 | 
			
		||||
  const REASON_IS_NONEXISTENT     = 'nonexistent';
 | 
			
		||||
  const REASON_BAD_COMMIT         = 'bad-commit';
 | 
			
		||||
  const REASON_IS_EMPTY           = 'empty';
 | 
			
		||||
 | 
			
		||||
  final private function __construct() {
 | 
			
		||||
    // <private>
 | 
			
		||||
 
 | 
			
		||||
@@ -53,7 +53,7 @@ class DiffusionPathChangeQuery {
 | 
			
		||||
        FROM %T c
 | 
			
		||||
          LEFT JOIN %T p ON c.pathID = p.id
 | 
			
		||||
          LEFT JOIN %T t on c.targetPathID = t.id
 | 
			
		||||
        WHERE c.commitID = %d',
 | 
			
		||||
        WHERE c.commitID = %d AND isDirect = 1',
 | 
			
		||||
      PhabricatorRepository::TABLE_PATHCHANGE,
 | 
			
		||||
      PhabricatorRepository::TABLE_PATH,
 | 
			
		||||
      PhabricatorRepository::TABLE_PATH,
 | 
			
		||||
@@ -61,6 +61,7 @@ class DiffusionPathChangeQuery {
 | 
			
		||||
 | 
			
		||||
    $changes = array();
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
    $raw_changes = isort($raw_changes, 'pathName');
 | 
			
		||||
    foreach ($raw_changes as $raw_change) {
 | 
			
		||||
      $type = $raw_change['changeType'];
 | 
			
		||||
 
 | 
			
		||||
@@ -120,6 +120,10 @@ class DiffusionRequest {
 | 
			
		||||
      $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
 | 
			
		||||
        'commitID = %d',
 | 
			
		||||
        $commit->getID());
 | 
			
		||||
      if (!$data) {
 | 
			
		||||
        $data = new PhabricatorRepositoryCommitData();
 | 
			
		||||
        $data->setCommitMessage('(This commit has not fully parsed yet.)');
 | 
			
		||||
      }
 | 
			
		||||
      $this->repositoryCommitData = $data;
 | 
			
		||||
    }
 | 
			
		||||
    return $this->repositoryCommitData;
 | 
			
		||||
 
 | 
			
		||||
@@ -27,19 +27,35 @@ final class DiffusionCommitChangeTableView extends DiffusionView {
 | 
			
		||||
 | 
			
		||||
  public function render() {
 | 
			
		||||
    $rows = array();
 | 
			
		||||
 | 
			
		||||
    // TODO: Experiment with path stack rendering.
 | 
			
		||||
 | 
			
		||||
    // TODO: Copy Away and Move Away are rendered junkily still.
 | 
			
		||||
 | 
			
		||||
    foreach ($this->pathChanges as $change) {
 | 
			
		||||
      $change_verb = DifferentialChangeType::getFullNameForChangeType(
 | 
			
		||||
        $change->getChangeType());
 | 
			
		||||
 | 
			
		||||
      $suffix = null;
 | 
			
		||||
      if ($change->getFileType() == DifferentialChangeType::FILE_DIRECTORY) {
 | 
			
		||||
        $suffix = '/';
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
      $path = $change->getPath();
 | 
			
		||||
      $hash = substr(sha1($path), 0, 7);
 | 
			
		||||
 | 
			
		||||
      $rows[] = array(
 | 
			
		||||
        $this->linkHistory($change->getPath()),
 | 
			
		||||
        $this->linkBrowse($change->getPath()),
 | 
			
		||||
        $this->linkChange(
 | 
			
		||||
          $change->getPath(),
 | 
			
		||||
          $change->getChangeType(),
 | 
			
		||||
          $change->getFileType()),
 | 
			
		||||
        phutil_render_tag(
 | 
			
		||||
          'a',
 | 
			
		||||
          array(
 | 
			
		||||
            'text' => $change_verb,
 | 
			
		||||
          )),
 | 
			
		||||
        phutil_escape_html($change->getPath()),
 | 
			
		||||
            'href' => '#'.$hash,
 | 
			
		||||
          ),
 | 
			
		||||
          phutil_escape_html($path).$suffix),
 | 
			
		||||
      );
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -203,12 +203,17 @@ class PhabricatorRepositorySvnCommitChangeParserWorker
 | 
			
		||||
 | 
			
		||||
              $source_file_type = $this->lookupPathFileType(
 | 
			
		||||
                $repository,
 | 
			
		||||
                $path,
 | 
			
		||||
                $copy_from,
 | 
			
		||||
                array(
 | 
			
		||||
                  'rawPath'   => $copy_from,
 | 
			
		||||
                  'rawCommit' => $copy_rev,
 | 
			
		||||
                ));
 | 
			
		||||
 | 
			
		||||
              if ($source_file_type == DifferentialChangeType::FILE_DELETED) {
 | 
			
		||||
                throw new Exception(
 | 
			
		||||
                  "Something is wrong; source of a copy must exist.");
 | 
			
		||||
              }
 | 
			
		||||
 | 
			
		||||
              if ($source_file_type != DifferentialChangeType::FILE_DIRECTORY) {
 | 
			
		||||
                if (isset($raw_paths[$copy_from])) {
 | 
			
		||||
                  break;
 | 
			
		||||
@@ -243,7 +248,7 @@ class PhabricatorRepositorySvnCommitChangeParserWorker
 | 
			
		||||
                      'rawPath'         => $full_to,
 | 
			
		||||
                      'rawTargetPath'   => $full_from,
 | 
			
		||||
                      'rawTargetCommit' => $copy_rev,
 | 
			
		||||
                      'rawDirect'       => true,
 | 
			
		||||
                      'rawDirect'       => false,
 | 
			
		||||
 | 
			
		||||
                      'changeType'      => $type,
 | 
			
		||||
                      'fileType'        => $from_file_type,
 | 
			
		||||
@@ -421,17 +426,24 @@ class PhabricatorRepositorySvnCommitChangeParserWorker
 | 
			
		||||
    foreach ($effects as $effect) {
 | 
			
		||||
      $type = $effect['changeType'];
 | 
			
		||||
 | 
			
		||||
      // Don't write COPY_AWAY to the filesystem table if it isn't a direct
 | 
			
		||||
      // event. We do write CHILD.
 | 
			
		||||
      if (!$effect['rawDirect']) {
 | 
			
		||||
        if ($type == DifferentialChangeType::TYPE_COPY_AWAY) {
 | 
			
		||||
          // Don't write COPY_AWAY to the filesystem table if it isn't a direct
 | 
			
		||||
          // event.
 | 
			
		||||
          continue;
 | 
			
		||||
        }
 | 
			
		||||
        if ($type == DifferentialChangeType::TYPE_CHILD) {
 | 
			
		||||
          // Don't write CHILD to the filesystem table. Although doing these
 | 
			
		||||
          // writes has the nice property of letting you see when a directory's
 | 
			
		||||
          // contents were last changed, it explodes the table tremendously
 | 
			
		||||
          // and makes Diffusion far slower.
 | 
			
		||||
          continue;
 | 
			
		||||
        }
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
      if ($effect['rawPath'] == '/') {
 | 
			
		||||
        // Don't bother writing the CHILD events on '/' to the filesystem
 | 
			
		||||
        // table; in particular, it doesn't have a meaningful parentID.
 | 
			
		||||
        // Don't write any events on '/' to the filesystem table; in
 | 
			
		||||
        // particular, it doesn't have a meaningful parentID.
 | 
			
		||||
        continue;
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user