diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 215a7a85..8676fb1f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -5,7 +5,6 @@ * @generated * @phutil-library-version 2 */ - phutil_register_library_map(array( '__library_version__' => 2, 'class' => diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php index f9f9e441..50d9c3bd 100644 --- a/src/parser/ArcanistDiffParser.php +++ b/src/parser/ArcanistDiffParser.php @@ -852,6 +852,14 @@ final class ArcanistDiffParser { } protected function parseChangeset(ArcanistDiffChange $change) { + // If a diff includes two sets of changes to the same file, let the + // second one win. In particular, this occurs when adding subdirectories + // in Subversion that contain files: the file text will be present in + // both the directory diff and the file diff. See T5555. Dropping the + // hunks lets whichever one shows up later win instead of showing changes + // twice. + $change->dropHunks(); + $all_changes = array(); do { $hunk = new ArcanistDiffHunk(); diff --git a/src/parser/__tests__/ArcanistDiffParserTestCase.php b/src/parser/__tests__/ArcanistDiffParserTestCase.php index 965468da..8e547a6e 100644 --- a/src/parser/__tests__/ArcanistDiffParserTestCase.php +++ b/src/parser/__tests__/ArcanistDiffParserTestCase.php @@ -588,6 +588,13 @@ EOTEXT ArcanistDiffChangeType::TYPE_CHANGE, $change->getType()); break; + case 'svn-double-diff.svndiff': + $this->assertEqual(1, count($changes)); + + $change = array_shift($changes); + $hunks = $change->getHunks(); + $this->assertEqual(1, count($hunks)); + break; default: throw new Exception("No test block for diff file {$diff_file}."); break; diff --git a/src/parser/__tests__/diff/svn-double-diff.svndiff b/src/parser/__tests__/diff/svn-double-diff.svndiff new file mode 100644 index 00000000..7bf96277 --- /dev/null +++ b/src/parser/__tests__/diff/svn-double-diff.svndiff @@ -0,0 +1,16 @@ +Index: subdir/newfile +=================================================================== +--- subdir/newfile (revision 0) ++++ subdir/newfile (working copy) +@@ -0,0 +1,3 @@ ++apple ++banana ++cherry +Index: subdir/newfile +=================================================================== +--- subdir/newfile (revision 0) ++++ subdir/newfile (working copy) +@@ -0,0 +1,3 @@ ++apple ++banana ++cherry diff --git a/src/parser/diff/ArcanistDiffChange.php b/src/parser/diff/ArcanistDiffChange.php index 3b8c04fa..c5e8de7e 100644 --- a/src/parser/diff/ArcanistDiffChange.php +++ b/src/parser/diff/ArcanistDiffChange.php @@ -192,6 +192,11 @@ final class ArcanistDiffChange { return $this; } + public function dropHunks() { + $this->hunks = array(); + return $this; + } + public function getHunks() { return $this->hunks; } diff --git a/src/repository/api/ArcanistSubversionAPI.php b/src/repository/api/ArcanistSubversionAPI.php index a4114bd0..00366688 100644 --- a/src/repository/api/ArcanistSubversionAPI.php +++ b/src/repository/api/ArcanistSubversionAPI.php @@ -272,6 +272,8 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI { } public function buildDiffFuture($path) { + $root = phutil_get_library_root('arcanist'); + // The "--depth empty" flag prevents us from picking up changes in // children when we run 'diff' against a directory. Specifically, when a // user has added or modified some directory "example/", we want to return @@ -279,8 +281,9 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI { // without "--depth empty", svn will give us changes to the directory // itself (such as property changes) and also give us changes to any // files within the directory (basically, implicit recursion). We don't - // want that, so prevent recursive diffing. - $root = phutil_get_library_root('arcanist'); + // want that, so prevent recursive diffing. This flag does not work if the + // directory is newly added (see T5555) so we need to filter the results + // out later as well. if (phutil_is_windows()) { // TODO: Provide a binary_safe_diff script for Windows.