{"thread":{"id":"59865","subject":"[PATCH v3] diff-lib: honor override_submodule_config flag bit","startedAt":"2023-06-14T16:32:22Z","lastAt":"2023-06-14T16:46:25Z","messageCount":2,"participants":["Josip Sokcevic","Eric Sunshine"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"478380","messageId":"20230614163127.3182986-2-sokcevic@google.com","threadId":"59865","inReplyTo":null,"subject":"[PATCH v3] diff-lib: honor override_submodule_config flag bit","fromName":"Josip Sokcevic","fromEmail":"sokcevic@google.com","sentAt":"2023-06-14T16:31:28Z","receivedAt":"2023-06-14T16:32:22Z","isPatch":true,"sender":{"key":"sokcevic@google.com","avatar":"https://avatars.githubusercontent.com/u/89933621?v=4"},"body":"When `diff.ignoreSubmodules = all` is set and submodule commits are\nmanually staged (e.g. via `git-update-index`), `git-commit` should\nrecord the commit  with updated submodules.\n\n`index_differs_from` is called from `prepare_to_commit` with flags set to\n`override_submodule_config = 1`. `index_differs_from` then merges the\ndefault diff flags and passed flags.\n\nWhen `diff.ignoreSubmodules` is set to \"all\", `flags` ends up having\nboth `override_submodule_config` and `ignore_submodules` set to 1. This\nresults in `git-commit` ignoring staged commits.\n\nThis patch restores original `flags.ignore_submodule` if\n`flags.override_submodule_config` is set.\n\nSigned-off-by: Josip Sokcevic <sokcevic@google.com>\n---\n diff-lib.c                  |  9 ++++++++-\n t/t7406-submodule-update.sh | 23 +++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 60e979dc1b..1918517ebd 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -669,8 +669,15 @@ int index_differs_from(struct repository *r,\n \tsetup_revisions(0, NULL, &rev, &opt);\n \trev.diffopt.flags.quick = 1;\n \trev.diffopt.flags.exit_with_status = 1;\n-\tif (flags)\n+\tif (flags) {\n \t\tdiff_flags_or(&rev.diffopt.flags, flags);\n+\t\t/*\n+\t\t * Now that flags are merged, honor override_submodule_config\n+\t\t * and ignore_submodules from passed flags.\n+\t\t */\n+\t\tif (flags->override_submodule_config)\n+\t\t\trev.diffopt.flags.ignore_submodules = flags->ignore_submodules;\n+\t}\n \trev.diffopt.ita_invisible_in_index = ita_invisible_in_index;\n \trun_diff_index(&rev, 1);\n \thas_changes = rev.diffopt.flags.has_changes;\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex f094e3d7f3..3c85ac2fbf 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -1179,4 +1179,27 @@ test_expect_success 'submodule update --recursive skip submodules with strategy=\n \ttest_cmp expect.err actual.err\n '\n \n+add_submodule_commit_and_validate () {\n+\tHASH=$(git rev-parse HEAD) &&\n+\tgit update-index --add --cacheinfo 160000,$HASH,sub &&\n+\tgit commit -m \"create submodule\" &&\n+\techo \"160000 commit $HASH\tsub\" > expect &&\n+\tgit ls-tree HEAD -- sub >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'commit with staged submodule change' '\n+\tadd_submodule_commit_and_validate\n+'\n+\n+test_expect_success 'commit with staged submodule change with ignoreSubmodules dirty' '\n+\ttest_config diff.ignoreSubmodules dirty &&\n+\tadd_submodule_commit_and_validate\n+'\n+\n+test_expect_success 'commit with staged submodule change with ignoreSubmodules all' '\n+\ttest_config diff.ignoreSubmodules all &&\n+\tadd_submodule_commit_and_validate\n+'\n+\n test_done\n-- \n2.41.0.162.gfafddb0af9-goog\n\n"},{"id":"478381","messageId":"CAPig+cR9Zv87ZmKaYKX2Acu=0vun+HonyXKOWd8juMPfE7by2Q@mail.gmail.com","threadId":"59865","inReplyTo":"20230614163127.3182986-2-sokcevic@google.com","subject":"Re: [PATCH v3] diff-lib: honor override_submodule_config flag bit","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-14T16:46:06Z","receivedAt":"2023-06-14T16:46:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 14, 2023 at 12:37 PM Josip Sokcevic <sokcevic@google.com> wrote:\n> When `diff.ignoreSubmodules = all` is set and submodule commits are\n> manually staged (e.g. via `git-update-index`), `git-commit` should\n> record the commit  with updated submodules.\n>\n> `index_differs_from` is called from `prepare_to_commit` with flags set to\n> `override_submodule_config = 1`. `index_differs_from` then merges the\n> default diff flags and passed flags.\n>\n> When `diff.ignoreSubmodules` is set to \"all\", `flags` ends up having\n> both `override_submodule_config` and `ignore_submodules` set to 1. This\n> results in `git-commit` ignoring staged commits.\n>\n> This patch restores original `flags.ignore_submodule` if\n> `flags.override_submodule_config` is set.\n>\n> Signed-off-by: Josip Sokcevic <sokcevic@google.com>\n\nLooking much better. Just a minor (nitpicky) style comment...\n\n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> @@ -1179,4 +1179,27 @@ test_expect_success 'submodule update --recursive skip submodules with strategy=\n> +add_submodule_commit_and_validate () {\n> +       HASH=$(git rev-parse HEAD) &&\n> +       git update-index --add --cacheinfo 160000,$HASH,sub &&\n> +       git commit -m \"create submodule\" &&\n> +       echo \"160000 commit $HASH       sub\" > expect &&\n\nDrop the space after the redirection operator:\n\n    echo \"160000 commit $HASH       sub\" >expect &&\n\n> +       git ls-tree HEAD -- sub >actual &&\n\nThis one correctly omits whitespace after the redirection operator.\n\n> +       test_cmp expect actual\n> +}\n"}]}