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

Re: [PATCH v2 10/15] diff: stop allowing diff to have submodules configured in .git/config

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 3, 2017, 20:40 UTC
Message-ID
<xmqqlgn0wfjt.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170803182000.179328-11-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
Show 8 quoted lines
> Traditionally a submodule is comprised of a gitlink as well as a
> corresponding entry in the .gitmodules file.  Diff doesn't follow this
> paradigm as its config callback routine falls back to populating the
> submodule-config if a config entry starts with 'submodule.'.
>
> Remove this behavior in order to be consistent with how the
> submodule-config is populated, via calling 'gitmodules_config()' or
> 'repo_read_gitmodules()'.

I am all for dropping special cases deep in the diff machinery, even though there may be submodule users who care about submodule.*.ignore

Does this change mean we can eventually get rid of the ugly DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG hack and also need for a patch like 03/15?

Show 113 quoted lines
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  diff.c                    |  3 ---
>  t/t4027-diff-submodule.sh | 67 -----------------------------------------------
>  2 files changed, 70 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 85e714f6c..e43519b88 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -346,9 +346,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  
> -	if (starts_with(var, "submodule."))
> -		return parse_submodule_config_option(var, value);
> -
>  	if (git_diff_heuristic_config(var, value, cb) < 0)
>  		return -1;
>  
> diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
> index 518bf9524..2ffd11a14 100755
> --- a/t/t4027-diff-submodule.sh
> +++ b/t/t4027-diff-submodule.sh
> @@ -113,35 +113,6 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
>  	! test -s actual4
>  '
>  
> -test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.git/config]' '
> -	git config diff.ignoreSubmodules all &&
> -	git diff HEAD >actual &&
> -	! test -s actual &&
> -	git config submodule.subname.ignore none &&
> -	git config submodule.subname.path sub &&
> -	git diff HEAD >actual &&
> -	sed -e "1,/^@@/d" actual >actual.body &&
> -	expect_from_to >expect.body $subprev $subprev-dirty &&
> -	test_cmp expect.body actual.body &&
> -	git config submodule.subname.ignore all &&
> -	git diff HEAD >actual2 &&
> -	! test -s actual2 &&
> -	git config submodule.subname.ignore untracked &&
> -	git diff HEAD >actual3 &&
> -	sed -e "1,/^@@/d" actual3 >actual3.body &&
> -	expect_from_to >expect.body $subprev $subprev-dirty &&
> -	test_cmp expect.body actual3.body &&
> -	git config submodule.subname.ignore dirty &&
> -	git diff HEAD >actual4 &&
> -	! test -s actual4 &&
> -	git diff HEAD --ignore-submodules=none >actual &&
> -	sed -e "1,/^@@/d" actual >actual.body &&
> -	expect_from_to >expect.body $subprev $subprev-dirty &&
> -	test_cmp expect.body actual.body &&
> -	git config --remove-section submodule.subname &&
> -	git config --unset diff.ignoreSubmodules
> -'
> -
>  test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '
>  	git config diff.ignoreSubmodules dirty &&
>  	git diff HEAD >actual &&
> @@ -208,24 +179,6 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
>  	! test -s actual4
>  '
>  
> -test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.git/config]' '
> -	git config submodule.subname.ignore all &&
> -	git config submodule.subname.path sub &&
> -	git diff HEAD >actual2 &&
> -	! test -s actual2 &&
> -	git config submodule.subname.ignore untracked &&
> -	git diff HEAD >actual3 &&
> -	! test -s actual3 &&
> -	git config submodule.subname.ignore dirty &&
> -	git diff HEAD >actual4 &&
> -	! test -s actual4 &&
> -	git diff --ignore-submodules=none HEAD >actual &&
> -	sed -e "1,/^@@/d" actual >actual.body &&
> -	expect_from_to >expect.body $subprev $subprev-dirty &&
> -	test_cmp expect.body actual.body &&
> -	git config --remove-section submodule.subname
> -'
> -
>  test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.gitmodules]' '
>  	git config --add -f .gitmodules submodule.subname.ignore all &&
>  	git config --add -f .gitmodules submodule.subname.path sub &&
> @@ -261,26 +214,6 @@ test_expect_success 'git diff between submodule commits' '
>  	! test -s actual
>  '
>  
> -test_expect_success 'git diff between submodule commits [.git/config]' '
> -	git diff HEAD^..HEAD >actual &&
> -	sed -e "1,/^@@/d" actual >actual.body &&
> -	expect_from_to >expect.body $subtip $subprev &&
> -	test_cmp expect.body actual.body &&
> -	git config submodule.subname.ignore dirty &&
> -	git config submodule.subname.path sub &&
> -	git diff HEAD^..HEAD >actual &&
> -	sed -e "1,/^@@/d" actual >actual.body &&
> -	expect_from_to >expect.body $subtip $subprev &&
> -	test_cmp expect.body actual.body &&
> -	git config submodule.subname.ignore all &&
> -	git diff HEAD^..HEAD >actual &&
> -	! test -s actual &&
> -	git diff --ignore-submodules=dirty HEAD^..HEAD >actual &&
> -	sed -e "1,/^@@/d" actual >actual.body &&
> -	expect_from_to >expect.body $subtip $subprev &&
> -	git config --remove-section submodule.subname
> -'
> -
>  test_expect_success 'git diff between submodule commits [.gitmodules]' '
>  	git diff HEAD^..HEAD >actual &&
>  	sed -e "1,/^@@/d" actual >actual.body &&
Previous: Brandon WilliamsNext: Brandon Williams
Message 50 of 61 in “submodule-config cleanup”
  1. 00/15 submodule-config cleanupBrandon Williams, Jul 25, 2017
  2. 02/15 submodule: don't use submodule_from_nameBrandon Williams, Jul 25, 2017
  3. Stefan BellerJul 25, 2017
  4. Junio C HamanoJul 26, 2017
  5. Jens LehmannJul 30, 2017
  6. Junio C HamanoJul 30, 2017
  7. Stefan BellerJul 31, 2017
  8. Heiko VoigtAug 11, 2017
  9. 04/15 submodule--helper: don't overlay config in remote_submodule_branchBrandon Williams, Jul 25, 2017
  10. Stefan BellerJul 25, 2017
  11. 05/15 submodule--helper: don't overlay config in update-cloneBrandon Williams, Jul 25, 2017
  12. Stefan BellerJul 25, 2017
  13. Brandon WilliamsJul 25, 2017
  14. 08/15 unpack-trees: don't rely on overlayed configBrandon Williams, Jul 25, 2017
  15. 09/15 submodule: remove submodule_config callback routineBrandon Williams, Jul 25, 2017
  16. Junio C HamanoJul 26, 2017
  17. 12/15 submodule-config: move submodule-config functions to submodule-config.cBrandon Williams, Jul 25, 2017
  18. 15/15 submodule: remove gitmodules_configBrandon Williams, Jul 25, 2017
  19. 14/15 unpack-trees: improve loading of .gitmodulesBrandon Williams, Jul 25, 2017
  20. 13/15 submodule-config: lazy-load a repository's .gitmodules fileBrandon Williams, Jul 25, 2017
  21. 11/15 submodule-config: remove support for overlaying repository configBrandon Williams, Jul 25, 2017
  22. 10/15 diff: stop allowing diff to have submodules configured in .git/configBrandon Williams, Jul 25, 2017
  23. 06/15 fetch: don't overlay config with submodule-configBrandon Williams, Jul 25, 2017
  24. Stefan BellerJul 25, 2017
  25. Brandon WilliamsJul 25, 2017
  26. 07/15 submodule: don't rely on overlayed config when setting diffoptsBrandon Williams, Jul 25, 2017
  27. Stefan BellerJul 25, 2017
  28. 01/15 t7411: check configuration parsing errorsBrandon Williams, Jul 25, 2017
  29. Junio C HamanoJul 26, 2017
  30. 03/15 add, reset: ensure submodules can be added or resetBrandon Williams, Jul 25, 2017
  31. Stefan BellerJul 25, 2017
  32. Brandon WilliamsJul 25, 2017
  33. Junio C HamanoJul 26, 2017
  34. Brandon WilliamsJul 31, 2017
  35. 00/15 submodule-config cleanupBrandon Williams, Aug 3, 2017
  36. 03/15 add, reset: ensure submodules can be added or resetBrandon Williams, Aug 3, 2017
  37. 05/15 submodule--helper: don't overlay config in update-cloneBrandon Williams, Aug 3, 2017
  38. 08/15 unpack-trees: don't respect submodule.updateBrandon Williams, Aug 3, 2017
  39. Stefan BellerAug 3, 2017
  40. Junio C HamanoAug 3, 2017
  41. Stefan BellerAug 3, 2017
  42. 07/15 submodule: don't rely on overlayed config when setting diffoptsBrandon Williams, Aug 3, 2017
  43. 09/15 submodule: remove submodule_config callback routineBrandon Williams, Aug 3, 2017
  44. 12/15 submodule-config: move submodule-config functions to submodule-config.cBrandon Williams, Aug 3, 2017
  45. 14/15 unpack-trees: improve loading of .gitmodulesBrandon Williams, Aug 3, 2017
  46. Heiko VoigtAug 11, 2017
  47. 15/15 submodule: remove gitmodules_configBrandon Williams, Aug 3, 2017
  48. 13/15 submodule-config: lazy-load a repository's .gitmodules fileBrandon Williams, Aug 3, 2017
  49. 10/15 diff: stop allowing diff to have submodules configured in .git/configBrandon Williams, Aug 3, 2017
  50. Junio C HamanoAug 3, 2017
  51. Brandon WilliamsAug 4, 2017
  52. 11/15 submodule-config: remove support for overlaying repository configBrandon Williams, Aug 3, 2017
  53. 06/15 fetch: don't overlay config with submodule-configBrandon Williams, Aug 3, 2017
  54. 04/15 submodule--helper: don't overlay config in remote_submodule_branchBrandon Williams, Aug 3, 2017
  55. 02/15 submodule: don't use submodule_from_nameBrandon Williams, Aug 3, 2017
  56. Stefan BellerAug 3, 2017
  57. Brandon WilliamsAug 4, 2017
  58. Heiko VoigtAug 11, 2017
  59. Junio C HamanoAug 3, 2017
  60. 01/15 t7411: check configuration parsing errorsBrandon Williams, Aug 3, 2017
  61. Junio C HamanoAug 3, 2017

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.