{"thread":{"id":"45314","subject":"diff.ignoreSubmoudles config setting broken?","startedAt":"2017-03-08T12:56:13Z","lastAt":"2017-03-09T16:15:55Z","messageCount":12,"participants":["Sebastian Schuberth","Jeff King","Stefan Beller","Junio C Hamano","Jacob Keller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"313492","messageId":"5e5b1b92-f7c6-2987-356e-1aab2bff557e@gmail.com","threadId":"45314","inReplyTo":null,"subject":"diff.ignoreSubmoudles config setting broken?","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2017-03-08T12:54:02Z","receivedAt":"2017-03-08T12:56:13Z","isPatch":false,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"Hi,\n\nwith\n\n$ git --version\ngit version 2.12.0.windows.1\n\nI'm getting\n\n$ git config --global diff.ignoreSubmodules all\n$ git diff\ndiff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit\nindex 65e5c9c..6b021a8 160000\n--- a/scanners/scancode-toolkit\n+++ b/scanners/scancode-toolkit\n@@ -1 +1 @@\n-Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30\n+Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec\n\nbut with\n\n$ git diff --ignore-submodules=all\n\nI'm getting the expected empty output.\n\nI can reproduce the same on Linux with \"git version 2.11.0\". Am I missing something, or is this a bug?\n\nRegards,\nSebastian\n\n"},{"id":"313494","messageId":"20170308140110.wgdedquqwm75zws2@sigill.intra.peff.net","threadId":"45314","inReplyTo":"CAHGBnuM3iM-kHdxdox_1i56uLbv7gQ5ZUY9Xqf4BG7G_kTf+jQ@mail.gmail.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-08T14:01:11Z","receivedAt":"2017-03-08T14:01:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 08, 2017 at 02:43:00PM +0100, Sebastian Schuberth wrote:\n\n> > Hrm. Isn't \"all\" the default? That's what git-diff(1) says (but I've\n> > never used the feature myself).\n> >\n> > That would imply to me that there's another config option set somewhere\n> > (perhaps in the repo-level config). What does:\n> >\n> > git config --show-origin --get-all diff.ignoresubmodules\n> >\n> > say?\n> \n> It says:\n> \n> file:/home/seschube/.gitconfig all\n\nOK, that looks right, so my guess is probably the wrong direction.\nPeeking at the code, it looks like there may be some per-submodule\nmagic, but I don't know how it all works. So I'll stop looking and wait\nfor somebody more clueful to respond.\n\n-Peff\n"},{"id":"313495","messageId":"20170308133348.2ovfsi44vq2rpgom@sigill.intra.peff.net","threadId":"45314","inReplyTo":"5e5b1b92-f7c6-2987-356e-1aab2bff557e@gmail.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-08T13:33:48Z","receivedAt":"2017-03-08T14:01:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 08, 2017 at 01:54:02PM +0100, Sebastian Schuberth wrote:\n\n> I'm getting\n> \n> $ git config --global diff.ignoreSubmodules all\n> $ git diff\n> diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit\n> index 65e5c9c..6b021a8 160000\n> --- a/scanners/scancode-toolkit\n> +++ b/scanners/scancode-toolkit\n> @@ -1 +1 @@\n> -Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30\n> +Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec\n>\n> but with\n> \n> $ git diff --ignore-submodules=all\n\nHrm. Isn't \"all\" the default? That's what git-diff(1) says (but I've\nnever used the feature myself).\n\nThat would imply to me that there's another config option set somewhere\n(perhaps in the repo-level config). What does:\n\n git config --show-origin --get-all diff.ignoresubmodules\n\nsay?\n\n-Peff\n"},{"id":"313497","messageId":"CAHGBnuPGPcWwbrZX_92XDJu47bpH=kj2PZ7yWHK=MRfZ_RHXrQ@mail.gmail.com","threadId":"45314","inReplyTo":"20170308140110.wgdedquqwm75zws2@sigill.intra.peff.net","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2017-03-08T15:07:31Z","receivedAt":"2017-03-08T15:07:43Z","isPatch":false,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Wed, Mar 8, 2017 at 3:01 PM, Jeff King wrote:\n\n>> > Hrm. Isn't \"all\" the default? That's what git-diff(1) says (but I've\n>> > never used the feature myself).\n>> >\n>> > That would imply to me that there's another config option set somewhere\n>> > (perhaps in the repo-level config). What does:\n>> >\n>> > git config --show-origin --get-all diff.ignoresubmodules\n>> >\n>> > say?\n>>\n>> It says:\n>>\n>> file:/home/seschube/.gitconfig all\n>\n> OK, that looks right, so my guess is probably the wrong direction.\n> Peeking at the code, it looks like there may be some per-submodule\n> magic, but I don't know how it all works. So I'll stop looking and wait\n> for somebody more clueful to respond.\n\n+ Jens\n\n-- \nSebastian Schuberth\n"},{"id":"313506","messageId":"CAHGBnuM3iM-kHdxdox_1i56uLbv7gQ5ZUY9Xqf4BG7G_kTf+jQ@mail.gmail.com","threadId":"45314","inReplyTo":"20170308133348.2ovfsi44vq2rpgom@sigill.intra.peff.net","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2017-03-08T13:43:00Z","receivedAt":"2017-03-08T15:45:12Z","isPatch":false,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Wed, Mar 8, 2017 at 2:33 PM, Jeff King wrote:\n\n>> I'm getting\n>>\n>> $ git config --global diff.ignoreSubmodules all\n>> $ git diff\n>> diff --git a/scanners/scancode-toolkit b/scanners/scancode-toolkit\n>> index 65e5c9c..6b021a8 160000\n>> --- a/scanners/scancode-toolkit\n>> +++ b/scanners/scancode-toolkit\n>> @@ -1 +1 @@\n>> -Subproject commit 65e5c9c9508441c5f62beff4749cf455c6eadc30\n>> +Subproject commit 6b021a8addf6d3c5f2a6ef1af6245e095c21d8ec\n>>\n>> but with\n>>\n>> $ git diff --ignore-submodules=all\n>\n> Hrm. Isn't \"all\" the default? That's what git-diff(1) says (but I've\n> never used the feature myself).\n>\n> That would imply to me that there's another config option set somewhere\n> (perhaps in the repo-level config). What does:\n>\n> git config --show-origin --get-all diff.ignoresubmodules\n>\n> say?\n\nIt says:\n\nfile:/home/seschube/.gitconfig all\n\n-- \nSebastian Schuberth\n"},{"id":"313536","messageId":"CAGZ79kbwMhL-ZnL-iYwPH=tWa8cNQbEGOYYQBw6OzFCMhOWE-w@mail.gmail.com","threadId":"45314","inReplyTo":"CAHGBnuPGPcWwbrZX_92XDJu47bpH=kj2PZ7yWHK=MRfZ_RHXrQ@mail.gmail.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-08T19:04:07Z","receivedAt":"2017-03-08T19:04:54Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 8, 2017 at 7:07 AM, Sebastian Schuberth\n wrote:\n>\n> + Jens\n>\n\n+ Jacob Keller, who touched submodule diff display code last.\n(I am thinking of fd47ae6a, diff: teach diff to display submodule\ndifference with an inline diff, 2016-08-31), which is first release as\npart of v2.11.0 (that would fit your observance)\n"},{"id":"313561","messageId":"xmqq8tof32x9.fsf@gitster.mtv.corp.google.com","threadId":"45314","inReplyTo":"CAGZ79kbwMhL-ZnL-iYwPH=tWa8cNQbEGOYYQBw6OzFCMhOWE-w@mail.gmail.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-08T20:54:10Z","receivedAt":"2017-03-08T22:08:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Wed, Mar 8, 2017 at 7:07 AM, Sebastian Schuberth\n> wrote:\n>>\n>> + Jens\n>>\n>\n> + Jacob Keller, who touched submodule diff display code last.\n> (I am thinking of fd47ae6a, diff: teach diff to display submodule\n> difference with an inline diff, 2016-08-31), which is first release as\n> part of v2.11.0 (that would fit your observance)\n\nBetween these two:\n\n\tgit -c diff.ignoresubmodules=all diff\n\tgit diff --ignore-submodules=all\n\none difference is that the latter disables reading extra config from\n.gitmodules (!) while the former makes the command honor it.\n\nThis comes from aee9c7d6 (\"Submodules: Add the new \"ignore\" config\noption for diff and status\", 2010-08-06), which is ancient and\npredates even v2.0, so if you see problems with v2.12 or v2.11 but\nnot with older ones, that would rule out this theory.\n"},{"id":"313565","messageId":"CA+P7+xp1nbXGAq_KfLpMeMbQEXMuA7oVSTPzY+ZhPKv4_9K2DQ@mail.gmail.com","threadId":"45314","inReplyTo":"CAGZ79kbwMhL-ZnL-iYwPH=tWa8cNQbEGOYYQBw6OzFCMhOWE-w@mail.gmail.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-03-08T22:27:51Z","receivedAt":"2017-03-08T22:28:30Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Mar 8, 2017 at 11:04 AM, Stefan Beller wrote:\n> On Wed, Mar 8, 2017 at 7:07 AM, Sebastian Schuberth\n> wrote:\n>>\n>> + Jens\n>>\n>\n> + Jacob Keller, who touched submodule diff display code last.\n> (I am thinking of fd47ae6a, diff: teach diff to display submodule\n> difference with an inline diff, 2016-08-31), which is first release as\n> part of v2.11.0 (that would fit your observance)\n\nI don't remember doing anything with ignoreSubmodules, but it's\npossible I broke it...\n\n-Jake\n"},{"id":"313569","messageId":"CAGZ79kZFGP0zMP5CtOH3poU9vx8FoT25UVr8ridQo0_VdH2cmA@mail.gmail.com","threadId":"45314","inReplyTo":"xmqq8tof32x9.fsf@gitster.mtv.corp.google.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-08T22:11:05Z","receivedAt":"2017-03-08T22:45:52Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 8, 2017 at 12:54 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> On Wed, Mar 8, 2017 at 7:07 AM, Sebastian Schuberth\n>> wrote:\n>>>\n>>> + Jens\n>>>\n>>\n>> + Jacob Keller, who touched submodule diff display code last.\n>> (I am thinking of fd47ae6a, diff: teach diff to display submodule\n>> difference with an inline diff, 2016-08-31), which is first release as\n>> part of v2.11.0 (that would fit your observance)\n>\n> Between these two:\n>\n> git -c diff.ignoresubmodules=all diff\n> git diff --ignore-submodules=all\n>\n> one difference is that the latter disables reading extra config from\n> .gitmodules (!) while the former makes the command honor it.\n>\n\nYeah the .gitmodules file is a good hint.\n\nHere is my understanding of the precedence:\n\n command line options > .git/config (in various forms) > .gitmodules\n\nwhere in the .git config we have precedence levels for different files\n\n .git/config > ~/.gitconfig\n\nas well as different settings:\n\n submodule..ignore > diff.ignoreSubmodules\n\nIt is not clear to me how a specific setting in .gitmodules\nwould interact with a submodule-global setting in the config,\ne.g.\n\n git config -f .gitmodules submodule. \\\n \"$(git submodule--helper name scanners/scancode-toolkit)\". \\\n ignore none\n git config diff.ignoreSubmodules all\n git diff scanners/scancode-toolkit\n\nFrom reading the code, I assume \"diff.ignoreSubmodules all\"\ntakes precedence here nevertheless because the diff.ignoreSubmodules\nsetting is treated on a higher level than the submodule specific setting,\ndespite the submodule specific setting being more specific.\n\nThis is a bad example, because it may be intuitive that the\nvalue in the .git/config file takes precedence over .gitmodules,\nbut we cannot set diff.ignoreSubmodules in .gitmodules.\n\nStefan\n"},{"id":"313572","messageId":"xmqqa88v1i5f.fsf@gitster.mtv.corp.google.com","threadId":"45314","inReplyTo":"CAGZ79kZFGP0zMP5CtOH3poU9vx8FoT25UVr8ridQo0_VdH2cmA@mail.gmail.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-08T23:08:12Z","receivedAt":"2017-03-08T23:23:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Yeah the .gitmodules file is a good hint.\n>\n> Here is my understanding of the precedence:\n>\n> command line options > .git/config (in various forms) > .gitmodules\n>\n> where in the .git config we have precedence levels for different files\n>\n> .git/config > ~/.gitconfig\n>\n> as well as different settings:\n>\n> submodule..ignore > diff.ignoreSubmodules\n\nI've never understood why people thought it a good idea to let\n.gitmodules supplied by the upstream override the configuration\nsetting the end user has like this. This is quite bad.\n\nPerhaps this is a good starting point?\n\n diff.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex a628ac3a95..75b7140c63 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -273,8 +273,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \tif (!strcmp(var, \"diff.orderfile\"))\n \t\treturn git_config_pathname(&diff_order_file_cfg, var, value);\n \n-\tif (!strcmp(var, \"diff.ignoresubmodules\"))\n+\tif (!strcmp(var, \"diff.ignoresubmodules\")) {\n \t\thandle_ignore_submodules_arg(&default_diff_options, value);\n+\t\tDIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);\n+\t\treturn 0;\n+\t}\n \n \tif (!strcmp(var, \"diff.submodule\")) {\n \t\tif (parse_submodule_params(&default_diff_options, value))\n"},{"id":"313589","messageId":"CAGZ79kZwiwhP-skEd6K+osXkX47fF-eN=psP88DBzjca7n85DQ@mail.gmail.com","threadId":"45314","inReplyTo":"xmqqa88v1i5f.fsf@gitster.mtv.corp.google.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-09T01:30:28Z","receivedAt":"2017-03-09T01:30:38Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 8, 2017 at 3:08 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Yeah the .gitmodules file is a good hint.\n\nAnd by that I meant that I am not sure if we're going\ndown the right rabbit hole here. So before we take action\nmaybe Sebastian can tell us more about his project (and all\nconfigurations and settings involved)\n\n>>\n>> Here is my understanding of the precedence:\n>>\n>> command line options > .git/config (in various forms) > .gitmodules\n>>\n>> where in the .git config we have precedence levels for different files\n>>\n>> .git/config > ~/.gitconfig\n>>\n>> as well as different settings:\n>>\n>> submodule..ignore > diff.ignoreSubmodules\n>\n> I've never understood why people thought it a good idea to let\n> .gitmodules supplied by the upstream override the configuration\n> setting the end user has like this. This is quite bad.\n\nApart from from the name <-> path mapping, the .gitmodules\nfile is a collection of suggestions, some more severe than\nothers.\n\nI think the issue here is to define the correct\nand clear order of precedence, specifically between along these\n2 different dimensions (different configuration settings vs different\nfiles with configuration), such that the .gitmodules file is only ever\nconsulted when the user has obviously nothing configured that\nwould contradict the .gitmodules file.\n\n>\n> Perhaps this is a good starting point?\n>\n> diff.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/diff.c b/diff.c\n> index a628ac3a95..75b7140c63 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -273,8 +273,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n> if (!strcmp(var, \"diff.orderfile\"))\n> return git_config_pathname(&diff_order_file_cfg, var, value);\n>\n> - if (!strcmp(var, \"diff.ignoresubmodules\"))\n> + if (!strcmp(var, \"diff.ignoresubmodules\")) {\n> handle_ignore_submodules_arg(&default_diff_options, value);\n> + DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);\n\ns/options/&default_diff_options/ makes it compile. (I did not think\nwhether that is\ncorrect though.)\n\nIn other occurrences of handle_ignore_submodules_arg, the DIFF_OPT_SET\nis set before the handle_ignore_submodules_arg, though.\n\nWhen trying these suggestions, ./t4027-diff-submodule.sh breaks.\nlog on that file yields e.g. 302ad7a9930 (Submodules: Use \"ignore\" settings\nfrom .gitmodules too for diff and status), which tells us that in 2010 people\nwere not as concerned by this, but the user had to use the exact option to\noverride the upstream default-suggestion.\n\nThanks,\nStefan\n"},{"id":"313650","messageId":"CAHGBnuNv--R3FMpRWsvhFJU5_R2Rji=nCjqQsuXrK0UU30eByw@mail.gmail.com","threadId":"45314","inReplyTo":"xmqq8tof32x9.fsf@gitster.mtv.corp.google.com","subject":"Re: diff.ignoreSubmoudles config setting broken?","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2017-03-09T16:15:47Z","receivedAt":"2017-03-09T16:15:55Z","isPatch":false,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Wed, Mar 8, 2017 at 9:54 PM, Junio C Hamano wrote:\n\n> Between these two:\n>\n> git -c diff.ignoresubmodules=all diff\n> git diff --ignore-submodules=all\n>\n> one difference is that the latter disables reading extra config from\n> .gitmodules (!) while the former makes the command honor it.\n>\n> This comes from aee9c7d6 (\"Submodules: Add the new \"ignore\" config\n> option for diff and status\", 2010-08-06), which is ancient and\n> predates even v2.0, so if you see problems with v2.12 or v2.11 but\n> not with older ones, that would rule out this theory.\n\nThanks for reminding me of possible settings .gitmodules. Indeed I have\n\n[submodule \"scanners/scancode-toolkit\"]\n path = scanners/scancode-toolkit\n url = https://github.com/sschuberth/scancode-toolkit.git\n ignore = untracked\n\nin .gitmodules, which explains why globally setting\n\"diff.ignoreSubmodules\" to \"all\" had no effect.\n\n-- \nSebastian Schuberth\n"}]}