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
BWBrandon Williams <bmwill@google.com>
Date
Aug 4, 2017, 21:59 UTC
Message-ID
<20170804215956.GC126093@google.com>
In-Reply-To
<xmqqlgn0wfjt.fsf@gitster.mtv.corp.google.com>
On 08/03, Junio C Hamano wrote:
Show 17 quoted lines
> Brandon Williams <bmwill@google.com> writes:
> 
> > 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?

I think that this is a step toward getting rid of that. We can either do two things: 1) deprecate submodule.*.ignore and don't respect it anymore or 2) flip the polarity of that flag so that by default we don't respect the submodule.*.ignore config and instead callers must opt in instead of the current opt out behavior.

Show 114 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 &&
-- 
Brandon Williams
Previous: Junio C HamanoNext: Brandon Williams
Message 51 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.