From d91da1da06ea066d1ca27ceec77157ae01ef3cb4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 20 Feb 2018 09:54:58 +0000 Subject: tests: diff::rename: use defines for commit OIDs While we frequently reuse commit OIDs throughout the file, we do not have any constants to refer to these commits. Make this a bit easier to read by giving the commit OIDs somewhat descriptive names of what kind of commit they refer to. --- tests/diff/rename.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/tests/diff/rename.c b/tests/diff/rename.c index c1cd25239..81f2d77dd 100644 --- a/tests/diff/rename.c +++ b/tests/diff/rename.c @@ -16,6 +16,11 @@ 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" + /* * Renames repo has: * @@ -40,8 +45,8 @@ void test_diff_rename__cleanup(void) 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 +144,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 +176,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 +438,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 +597,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 +1430,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 +1513,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 +1568,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 +1715,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; -- cgit v1.2.1 From 80e77b870444ece5c21382185ac6fe5e2842618d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 20 Feb 2018 10:03:48 +0000 Subject: tests: add rename-rewrite scenarios to "renames" repository Add two more scenarios to the "renames" repository. The first scenario has a major rewrite of a file and a delete of another file, the second scenario has a deletion of a file and rename of another file to the deleted file. Both scenarios will be used in the following commit. --- tests/diff/rename.c | 12 ++++++++++++ .../objects/2c/136d294960f7d939f1ed1903f1ced78b874c87 | Bin 0 -> 126 bytes .../objects/84/d8efa38af7ace2b302de0adbda16b1f1cd2e1b | 1 + .../objects/89/7dda8ecb7fa2e092bc3f9e2a179d7c1b0364db | Bin 0 -> 130 bytes .../objects/95/ceb126bf79e76020d8879a8b0d50a73307a97f | Bin 0 -> 1189 bytes .../objects/be/053a189b0bbde545e0a3f59ce00b46ad29ce0d | Bin 0 -> 163 bytes .../renames/.gitted/refs/heads/delete-and-rename | 1 + .../renames/.gitted/refs/heads/rewrite-and-delete | 1 + 8 files changed, 15 insertions(+) create mode 100644 tests/resources/renames/.gitted/objects/2c/136d294960f7d939f1ed1903f1ced78b874c87 create mode 100644 tests/resources/renames/.gitted/objects/84/d8efa38af7ace2b302de0adbda16b1f1cd2e1b create mode 100644 tests/resources/renames/.gitted/objects/89/7dda8ecb7fa2e092bc3f9e2a179d7c1b0364db create mode 100644 tests/resources/renames/.gitted/objects/95/ceb126bf79e76020d8879a8b0d50a73307a97f create mode 100644 tests/resources/renames/.gitted/objects/be/053a189b0bbde545e0a3f59ce00b46ad29ce0d create mode 100644 tests/resources/renames/.gitted/refs/heads/delete-and-rename create mode 100644 tests/resources/renames/.gitted/refs/heads/rewrite-and-delete diff --git a/tests/diff/rename.c b/tests/diff/rename.c index 81f2d77dd..3625d6884 100644 --- a/tests/diff/rename.c +++ b/tests/diff/rename.c @@ -20,6 +20,8 @@ void test_diff_rename__cleanup(void) #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: @@ -41,6 +43,16 @@ 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) diff --git a/tests/resources/renames/.gitted/objects/2c/136d294960f7d939f1ed1903f1ced78b874c87 b/tests/resources/renames/.gitted/objects/2c/136d294960f7d939f1ed1903f1ced78b874c87 new file mode 100644 index 000000000..51b7cea41 Binary files /dev/null and b/tests/resources/renames/.gitted/objects/2c/136d294960f7d939f1ed1903f1ced78b874c87 differ diff --git a/tests/resources/renames/.gitted/objects/84/d8efa38af7ace2b302de0adbda16b1f1cd2e1b b/tests/resources/renames/.gitted/objects/84/d8efa38af7ace2b302de0adbda16b1f1cd2e1b new file mode 100644 index 000000000..56f98fe3b --- /dev/null +++ b/tests/resources/renames/.gitted/objects/84/d8efa38af7ace2b302de0adbda16b1f1cd2e1b @@ -0,0 +1 @@ +xQj0SX+۲ !4'XiHeCߐtgr_fZRT%*!q.&40.Of? )g,Z0>8y$[жt5:l' Date: Tue, 20 Feb 2018 10:38:27 +0000 Subject: diff_tform: fix rename detection with rewrite/delete pair A rewritten file can either be classified as a modification of its contents or of a delete of the complete file followed by an addition of the new content. This distinction becomes important when we want to detect renames for rewrites. Given a scenario where a file "a" has been deleted and another file "b" has been renamed to "a", this should be detected as a deletion of "a" followed by a rename of "a" -> "b". Thus, splitting of the original rewrite into a delete/add pair is important here. This splitting is represented by a flag we can set at the current delta. While the flag is already being set in case we want to break rewrites, we do not do so in case where the `GIT_DIFF_FIND_RENAMES_FROM_REWRITES` flag is set. This can trigger an assert when we try to match the source and target deltas. Fix the issue by setting the `GIT_DIFF_FLAG__TO_SPLIT` flag at the delta when it is a rename target and `GIT_DIFF_FIND_RENAMES_FROM_REWRITES` is set. --- src/diff_tform.c | 4 +- tests/diff/rename.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) diff --git a/src/diff_tform.c b/src/diff_tform.c index c0461a3a3..bc664dd05 100644 --- a/src/diff_tform.c +++ b/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; diff --git a/tests/diff/rename.c b/tests/diff/rename.c index 3625d6884..ddc1d5d78 100644 --- a/tests/diff/rename.c +++ b/tests/diff/rename.c @@ -1765,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); +} -- cgit v1.2.1