@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Add basic test coverage for the Subversion parser
Summary:
Ref T4327. Adds the same basic battery of tests to the SVN parser as the other parsers have.
cowsay {{{ THIS IS AN AMAZING DIFF }}}
Test Plan:
Ran unit tests against SVN change parsing.
bwahaha
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4327
Differential Revision: https://secure.phabricator.com/D8016
+235
-48
src/applications/repository/engine/__tests__/data/CHC.svn.tgz
src/applications/repository/engine/__tests__/data/CHC.svn.tgz
This is a binary file and will not be displayed.
+197
-1
src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php
+197
-1
src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php
···
393
393
));
394
394
}
395
395
396
+
public function testSubversionParser() {
397
+
$repository = $this->buildDiscoveredRepository('CHC');
398
+
$viewer = PhabricatorUser::getOmnipotentUser();
399
+
400
+
$commits = id(new DiffusionCommitQuery())
401
+
->setViewer($viewer)
402
+
->withRepositoryIDs(array($repository->getID()))
403
+
->execute();
404
+
405
+
$this->expectChanges(
406
+
$repository,
407
+
$commits,
408
+
array(
409
+
'7' => array(
410
+
array(
411
+
'/',
412
+
null,
413
+
null,
414
+
DifferentialChangeType::TYPE_CHILD,
415
+
DifferentialChangeType::FILE_DIRECTORY,
416
+
0,
417
+
7,
418
+
),
419
+
array(
420
+
'/file_moved',
421
+
null,
422
+
null,
423
+
DifferentialChangeType::TYPE_CHANGE,
424
+
DifferentialChangeType::FILE_NORMAL,
425
+
1,
426
+
7,
427
+
),
428
+
),
429
+
430
+
'6' => array(
431
+
array(
432
+
'/',
433
+
null,
434
+
null,
435
+
DifferentialChangeType::TYPE_CHILD,
436
+
DifferentialChangeType::FILE_DIRECTORY,
437
+
0,
438
+
6,
439
+
),
440
+
array(
441
+
'/file_link',
442
+
null,
443
+
null,
444
+
DifferentialChangeType::TYPE_ADD,
445
+
// TODO: This is not correct, and should be FILE_SYMLINK.
446
+
DifferentialChangeType::FILE_NORMAL,
447
+
1,
448
+
6,
449
+
),
450
+
),
451
+
452
+
'5' => array(
453
+
array(
454
+
'/',
455
+
null,
456
+
null,
457
+
DifferentialChangeType::TYPE_CHILD,
458
+
DifferentialChangeType::FILE_DIRECTORY,
459
+
0,
460
+
5,
461
+
),
462
+
array(
463
+
'/dir',
464
+
null,
465
+
null,
466
+
DifferentialChangeType::TYPE_ADD,
467
+
DifferentialChangeType::FILE_DIRECTORY,
468
+
1,
469
+
5,
470
+
),
471
+
array(
472
+
'/dir/subfile',
473
+
null,
474
+
null,
475
+
DifferentialChangeType::TYPE_ADD,
476
+
DifferentialChangeType::FILE_NORMAL,
477
+
1,
478
+
5,
479
+
),
480
+
),
481
+
482
+
'4' => array(
483
+
array(
484
+
'/',
485
+
null,
486
+
null,
487
+
DifferentialChangeType::TYPE_CHILD,
488
+
DifferentialChangeType::FILE_DIRECTORY,
489
+
0,
490
+
4,
491
+
),
492
+
array(
493
+
'/file',
494
+
null,
495
+
null,
496
+
DifferentialChangeType::TYPE_MOVE_AWAY,
497
+
DifferentialChangeType::FILE_NORMAL,
498
+
1,
499
+
4,
500
+
),
501
+
array(
502
+
'/file_moved',
503
+
'/file',
504
+
'2',
505
+
DifferentialChangeType::TYPE_MOVE_HERE,
506
+
DifferentialChangeType::FILE_NORMAL,
507
+
1,
508
+
4,
509
+
),
510
+
),
511
+
512
+
'3' => array(
513
+
array(
514
+
'/',
515
+
null,
516
+
null,
517
+
DifferentialChangeType::TYPE_CHILD,
518
+
DifferentialChangeType::FILE_DIRECTORY,
519
+
0,
520
+
3,
521
+
),
522
+
array(
523
+
'/file',
524
+
null,
525
+
null,
526
+
DifferentialChangeType::TYPE_COPY_AWAY,
527
+
DifferentialChangeType::FILE_NORMAL,
528
+
0,
529
+
3,
530
+
),
531
+
array(
532
+
'/file_copy',
533
+
'/file',
534
+
'2',
535
+
DifferentialChangeType::TYPE_COPY_HERE,
536
+
DifferentialChangeType::FILE_NORMAL,
537
+
1,
538
+
3,
539
+
),
540
+
),
541
+
542
+
'2' => array(
543
+
array(
544
+
'/',
545
+
null,
546
+
null,
547
+
DifferentialChangeType::TYPE_CHILD,
548
+
DifferentialChangeType::FILE_DIRECTORY,
549
+
0,
550
+
2,
551
+
),
552
+
array(
553
+
'/file',
554
+
null,
555
+
null,
556
+
DifferentialChangeType::TYPE_CHANGE,
557
+
DifferentialChangeType::FILE_NORMAL,
558
+
1,
559
+
2,
560
+
),
561
+
),
562
+
563
+
'1' => array(
564
+
array(
565
+
'/',
566
+
null,
567
+
null,
568
+
// The Git and Svn parsers don't recognize the first commit as
569
+
// creating "/", while the Mercurial parser does. All the parsers
570
+
// should probably behave like the Mercurial parser.
571
+
DifferentialChangeType::TYPE_CHILD,
572
+
DifferentialChangeType::FILE_DIRECTORY,
573
+
0,
574
+
1,
575
+
),
576
+
array(
577
+
'/file',
578
+
null,
579
+
null,
580
+
DifferentialChangeType::TYPE_ADD,
581
+
DifferentialChangeType::FILE_NORMAL,
582
+
1,
583
+
1,
584
+
),
585
+
),
586
+
));
587
+
}
588
+
396
589
private function expectChanges(
397
590
PhabricatorRepository $repository,
398
591
array $commits,
···
404
597
break;
405
598
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
406
599
$parser = 'PhabricatorRepositoryMercurialCommitChangeParserWorker';
600
+
break;
601
+
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
602
+
$parser = 'PhabricatorRepositorySvnCommitChangeParserWorker';
407
603
break;
408
604
default:
409
605
throw new Exception(pht('No support yet.'));
···
448
644
$dicts[$key] = array(
449
645
$path_map[(int)$change->getPathID()],
450
646
$target_path,
451
-
$target_commit,
647
+
$target_commit ? (string)$target_commit : null,
452
648
(int)$change->getChangeType(),
453
649
(int)$change->getFileType(),
454
650
(int)$change->getIsDirect(),
+38
-47
src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php
+38
-47
src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php
···
29
29
30
30
// Pull the top-level path changes out of "svn log". This is pretty
31
31
// straightforward; just parse the XML log.
32
-
$log = $this->getSVNLogXMLObject($uri, $svn_commit);
32
+
$log = $this->getSVNLogXMLObject($repository, $uri, $svn_commit);
33
33
34
34
$entry = $log->logentry[0];
35
35
···
357
357
$path_map = $this->lookupOrCreatePaths($lookup_paths);
358
358
$commit_map = $this->lookupSvnCommits($repository, $lookup_commits);
359
359
360
-
$this->writeChanges($repository, $commit, $effects, $path_map, $commit_map);
361
360
$this->writeBrowse($repository, $commit, $effects, $path_map);
362
361
363
-
return array();
362
+
return $this->buildChanges(
363
+
$repository,
364
+
$commit,
365
+
$effects,
366
+
$path_map,
367
+
$commit_map);
364
368
}
365
369
366
-
private function writeChanges(
370
+
private function buildChanges(
367
371
PhabricatorRepository $repository,
368
372
PhabricatorRepositoryCommit $commit,
369
373
array $effects,
370
374
array $path_map,
371
375
array $commit_map) {
372
376
373
-
$conn_w = $repository->establishConnection('w');
377
+
$results = array();
378
+
foreach ($effects as $effect) {
379
+
$path_id = $path_map[$effect['rawPath']];
380
+
381
+
$target_path_id = null;
382
+
if ($effect['rawTargetPath']) {
383
+
$target_path_id = $path_map[$effect['rawTargetPath']];
384
+
}
385
+
386
+
$target_commit_id = null;
387
+
if ($effect['rawTargetCommit']) {
388
+
$target_commit_id = $commit_map[$effect['rawTargetCommit']];
389
+
}
374
390
375
-
$sql = array();
376
-
foreach ($effects as $effect) {
377
-
$sql[] = qsprintf(
378
-
$conn_w,
379
-
'(%d, %d, %d, %nd, %nd, %d, %d, %d, %d)',
380
-
$repository->getID(),
381
-
$path_map[$effect['rawPath']],
382
-
$commit->getID(),
383
-
$effect['rawTargetPath']
384
-
? $path_map[$effect['rawTargetPath']]
385
-
: null,
386
-
$effect['rawTargetCommit']
387
-
? $commit_map[$effect['rawTargetCommit']]
388
-
: null,
389
-
$effect['changeType'],
390
-
$effect['fileType'],
391
-
$effect['rawDirect']
392
-
? 1
393
-
: 0,
394
-
$commit->getCommitIdentifier());
391
+
$result = id(new PhabricatorRepositoryParsedChange())
392
+
->setPathID($path_id)
393
+
->setTargetPathID($target_path_id)
394
+
->setTargetCommitID($target_commit_id)
395
+
->setChangeType($effect['changeType'])
396
+
->setFileType($effect['fileType'])
397
+
->setIsDirect($effect['rawDirect'])
398
+
->setCommitSequence($commit->getCommitIdentifier());
399
+
400
+
$results[] = $result;
395
401
}
396
402
397
-
queryfx(
398
-
$conn_w,
399
-
'DELETE FROM %T WHERE commitID = %d',
400
-
PhabricatorRepository::TABLE_PATHCHANGE,
401
-
$commit->getID());
402
-
foreach (array_chunk($sql, 512) as $sql_chunk) {
403
-
queryfx(
404
-
$conn_w,
405
-
'INSERT INTO %T
406
-
(repositoryID, pathID, commitID, targetPathID, targetCommitID,
407
-
changeType, fileType, isDirect, commitSequence)
408
-
VALUES %Q',
409
-
PhabricatorRepository::TABLE_PATHCHANGE,
410
-
implode(', ', $sql_chunk));
411
-
}
403
+
return $results;
412
404
}
413
405
414
406
private function writeBrowse(
···
771
763
return $parents;
772
764
}
773
765
774
-
/**
775
-
* This method is kind of awkward here but both the SVN message and
776
-
* change parsers use it.
777
-
*/
778
-
private function getSVNLogXMLObject($uri, $revision) {
779
-
list($xml) = $this->repository->execxRemoteCommand(
780
-
"log --xml --verbose --limit 1 %s@%d",
766
+
private function getSVNLogXMLObject(
767
+
PhabricatorRepository $repository,
768
+
$uri,
769
+
$revision) {
770
+
list($xml) = $repository->execxRemoteCommand(
771
+
'log --xml --verbose --limit 1 %s@%d',
781
772
$uri,
782
773
$revision);
783
774