git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] range-diff: show submodule changes irrespective of diff.submodule

From
Philippe Blain via GitGitGadget <gitgitgadget@gmail.com>
Date
Jun 6, 2022, 20:59 UTC
Message-ID
<pull.1244.v2.git.1654549153769.gitgitgadget@gmail.com>
In-Reply-To
<pull.1244.git.1653916145441.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>

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>
---
    range-diff: show submodule changes irrespective of diff.submodule
    
    Changes since v1:
    
     * added a comparison without '--creation-factor' to the test, as
       suggested by Ævar
     * remove separate 'git add sub' invocations in favor of 'git commit -m
       msg sub', as suggested by Dscho
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1244%2Fphil-blain%2Frange-diff-submodule-diff-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1244/phil-blain/range-diff-submodule-diff-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1244
Range-diff vs v1:
 1:  4ae11694ac6 ! 1:  1982b720a04 range-diff: show submodule changes irrespective of diff.submodule
     @@ Commit message
      
          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 shown.
     +    changes are always detected.
      
     -    Note that the test must use '--creation-factor=100' to force the second
     -    commit in the range not to be considered a complete rewrite.
     +    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>
      
     @@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '
      +	git checkout -b main-sub topic &&
      +	git submodule add ./sub-repo sub &&
      +	git -C sub checkout --detach sub-first &&
     -+	git add sub &&
     -+	git commit -m "add sub" &&
     ++	git commit -m "add sub" sub &&
      +	sup_oid1=$(git rev-parse --short HEAD) &&
      +	git checkout -b topic-sub &&
      +	git -C sub checkout sub-second &&
     -+	git add sub &&
     -+	git commit -m "change sub" &&
     ++	git commit -m "change sub" sub &&
      +	sup_oid2=$(git rev-parse --short HEAD) &&
      +	git checkout -b modified-sub main-sub &&
      +	git -C sub checkout sub-third &&
     -+	git add sub &&
     -+	git commit -m "change sub" &&
     ++	git commit -m "change sub" sub &&
      +	sup_oid3=$(git rev-parse --short HEAD) &&
     ++	sup_oid0=$(test_oid __) &&
      +
      +	test_config diff.submodule log &&
     ++	git range-diff topic topic-sub modified-sub >actual &&
     ++	cat >expect <<-EOF &&
     ++	1:  $sup_oid1 = 1:  $sup_oid1 add sub
     ++	2:  $sup_oid2 < -:  $sup_oid0 change sub
     ++	-:  $sup_oid0 > 2:  $sup_oid3 change sub
     ++	EOF
     ++	test_cmp expect actual &&
     ++	test_config diff.submodule diff &&
     ++	git range-diff topic topic-sub modified-sub >actual &&
      +	git range-diff --creation-factor=100 topic topic-sub modified-sub >actual &&
      +	cat >expect <<-EOF &&
      +	1:  $sup_oid1 = 1:  $sup_oid1 add sub
 range-diff.c          |  2 +-
 t/t3206-range-diff.sh | 51 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/range-diff.c b/range-diff.c
index b72eb9fdbee..068bf214544 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -44,7 +44,7 @@ static int read_patches(const char *range, struct string_list *list,
 
 	strvec_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
 		     "--reverse", "--date-order", "--decorate=no",
-		     "--no-prefix",
+		     "--no-prefix", "--submodule=short",
 		     /*
 		      * Choose indicators that are not used anywhere
 		      * else in diffs, but still look reasonable
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index e30bc48a290..d12e4e4cc6c 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -772,4 +772,55 @@ test_expect_success '--left-only/--right-only' '
 	test_cmp expect actual
 '
 
+test_expect_success 'submodule changes are shown irrespective of diff.submodule' '
+	git init sub-repo &&
+	test_commit -C sub-repo sub-first &&
+	sub_oid1=$(git -C sub-repo rev-parse HEAD) &&
+	test_commit -C sub-repo sub-second &&
+	sub_oid2=$(git -C sub-repo rev-parse HEAD) &&
+	test_commit -C sub-repo sub-third &&
+	sub_oid3=$(git -C sub-repo rev-parse HEAD) &&
+
+	git checkout -b main-sub topic &&
+	git submodule add ./sub-repo sub &&
+	git -C sub checkout --detach sub-first &&
+	git commit -m "add sub" sub &&
+	sup_oid1=$(git rev-parse --short HEAD) &&
+	git checkout -b topic-sub &&
+	git -C sub checkout sub-second &&
+	git commit -m "change sub" sub &&
+	sup_oid2=$(git rev-parse --short HEAD) &&
+	git checkout -b modified-sub main-sub &&
+	git -C sub checkout sub-third &&
+	git commit -m "change sub" sub &&
+	sup_oid3=$(git rev-parse --short HEAD) &&
+	sup_oid0=$(test_oid __) &&
+
+	test_config diff.submodule log &&
+	git range-diff topic topic-sub modified-sub >actual &&
+	cat >expect <<-EOF &&
+	1:  $sup_oid1 = 1:  $sup_oid1 add sub
+	2:  $sup_oid2 < -:  $sup_oid0 change sub
+	-:  $sup_oid0 > 2:  $sup_oid3 change sub
+	EOF
+	test_cmp expect actual &&
+	test_config diff.submodule diff &&
+	git range-diff topic topic-sub modified-sub >actual &&
+	git range-diff --creation-factor=100 topic topic-sub modified-sub >actual &&
+	cat >expect <<-EOF &&
+	1:  $sup_oid1 = 1:  $sup_oid1 add sub
+	2:  $sup_oid2 ! 2:  $sup_oid3 change sub
+	    @@ Commit message
+	      ## sub ##
+	     @@
+	     -Subproject commit $sub_oid1
+	    -+Subproject commit $sub_oid2
+	    ++Subproject commit $sub_oid3
+	EOF
+	test_cmp expect actual &&
+	test_config diff.submodule diff &&
+	git range-diff --creation-factor=100 topic topic-sub modified-sub >actual &&
+	test_cmp expect actual
+'
+
 test_done

base-commit: 7a3eb286977746bc09a5de7682df0e5a7085e17c
-- 
gitgitgadget
Previous: Philippe Blain
Message 9 of 9 in “range-diff: show submodule changes irrespective of diff.submodule”
  1. range-diff: show submodule changes irrespective of diff.submodulePhilippe Blain via GitGitGadget, May 30, 2022
  2. Ævar Arnfjörð BjarmasonMay 30, 2022
  3. Philippe BlainMay 31, 2022
  4. Johannes SchindelinJun 2, 2022
  5. Junio C HamanoJun 2, 2022
  6. Philippe BlainJun 6, 2022
  7. Junio C HamanoJun 7, 2022
  8. Philippe BlainJun 6, 2022
  9. range-diff: show submodule changes irrespective of diff.submodulePhilippe Blain via GitGitGadget, Jun 6, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.