Merge pull request #4539 from pks-t/pks/diff_renames_with_rewrites

diff_tform: fix rename detection with rewrite/delete pair

src/diff_tform.c

@@ -686,8 +686,10 @@ static bool is_rename_target(
 			break;
 		}
 		if (FLAG_SET(opts, GIT_DIFF_FIND_RENAMES_FROM_REWRITES) &&
-			delta->similarity < opts->rename_from_rewrite_threshold)
+			delta->similarity < opts->rename_from_rewrite_threshold) {
+			delta->flags |= GIT_DIFF_FLAG__TO_SPLIT;
 			break;
+		}
 
 		return false;
 

tests/diff/rename.c

@@ -16,6 +16,13 @@ void test_diff_rename__cleanup(void)
 	cl_git_sandbox_cleanup();
 }
 
+#define INITIAL_COMMIT "31e47d8c1fa36d7f8d537b96158e3f024de0a9f2"
+#define COPY_RENAME_COMMIT "2bc7f351d20b53f1c72c16c4b036e491c478c49a"
+#define REWRITE_COPY_COMMIT "1c068dee5790ef1580cfc4cd670915b48d790084"
+#define RENAME_MODIFICATION_COMMIT "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13"
+#define REWRITE_DELETE_COMMIT "84d8efa38af7ace2b302de0adbda16b1f1cd2e1b"
+#define DELETE_RENAME_COMMIT "be053a189b0bbde545e0a3f59ce00b46ad29ce0d"
+
 /*
  * Renames repo has:
  *
@@ -36,12 +43,22 @@ void test_diff_rename__cleanup(void)
  *   ikeepsix.txt    -> ikeepsix.txt    (reorder sections in file)
  *   sixserving.txt  -> sixserving.txt  (whitespace change - not just indent)
  *   sevencities.txt -> songof7cities.txt (rename, small text changes)
+ * commit 84d8efa38af7ace2b302de0adbda16b1f1cd2e1b
+ *   songof7cities.txt -> songof7citie.txt (major rewrite, <20% match)
+ *   ikeepsix.txt      ->                  (deleted)
+ *   untimely.txt                          (no change)
+ *   sixserving.txt                        (no change)
+ * commit be053a189b0bbde545e0a3f59ce00b46ad29ce0d
+ *   ikeepsix.txt      ->              (deleted)
+ *   songof7cities.txt -> ikeepsix.txt (rename, 100% match)
+ *   untimely.txt                      (no change)
+ *   sixserving.txt                    (no change)
  */
 
 void test_diff_rename__match_oid(void)
 {
-	const char *old_sha = "31e47d8c1fa36d7f8d537b96158e3f024de0a9f2";
-	const char *new_sha = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
+	const char *old_sha = INITIAL_COMMIT;
+	const char *new_sha = COPY_RENAME_COMMIT;
 	git_tree *old_tree, *new_tree;
 	git_diff *diff;
 	git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT;
@@ -139,8 +156,8 @@ void test_diff_rename__match_oid(void)
 
 void test_diff_rename__checks_options_version(void)
 {
-	const char *old_sha = "31e47d8c1fa36d7f8d537b96158e3f024de0a9f2";
-	const char *new_sha = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
+	const char *old_sha = INITIAL_COMMIT;
+	const char *new_sha = COPY_RENAME_COMMIT;
 	git_tree *old_tree, *new_tree;
 	git_diff *diff;
 	git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT;
@@ -171,9 +188,9 @@ void test_diff_rename__checks_options_version(void)
 
 void test_diff_rename__not_exact_match(void)
 {
-	const char *sha0 = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
-	const char *sha1 = "1c068dee5790ef1580cfc4cd670915b48d790084";
-	const char *sha2 = "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13";
+	const char *sha0 = COPY_RENAME_COMMIT;
+	const char *sha1 = REWRITE_COPY_COMMIT;
+	const char *sha2 = RENAME_MODIFICATION_COMMIT;
 	git_tree *old_tree, *new_tree;
 	git_diff *diff;
 	git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT;
@@ -433,7 +450,7 @@ void test_diff_rename__test_small_files(void)
 
 void test_diff_rename__working_directory_changes(void)
 {
-	const char *sha0 = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
+	const char *sha0 = COPY_RENAME_COMMIT;
 	const char *blobsha = "66311f5cfbe7836c27510a3ba2f43e282e2c8bba";
 	git_oid id;
 	git_tree *tree;
@@ -592,8 +609,8 @@ void test_diff_rename__working_directory_changes(void)
 
 void test_diff_rename__patch(void)
 {
-	const char *sha0 = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
-	const char *sha1 = "1c068dee5790ef1580cfc4cd670915b48d790084";
+	const char *sha0 = COPY_RENAME_COMMIT;
+	const char *sha1 = REWRITE_COPY_COMMIT;
 	git_tree *old_tree, *new_tree;
 	git_diff *diff;
 	git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT;
@@ -1425,9 +1442,9 @@ void test_diff_rename__can_delete_unmodified_deltas(void)
 
 void test_diff_rename__matches_config_behavior(void)
 {
-	const char *sha0 = "31e47d8c1fa36d7f8d537b96158e3f024de0a9f2";
-	const char *sha1 = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
-	const char *sha2 = "1c068dee5790ef1580cfc4cd670915b48d790084";
+	const char *sha0 = INITIAL_COMMIT;
+	const char *sha1 = COPY_RENAME_COMMIT;
+	const char *sha2 = REWRITE_COPY_COMMIT;
 
 	git_tree *tree0, *tree1, *tree2;
 	git_config *cfg;
@@ -1508,8 +1525,8 @@ void test_diff_rename__matches_config_behavior(void)
 
 void test_diff_rename__can_override_thresholds_when_obeying_config(void)
 {
-	const char *sha1 = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
-	const char *sha2 = "1c068dee5790ef1580cfc4cd670915b48d790084";
+	const char *sha1 = COPY_RENAME_COMMIT;
+	const char *sha2 = REWRITE_COPY_COMMIT;
 
 	git_tree *tree1, *tree2;
 	git_config *cfg;
@@ -1563,8 +1580,8 @@ void test_diff_rename__can_override_thresholds_when_obeying_config(void)
 
 void test_diff_rename__by_config_doesnt_mess_with_whitespace_settings(void)
 {
-	const char *sha1 = "1c068dee5790ef1580cfc4cd670915b48d790084";
-	const char *sha2 = "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13";
+	const char *sha1 = REWRITE_COPY_COMMIT;
+	const char *sha2 = RENAME_MODIFICATION_COMMIT;
 
 	git_tree *tree1, *tree2;
 	git_config *cfg;
@@ -1710,8 +1727,8 @@ void test_diff_rename__blank_files_not_renamed_when_not_ignoring_whitespace(void
  */
 void test_diff_rename__identical(void)
 {
-	const char *old_sha = "31e47d8c1fa36d7f8d537b96158e3f024de0a9f2";
-	const char *new_sha = "2bc7f351d20b53f1c72c16c4b036e491c478c49a";
+	const char *old_sha = INITIAL_COMMIT;
+	const char *new_sha = COPY_RENAME_COMMIT;
 	git_tree *old_tree, *new_tree;
     git_diff *diff;
 	git_diff_options diff_opts = GIT_DIFF_OPTIONS_INIT;
@@ -1748,3 +1765,216 @@ void test_diff_rename__identical(void)
 	git_tree_free(new_tree);
 }
 
+void test_diff_rename__rewrite_and_delete(void)
+{
+	const char *old_sha = RENAME_MODIFICATION_COMMIT;
+	const char *new_sha = REWRITE_DELETE_COMMIT;
+	git_tree *old_tree, *new_tree;
+	git_diff *diff;
+	git_diff_find_options find_opts = GIT_DIFF_FIND_OPTIONS_INIT;
+	git_buf diff_buf = GIT_BUF_INIT;
+	const char *expected =
+		"diff --git a/ikeepsix.txt b/ikeepsix.txt\n"
+		"deleted file mode 100644\n"
+		"index eaf4a3e..0000000\n"
+		"--- a/ikeepsix.txt\n"
+		"+++ /dev/null\n"
+		"@@ -1,27 +0,0 @@\n"
+		"-I Keep Six Honest Serving-Men\n"
+		"-=============================\n"
+		"-\n"
+		"-She sends'em abroad on her own affairs,\n"
+		"- From the second she opens her eyes—\n"
+		"-One million Hows, two million Wheres,\n"
+		"-And seven million Whys!\n"
+		"-\n"
+		"-I let them rest from nine till five,\n"
+		"- For I am busy then,\n"
+		"-As well as breakfast, lunch, and tea,\n"
+		"- For they are hungry men.\n"
+		"-But different folk have different views;\n"
+		"-I know a person small—\n"
+		"-She keeps ten million serving-men,\n"
+		"-Who get no rest at all!\n"
+		"-\n"
+		"-  -- Rudyard Kipling\n"
+		"-\n"
+		"-I KEEP six honest serving-men\n"
+		"- (They taught me all I knew);\n"
+		"-Their names are What and Why and When\n"
+		"- And How and Where and Who.\n"
+		"-I send them over land and sea,\n"
+		"- I send them east and west;\n"
+		"-But after they have worked for me,\n"
+		"- I give them all a rest.\n"
+		"diff --git a/songof7cities.txt b/songof7cities.txt\n"
+		"index 4210ffd..95ceb12 100644\n"
+		"--- a/songof7cities.txt\n"
+		"+++ b/songof7cities.txt\n"
+		"@@ -1,45 +1,45 @@\n"
+		"-The Song of Seven Cities\n"
+		"+THE SONG OF SEVEN CITIES\n"
+		" ------------------------\n"
+		" \n"
+		"-I WAS Lord of Cities very sumptuously builded.\n"
+		"-Seven roaring Cities paid me tribute from afar.\n"
+		"-Ivory their outposts were--the guardrooms of them gilded,\n"
+		"-And garrisoned with Amazons invincible in war.\n"
+		"-\n"
+		"-All the world went softly when it walked before my Cities--\n"
+		"-Neither King nor Army vexed my peoples at their toil,\n"
+		"-Never horse nor chariot irked or overbore my Cities,\n"
+		"-Never Mob nor Ruler questioned whence they drew their spoil.\n"
+		"-\n"
+		"-Banded, mailed and arrogant from sunrise unto sunset;\n"
+		"-Singing while they sacked it, they possessed the land at large.\n"
+		"-Yet when men would rob them, they resisted, they made onset\n"
+		"-And pierced the smoke of battle with a thousand-sabred charge.\n"
+		"-\n"
+		"-So they warred and trafficked only yesterday, my Cities.\n"
+		"-To-day there is no mark or mound of where my Cities stood.\n"
+		"-For the River rose at midnight and it washed away my Cities.\n"
+		"-They are evened with Atlantis and the towns before the Flood.\n"
+		"-\n"
+		"-Rain on rain-gorged channels raised the water-levels round them,\n"
+		"-Freshet backed on freshet swelled and swept their world from sight,\n"
+		"-Till the emboldened floods linked arms and, flashing forward, drowned them--\n"
+		"-Drowned my Seven Cities and their peoples in one night!\n"
+		"-\n"
+		"-Low among the alders lie their derelict foundations,\n"
+		"-The beams wherein they trusted and the plinths whereon they built--\n"
+		"-My rulers and their treasure and their unborn populations,\n"
+		"-Dead, destroyed, aborted, and defiled with mud and silt!\n"
+		"-\n"
+		"-The Daughters of the Palace whom they cherished in my Cities,\n"
+		"-My silver-tongued Princesses, and the promise of their May--\n"
+		"-Their bridegrooms of the June-tide--all have perished in my Cities,\n"
+		"-With the harsh envenomed virgins that can neither love nor play.\n"
+		"-\n"
+		"-I was Lord of Cities--I will build anew my Cities,\n"
+		"-Seven, set on rocks, above the wrath of any flood.\n"
+		"-Nor will I rest from search till I have filled anew my Cities\n"
+		"-With peoples undefeated of the dark, enduring blood.\n"
+		"+I WAS LORD OF CITIES VERY SUMPTUOUSLY BUILDED.\n"
+		"+SEVEN ROARING CITIES PAID ME TRIBUTE FROM AFAR.\n"
+		"+IVORY THEIR OUTPOSTS WERE--THE GUARDROOMS OF THEM GILDED,\n"
+		"+AND GARRISONED WITH AMAZONS INVINCIBLE IN WAR.\n"
+		"+\n"
+		"+ALL THE WORLD WENT SOFTLY WHEN IT WALKED BEFORE MY CITIES--\n"
+		"+NEITHER KING NOR ARMY VEXED MY PEOPLES AT THEIR TOIL,\n"
+		"+NEVER HORSE NOR CHARIOT IRKED OR OVERBORE MY CITIES,\n"
+		"+NEVER MOB NOR RULER QUESTIONED WHENCE THEY DREW THEIR SPOIL.\n"
+		"+\n"
+		"+BANDED, MAILED AND ARROGANT FROM SUNRISE UNTO SUNSET;\n"
+		"+SINGING WHILE THEY SACKED IT, THEY POSSESSED THE LAND AT LARGE.\n"
+		"+YET WHEN MEN WOULD ROB THEM, THEY RESISTED, THEY MADE ONSET\n"
+		"+AND PIERCED THE SMOKE OF BATTLE WITH A THOUSAND-SABRED CHARGE.\n"
+		"+\n"
+		"+SO THEY WARRED AND TRAFFICKED ONLY YESTERDAY, MY CITIES.\n"
+		"+TO-DAY THERE IS NO MARK OR MOUND OF WHERE MY CITIES STOOD.\n"
+		"+FOR THE RIVER ROSE AT MIDNIGHT AND IT WASHED AWAY MY CITIES.\n"
+		"+THEY ARE EVENED WITH ATLANTIS AND THE TOWNS BEFORE THE FLOOD.\n"
+		"+\n"
+		"+RAIN ON RAIN-GORGED CHANNELS RAISED THE WATER-LEVELS ROUND THEM,\n"
+		"+FRESHET BACKED ON FRESHET SWELLED AND SWEPT THEIR WORLD FROM SIGHT,\n"
+		"+TILL THE EMBOLDENED FLOODS LINKED ARMS AND, FLASHING FORWARD, DROWNED THEM--\n"
+		"+DROWNED MY SEVEN CITIES AND THEIR PEOPLES IN ONE NIGHT!\n"
+		"+\n"
+		"+LOW AMONG THE ALDERS LIE THEIR DERELICT FOUNDATIONS,\n"
+		"+THE BEAMS WHEREIN THEY TRUSTED AND THE PLINTHS WHEREON THEY BUILT--\n"
+		"+MY RULERS AND THEIR TREASURE AND THEIR UNBORN POPULATIONS,\n"
+		"+DEAD, DESTROYED, ABORTED, AND DEFILED WITH MUD AND SILT!\n"
+		"+\n"
+		"+THE DAUGHTERS OF THE PALACE WHOM THEY CHERISHED IN MY CITIES,\n"
+		"+MY SILVER-TONGUED PRINCESSES, AND THE PROMISE OF THEIR MAY--\n"
+		"+THEIR BRIDEGROOMS OF THE JUNE-TIDE--ALL HAVE PERISHED IN MY CITIES,\n"
+		"+WITH THE HARSH ENVENOMED VIRGINS THAT CAN NEITHER LOVE NOR PLAY.\n"
+		"+\n"
+		"+I WAS LORD OF CITIES--I WILL BUILD ANEW MY CITIES,\n"
+		"+SEVEN, SET ON ROCKS, ABOVE THE WRATH OF ANY FLOOD.\n"
+		"+NOR WILL I REST FROM SEARCH TILL I HAVE FILLED ANEW MY CITIES\n"
+		"+WITH PEOPLES UNDEFEATED OF THE DARK, ENDURING BLOOD.\n"
+		" \n"
+		" To the sound of trumpets shall their seed restore my Cities\n"
+		" Wealthy and well-weaponed, that once more may I behold\n";
+
+	old_tree = resolve_commit_oid_to_tree(g_repo, old_sha);
+	new_tree = resolve_commit_oid_to_tree(g_repo, new_sha);
+
+	find_opts.flags = GIT_DIFF_FIND_RENAMES_FROM_REWRITES;
+
+	cl_git_pass(git_diff_tree_to_tree(&diff, g_repo, old_tree, new_tree, NULL));
+	cl_git_pass(git_diff_find_similar(diff, &find_opts));
+
+	cl_git_pass(git_diff_to_buf(&diff_buf, diff, GIT_DIFF_FORMAT_PATCH));
+
+	cl_assert_equal_s(expected, diff_buf.ptr);
+
+	git_buf_free(&diff_buf);
+	git_diff_free(diff);
+	git_tree_free(old_tree);
+	git_tree_free(new_tree);
+}
+
+void test_diff_rename__delete_and_rename(void)
+{
+	const char *old_sha = RENAME_MODIFICATION_COMMIT;
+	const char *new_sha = DELETE_RENAME_COMMIT;
+	git_tree *old_tree, *new_tree;
+	git_diff *diff;
+	git_diff_find_options find_opts = GIT_DIFF_FIND_OPTIONS_INIT;
+	git_buf diff_buf = GIT_BUF_INIT;
+	const char *expected =
+		"diff --git a/sixserving.txt b/sixserving.txt\n"
+		"deleted file mode 100644\n"
+		"index f90d4fc..0000000\n"
+		"--- a/sixserving.txt\n"
+		"+++ /dev/null\n"
+		"@@ -1,25 +0,0 @@\n"
+		"-I  KEEP  six  honest  serving-men\n"
+		"-  (They  taught  me  all  I  knew);\n"
+		"-Their  names  are  What  and  Why  and  When\n"
+		"-  And  How  and  Where  and  Who.\n"
+		"-I  send  them  over  land  and  sea,\n"
+		"-  I  send  them  east  and  west;\n"
+		"-But  after  they  have  worked  for  me,\n"
+		"-  I  give  them  all  a  rest.\n"
+		"-\n"
+		"-I  let  them  rest  from  nine  till  five,\n"
+		"-  For  I  am  busy  then,\n"
+		"-As  well  as  breakfast,  lunch,  and  tea,\n"
+		"-  For  they  are  hungry  men.\n"
+		"-But  different  folk  have  different  views;\n"
+		"-I  know  a  person  small—\n"
+		"-She  keeps  ten  million  serving-men,\n"
+		"-Who  get  no  rest  at  all!\n"
+		"-\n"
+		"-She  sends'em  abroad  on  her  own  affairs,\n"
+		"-  From  the  second  she  opens  her  eyes—\n"
+		"-One  million  Hows,  two  million  Wheres,\n"
+		"-And  seven  million  Whys!\n"
+		"-\n"
+		"-    --  Rudyard  Kipling\n"
+		"-\n"
+		"diff --git a/songof7cities.txt b/sixserving.txt\n"
+		"similarity index 100%\n"
+		"rename from songof7cities.txt\n"
+		"rename to sixserving.txt\n";
+
+	old_tree = resolve_commit_oid_to_tree(g_repo, old_sha);
+	new_tree = resolve_commit_oid_to_tree(g_repo, new_sha);
+
+	find_opts.flags = GIT_DIFF_FIND_RENAMES_FROM_REWRITES;
+
+	cl_git_pass(git_diff_tree_to_tree(&diff, g_repo, old_tree, new_tree, NULL));
+	cl_git_pass(git_diff_find_similar(diff, &find_opts));
+
+	cl_git_pass(git_diff_to_buf(&diff_buf, diff, GIT_DIFF_FORMAT_PATCH));
+
+	cl_assert_equal_s(expected, diff_buf.ptr);
+
+	git_buf_free(&diff_buf);
+	git_diff_free(diff);
+	git_tree_free(old_tree);
+	git_tree_free(new_tree);
+}

tests/resources/renames/.gitted/refs/heads/delete-and-rename

+be053a189b0bbde545e0a3f59ce00b46ad29ce0d

tests/resources/renames/.gitted/refs/heads/rewrite-and-delete

+84d8efa38af7ace2b302de0adbda16b1f1cd2e1b