range-diff: show submodule changes irrespective of diff.submodule

After generating diffs for each range to be compared using a 'git log' invocation, range-diff.c::read_patches looks for the "diff --git" header in those diffs to recognize the beginning of a new change. In a project with submodules, and with 'diff.submodule=log' set in the config, this header is missing for the diff of a changed submodule, so any submodule changes are quietly ignored in the range-diff. When 'diff.submodule=diff' is set in the config, the "diff --git" header is also missing for the submodule itself, but is shown for submodule content changes, which can easily confuse 'git range-diff' and lead to errors such as: error: git apply: bad git-diff - inconsistent old filename on line 1 error: could not parse git header 'diff --git path/to/submodule/and/some/file/within ' error: could not parse log for '@{u}..@{1}' Force the submodule diff format to its default ("short") when invoking 'git log' to generate the patches for each range, such that submodule changes are always detected. Add a test, including an invocation with '--creation-factor=100' to force the second commit in the range not to be considered a complete rewrite, in order to verify we do indeed get the "short" format. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Philippe Blain committed Jun 6, 2022 at 20:59 UTC 04b1f1fd9d9faeb90d7bd25d8eb4d3ffbb28b9f1
2 files changed +52 -1
range-diff.c
+1 -1
@@ -44,7 +44,7 @@ static int read_patches(const char *range, struct string_list *list,
44
45 strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
46 "--reverse", "--date-order", "--decorate=no",
47 - "--no-prefix",
47 + "--no-prefix", "--submodule=short",
48 /*
49 * Choose indicators that are not used anywhere
50 * else in diffs, but still look reasonable
t/t3206-range-diff.sh
+51
@@ -772,4 +772,55 @@ test_expect_success '--left-only/--right-only' '
772 test_cmp expect actual
773 '
774
775 +test_expect_success 'submodule changes are shown irrespective of diff.submodule' '
776 + git init sub-repo &&
777 + test_commit -C sub-repo sub-first &&
778 + sub_oid1=$(git -C sub-repo rev-parse HEAD) &&
779 + test_commit -C sub-repo sub-second &&
780 + sub_oid2=$(git -C sub-repo rev-parse HEAD) &&
781 + test_commit -C sub-repo sub-third &&
782 + sub_oid3=$(git -C sub-repo rev-parse HEAD) &&
783 +
784 + git checkout -b main-sub topic &&
785 + git submodule add ./sub-repo sub &&
786 + git -C sub checkout --detach sub-first &&
787 + git commit -m "add sub" sub &&
788 + sup_oid1=$(git rev-parse --short HEAD) &&
789 + git checkout -b topic-sub &&
790 + git -C sub checkout sub-second &&
791 + git commit -m "change sub" sub &&
792 + sup_oid2=$(git rev-parse --short HEAD) &&
793 + git checkout -b modified-sub main-sub &&
794 + git -C sub checkout sub-third &&
795 + git commit -m "change sub" sub &&
796 + sup_oid3=$(git rev-parse --short HEAD) &&
797 + sup_oid0=$(test_oid __) &&
798 +
799 + test_config diff.submodule log &&
800 + git range-diff topic topic-sub modified-sub >actual &&
801 + cat >expect <<-EOF &&
802 + 1: $sup_oid1 = 1: $sup_oid1 add sub
803 + 2: $sup_oid2 < -: $sup_oid0 change sub
804 + -: $sup_oid0 > 2: $sup_oid3 change sub
805 + EOF
806 + test_cmp expect actual &&
807 + test_config diff.submodule diff &&
808 + git range-diff topic topic-sub modified-sub >actual &&
809 + git range-diff --creation-factor=100 topic topic-sub modified-sub >actual &&
810 + cat >expect <<-EOF &&
811 + 1: $sup_oid1 = 1: $sup_oid1 add sub
812 + 2: $sup_oid2 ! 2: $sup_oid3 change sub
813 + @@ Commit message
814 + ## sub ##
815 + @@
816 + -Subproject commit $sub_oid1
817 + -+Subproject commit $sub_oid2
818 + ++Subproject commit $sub_oid3
819 + EOF
820 + test_cmp expect actual &&
821 + test_config diff.submodule diff &&
822 + git range-diff --creation-factor=100 topic topic-sub modified-sub >actual &&
823 + test_cmp expect actual
824 +'
825 +
826 test_done