{"thread":{"id":"35390","subject":"Git issues with submodules","startedAt":"2013-11-22T07:53:33Z","lastAt":"2013-12-09T22:25:22Z","messageCount":51,"participants":["Sergey Sharybin","Ramkumar Ramachandra","Jeff King","Jens Lehmann","Heiko Voigt","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"230933","messageId":"CAErtv26Q_YN+U+trjNac1aKLi9BvNHNNuaUkrr2RE0nB+yxWsw@mail.gmail.com","threadId":"35390","inReplyTo":null,"subject":"Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-22T07:53:33Z","receivedAt":"2013-11-22T07:53:33Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Hey everyone from Blender developers!\n\nAs you might already know, we've recently switched from SVN to Git to\nhost Blender sources. In general it works really awesome, but we've\ngot some issues with submodules.\n\nin SVN we had separate repositories for addons and translations which\nwere attached to main tree as svn:external. The reason for this was:\n\n1. Separate commit access between core sources and addons so nobody\naccidentally breaks anything in the core.\n2. Separate commit history to help tracking issues down.\n\nFor the most developers and all artists (yes, we've got loads of\nartists who builds blender on their own) it makes sense to always\ncheckout latest versions of addons and translations when updating\nworking tree.\n\nWe used Git submodules as a replacement for svn:external, with some\ntweaks and specific of update procedure.\n\nNamely, we always do `git submodule update --remote` to pull all the\nlatest changes from submodules. This will mark checkout as modified\nbecause submodule hash changes. To avoid infinite commits of submodule\nhash we've added ignore=all to their configuration.\n\nIn most cases it works fine, but there're some circumstances when it\ngives weirdo issues.\n\nNamely, `git ls-files -m` will show addons as modified, regardless\nignore=all configuration. In the same time `git diff-index --name-only\nHEAD --` will show no changes at all.\n\nThis leads to issues with Arcanist (which is a Phabricator's tool) who\nconsiders addons as uncommited changes and either complains on this or\njust adds this to commits.\n\nThis issue i might easily reproduce on my laptop with latest Git\n1.8.4.3. There're also some more issues which happens to our\ndevelopers and which i can not quite reproduce.\n\nSometimes it happens so git checkout to another branch yields about\nuncommited changes to addons and doesn't checkout to another branch.\n\nMy guess here is that submodule hash in master and branch was\ndifferent and having hash modified in master somehow prevented changes\nfrom another branch to be checked out. In this case question would be:\nwhat would be the proper way to checkout branches when having\nsubmodules configured this way?\n\nSecond issue is that some developers still manages to commit changes\nto submodule hash, which i have totally no idea why Git allows to\ninclude such a changes. I could not do such a commits on purpose even.\n\nHere're some links to help understanding what's going on:\n\n- Blender repository browser: http://developer.blender.org/diffusion/B/\n- Task in our tracker about issues we've got with Git:\nhttp://developer.blender.org/T37528\n- History of changes to addons hash:\nhttp://developer.blender.org/diffusion/B/history/master/release/scripts/addons\n\nWe're totally new to git submodules and clarification (and maybe even\nconfirmed bug with ls-files -m :) would really be appreciated. We're\nalso open for suggestions about re-configuring our submodules so they\nworks in a way we'd expect this.\n\nThanks in advance!\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"230937","messageId":"CALkWK0n7jdLKOAFoFjuRz0aTCssorAgk2y=Vce76Y5aHWbj53Q@mail.gmail.com","threadId":"35390","inReplyTo":"CAErtv26Q_YN+U+trjNac1aKLi9BvNHNNuaUkrr2RE0nB+yxWsw@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-22T11:16:27Z","receivedAt":"2013-11-22T11:16:27Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"[+CC: Jens, the goto-guy for submodules]\n\nSergey Sharybin wrote:\n> Namely, `git ls-files -m` will show addons as modified, regardless\n> ignore=all configuration. In the same time `git diff-index --name-only\n> HEAD --` will show no changes at all.\n\nThis happens because diff-index handles submodules explicitly (see\ndiff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My\nopinion is that this is a bug, and git ls-files needs to be taught to\nhandle submodules properly.\n\n> This leads to issues with Arcanist (which is a Phabricator's tool) who\n> considers addons as uncommited changes and either complains on this or\n> just adds this to commits.\n\nDoes Arcanist use `git ls-files -m` to check?\n\n> There're also some more issues which happens to our\n> developers and which i can not quite reproduce.\n\nDo try to track down the other issues and let us know.\n\n> Sometimes it happens so git checkout to another branch yields about\n> uncommited changes to addons and doesn't checkout to another branch.\n\nI've seldom used submodules with branches, so I'll let others chime in.\n\nCheers.\n"},{"id":"230939","messageId":"CAErtv27dMepNSbBVdOokn6OF858ENaKooL+FzD7JHtp9nRPufw@mail.gmail.com","threadId":"35390","inReplyTo":"CALkWK0n7jdLKOAFoFjuRz0aTCssorAgk2y=Vce76Y5aHWbj53Q@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-22T11:35:28Z","receivedAt":"2013-11-22T11:35:28Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Hey,\n\nAnswers are inlined.\n\n\nOn Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra\n wrote:\n>\n> [+CC: Jens, the goto-guy for submodules]\n>\n> Sergey Sharybin wrote:\n> > Namely, `git ls-files -m` will show addons as modified, regardless\n> > ignore=all configuration. In the same time `git diff-index --name-only\n> > HEAD --` will show no changes at all.\n>\n> This happens because diff-index handles submodules explicitly (see\n> diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My\n> opinion is that this is a bug, and git ls-files needs to be taught to\n> handle submodules properly.\n\nShall i fire report somewhere or it's being handled by the folks\nreading this ML?\n\n> > This leads to issues with Arcanist (which is a Phabricator's tool) who\n> > considers addons as uncommited changes and either complains on this or\n> > just adds this to commits.\n>\n> Does Arcanist use `git ls-files -m` to check?\n\nYes, Arcanist uses `git ls-files -m` to check whether there're local\nmodifications. We might also contact phab developers asking to change\nit to `git diff --name-only HEAD --`. Is there a preferable way to\nget list of modified files and are this command intended to output the\nsame results?\n\n> > There're also some more issues which happens to our\n> > developers and which i can not quite reproduce.\n>\n> Do try to track down the other issues and let us know.\n\nI'm trying, but doesn't happen here on laptop yet. Will give it\nanother try (do have some ideas). Also directed our developers here\nwho experienced the issue and might give some details,\n\n> > Sometimes it happens so git checkout to another branch yields about\n> > uncommited changes to addons and doesn't checkout to another branch.\n>\n> I've seldom used submodules with branches, so I'll let others chime in.\n\nOk, thanks anyway :)\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"230942","messageId":"CALkWK0nDME-z7G4kcag=ad3qH5FL9FawrYFyVLQB6Z_g+TV+vQ@mail.gmail.com","threadId":"35390","inReplyTo":"CAErtv27dMepNSbBVdOokn6OF858ENaKooL+FzD7JHtp9nRPufw@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-22T13:08:47Z","receivedAt":"2013-11-22T13:08:47Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Sergey Sharybin wrote:\n> On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra\n> wrote:\n>>\n>> [+CC: Jens, the goto-guy for submodules]\n>>\n>> Sergey Sharybin wrote:\n>> > Namely, `git ls-files -m` will show addons as modified, regardless\n>> > ignore=all configuration. In the same time `git diff-index --name-only\n>> > HEAD --` will show no changes at all.\n>>\n>> This happens because diff-index handles submodules explicitly (see\n>> diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My\n>> opinion is that this is a bug, and git ls-files needs to be taught to\n>> handle submodules properly.\n>\n> Shall i fire report somewhere or it's being handled by the folks\n> reading this ML?\n\nBugs are reported and tackled on the list.\n\n>> > This leads to issues with Arcanist (which is a Phabricator's tool) who\n>> > considers addons as uncommited changes and either complains on this or\n>> > just adds this to commits.\n>>\n>> Does Arcanist use `git ls-files -m` to check?\n>\n> Yes, Arcanist uses `git ls-files -m` to check whether there're local\n> modifications. We might also contact phab developers asking to change\n> it to `git diff --name-only HEAD --`. Is there a preferable way to\n> get list of modified files and are this command intended to output the\n> same results?\n\nI just checked it out: it uses `git ls-files -m` to get the list of\nunstaged changes; `git diff --name-only HEAD --` will list staged\nchanges as well.\n"},{"id":"230948","messageId":"20131122151120.GA32361@sigill.intra.peff.net","threadId":"35390","inReplyTo":"CALkWK0nDME-z7G4kcag=ad3qH5FL9FawrYFyVLQB6Z_g+TV+vQ@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-22T15:11:20Z","receivedAt":"2013-11-22T15:11:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 22, 2013 at 06:38:47PM +0530, Ramkumar Ramachandra wrote:\n\n> >> Does Arcanist use `git ls-files -m` to check?\n> >\n> > Yes, Arcanist uses `git ls-files -m` to check whether there're local\n> > modifications. We might also contact phab developers asking to change\n> > it to `git diff --name-only HEAD --`. Is there a preferable way to\n> > get list of modified files and are this command intended to output the\n> > same results?\n> \n> I just checked it out: it uses `git ls-files -m` to get the list of\n> unstaged changes; `git diff --name-only HEAD --` will list staged\n> changes as well.\n\nThat diff command compares the working tree and HEAD; if you are trying\nto match `ls-files -m`, you probably wanted just `git diff --name-only`\nto compare the working tree and the index. Although in a script you'd\nprobably want to use the plumbing `git diff-files` instead.\n\n-Peff\n"},{"id":"230949","messageId":"CAErtv25zrsde7wYg+VUZebow2pmhDnDQG53Dmz_gbjavC-D2cA@mail.gmail.com","threadId":"35390","inReplyTo":"20131122151120.GA32361@sigill.intra.peff.net","subject":"Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-22T15:42:20Z","receivedAt":"2013-11-22T15:42:20Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Ramkumar, not actually sure what you mean?\n\nFor me `git diff --name-only HEAD --` ignores changes to submodules\nhash changes. Also apparently it became a known TODO for phabricator\ndevelopers [1].\n\nJeff, kinda trying to match yes. Just don't want changes to submodules\nhash to be included.\n\nSo, after all is it expected behavior of ls-files or not and if not\nshall i report it as a separate thread? :)\n\n[1] https://secure.phabricator.com/rARCe62b23e67deacc24469525cc5dea2b297a5073fb\n\n\nOn Fri, Nov 22, 2013 at 9:11 PM, Jeff King wrote:\n> On Fri, Nov 22, 2013 at 06:38:47PM +0530, Ramkumar Ramachandra wrote:\n>\n>> >> Does Arcanist use `git ls-files -m` to check?\n>> >\n>> > Yes, Arcanist uses `git ls-files -m` to check whether there're local\n>> > modifications. We might also contact phab developers asking to change\n>> > it to `git diff --name-only HEAD --`. Is there a preferable way to\n>> > get list of modified files and are this command intended to output the\n>> > same results?\n>>\n>> I just checked it out: it uses `git ls-files -m` to get the list of\n>> unstaged changes; `git diff --name-only HEAD --` will list staged\n>> changes as well.\n>\n> That diff command compares the working tree and HEAD; if you are trying\n> to match `ls-files -m`, you probably wanted just `git diff --name-only`\n> to compare the working tree and the index. Although in a script you'd\n> probably want to use the plumbing `git diff-files` instead.\n>\n> -Peff\n\n\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"230950","messageId":"CALkWK0myk5fWcHdwNtjct4Ji4y4=iVT36wZxqOqZS6S7kem+OQ@mail.gmail.com","threadId":"35390","inReplyTo":"20131122151120.GA32361@sigill.intra.peff.net","subject":"Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-22T16:12:48Z","receivedAt":"2013-11-22T16:12:48Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jeff King wrote:\n>> I just checked it out: it uses `git ls-files -m` to get the list of\n>> unstaged changes; `git diff --name-only HEAD --` will list staged\n>> changes as well.\n>\n> That diff command compares the working tree and HEAD; if you are trying\n> to match `ls-files -m`, you probably wanted just `git diff --name-only`\n> to compare the working tree and the index. Although in a script you'd\n> probably want to use the plumbing `git diff-files` instead.\n\nThanks for that. It's probably not worth fixing ls-files; I'll patch\nArcanist to use diff-files instead.\n"},{"id":"230953","messageId":"CALkWK0m9MK=RBBor-ZeGrGU9KA6tZa89UUi0J7j9fxr1g6uJtQ@mail.gmail.com","threadId":"35390","inReplyTo":"CAErtv25zrsde7wYg+VUZebow2pmhDnDQG53Dmz_gbjavC-D2cA@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-22T16:35:29Z","receivedAt":"2013-11-22T16:35:29Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Sergey Sharybin wrote:\n> Ramkumar, not actually sure what you mean?\n>\n> For me `git diff --name-only HEAD --` ignores changes to submodules\n> hash changes.\n\n`git diff --name-only HEAD --` compares the worktree to HEAD (listing\nboth staged and unstaged changes); we want `git diff --name-only --`\nto compare the worktree to the index (listing only unstaged changes),\nas Peff notes.\n\n> Also apparently it became a known TODO for phabricator\n> developers [1].\n\nThat was me :)\n\n> So, after all is it expected behavior of ls-files or not and if not\n> shall i report it as a separate thread? :)\n\nActually, I doubt it's worth fixing ls-files. Your problem should be\nfixed when this is merged (hopefully in a few hours):\n\n https://github.com/facebook/arcanist/pull/121\n\nCheers.\n"},{"id":"230955","messageId":"CAErtv24Lv1JegCBQ=TXvOsgBNHp=Rphk5YVAq2qqRbNmqfNSkw@mail.gmail.com","threadId":"35390","inReplyTo":"CALkWK0m9MK=RBBor-ZeGrGU9KA6tZa89UUi0J7j9fxr1g6uJtQ@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-22T17:01:20Z","receivedAt":"2013-11-22T17:01:20Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Ah, didn't notice you're the author of that pull-request Ramkumar :)\n\nSo guess issue with arc can be considered solved now. But i'm still\ncollecting more details about how to manage to commit change addons\nhash without arc command even (it happens to Campbell Barton really\noften).\n\nWill report back when we'll know something.\n\nOn Fri, Nov 22, 2013 at 10:35 PM, Ramkumar Ramachandra\n wrote:\n> Sergey Sharybin wrote:\n>> Ramkumar, not actually sure what you mean?\n>>\n>> For me `git diff --name-only HEAD --` ignores changes to submodules\n>> hash changes.\n>\n> `git diff --name-only HEAD --` compares the worktree to HEAD (listing\n> both staged and unstaged changes); we want `git diff --name-only --`\n> to compare the worktree to the index (listing only unstaged changes),\n> as Peff notes.\n>\n>> Also apparently it became a known TODO for phabricator\n>> developers [1].\n>\n> That was me :)\n>\n>> So, after all is it expected behavior of ls-files or not and if not\n>> shall i report it as a separate thread? :)\n>\n> Actually, I doubt it's worth fixing ls-files. Your problem should be\n> fixed when this is merged (hopefully in a few hours):\n>\n> https://github.com/facebook/arcanist/pull/121\n>\n> Cheers.\n\n\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"230960","messageId":"CAErtv24P+wyZKvvuuPJJ0oxzMif7XtOwJDtKcTKQdKHZaAUbig@mail.gmail.com","threadId":"35390","inReplyTo":"CAErtv24Lv1JegCBQ=TXvOsgBNHp=Rphk5YVAq2qqRbNmqfNSkw@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-22T17:40:24Z","receivedAt":"2013-11-22T17:40:24Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Ok, got it now.\n\nTo reproduce the issue:\n\n- Run git submodule update --recursive to make sure their SHA is\nchanged. Then `git add /path/to/changed submodule` or just `git add .`\n- Modify any file from the parent repository\n- Neither of `git status`, `git diff` and `git diff-files --name-only`\nwill show changes to a submodule, only changes to that file which was\nchanged in parent repo.\n- Make a git commit. It will not list changes to submodule as wll.\n- `git show HEAD` will show changes to both file from and parent\nrepository (which is expected) and will also show changes to the\nsubmodule hash (which is unexpected i'd say).\n\nOn Fri, Nov 22, 2013 at 11:01 PM, Sergey Sharybin wrote:\n> Ah, didn't notice you're the author of that pull-request Ramkumar :)\n>\n> So guess issue with arc can be considered solved now. But i'm still\n> collecting more details about how to manage to commit change addons\n> hash without arc command even (it happens to Campbell Barton really\n> often).\n>\n> Will report back when we'll know something.\n>\n> On Fri, Nov 22, 2013 at 10:35 PM, Ramkumar Ramachandra\n> wrote:\n>> Sergey Sharybin wrote:\n>>> Ramkumar, not actually sure what you mean?\n>>>\n>>> For me `git diff --name-only HEAD --` ignores changes to submodules\n>>> hash changes.\n>>\n>> `git diff --name-only HEAD --` compares the worktree to HEAD (listing\n>> both staged and unstaged changes); we want `git diff --name-only --`\n>> to compare the worktree to the index (listing only unstaged changes),\n>> as Peff notes.\n>>\n>>> Also apparently it became a known TODO for phabricator\n>>> developers [1].\n>>\n>> That was me :)\n>>\n>>> So, after all is it expected behavior of ls-files or not and if not\n>>> shall i report it as a separate thread? :)\n>>\n>> Actually, I doubt it's worth fixing ls-files. Your problem should be\n>> fixed when this is merged (hopefully in a few hours):\n>>\n>> https://github.com/facebook/arcanist/pull/121\n>>\n>> Cheers.\n>\n>\n>\n> --\n> With best regards, Sergey Sharybin\n\n\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"230962","messageId":"CALkWK0muxsRUtO6KYk5G3=RVN0nqd=8gOZn=jsNbTc4B9KCATQ@mail.gmail.com","threadId":"35390","inReplyTo":"CAErtv24P+wyZKvvuuPJJ0oxzMif7XtOwJDtKcTKQdKHZaAUbig@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-22T18:11:25Z","receivedAt":"2013-11-22T18:11:25Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Sergey Sharybin wrote:\n> To reproduce the issue:\n>\n> - Run git submodule update --recursive to make sure their SHA is\n> changed. Then `git add /path/to/changed submodule` or just `git add .`\n> - Modify any file from the parent repository\n> - Neither of `git status`, `git diff` and `git diff-files --name-only`\n> will show changes to a submodule, only changes to that file which was\n> changed in parent repo.\n> - Make a git commit. It will not list changes to submodule as wll.\n> - `git show HEAD` will show changes to both file from and parent\n> repository (which is expected) and will also show changes to the\n> submodule hash (which is unexpected i'd say).\n\nThanks Sergey; I can confirm that this is a bug. For some reason, the\n`git add .` is adding the ignored submodule to the index. After that,\n\n $ git diff-index @\n\nis not showing the ignored submodule. Let me see if I can dig through\nthis in greater detail.\n"},{"id":"230971","messageId":"528FBC76.5060309@web.de","threadId":"35390","inReplyTo":"CALkWK0myk5fWcHdwNtjct4Ji4y4=iVT36wZxqOqZS6S7kem+OQ@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-22T20:20:06Z","receivedAt":"2013-11-22T20:20:06Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 22.11.2013 17:12, schrieb Ramkumar Ramachandra:\n> Jeff King wrote:\n>>> I just checked it out: it uses `git ls-files -m` to get the list of\n>>> unstaged changes; `git diff --name-only HEAD --` will list staged\n>>> changes as well.\n>>\n>> That diff command compares the working tree and HEAD; if you are trying\n>> to match `ls-files -m`, you probably wanted just `git diff --name-only`\n>> to compare the working tree and the index. Although in a script you'd\n>> probably want to use the plumbing `git diff-files` instead.\n> \n> Thanks for that. It's probably not worth fixing ls-files; I'll patch\n> Arcanist to use diff-files instead.\n\nGood to have an short term solution for Sergey, but Heiko and I\ndiscussed this issue and agreed that we should fix ls-files. After\nall the user explicitly asked not to be bothered with submodule\ndifferences by configuring the ignore setting.\n"},{"id":"230972","messageId":"528FC638.5060403@web.de","threadId":"35390","inReplyTo":"CALkWK0muxsRUtO6KYk5G3=RVN0nqd=8gOZn=jsNbTc4B9KCATQ@mail.gmail.com","subject":"Re: Git issues with submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-22T21:01:44Z","receivedAt":"2013-11-22T21:01:44Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 22.11.2013 19:11, schrieb Ramkumar Ramachandra:\n> Sergey Sharybin wrote:\n>> To reproduce the issue:\n>>\n>> - Run git submodule update --recursive to make sure their SHA is\n>> changed. Then `git add /path/to/changed submodule` or just `git add .`\n>> - Modify any file from the parent repository\n>> - Neither of `git status`, `git diff` and `git diff-files --name-only`\n>> will show changes to a submodule, only changes to that file which was\n>> changed in parent repo.\n>> - Make a git commit. It will not list changes to submodule as wll.\n>> - `git show HEAD` will show changes to both file from and parent\n>> repository (which is expected) and will also show changes to the\n>> submodule hash (which is unexpected i'd say).\n> \n> Thanks Sergey; I can confirm that this is a bug.\n\nHmm, looks like git show also needs to be fixed to honor the\nignore setting from .gitmodules. It already does that for\ndiff.ignoreSubmodules from either .git/config or git -c and\nalso supports the --ignore-submodules command line option.\nThe following fixes this inconsistency for me:\n\n---------------------->8-------------------\ndiff --git a/builtin/log.c b/builtin/log.c\nindex b708517..ca97cfb 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -25,6 +25,7 @@\n #include \"version.h\"\n #include \"mailmap.h\"\n #include \"gpg-interface.h\"\n+#include \"submodule.h\"\n\n /* Set a default date-time format for git log (\"log.date\" config variable) */\n static const char *default_date_mode = NULL;\n@@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char *prefix\n int i, count, ret = 0;\n\n init_grep_defaults();\n+ gitmodules_config();\n git_config(git_log_config, NULL);\n\n memset(&match_all, 0, sizeof(match_all));\n---------------------->8-------------------\n\nBut the question is if that is the right thing to do: should\ndiff.ignoreSubmodules and submodule..ignore only affect\nthe diff family or also git log & friends? That would make\nusers blind for submodule history (which they already are\nwhen using diff & friends, so that might be ok here too).\n\n> For some reason, the\n> `git add .` is adding the ignored submodule to the index.\n\nThe ignore setting is documented to only affect diff output\n(including what checkout, commit and status show as modified).\nWhile I agree that this behavior is confusing for Sergey and\nnot optimal for the floating branch model he uses, git is\ncurrently doing exactly what it should. And for people using\nthe ignore setting to not having to stat submodules with huge\nand/or many files that behavior is what they want: don't bother\nme with what changed, but commit what I did change on purpose.\nWe may have to rethink what should happen for users of the\nfloating branch model though.\n\n> After that,\n> \n> $ git diff-index @\n> \n> is not showing the ignored submodule.\n\nOf course it isn't, it's configured not to. You'll have to use\n--ignore-submodules=dirty to override the configuration to make\nit show differences in the recorded hash.\n"},{"id":"230973","messageId":"CAErtv24_M-h-4T187Tj0gtsaqK7U76T2PwXRwLu1+FP6a6nFfg@mail.gmail.com","threadId":"35390","inReplyTo":"528FC638.5060403@web.de","subject":"Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-22T21:46:20Z","receivedAt":"2013-11-22T21:46:20Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"> > For some reason, the\n> > `git add .` is adding the ignored submodule to the index.\n>\n> The ignore setting is documented to only affect diff output\n> (including what checkout, commit and status show as modified).\n> While I agree that this behavior is confusing for Sergey and\n> not optimal for the floating branch model he uses, git is\n> currently doing exactly what it should. And for people using\n> the ignore setting to not having to stat submodules with huge\n> and/or many files that behavior is what they want: don't bother\n> me with what changed, but commit what I did change on purpose.\n> We may have to rethink what should happen for users of the\n> floating branch model though.\n>\n\nI totally see what's happening here and indeed current logic of `git\nadd .` agree is correct from how it was designed to. I could also see\nwhy it might be useful to keep `git add .` and `git commit .` not to\nrespect submodule ignore flag. The only confusing thing here is that\nif i stage changed submodule with this command i wouldn't see this\nsubmodule in \"changes to be committed\" wen doing a commit.\n\nSo seems it's just matter of better communication of what's gonna to\nbe committed in \"changes to be committed\" section? Or maybe even make\nit so `git status` will show staged changes from submdules hash\nregardless ignore flag? Just an ideas how to make communication what's\ngoing on a bit better :)\n\nAnd for sure don't think suppressing stuff from git show is a nice\nidea (if i understand your proposal f making submodule ignore option\naffect on other commands).\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"230974","messageId":"20131122215454.GA4952@sandbox-ub","threadId":"35390","inReplyTo":"528FC638.5060403@web.de","subject":"Re: Re: Git issues with submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-22T21:54:54Z","receivedAt":"2013-11-22T21:54:54Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Fri, Nov 22, 2013 at 10:01:44PM +0100, Jens Lehmann wrote:\n> Hmm, looks like git show also needs to be fixed to honor the\n> ignore setting from .gitmodules. It already does that for\n> diff.ignoreSubmodules from either .git/config or git -c and\n> also supports the --ignore-submodules command line option.\n> The following fixes this inconsistency for me:\n> \n> ---------------------->8-------------------\n> diff --git a/builtin/log.c b/builtin/log.c\n> index b708517..ca97cfb 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -25,6 +25,7 @@\n> #include \"version.h\"\n> #include \"mailmap.h\"\n> #include \"gpg-interface.h\"\n> +#include \"submodule.h\"\n> \n> /* Set a default date-time format for git log (\"log.date\" config variable) */\n> static const char *default_date_mode = NULL;\n> @@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char *prefix\n> int i, count, ret = 0;\n> \n> init_grep_defaults();\n> + gitmodules_config();\n> git_config(git_log_config, NULL);\n> \n> memset(&match_all, 0, sizeof(match_all));\n> ---------------------->8-------------------\n> \n> But the question is if that is the right thing to do: should\n> diff.ignoreSubmodules and submodule..ignore only affect\n> the diff family or also git log & friends? That would make\n> users blind for submodule history (which they already are\n> when using diff & friends, so that might be ok here too).\n> \n> > For some reason, the\n> > `git add .` is adding the ignored submodule to the index.\n> \n> The ignore setting is documented to only affect diff output\n> (including what checkout, commit and status show as modified).\n> While I agree that this behavior is confusing for Sergey and\n> not optimal for the floating branch model he uses, git is\n> currently doing exactly what it should. And for people using\n> the ignore setting to not having to stat submodules with huge\n> and/or many files that behavior is what they want: don't bother\n> me with what changed, but commit what I did change on purpose.\n> We may have to rethink what should happen for users of the\n> floating branch model though.\n\nThis gets more nasty. When using 'git add .' you secretly add the\nsubmodule to the index. But it is neither shown in status nor diff\n--cached. commit actually complains there is nothing to add. But then\nonce you add a local file to the index you can commit and secretly take\nthe submodule change with you.\n\nWhat I think needs fixing here first is that the ignore setting should not\napply to any diffs between HEAD and index. IMO, it should only apply\nto the diff between worktree and index.\n\nWhen we have that the user does not see the submodule changed when\nnormally working. But after doing git add . the change to the submodule\nshould be shown in status and diff regardless of the configuration.\n\nI will have a look at that.\n\nAfter that we can discuss whether add should add submodules that are\ntracked but not shown. How about commit -a ? Should it also ignore the\nchange? I am undecided here. There does not seem to be any good\ndecision. From the users point of view we should probably not add it\nsince its not visible in status. What do others think?\n\nCheers Heiko\n"},{"id":"230975","messageId":"20131122220953.GI4212@google.com","threadId":"35390","inReplyTo":"20131122215454.GA4952@sandbox-ub","subject":"Re: Re: Git issues with submodules","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-11-22T22:09:53Z","receivedAt":"2013-11-22T22:09:53Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Heiko Voigt wrote:\n\n> After that we can discuss whether add should add submodules that are\n> tracked but not shown. How about commit -a ? Should it also ignore the\n> change? I am undecided here. There does not seem to be any good\n> decision. From the users point of view we should probably not add it\n> since its not visible in status. What do others think?\n\nI agree --- it should not add.\n\nThat leaves the question of how to add explicitly. \"git add -f\"?\n\"git add --ignore-submodules=none\"?\n\nThanks,\nJonathan\n"},{"id":"230984","messageId":"20131123011145.GB4952@sandbox-ub","threadId":"35390","inReplyTo":"20131122215454.GA4952@sandbox-ub","subject":"[RFC PATCH] disable complete ignorance of submodules for index <-> HEAD diff","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-23T01:11:45Z","receivedAt":"2013-11-23T01:11:45Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If the value of ignore for submodules is set to \"all\" we would not show\nwhats actually committed during status or diff. This can result in the\nuser committing unexpected submodule references. Lets be nicer and always\nshow whats in the index.\n\nSigned-off-by: Heiko Voigt \n---\nThis probably needs splitting up into two patches one for the\nrefactoring and one for the actual fix. It is also missing tests, but I\nwould first like to know what you think about this approach.\n\n builtin/diff.c | 43 +++++++++++++++++++++++++++----------------\n diff.h | 2 +-\n submodule.c | 6 ++++--\n wt-status.c | 3 +++\n 4 files changed, 35 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex adb93a9..e9a356c 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv\n \treturn run_diff_files(revs, options);\n }\n \n+static int have_cached_option(int argc, const char **argv)\n+{\n+\tint i;\n+\tfor (i = 1; i < argc; i++) {\n+\t\tconst char *arg = argv[i];\n+\t\tif (!strcmp(arg, \"--\"))\n+\t\t\treturn 0;\n+\t\telse if (!strcmp(arg, \"--cached\") ||\n+\t\t\t !strcmp(arg, \"--staged\")) {\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_diff(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tstruct blobinfo blob[2];\n \tint nongit;\n \tint result = 0;\n+\tint have_cached;\n \n \t/*\n \t * We could get N tree-ish in the rev.pending_objects list.\n@@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n \tif (nongit)\n \t\tdie(_(\"Not a git repository\"));\n+\n+\thave_cached = have_cached_option(argc, argv);\n+\tif (have_cached)\n+\t\tDIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE);\n+\n \targc = setup_revisions(argc, argv, &rev, NULL);\n \tif (!rev.diffopt.output_format) {\n \t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n@@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t * Do we have --cached and not have a pending object, then\n \t * default to HEAD by hand. Eek.\n \t */\n-\tif (!rev.pending.nr) {\n-\t\tint i;\n-\t\tfor (i = 1; i < argc; i++) {\n-\t\t\tconst char *arg = argv[i];\n-\t\t\tif (!strcmp(arg, \"--\"))\n-\t\t\t\tbreak;\n-\t\t\telse if (!strcmp(arg, \"--cached\") ||\n-\t\t\t\t !strcmp(arg, \"--staged\")) {\n-\t\t\t\tadd_head_to_pending(&rev);\n-\t\t\t\tif (!rev.pending.nr) {\n-\t\t\t\t\tstruct tree *tree;\n-\t\t\t\t\ttree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n-\t\t\t\t\tadd_pending_object(&rev, &tree->object, \"HEAD\");\n-\t\t\t\t}\n-\t\t\t\tbreak;\n-\t\t\t}\n+\tif (!rev.pending.nr && have_cached) {\n+\t\tadd_head_to_pending(&rev);\n+\t\tif (!rev.pending.nr) {\n+\t\t\tstruct tree *tree;\n+\t\t\ttree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n+\t\t\tadd_pending_object(&rev, &tree->object, \"HEAD\");\n \t\t}\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex e342325..81561b3 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)\n #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)\n #define DIFF_OPT_RENAME_EMPTY (1 << 8)\n-/* (1 << 9) unused */\n+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 << 9)\n #define DIFF_OPT_HAS_CHANGES (1 << 10)\n #define DIFF_OPT_QUICK (1 << 11)\n #define DIFF_OPT_NO_INDEX (1 << 12)\ndiff --git a/submodule.c b/submodule.c\nindex 1905d75..9d81712 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \tDIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \tDIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);\n \n-\tif (!strcmp(arg, \"all\"))\n+\tif (!strcmp(arg, \"all\")) {\n+\t\tif (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))\n+\t\t\treturn;\n \t\tDIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);\n-\telse if (!strcmp(arg, \"untracked\"))\n+\t} else if (!strcmp(arg, \"untracked\"))\n \t\tDIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \telse if (!strcmp(arg, \"dirty\"))\n \t\tDIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);\ndiff --git a/wt-status.c b/wt-status.c\nindex b4e44ba..34be1cc 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -462,6 +462,9 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \t\thandle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n \t}\n \n+\t/* for the index we need to disable complete ignorance of submodules */\n+\tDIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE);\n+\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_collect_updated_cb;\n \trev.diffopt.format_callback_data = s;\n-- \n1.8.5.rc3.1.gcd6363f\n"},{"id":"230986","messageId":"CALkWK0mt17FKUQUXCrL41E0dFx2XdxQQWP05CHjQjUf+g13rmg@mail.gmail.com","threadId":"35390","inReplyTo":"528FC638.5060403@web.de","subject":"Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-23T06:53:29Z","receivedAt":"2013-11-23T06:53:29Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jens Lehmann wrote:\n> But the question is if that is the right thing to do: should\n> diff.ignoreSubmodules and submodule..ignore only affect\n> the diff family or also git log & friends? That would make\n> users blind for submodule history (which they already are\n> when using diff & friends, so that might be ok here too).\n\nNo, I think it's the wrong thing to do. We don't want to show false history.\n\n> The ignore setting is documented to only affect diff output\n> (including what checkout, commit and status show as modified).\n> While I agree that this behavior is confusing for Sergey and\n> not optimal for the floating branch model he uses, git is\n> currently doing exactly what it should. And for people using\n> the ignore setting to not having to stat submodules with huge\n> and/or many files that behavior is what they want: don't bother\n> me with what changed, but commit what I did change on purpose.\n> We may have to rethink what should happen for users of the\n> floating branch model though.\n\nI'd argue that the only reason the diff-family is blind is because the\ncommit hash changes in the first place; if the hash didn't change (ie.\nfloating submodules were represented by 0s hash or something), we\nwouldn't have this problem. The correct solution is to also make `git\nadd' blind.\n"},{"id":"230987","messageId":"CALkWK0nNu=XtYEedJbppsz-3+ShYa1yUrS28GA0JBbmLuF1Caw@mail.gmail.com","threadId":"35390","inReplyTo":"20131122215454.GA4952@sandbox-ub","subject":"Re: Re: Git issues with submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-11-23T07:04:23Z","receivedAt":"2013-11-23T07:04:23Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Heiko Voigt wrote:\n> What I think needs fixing here first is that the ignore setting should not\n> apply to any diffs between HEAD and index. IMO, it should only apply\n> to the diff between worktree and index.\n>\n> When we have that the user does not see the submodule changed when\n> normally working. But after doing git add . the change to the submodule\n> should be shown in status and diff regardless of the configuration.\n\nYeah, I think this is a good direction.\n\n> After that we can discuss whether add should add submodules that are\n> tracked but not shown. How about commit -a ? Should it also ignore the\n> change?\n\nHere, I think ignored submodules should behave like files matched by\n.gitignore: add should not add (`add -f` would be a good way to force\nit), and `commit -a` should also exclude it.\n"},{"id":"231001","messageId":"52910BC4.1030800@web.de","threadId":"35390","inReplyTo":"20131122220953.GI4212@google.com","subject":"Re: Git issues with submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-23T20:10:44Z","receivedAt":"2013-11-23T20:10:44Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 22.11.2013 23:09, schrieb Jonathan Nieder:\n> Heiko Voigt wrote:\n> \n>> After that we can discuss whether add should add submodules that are\n>> tracked but not shown. How about commit -a ? Should it also ignore the\n>> change? I am undecided here. There does not seem to be any good\n>> decision. From the users point of view we should probably not add it\n>> since its not visible in status. What do others think?\n> \n> I agree --- it should not add.\n\nI concur: adding a change that is hidden from the user during\nthe process is not a good idea.\n\n> That leaves the question of how to add explicitly. \"git add -f\"?\n> \"git add --ignore-submodules=none\"?\n\nI suspect \"git add\" and \"git commit -a\" have to learn the\n--ignore-submodules option anyway if we go that route. There\nare points in time (e.g. releasing a new version or having run\nan expansive test successfully) where some users want to update\nthe submodules that are normally ignored to record the exact\nversions involved.\n"},{"id":"231003","messageId":"529110ED.8000501@web.de","threadId":"35390","inReplyTo":"20131122215454.GA4952@sandbox-ub","subject":"Re: Git issues with submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-23T20:32:45Z","receivedAt":"2013-11-23T20:32:45Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 22.11.2013 22:54, schrieb Heiko Voigt:\n> What I think needs fixing here first is that the ignore setting should not\n> apply to any diffs between HEAD and index. IMO, it should only apply\n> to the diff between worktree and index.\n\nNot only that. It should also apply to diffs between commits/trees\nand work tree but not between commits/trees. The reason the ignore\nsetting was added three years ago was to avoid expensive work tree\noperations when it was clear that either the information wasn't\nwanted or it took too much time to determine that. And I doubt you\nwant to see modifications to submodules in your work tree when\ndiffing against HEAD but not when diffing against the index.\n\nAnd this behavior happens to be just what the floating branch model\nneeds too. I'm not sure there isn't a use case out there that also\nneeds to silence diff & friends regarding submodule changes between\ncommits/trees and/or index too (even though I cannot come up with\none at the moment). So I propose to add \"worktree\" as another value\nfor the ignore option - which ignores submodule modifications in\nthe work tree - and leave \"all\" as it is.\n"},{"id":"231004","messageId":"20131124005256.GA3500@sandbox-ub","threadId":"35390","inReplyTo":"52910BC4.1030800@web.de","subject":"Re: Re: Git issues with submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-24T00:52:56Z","receivedAt":"2013-11-24T00:52:56Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote:\n> Am 22.11.2013 23:09, schrieb Jonathan Nieder:\n> > Heiko Voigt wrote:\n> > \n> >> After that we can discuss whether add should add submodules that are\n> >> tracked but not shown. How about commit -a ? Should it also ignore the\n> >> change? I am undecided here. There does not seem to be any good\n> >> decision. From the users point of view we should probably not add it\n> >> since its not visible in status. What do others think?\n> > \n> > I agree --- it should not add.\n> \n> I concur: adding a change that is hidden from the user during\n> the process is not a good idea.\n\nHere is a patch achieving that. Still missing a test which I will add.\n\nCheers Heiko\n\n---8<----\nSubject: [PATCH] fix 'git add' to skip submodules configured as ignored\n\nIf submodules are configured as ignore=all they are not shown by status.\nLets also ignore them when adding files to the index. This avoids that\nusers accidentially add ignored submodules with: git add .\n\nWe achieve this by reading the submodule config and thus correctly\ninitializing the infrastructure to take the ignore decision.\n\nSigned-off-by: Heiko Voigt \n---\n builtin/add.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 226f758..2d0d2ef 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -15,6 +15,7 @@\n #include \"diffcore.h\"\n #include \"revision.h\"\n #include \"bulk-checkin.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [options] [--] ...\"),\n@@ -378,6 +379,10 @@ static int add_config(const char *var, const char *value, void *cb)\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\n+\tif (!prefixcmp(var, \"submodule.\"))\n+\t\treturn parse_submodule_config_option(var, value);\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint implicit_dot = 0;\n \tstruct update_callback_data update_data;\n \n+\tgitmodules_config();\n \tgit_config(add_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, builtin_add_options,\n-- \n1.8.5.rc3.1.gbe2a8c7\n"},{"id":"231005","messageId":"20131124010642.GB3500@sandbox-ub","threadId":"35390","inReplyTo":"529110ED.8000501@web.de","subject":"Re: Re: Git issues with submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-24T01:06:42Z","receivedAt":"2013-11-24T01:06:42Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Sat, Nov 23, 2013 at 09:32:45PM +0100, Jens Lehmann wrote:\n> Am 22.11.2013 22:54, schrieb Heiko Voigt:\n> > What I think needs fixing here first is that the ignore setting should not\n> > apply to any diffs between HEAD and index. IMO, it should only apply\n> > to the diff between worktree and index.\n> \n> Not only that. It should also apply to diffs between commits/trees\n> and work tree but not between commits/trees. The reason the ignore\n> setting was added three years ago was to avoid expensive work tree\n> operations when it was clear that either the information wasn't\n> wanted or it took too much time to determine that. And I doubt you\n> want to see modifications to submodules in your work tree when\n> diffing against HEAD but not when diffing against the index.\n> \n> And this behavior happens to be just what the floating branch model\n> needs too. I'm not sure there isn't a use case out there that also\n> needs to silence diff & friends regarding submodule changes between\n> commits/trees and/or index too (even though I cannot come up with\n> one at the moment). So I propose to add \"worktree\" as another value\n> for the ignore option - which ignores submodule modifications in\n> the work tree - and leave \"all\" as it is.\n\nI am not so sure about that. Only finding out what has changed (commit\nwise) in a submodule is expensive. Just finding out whether a submodule\nsha1 has changed is not expensive. Maybe we should completely stop\nrespecting the ignore=all setting for history and diff between index and\nHEAD. AFAIK, we do not have any other setting that instruct git to\nignore specific parts of the history unless explicitly asked for by\nspecifying a pathspec.\n\nAnd I think a user should never miss by accident that something has\nchanged in the repository.\n\nCheers Heiko\n"},{"id":"231020","messageId":"52922962.3090407@web.de","threadId":"35390","inReplyTo":"20131124005256.GA3500@sandbox-ub","subject":"Re: Git issues with submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-24T16:29:22Z","receivedAt":"2013-11-24T16:29:22Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.11.2013 01:52, schrieb Heiko Voigt:\n> Hi,\n> \n> On Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote:\n>> Am 22.11.2013 23:09, schrieb Jonathan Nieder:\n>>> Heiko Voigt wrote:\n>>>\n>>>> After that we can discuss whether add should add submodules that are\n>>>> tracked but not shown. How about commit -a ? Should it also ignore the\n>>>> change? I am undecided here. There does not seem to be any good\n>>>> decision. From the users point of view we should probably not add it\n>>>> since its not visible in status. What do others think?\n>>>\n>>> I agree --- it should not add.\n>>\n>> I concur: adding a change that is hidden from the user during\n>> the process is not a good idea.\n> \n> Here is a patch achieving that. Still missing a test which I will add.\n\nLooking good to me. Please add tests for \"diff.ignoreSubmodules\"\nand \"submodule..ignore\", the latter both in .gitmodules and\n.git/config. While doing some testing for this thread I found an\ninconsistency in git show which currently honors the submodule\nspecific option only from .git/config and ignores it in the\n.gitmodules file (depending on the outcome of the discussion on\nwhat '--ignore-submodules=all' should ignore we might have to fix\nthat one afterwards).\n\nI'd suggest to also add the --ignore-submodules option in another\npatch on top, because the user should be able to override the\nconfiguration either way. And what about having the '-f' option\nimply '--ignore-submodules=none'?\n\n> Cheers Heiko\n> \n> ---8<----\n> Subject: [PATCH] fix 'git add' to skip submodules configured as ignored\n> \n> If submodules are configured as ignore=all they are not shown by status.\n> Lets also ignore them when adding files to the index. This avoids that\n> users accidentially add ignored submodules with: git add .\n> \n> We achieve this by reading the submodule config and thus correctly\n> initializing the infrastructure to take the ignore decision.\n> \n> Signed-off-by: Heiko Voigt \n> ---\n> builtin/add.c | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/builtin/add.c b/builtin/add.c\n> index 226f758..2d0d2ef 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -15,6 +15,7 @@\n> #include \"diffcore.h\"\n> #include \"revision.h\"\n> #include \"bulk-checkin.h\"\n> +#include \"submodule.h\"\n> \n> static const char * const builtin_add_usage[] = {\n> \tN_(\"git add [options] [--] ...\"),\n> @@ -378,6 +379,10 @@ static int add_config(const char *var, const char *value, void *cb)\n> \t\tignore_add_errors = git_config_bool(var, value);\n> \t\treturn 0;\n> \t}\n> +\n> +\tif (!prefixcmp(var, \"submodule.\"))\n> +\t\treturn parse_submodule_config_option(var, value);\n> +\n> \treturn git_default_config(var, value, cb);\n> }\n> \n> @@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \tint implicit_dot = 0;\n> \tstruct update_callback_data update_data;\n> \n> +\tgitmodules_config();\n> \tgit_config(add_config, NULL);\n> \n> \targc = parse_options(argc, argv, prefix, builtin_add_options,\n> \n"},{"id":"231060","messageId":"CAErtv26e1NxmsBLH_2KuzBECiwZvyvstqXoK5Vybk9xpsaaO9Q@mail.gmail.com","threadId":"35390","inReplyTo":"20131123011145.GB4952@sandbox-ub","subject":"Re: [RFC PATCH] disable complete ignorance of submodules for index <-> HEAD diff","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-25T09:01:34Z","receivedAt":"2013-11-25T09:01:34Z","isPatch":true,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Hi,\n\nTested the patch. `git status` now shows the changes to the\nsubmodules, which is nice :)\n\nHowever, is it possible to make it so `git commit` lists submodules in\n\"changes to be committed\" section, so you'll see what's gonna to be in\nthe commit while typing the commit message as well?\n\nOn Sat, Nov 23, 2013 at 7:11 AM, Heiko Voigt wrote:\n> If the value of ignore for submodules is set to \"all\" we would not show\n> whats actually committed during status or diff. This can result in the\n> user committing unexpected submodule references. Lets be nicer and always\n> show whats in the index.\n>\n> Signed-off-by: Heiko Voigt \n> ---\n> This probably needs splitting up into two patches one for the\n> refactoring and one for the actual fix. It is also missing tests, but I\n> would first like to know what you think about this approach.\n>\n> builtin/diff.c | 43 +++++++++++++++++++++++++++----------------\n> diff.h | 2 +-\n> submodule.c | 6 ++++--\n> wt-status.c | 3 +++\n> 4 files changed, 35 insertions(+), 19 deletions(-)\n>\n> diff --git a/builtin/diff.c b/builtin/diff.c\n> index adb93a9..e9a356c 100644\n> --- a/builtin/diff.c\n> +++ b/builtin/diff.c\n> @@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv\n> return run_diff_files(revs, options);\n> }\n>\n> +static int have_cached_option(int argc, const char **argv)\n> +{\n> + int i;\n> + for (i = 1; i < argc; i++) {\n> + const char *arg = argv[i];\n> + if (!strcmp(arg, \"--\"))\n> + return 0;\n> + else if (!strcmp(arg, \"--cached\") ||\n> + !strcmp(arg, \"--staged\")) {\n> + return 1;\n> + }\n> + }\n> + return 0;\n> +}\n> +\n> int cmd_diff(int argc, const char **argv, const char *prefix)\n> {\n> int i;\n> @@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n> struct blobinfo blob[2];\n> int nongit;\n> int result = 0;\n> + int have_cached;\n>\n> /*\n> * We could get N tree-ish in the rev.pending_objects list.\n> @@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n>\n> if (nongit)\n> die(_(\"Not a git repository\"));\n> +\n> + have_cached = have_cached_option(argc, argv);\n> + if (have_cached)\n> + DIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE);\n> +\n> argc = setup_revisions(argc, argv, &rev, NULL);\n> if (!rev.diffopt.output_format) {\n> rev.diffopt.output_format = DIFF_FORMAT_PATCH;\n> @@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n> * Do we have --cached and not have a pending object, then\n> * default to HEAD by hand. Eek.\n> */\n> - if (!rev.pending.nr) {\n> - int i;\n> - for (i = 1; i < argc; i++) {\n> - const char *arg = argv[i];\n> - if (!strcmp(arg, \"--\"))\n> - break;\n> - else if (!strcmp(arg, \"--cached\") ||\n> - !strcmp(arg, \"--staged\")) {\n> - add_head_to_pending(&rev);\n> - if (!rev.pending.nr) {\n> - struct tree *tree;\n> - tree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n> - add_pending_object(&rev, &tree->object, \"HEAD\");\n> - }\n> - break;\n> - }\n> + if (!rev.pending.nr && have_cached) {\n> + add_head_to_pending(&rev);\n> + if (!rev.pending.nr) {\n> + struct tree *tree;\n> + tree = lookup_tree(EMPTY_TREE_SHA1_BIN);\n> + add_pending_object(&rev, &tree->object, \"HEAD\");\n> }\n> }\n>\n> diff --git a/diff.h b/diff.h\n> index e342325..81561b3 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n> #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)\n> #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)\n> #define DIFF_OPT_RENAME_EMPTY (1 << 8)\n> -/* (1 << 9) unused */\n> +#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 << 9)\n> #define DIFF_OPT_HAS_CHANGES (1 << 10)\n> #define DIFF_OPT_QUICK (1 << 11)\n> #define DIFF_OPT_NO_INDEX (1 << 12)\n> diff --git a/submodule.c b/submodule.c\n> index 1905d75..9d81712 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n> DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n> DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);\n>\n> - if (!strcmp(arg, \"all\"))\n> + if (!strcmp(arg, \"all\")) {\n> + if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))\n> + return;\n> DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);\n> - else if (!strcmp(arg, \"untracked\"))\n> + } else if (!strcmp(arg, \"untracked\"))\n> DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n> else if (!strcmp(arg, \"dirty\"))\n> DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);\n> diff --git a/wt-status.c b/wt-status.c\n> index b4e44ba..34be1cc 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -462,6 +462,9 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n> handle_ignore_submodules_arg(&rev.diffopt, s->ignore_submodule_arg);\n> }\n>\n> + /* for the index we need to disable complete ignorance of submodules */\n> + DIFF_OPT_SET(&rev.diffopt, NO_IGNORE_SUBMODULE);\n> +\n> rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n> rev.diffopt.format_callback = wt_status_collect_updated_cb;\n> rev.diffopt.format_callback_data = s;\n> --\n> 1.8.5.rc3.1.gcd6363f\n>\n\n\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"231059","messageId":"CAErtv2729o-xf=49xY06aVL1ZJzJpeH+cc_Pd1cAP52r32Ss_g@mail.gmail.com","threadId":"35390","inReplyTo":"52922962.3090407@web.de","subject":"Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-25T09:02:51Z","receivedAt":"2013-11-25T09:02:51Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Hey!\n\nSorry for the delayed reply.\n\nAm i right the intention is to make it so `git add .` and `git commit\n.` doesn't include changes to submodule hash unless -f argument is\nprovided?\n\nOn Sun, Nov 24, 2013 at 10:29 PM, Jens Lehmann wrote:\n> Am 24.11.2013 01:52, schrieb Heiko Voigt:\n>> Hi,\n>>\n>> On Sat, Nov 23, 2013 at 09:10:44PM +0100, Jens Lehmann wrote:\n>>> Am 22.11.2013 23:09, schrieb Jonathan Nieder:\n>>>> Heiko Voigt wrote:\n>>>>\n>>>>> After that we can discuss whether add should add submodules that are\n>>>>> tracked but not shown. How about commit -a ? Should it also ignore the\n>>>>> change? I am undecided here. There does not seem to be any good\n>>>>> decision. From the users point of view we should probably not add it\n>>>>> since its not visible in status. What do others think?\n>>>>\n>>>> I agree --- it should not add.\n>>>\n>>> I concur: adding a change that is hidden from the user during\n>>> the process is not a good idea.\n>>\n>> Here is a patch achieving that. Still missing a test which I will add.\n>\n> Looking good to me. Please add tests for \"diff.ignoreSubmodules\"\n> and \"submodule..ignore\", the latter both in .gitmodules and\n> .git/config. While doing some testing for this thread I found an\n> inconsistency in git show which currently honors the submodule\n> specific option only from .git/config and ignores it in the\n> .gitmodules file (depending on the outcome of the discussion on\n> what '--ignore-submodules=all' should ignore we might have to fix\n> that one afterwards).\n>\n> I'd suggest to also add the --ignore-submodules option in another\n> patch on top, because the user should be able to override the\n> configuration either way. And what about having the '-f' option\n> imply '--ignore-submodules=none'?\n>\n>> Cheers Heiko\n>>\n>> ---8<----\n>> Subject: [PATCH] fix 'git add' to skip submodules configured as ignored\n>>\n>> If submodules are configured as ignore=all they are not shown by status.\n>> Lets also ignore them when adding files to the index. This avoids that\n>> users accidentially add ignored submodules with: git add .\n>>\n>> We achieve this by reading the submodule config and thus correctly\n>> initializing the infrastructure to take the ignore decision.\n>>\n>> Signed-off-by: Heiko Voigt \n>> ---\n>> builtin/add.c | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>>\n>> diff --git a/builtin/add.c b/builtin/add.c\n>> index 226f758..2d0d2ef 100644\n>> --- a/builtin/add.c\n>> +++ b/builtin/add.c\n>> @@ -15,6 +15,7 @@\n>> #include \"diffcore.h\"\n>> #include \"revision.h\"\n>> #include \"bulk-checkin.h\"\n>> +#include \"submodule.h\"\n>>\n>> static const char * const builtin_add_usage[] = {\n>> N_(\"git add [options] [--] ...\"),\n>> @@ -378,6 +379,10 @@ static int add_config(const char *var, const char *value, void *cb)\n>> ignore_add_errors = git_config_bool(var, value);\n>> return 0;\n>> }\n>> +\n>> + if (!prefixcmp(var, \"submodule.\"))\n>> + return parse_submodule_config_option(var, value);\n>> +\n>> return git_default_config(var, value, cb);\n>> }\n>>\n>> @@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>> int implicit_dot = 0;\n>> struct update_callback_data update_data;\n>>\n>> + gitmodules_config();\n>> git_config(add_config, NULL);\n>>\n>> argc = parse_options(argc, argv, prefix, builtin_add_options,\n>>\n>\n\n\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"231077","messageId":"20131125174945.GA3847@sandbox-ub","threadId":"35390","inReplyTo":"CAErtv2729o-xf=49xY06aVL1ZJzJpeH+cc_Pd1cAP52r32Ss_g@mail.gmail.com","subject":"Re: Re: Git issues with submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-25T17:49:45Z","receivedAt":"2013-11-25T17:49:45Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Nov 25, 2013 at 03:02:51PM +0600, Sergey Sharybin wrote:\n> Am i right the intention is to make it so `git add .` and `git commit\n> .` doesn't include changes to submodule hash unless -f argument is\n> provided?\n\nYes thats the goal. My patch currently only disables it when ignore is\nset to all. I will add another patch that implements the -f and\n--submodule-ignore option to both of them so the user has an easy way to\nbypass that. But having said that we changing existing behavior here so\nwe have to investigate carefully whether we are not breaking peoples\nexpectations (and script). That also applies to the other patch\nthat enables showing them in diff and friends again.\n\nCheers Heiko\n"},{"id":"231078","messageId":"CAErtv259jxCtvbJYZHgQZv-VJ9U+JwNzWo0tn007SDTCCBScrA@mail.gmail.com","threadId":"35390","inReplyTo":"20131125174945.GA3847@sandbox-ub","subject":"Re: Re: Git issues with submodules","fromName":"Sergey Sharybin","fromEmail":"sergey.vfx@gmail.com","sentAt":"2013-11-25T17:57:45Z","receivedAt":"2013-11-25T17:57:45Z","isPatch":false,"sender":{"key":"sergey.vfx@gmail.com","avatar":"https://gravatar.com/avatar/26027c72ccaf17049ce0f691381f780c049736d0ed89bd45bd812dec74b78bb8?d=mp&s=160"},"body":"Heiko, yeah sure see what you mean. Changing existing behavior is pretty PITA.\n\nJust one more question for now, are you referencing to the patch \"[RFC\nPATCH] disable complete ignorance of submodules for index <-> HEAD\ndiff\"? Coz i tested it and seems it doesn't change behavior of\nadd/commit.\n\nAlso, i'm around to test the all patches which are related on submodules :)\n\nOn Mon, Nov 25, 2013 at 11:49 PM, Heiko Voigt wrote:\n> On Mon, Nov 25, 2013 at 03:02:51PM +0600, Sergey Sharybin wrote:\n>> Am i right the intention is to make it so `git add .` and `git commit\n>> .` doesn't include changes to submodule hash unless -f argument is\n>> provided?\n>\n> Yes thats the goal. My patch currently only disables it when ignore is\n> set to all. I will add another patch that implements the -f and\n> --submodule-ignore option to both of them so the user has an easy way to\n> bypass that. But having said that we changing existing behavior here so\n> we have to investigate carefully whether we are not breaking peoples\n> expectations (and script). That also applies to the other patch\n> that enables showing them in diff and friends again.\n>\n> Cheers Heiko\n\n\n\n-- \nWith best regards, Sergey Sharybin\n"},{"id":"231079","messageId":"20131125181542.GA9761@sandbox-ub","threadId":"35390","inReplyTo":"CAErtv259jxCtvbJYZHgQZv-VJ9U+JwNzWo0tn007SDTCCBScrA@mail.gmail.com","subject":"Re: Re: Re: Git issues with submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-25T18:15:42Z","receivedAt":"2013-11-25T18:15:42Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Nov 25, 2013 at 11:57:45PM +0600, Sergey Sharybin wrote:\n> Heiko, yeah sure see what you mean. Changing existing behavior is pretty PITA.\n> \n> Just one more question for now, are you referencing to the patch \"[RFC\n> PATCH] disable complete ignorance of submodules for index <-> HEAD\n> diff\"? Coz i tested it and seems it doesn't change behavior of\n> add/commit.\n\nYep, that was just an RFC for status and diff. I think teaching add and\ncommit to skip submodules if ignored are a separate topic and thus will\nbe in a separate patch. I have to add tests and probably some more\ncommands. The logic of ignoring submodules is implemented quite deep in\nthe diff code. So changing it can affect quite some commands so we\nhave to check quite carefully what will be affected and if we can change\nit without to much fallout.\n\n> Also, i'm around to test the all patches which are related on submodules :)\n\nThanks, good to know. Stay tuned!\n\nCheers Heiko\n"},{"id":"231101","messageId":"xmqq61rgxkx2.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"20131122215454.GA4952@sandbox-ub","subject":"Re: Git issues with submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-25T20:53:45Z","receivedAt":"2013-11-25T20:53:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> What I think needs fixing here first is that the ignore setting should not\n> apply to any diffs between HEAD and index. IMO, it should only apply\n> to the diff between worktree and index.\n\nHmph. How about \"git diff $commit\", the diff between the worktree and\na named commit (which may or may not be HEAD)?\n\n> When we have that the user does not see the submodule changed when\n> normally working. But after doing git add . the change to the submodule\n> should be shown in status and diff regardless of the configuration.\n\nYes, that sounds sensible.\n"},{"id":"231108","messageId":"xmqq1u24xkjq.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"52922962.3090407@web.de","subject":"Re: Git issues with submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-25T21:01:45Z","receivedAt":"2013-11-25T21:01:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Looking good to me. Please add tests for \"diff.ignoreSubmodules\"\n> and \"submodule..ignore\", the latter both in .gitmodules and\n> .git/config. While doing some testing for this thread I found an\n> inconsistency in git show which currently honors the submodule\n> specific option only from .git/config and ignores it in the\n> .gitmodules file ...\n\nSorry, but isn't that what should happen? .git/config is the\nultimate source of the truth, and .gitmodules is a hint to prime\nthat when the user does \"git submodule init\", no?\n\n> I'd suggest to also add the --ignore-submodules option in another\n> patch on top, because the user should be able to override the\n> configuration either way. And what about having the '-f' option\n> imply '--ignore-submodules=none'?\n\nYeah, this sudden change of semantics, which I think is going in the\nright direction in the longer run, does look like it may be robbing\nfrom those from the \"want specific revision, but not want to see the\ncruft in the top-level\" camp to pay those in the \"floating\" school.\nAt least, with \"add -f\", it allows people to add such ignored ones,\njust like you can \"git add -f cruft\" when cruft is not tracked and\nmarked as ignored in the .gitignore mechansim.\n"},{"id":"231139","messageId":"5294EC11.2010405@web.de","threadId":"35390","inReplyTo":"xmqq1u24xkjq.fsf@gitster.dls.corp.google.com","subject":"Re: Git issues with submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-26T18:44:33Z","receivedAt":"2013-11-26T18:44:33Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 25.11.2013 22:01, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> Looking good to me. Please add tests for \"diff.ignoreSubmodules\"\n>> and \"submodule..ignore\", the latter both in .gitmodules and\n>> .git/config. While doing some testing for this thread I found an\n>> inconsistency in git show which currently honors the submodule\n>> specific option only from .git/config and ignores it in the\n>> .gitmodules file ...\n> \n> Sorry, but isn't that what should happen? .git/config is the\n> ultimate source of the truth, and .gitmodules is a hint to prime\n> that when the user does \"git submodule init\", no?\n\n\"git submodule init\" only copies the \"update\" and \"url\" settings\nto .git/config, all others default to the value they have in the\n.gitmodules file if they aren't found in .git/config. This allows\nupstream to change these settings unless the user copies them to\n.git/config himself.\n"},{"id":"231142","messageId":"xmqq1u23vty1.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"5294EC11.2010405@web.de","subject":"Re: Git issues with submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-26T19:33:58Z","receivedAt":"2013-11-26T19:33:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Am 25.11.2013 22:01, schrieb Junio C Hamano:\n>> Jens Lehmann writes:\n>> \n>>> Looking good to me. Please add tests for \"diff.ignoreSubmodules\"\n>>> and \"submodule..ignore\", the latter both in .gitmodules and\n>>> .git/config. While doing some testing for this thread I found an\n>>> inconsistency in git show which currently honors the submodule\n>>> specific option only from .git/config and ignores it in the\n>>> .gitmodules file ...\n>> \n>> Sorry, but isn't that what should happen? .git/config is the\n>> ultimate source of the truth, and .gitmodules is a hint to prime\n>> that when the user does \"git submodule init\", no?\n>\n> \"git submodule init\" only copies the \"update\" and \"url\" settings\n> to .git/config, all others default to the value they have in the\n> .gitmodules file if they aren't found in .git/config. This allows\n> upstream to change these settings unless the user copies them to\n> .git/config himself.\n\nI know what the code does. I was questioning if \"only copies X and\nY\" is a sensible thing.\n\nCopying at init time will fix the values when copied and give the\nuser a stable and dependable behaviour. I have a feeling that the\ncurrent \"not copy to fix it to a stable value, but look into\n.gitmodules as a fallback\" was not a designed behaviour for the\nother properties, but was done by accident and/or laziness.\n"},{"id":"231144","messageId":"20131126195126.GC4212@google.com","threadId":"35390","inReplyTo":"xmqq1u23vty1.fsf@gitster.dls.corp.google.com","subject":"Re: Git issues with submodules","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-11-26T19:51:26Z","receivedAt":"2013-11-26T19:51:26Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> I have a feeling that the\n> current \"not copy to fix it to a stable value, but look into\n> .gitmodules as a fallback\" was not a designed behaviour for the\n> other properties, but was done by accident and/or laziness.\n\nIt was designed. See for example the thread surrounding [1]:\n\n| And when you are on a superproject branch actively developing inside a\n| submodule, you may want to increase fetch-activity to fetch all new\n| commits in the submodule even if they aren't referenced in the\n| superproject (yet), as that might be just what your fellow developers\n| are about to do. And the person setting up that branch could do that\n| once for all users so they don't have to repeat it in every clone. And\n| when switching away from that branch all those developers cannot forget\n| to reconfigure to fetch-on-demand, so not having that in .git/config is\n| a plus here too.\n\nThanks,\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/161193/focus=161357\n"},{"id":"231150","messageId":"xmqqbo16vma2.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"20131126195126.GC4212@google.com","subject":"Re: Git issues with submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-26T22:19:33Z","receivedAt":"2013-11-26T22:19:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> I have a feeling that the\n>> current \"not copy to fix it to a stable value, but look into\n>> .gitmodules as a fallback\" was not a designed behaviour for the\n>> other properties, but was done by accident and/or laziness.\n>\n> It was designed. See for example the thread surrounding [1]:\n\nOK, thanks.\n\n>\n> | And when you are on a superproject branch actively developing inside a\n> | submodule, you may want to increase fetch-activity to fetch all new\n> | commits in the submodule even if they aren't referenced in the\n> | superproject (yet), as that might be just what your fellow developers\n> | are about to do. And the person setting up that branch could do that\n> | once for all users so they don't have to repeat it in every clone. And\n> | when switching away from that branch all those developers cannot forget\n> | to reconfigure to fetch-on-demand, so not having that in .git/config is\n> | a plus here too.\n>\n> Thanks,\n> Jonathan\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/161193/focus=161357\n"},{"id":"231222","messageId":"20131128071001.GA1057@book.hvoigt.net","threadId":"35390","inReplyTo":"CAErtv26e1NxmsBLH_2KuzBECiwZvyvstqXoK5Vybk9xpsaaO9Q@mail.gmail.com","subject":"Re: Re: [RFC PATCH] disable complete ignorance of submodules for index <-> HEAD diff","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-28T07:10:01Z","receivedAt":"2013-11-28T07:10:01Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Nov 25, 2013 at 03:01:34PM +0600, Sergey Sharybin wrote:\n> Tested the patch. `git status` now shows the changes to the\n> submodules, which is nice :)\n> \n> However, is it possible to make it so `git commit` lists submodules in\n> \"changes to be committed\" section, so you'll see what's gonna to be in\n> the commit while typing the commit message as well?\n\nYes, of course that should be shown. Will add in the next iteration.\nWhich will hopefully be a much simpler implementation. Possibly getting\nrid of this new flag.\n\nCheers Heiko\n"},{"id":"231284","messageId":"20131129225040.GB31636@sandbox-ub","threadId":"35390","inReplyTo":"xmqq61rgxkx2.fsf@gitster.dls.corp.google.com","subject":"Re: Re: Git issues with submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-29T22:50:40Z","receivedAt":"2013-11-29T22:50:40Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Nov 25, 2013 at 12:53:45PM -0800, Junio C Hamano wrote:\n> Heiko Voigt writes:\n> \n> > What I think needs fixing here first is that the ignore setting should not\n> > apply to any diffs between HEAD and index. IMO, it should only apply\n> > to the diff between worktree and index.\n> \n> Hmph. How about \"git diff $commit\", the diff between the worktree and\n> a named commit (which may or may not be HEAD)?\n\nThats an interesting question. My first thought was that I would expect\nit to not show submodules since it involves the worktree, but then I could\nalso argue that it should only show differences between whats in the\nindex and the given commit. That would make matters more complicated but\nI image the use case (floating submodules) involves not caring\nabout submodules except for some integration points when submodule sha1's\nare explicitly recorded. I would expect to only see diffs between these\nintegration points. But then I am not a big user (none at all at the\nmoment) of the floating model.\n\nCheers Heiko\n"},{"id":"231285","messageId":"20131129231125.GC31636@sandbox-ub","threadId":"35390","inReplyTo":"20131128071001.GA1057@book.hvoigt.net","subject":"[RFC/WIP PATCH v2] disable complete ignorance of submodules for index <-> HEAD diff","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-29T23:11:26Z","receivedAt":"2013-11-29T23:11:26Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If the value of ignore for submodules is set to \"all\" we would not show\nwhats actually committed during status or diff. This can result in the\nuser committing unexpected submodule references. Lets be nicer and always\nshow whats in the index.\n\nSigned-off-by: Heiko Voigt \n---\nOn Thu, Nov 28, 2013 at 08:10:01AM +0100, Heiko Voigt wrote:\n> On Mon, Nov 25, 2013 at 03:01:34PM +0600, Sergey Sharybin wrote:\n> > Tested the patch. `git status` now shows the changes to the\n> > submodules, which is nice :)\n> > \n> > However, is it possible to make it so `git commit` lists submodules in\n> > \"changes to be committed\" section, so you'll see what's gonna to be in\n> > the commit while typing the commit message as well?\n> \n> Yes, of course that should be shown. Will add in the next iteration.\n> Which will hopefully be a much simpler implementation. Possibly getting\n> rid of this new flag.\n\nHere is an updated version of this patch. The code is a little bit more\nsimplified and I changed the existing tests to account for the new\nbehavior we are discussing. If everyone agrees that this a desired\nchange in behavior I would continue adding more tests so commit, status\nand so on are more explicitly tested.\n\nCheers Heiko\n\nP.S.: This is still work in progress, the complete series should contain\nboth my patches from this thread.\n\n builtin/diff.c | 2 ++\n diff-lib.c | 3 +++\n diff.h | 2 +-\n submodule.c | 16 ++++++++++++++--\n submodule.h | 1 +\n t/t4027-diff-submodule.sh | 12 +++++++++---\n t/t7508-status.sh | 6 +++++-\n 7 files changed, 35 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex adb93a9..c47614d 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -162,6 +162,8 @@ static int builtin_diff_tree(struct rev_info *revs,\n \tif (argc > 1)\n \t\tusage(builtin_diff_usage);\n \n+\tenforce_no_complete_ignore_submodule(&revs->diffopt);\n+\n \t/*\n \t * We saw two trees, ent0 and ent1. If ent1 is uninteresting,\n \t * swap them.\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 346cac6..c5219cb 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -483,6 +483,9 @@ int run_diff_index(struct rev_info *revs, int cached)\n {\n \tstruct object_array_entry *ent;\n \n+\tif (cached)\n+\t\tenforce_no_complete_ignore_submodule(&revs->diffopt);\n+\n \tent = revs->pending.objects;\n \tif (diff_cache(revs, ent->item->sha1, ent->name, cached))\n \t\texit(128);\ndiff --git a/diff.h b/diff.h\nindex e342325..81561b3 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)\n #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)\n #define DIFF_OPT_RENAME_EMPTY (1 << 8)\n-/* (1 << 9) unused */\n+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 << 9)\n #define DIFF_OPT_HAS_CHANGES (1 << 10)\n #define DIFF_OPT_QUICK (1 << 11)\n #define DIFF_OPT_NO_INDEX (1 << 12)\ndiff --git a/submodule.c b/submodule.c\nindex 1905d75..e0719b6 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -294,6 +294,16 @@ int parse_submodule_config_option(const char *var, const char *value)\n \treturn 0;\n }\n \n+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt)\n+{\n+\tDIFF_OPT_SET(diffopt, NO_IGNORE_SUBMODULE);\n+\tif (DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG) &&\n+\t DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)) {\n+\t\tDIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);\n+\t\tDIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);\n+\t}\n+}\n+\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t const char *arg)\n {\n@@ -301,9 +311,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \tDIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \tDIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);\n \n-\tif (!strcmp(arg, \"all\"))\n+\tif (!strcmp(arg, \"all\")) {\n+\t\tif (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))\n+\t\t\treturn;\n \t\tDIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);\n-\telse if (!strcmp(arg, \"untracked\"))\n+\t} else if (!strcmp(arg, \"untracked\"))\n \t\tDIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \telse if (!strcmp(arg, \"dirty\"))\n \t\tDIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);\ndiff --git a/submodule.h b/submodule.h\nindex 7beec48..2c8087e 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -20,6 +20,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n int parse_submodule_config_option(const char *var, const char *value);\n+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n void show_submodule_summary(FILE *f, const char *path,\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 518bf95..bd84ea7 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -258,7 +258,9 @@ test_expect_success 'git diff between submodule commits' '\n \texpect_from_to >expect.body $subtip $subprev &&\n \ttest_cmp expect.body actual.body &&\n \tgit diff --ignore-submodules HEAD^..HEAD >actual &&\n-\t! test -s actual\n+\tsed -e \"1,/^@@/d\" actual >actual.body &&\n+\texpect_from_to >expect.body $subtip $subprev &&\n+\ttest_cmp expect.body actual.body\n '\n \n test_expect_success 'git diff between submodule commits [.git/config]' '\n@@ -274,7 +276,9 @@ test_expect_success 'git diff between submodule commits [.git/config]' '\n \ttest_cmp expect.body actual.body &&\n \tgit config submodule.subname.ignore all &&\n \tgit diff HEAD^..HEAD >actual &&\n-\t! test -s actual &&\n+\tsed -e \"1,/^@@/d\" actual >actual.body &&\n+\texpect_from_to >expect.body $subtip $subprev &&\n+\ttest_cmp expect.body actual.body &&\n \tgit diff --ignore-submodules=dirty HEAD^..HEAD >actual &&\n \tsed -e \"1,/^@@/d\" actual >actual.body &&\n \texpect_from_to >expect.body $subtip $subprev &&\n@@ -294,7 +298,9 @@ test_expect_success 'git diff between submodule commits [.gitmodules]' '\n \ttest_cmp expect.body actual.body &&\n \tgit config -f .gitmodules submodule.subname.ignore all &&\n \tgit diff HEAD^..HEAD >actual &&\n-\t! test -s actual &&\n+\tsed -e \"1,/^@@/d\" actual >actual.body &&\n+\texpect_from_to >expect.body $subtip $subprev &&\n+\ttest_cmp expect.body actual.body &&\n \tgit config submodule.subname.ignore dirty &&\n \tgit config submodule.subname.path sub &&\n \tgit diff HEAD^..HEAD >actual &&\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex c987b5e..977295f 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -1357,6 +1357,11 @@ test_expect_success \"status (core.commentchar with two chars with submodule summ\n test_expect_success \"--ignore-submodules=all suppresses submodule summary\" '\n \tcat > expect << EOF &&\n On branch master\n+Changes to be committed:\n+ (use \"git reset HEAD ...\" to unstage)\n+\n+\tmodified: sm\n+\n Changes not staged for commit:\n (use \"git add ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n@@ -1374,7 +1379,6 @@ Untracked files:\n \toutput\n \tuntracked\n \n-no changes added to commit (use \"git add\" and/or \"git commit -a\")\n EOF\n \tgit status --ignore-submodules=all > output &&\n \ttest_i18ncmp expect output\n-- \n1.8.5.rc3.1.g223caec\n"},{"id":"231545","messageId":"20131204221659.GA7326@sandbox-ub","threadId":"35390","inReplyTo":"CAErtv259jxCtvbJYZHgQZv-VJ9U+JwNzWo0tn007SDTCCBScrA@mail.gmail.com","subject":"[RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T22:16:59Z","receivedAt":"2013-12-04T22:16:59Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"This is my current work in progress. Sergey it would be awesome if you\ncould test these and tell me whether the behaviour is what you would\nexpect. Once that is settled I will add some tests and possibly clean up\nsome code.\n\nSince nobody spoke against this change of behavior I assume that we\nagree on the general approach I am taking here. If not please speak up\nnow so we can work something out and save me implementation time ;-)\n\nWhats still missing is:\n\n * it seems reset does not care at all about the ignore settings. It\n still shows a\n\n M\tsubmodule\n\n line even when the submodule in question was not in the index and is\n marked as ignored. Have not looked at the code yet.\n\n * The git diff $commit question Junio mentioned here[1] it does not yet\n show diffs of ignore=all submodules.\n\nFor testing convenience you can also find all patches applied to Junio's\ncurrent master here:\n\nhttps://github.com/hvoigt/git/commits/hv/fix_ignore_all_submodules\n\nCheers Heiko\n\nHeiko Voigt (4):\n disable complete ignorance of submodules for index <-> HEAD diff\n fix 'git add' to skip submodules configured as ignored\n teach add -f option for ignored submodules\n always show committed submodules in summary after commit\n\n builtin/add.c | 55 ++++++++++++++++++++++++++++++++++++-----------\n builtin/commit.c | 1 +\n builtin/diff.c | 2 ++\n diff-lib.c | 3 +++\n diff.h | 2 +-\n submodule.c | 26 ++++++++++++++++++++--\n submodule.h | 2 ++\n t/t4027-diff-submodule.sh | 12 ++++++++---\n t/t7508-status.sh | 6 +++++-\n 9 files changed, 90 insertions(+), 19 deletions(-)\n\n-- \n1.8.5.1.43.gf00fb86\n"},{"id":"231546","messageId":"20131204221959.GB7326@sandbox-ub","threadId":"35390","inReplyTo":"20131204221659.GA7326@sandbox-ub","subject":"[RFC/WIP PATCH 1/4] disable complete ignorance of submodules for index <-> HEAD diff","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T22:19:59Z","receivedAt":"2013-12-04T22:19:59Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If the value of ignore for submodules is set to \"all\" we would not show\nwhats actually committed during status or diff. This can result in the\nuser committing unexpected submodule references. Lets be nicer and always\nshow whats in the index.\n\nSigned-off-by: Heiko Voigt \n---\n builtin/diff.c | 2 ++\n diff-lib.c | 3 +++\n diff.h | 2 +-\n submodule.c | 16 ++++++++++++++--\n submodule.h | 1 +\n t/t4027-diff-submodule.sh | 12 +++++++++---\n t/t7508-status.sh | 6 +++++-\n 7 files changed, 35 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex adb93a9..c47614d 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -162,6 +162,8 @@ static int builtin_diff_tree(struct rev_info *revs,\n \tif (argc > 1)\n \t\tusage(builtin_diff_usage);\n \n+\tenforce_no_complete_ignore_submodule(&revs->diffopt);\n+\n \t/*\n \t * We saw two trees, ent0 and ent1. If ent1 is uninteresting,\n \t * swap them.\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 346cac6..c5219cb 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -483,6 +483,9 @@ int run_diff_index(struct rev_info *revs, int cached)\n {\n \tstruct object_array_entry *ent;\n \n+\tif (cached)\n+\t\tenforce_no_complete_ignore_submodule(&revs->diffopt);\n+\n \tent = revs->pending.objects;\n \tif (diff_cache(revs, ent->item->sha1, ent->name, cached))\n \t\texit(128);\ndiff --git a/diff.h b/diff.h\nindex e342325..81561b3 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)\n #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)\n #define DIFF_OPT_RENAME_EMPTY (1 << 8)\n-/* (1 << 9) unused */\n+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1 << 9)\n #define DIFF_OPT_HAS_CHANGES (1 << 10)\n #define DIFF_OPT_QUICK (1 << 11)\n #define DIFF_OPT_NO_INDEX (1 << 12)\ndiff --git a/submodule.c b/submodule.c\nindex 1905d75..e0719b6 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -294,6 +294,16 @@ int parse_submodule_config_option(const char *var, const char *value)\n \treturn 0;\n }\n \n+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt)\n+{\n+\tDIFF_OPT_SET(diffopt, NO_IGNORE_SUBMODULE);\n+\tif (DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG) &&\n+\t DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)) {\n+\t\tDIFF_OPT_CLR(diffopt, IGNORE_SUBMODULES);\n+\t\tDIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);\n+\t}\n+}\n+\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t const char *arg)\n {\n@@ -301,9 +311,11 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \tDIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \tDIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);\n \n-\tif (!strcmp(arg, \"all\"))\n+\tif (!strcmp(arg, \"all\")) {\n+\t\tif (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))\n+\t\t\treturn;\n \t\tDIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);\n-\telse if (!strcmp(arg, \"untracked\"))\n+\t} else if (!strcmp(arg, \"untracked\"))\n \t\tDIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);\n \telse if (!strcmp(arg, \"dirty\"))\n \t\tDIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);\ndiff --git a/submodule.h b/submodule.h\nindex 7beec48..2c8087e 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -20,6 +20,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n int parse_submodule_config_option(const char *var, const char *value);\n+void enforce_no_complete_ignore_submodule(struct diff_options *diffopt);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n void show_submodule_summary(FILE *f, const char *path,\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 518bf95..bd84ea7 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -258,7 +258,9 @@ test_expect_success 'git diff between submodule commits' '\n \texpect_from_to >expect.body $subtip $subprev &&\n \ttest_cmp expect.body actual.body &&\n \tgit diff --ignore-submodules HEAD^..HEAD >actual &&\n-\t! test -s actual\n+\tsed -e \"1,/^@@/d\" actual >actual.body &&\n+\texpect_from_to >expect.body $subtip $subprev &&\n+\ttest_cmp expect.body actual.body\n '\n \n test_expect_success 'git diff between submodule commits [.git/config]' '\n@@ -274,7 +276,9 @@ test_expect_success 'git diff between submodule commits [.git/config]' '\n \ttest_cmp expect.body actual.body &&\n \tgit config submodule.subname.ignore all &&\n \tgit diff HEAD^..HEAD >actual &&\n-\t! test -s actual &&\n+\tsed -e \"1,/^@@/d\" actual >actual.body &&\n+\texpect_from_to >expect.body $subtip $subprev &&\n+\ttest_cmp expect.body actual.body &&\n \tgit diff --ignore-submodules=dirty HEAD^..HEAD >actual &&\n \tsed -e \"1,/^@@/d\" actual >actual.body &&\n \texpect_from_to >expect.body $subtip $subprev &&\n@@ -294,7 +298,9 @@ test_expect_success 'git diff between submodule commits [.gitmodules]' '\n \ttest_cmp expect.body actual.body &&\n \tgit config -f .gitmodules submodule.subname.ignore all &&\n \tgit diff HEAD^..HEAD >actual &&\n-\t! test -s actual &&\n+\tsed -e \"1,/^@@/d\" actual >actual.body &&\n+\texpect_from_to >expect.body $subtip $subprev &&\n+\ttest_cmp expect.body actual.body &&\n \tgit config submodule.subname.ignore dirty &&\n \tgit config submodule.subname.path sub &&\n \tgit diff HEAD^..HEAD >actual &&\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex c987b5e..977295f 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -1357,6 +1357,11 @@ test_expect_success \"status (core.commentchar with two chars with submodule summ\n test_expect_success \"--ignore-submodules=all suppresses submodule summary\" '\n \tcat > expect << EOF &&\n On branch master\n+Changes to be committed:\n+ (use \"git reset HEAD ...\" to unstage)\n+\n+\tmodified: sm\n+\n Changes not staged for commit:\n (use \"git add ...\" to update what will be committed)\n (use \"git checkout -- ...\" to discard changes in working directory)\n@@ -1374,7 +1379,6 @@ Untracked files:\n \toutput\n \tuntracked\n \n-no changes added to commit (use \"git add\" and/or \"git commit -a\")\n EOF\n \tgit status --ignore-submodules=all > output &&\n \ttest_i18ncmp expect output\n-- \n1.8.5.1.43.gf00fb86\n"},{"id":"231547","messageId":"20131204222105.GC7326@sandbox-ub","threadId":"35390","inReplyTo":"20131204221659.GA7326@sandbox-ub","subject":"[RFC/WIP PATCH 2/4] fix 'git add' to skip submodules configured as ignored","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T22:21:05Z","receivedAt":"2013-12-04T22:21:05Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If submodules are configured as ignore=all they are not shown by status.\nLets also ignore them when adding files to the index. This avoids that\nusers accidentially add ignored submodules with: git add .\n\nWe achieve this by reading the submodule config and thus correctly\ninitializing the infrastructure to take the ignore decision.\n\nSigned-off-by: Heiko Voigt \n---\n builtin/add.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 226f758..2d0d2ef 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -15,6 +15,7 @@\n #include \"diffcore.h\"\n #include \"revision.h\"\n #include \"bulk-checkin.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [options] [--] ...\"),\n@@ -378,6 +379,10 @@ static int add_config(const char *var, const char *value, void *cb)\n \t\tignore_add_errors = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\n+\tif (!prefixcmp(var, \"submodule.\"))\n+\t\treturn parse_submodule_config_option(var, value);\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -415,6 +420,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint implicit_dot = 0;\n \tstruct update_callback_data update_data;\n \n+\tgitmodules_config();\n \tgit_config(add_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, builtin_add_options,\n-- \n1.8.5.1.43.gf00fb86\n"},{"id":"231548","messageId":"20131204222156.GD7326@sandbox-ub","threadId":"35390","inReplyTo":"20131204221659.GA7326@sandbox-ub","subject":"[RFC/WIP PATCH 3/4] teach add -f option for ignored submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T22:21:56Z","receivedAt":"2013-12-04T22:21:56Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"When the user wants to bypass the ignored status configured by\nsubmodule..ignore=all it is now allowed by using the -f option.\n\nSigned-off-by: Heiko Voigt \n---\n builtin/add.c | 49 +++++++++++++++++++++++++++++++++++++------------\n submodule.c | 10 ++++++++++\n submodule.h | 1 +\n 3 files changed, 48 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 2d0d2ef..d6cab7f 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -16,6 +16,7 @@\n #include \"revision.h\"\n #include \"bulk-checkin.h\"\n #include \"submodule.h\"\n+#include \"string-list.h\"\n \n static const char * const builtin_add_usage[] = {\n \tN_(\"git add [options] [--] ...\"),\n@@ -37,6 +38,20 @@ struct update_callback_data {\n static const char *option_with_implicit_dot;\n static const char *short_option_with_implicit_dot;\n \n+static struct lock_file lock_file;\n+\n+static const char ignore_error[] =\n+N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n+static const char submodule_ignore_error[] =\n+N_(\"The following paths are ignored submodules:\\n\");\n+\n+static int verbose, show_only, ignored_too, refresh_only;\n+static int ignore_add_errors, intent_to_add, ignore_missing;\n+\n+#define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */\n+static int addremove = ADDREMOVE_DEFAULT;\n+static int addremove_explicit = -1; /* unspecified */\n+\n static void warn_pathless_add(void)\n {\n \tstatic int shown;\n@@ -140,6 +155,9 @@ static void update_callback(struct diff_queue_struct *q,\n \t\t\twarn_pathless_add();\n \t\t\tcontinue;\n \t\t}\n+\t\tif (is_ignored_submodule(path) && !ignored_too)\n+\t\t\tcontinue;\n+\n \t\tswitch (fix_unmerged_status(p, data)) {\n \t\tdefault:\n \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n@@ -174,6 +192,7 @@ static void update_files_in_cache(const char *prefix,\n \tstruct rev_info rev;\n \n \tinit_revisions(&rev, prefix);\n+\tenforce_no_complete_ignore_submodule(&rev.diffopt);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \tif (pathspec)\n \t\tcopy_pathspec(&rev.prune_data, pathspec);\n@@ -332,18 +351,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static struct lock_file lock_file;\n-\n-static const char ignore_error[] =\n-N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n-\n-static int verbose, show_only, ignored_too, refresh_only;\n-static int ignore_add_errors, intent_to_add, ignore_missing;\n-\n-#define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */\n-static int addremove = ADDREMOVE_DEFAULT;\n-static int addremove_explicit = -1; /* unspecified */\n-\n static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)\n {\n \t/* if we are told to ignore, we are not adding removals */\n@@ -407,6 +414,17 @@ static int add_files(struct dir_struct *dir, int flags)\n \treturn exit_status;\n }\n \n+static void die_ignored_submodules(struct string_list *ignored_submodules)\n+{\n+\tstruct string_list_item *path;\n+\n+\tfprintf(stderr, _(submodule_ignore_error));\n+\tfor_each_string_list_item(path, ignored_submodules)\n+\t\tfprintf(stderr, \"%s\\n\", path->string);\n+\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n+\tdie(_(\"no files added\"));\n+}\n+\n int cmd_add(int argc, const char **argv, const char *prefix)\n {\n \tint exit_status = 0;\n@@ -419,6 +437,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tchar *seen = NULL;\n \tint implicit_dot = 0;\n \tstruct update_callback_data update_data;\n+\tstruct string_list ignored_submodules = STRING_LIST_INIT_NODUP;\n \n \tgitmodules_config();\n \tgit_config(add_config, NULL);\n@@ -550,6 +569,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \t\tfor (i = 0; i < pathspec.nr; i++) {\n \t\t\tconst char *path = pathspec.items[i].match;\n+\t\t\tchar path_copy[PATH_MAX];\n \t\t\tif (!seen[i] &&\n \t\t\t ((pathspec.items[i].magic &\n \t\t\t (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n@@ -562,6 +582,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n \t\t\t\t\t pathspec.items[i].original);\n \t\t\t}\n+\t\t\tnormalize_path_copy(path_copy, path);\n+\t\t\tif (is_ignored_submodule(path_copy))\n+\t\t\t\tstring_list_insert(&ignored_submodules, path);\n \t\t}\n \t\tfree(seen);\n \t}\n@@ -583,6 +606,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tupdate_files_in_cache(prefix, &pathspec, &update_data);\n \n \texit_status |= !!update_data.add_errors;\n+\tif (!ignored_too && ignored_submodules.nr)\n+\t\tdie_ignored_submodules(&ignored_submodules);\n \tif (add_new_files)\n \t\texit_status |= add_files(&dir, flags);\n \ndiff --git a/submodule.c b/submodule.c\nindex e0719b6..c28a926 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -199,6 +199,16 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t}\n }\n \n+int is_ignored_submodule(const char *path)\n+{\n+\tstruct diff_options diffopt;\n+\tmemset(&diffopt, 0, sizeof(diffopt));\n+\tset_diffopt_flags_from_submodule_config(&diffopt, path);\n+\tif (DIFF_OPT_TST(&diffopt, IGNORE_SUBMODULES))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n int submodule_config(const char *var, const char *value, void *cb)\n {\n \tif (!prefixcmp(var, \"submodule.\"))\ndiff --git a/submodule.h b/submodule.h\nindex 2c8087e..e067580 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -17,6 +17,7 @@ int remove_path_from_gitmodules(const char *path);\n void stage_updated_gitmodules(void);\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n+int is_ignored_submodule(const char *path);\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n int parse_submodule_config_option(const char *var, const char *value);\n-- \n1.8.5.1.43.gf00fb86\n"},{"id":"231549","messageId":"20131204222348.GE7326@sandbox-ub","threadId":"35390","inReplyTo":"20131204221659.GA7326@sandbox-ub","subject":"[RFC/WIP PATCH 4/4] always show committed submodules in summary after commit","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T22:23:48Z","receivedAt":"2013-12-04T22:23:48Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"If an ignored submodule is committed because is was registered in the\nindex we should always show that to the user in the printed summary\nafter commit.\n\nSigned-off-by: Heiko Voigt \n---\n builtin/commit.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6ab4605..e551566 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1361,6 +1361,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,\n \tstrbuf_release(&committer_ident);\n \n \tinit_revisions(&rev, prefix);\n+\tenforce_no_complete_ignore_submodule(&rev.diffopt);\n \tsetup_revisions(0, NULL, &rev, NULL);\n \n \trev.diff = 1;\n-- \n1.8.5.1.43.gf00fb86\n"},{"id":"231551","messageId":"20131204222629.GF7326@sandbox-ub","threadId":"35390","inReplyTo":"20131204221659.GA7326@sandbox-ub","subject":"Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T22:26:29Z","receivedAt":"2013-12-04T22:26:29Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Wed, Dec 04, 2013 at 11:16:59PM +0100, Heiko Voigt wrote:\n> * The git diff $commit question Junio mentioned here[1] it does not yet\n> show diffs of ignore=all submodules.\n\nForgot to add this here:\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/238348\n"},{"id":"231552","messageId":"xmqq7gbkjlgx.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"20131204221659.GA7326@sandbox-ub","subject":"Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-04T22:32:46Z","receivedAt":"2013-12-04T22:32:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> This is my current work in progress. Sergey it would be awesome if you\n> could test these and tell me whether the behaviour is what you would\n> expect. Once that is settled I will add some tests and possibly clean up\n> some code.\n>\n> Since nobody spoke against this change of behavior I assume that we\n> agree on the general approach I am taking here. If not please speak up\n> now so we can work something out and save me implementation time ;-)\n>\n> Whats still missing is:\n\nBefore listing what's missing, can you describe what \"the general\napproach\" is? After all, that is what you are assuming that has got\na silent concensus, but without getting it spelled out, others would\neasily miss what they \"agreed\" to.\n\nI do think that it is a good thing to make what \"git add .\" does and\nwhat \"git status .\" reports consistent, and \"git add .\" that does\nnot add everything may be a good step in that direction (another\npossible solution may be to admit that ignore=all was a mistake and\nremove that special case altogether, so that \"git status\" will\nalways report a submodule that does not match what is in the HEAD\nand/or index).\n"},{"id":"231556","messageId":"20131204231932.GG7326@sandbox-ub","threadId":"35390","inReplyTo":"xmqq7gbkjlgx.fsf@gitster.dls.corp.google.com","subject":"Re: Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-04T23:19:32Z","receivedAt":"2013-12-04T23:19:32Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Wed, Dec 04, 2013 at 02:32:46PM -0800, Junio C Hamano wrote:\n> Heiko Voigt writes:\n> \n> > This is my current work in progress. Sergey it would be awesome if you\n> > could test these and tell me whether the behaviour is what you would\n> > expect. Once that is settled I will add some tests and possibly clean up\n> > some code.\n> >\n> > Since nobody spoke against this change of behavior I assume that we\n> > agree on the general approach I am taking here. If not please speak up\n> > now so we can work something out and save me implementation time ;-)\n> >\n> > Whats still missing is:\n> \n> Before listing what's missing, can you describe what \"the general\n> approach\" is? After all, that is what you are assuming that has got\n> a silent concensus, but without getting it spelled out, others would\n> easily miss what they \"agreed\" to.\n\nDefinitely, sorry I missed that (isn't it obvious ;-)):\n\nThis series tries to achieve the following goals for the\nsubmodule..ignore=all configuration or the --ignore-submodules=all\ncommand line switch.\n\n * Make git status never ignore submodule changes that got somehow in the\n index. Currently when ignore=all is specified they are and thus\n secretly committed. Basically always show exactly what will be\n committed.\n\n * Make add ignore submodules that have the ignore=all configuration when\n not explicitly naming a certain submodule (i.e. using git add .).\n That way ignore=all submodules are not added to the index by default.\n That can be overridden by using the -f switch so it behaves the same\n as with untracked files specified in one of the ignore files except\n that submodules are actually tracked.\n\n * Let diff always show submodule changes between revisions or\n between a revision and the index. Only worktree changes should be\n ignored with ignore=all.\n\n * Generally speaking: Make everything that displays diffs in history,\n diffs between revisions or between a revision and the index always\n show submodules changes (only the commit ids) even if a submodule is\n specified as ignore=all.\n\n * If ignore=all for a submodule and a diff would usually involve the\n worktree we will show the diff of the commit ids between the current\n index and the requested revision.\n\n> I do think that it is a good thing to make what \"git add .\" does and\n> what \"git status .\" reports consistent, and \"git add .\" that does\n> not add everything may be a good step in that direction (another\n> possible solution may be to admit that ignore=all was a mistake and\n> remove that special case altogether, so that \"git status\" will\n> always report a submodule that does not match what is in the HEAD\n> and/or index).\n\nI think it was too early to add ignore=all back then when the ignoring\nwas implemented. We did not think through all implications. Since people\nhave always been requesting the floating model and as it seems started\nusing it I am not so sure whether there is not a valid use case. Maybe\nSergey can shed some light on their actual use case and why they do not\ncare about the precise revision most of the time.\n\nFor example the case that all developers always want to work with some\nHEAD revision of all submodules and the build system then integrates\ntheir changes on a regular basis. When all went well it creates commits\nwith the precise revisions. This way they have some stable points as\nfallback or for releases. Thats at least the use case I can think of but\nmaybe there are others.\n\nCheers Heiko\n"},{"id":"231636","messageId":"52A0E753.5090908@web.de","threadId":"35390","inReplyTo":"20131204231932.GG7326@sandbox-ub","subject":"Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-12-05T20:51:31Z","receivedAt":"2013-12-05T20:51:31Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 05.12.2013 00:19, schrieb Heiko Voigt:\n> On Wed, Dec 04, 2013 at 02:32:46PM -0800, Junio C Hamano wrote:\n> This series tries to achieve the following goals for the\n> submodule..ignore=all configuration or the --ignore-submodules=all\n> command line switch.\n\nThanks for the summary.\n\n> * Make git status never ignore submodule changes that got somehow in the\n> index. Currently when ignore=all is specified they are and thus\n> secretly committed. Basically always show exactly what will be\n> committed.\n\nYes, what's in the index should always be shown as such even when the\nuser chose to ignore the work tree differences of the submodule.\n\n> * Make add ignore submodules that have the ignore=all configuration when\n> not explicitly naming a certain submodule (i.e. using git add .).\n> That way ignore=all submodules are not added to the index by default.\n> That can be overridden by using the -f switch so it behaves the same\n> as with untracked files specified in one of the ignore files except\n> that submodules are actually tracked.\n\nI think we should do this part in a different series, as everybody\nseems to agree that this should be fixed that way and it has nothing\nto do with what is ignored in submodule history.\n\n> * Let diff always show submodule changes between revisions or\n> between a revision and the index. Only worktree changes should be\n> ignored with ignore=all.\n> \n> * Generally speaking: Make everything that displays diffs in history,\n> diffs between revisions or between a revision and the index always\n> show submodules changes (only the commit ids) even if a submodule is\n> specified as ignore=all.\n\nI'm not so sure about that. Some scripts really want to ignore the\nhistory of submodules when comparing a rev to the index:\n\ngit-filter-branch.sh:\t\t\tgit diff-index -r --name-only --ignore-submodules $commit &&\ngit-pull.sh: git diff-index --cached --name-status -r --ignore-submodules HEAD --\ngit-rebase--merge.sh:\tif ! git diff-index --quiet --ignore-submodules HEAD --\ngit-sh-setup.sh:\tif ! git diff-index --cached --quiet --ignore-submodules HEAD --\ngit-stash.sh:\tgit diff-index --quiet --cached HEAD --ignore-submodules -- &&\n\nI didn't check each site in detail, but I suspect each ignore option\nwas added on purpose to fix a problem. That means we still need \"all\"\n(at least when diffing rev<->index). Unfortunately that area is not\ncovered well in our tests, I only got breakage from the filter-branch\ntests when teaching \"all\" to only ignore work tree changes (see at the\nend on how I did that).\n\nSo I'm currently in favor of adding a new \"worktree\"-value which will\nonly ignore the work tree changes of submodules, which seems just what\nthe floating submodule use case needs. But it looks like we need to\nkeep \"all\".\n\n> * If ignore=all for a submodule and a diff would usually involve the\n> worktree we will show the diff of the commit ids between the current\n> index and the requested revision.\n\nI agree if we make that \"ignore=worktree\".\n\n>> I do think that it is a good thing to make what \"git add .\" does and\n>> what \"git status .\" reports consistent, and \"git add .\" that does\n>> not add everything may be a good step in that direction\n\nYup, as written above I'd propose to start with that too.\n\n>> (another\n>> possible solution may be to admit that ignore=all was a mistake and\n>> remove that special case altogether, so that \"git status\" will\n>> always report a submodule that does not match what is in the HEAD\n>> and/or index).\n\nNo, looking at the git-scripts that use it together with diff-index it\nwasn't a mistake. But we might be missing a less drastic option ;-)\n\n> I think it was too early to add ignore=all back then when the ignoring\n> was implemented. We did not think through all implications. Since people\n> have always been requesting the floating model and as it seems started\n> using it I am not so sure whether there is not a valid use case. Maybe\n> Sergey can shed some light on their actual use case and why they do not\n> care about the precise revision most of the time.\n\nYou maybe right about not thinking things thoroughly through, but we\nhelped people that rightfully complained when the (then new) submodule\nawareness broke their scripts.\n\n> For example the case that all developers always want to work with some\n> HEAD revision of all submodules and the build system then integrates\n> their changes on a regular basis. When all went well it creates commits\n> with the precise revisions. This way they have some stable points as\n> fallback or for releases. Thats at least the use case I can think of but\n> maybe there are others.\n\nAnd that could be the \"worktree\" value.\n\nBelow is a hack that disables the diffing of rev and index, but not\nthat against the work tree. It breaks t4027-diff-submodule.sh,\nt7003-filter-branch.sh and t7508-status.sh as expected:\n\n---------------------->8--------------------\ndiff --git a/diff.c b/diff.c\nindex e34bf97..ed66a01 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4813,7 +4813,7 @@ static int is_submodule_ignored(const char *path, struct d\n if (DIFF_OPT_TST(options, IGNORE_SUBMODULES))\n ignored = 1;\n options->flags = orig_flags;\n- return ignored;\n+ return 0;\n }\n\n void diff_addremove(struct diff_options *options,\n"},{"id":"231685","messageId":"xmqqob4t1spk.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"20131204222156.GD7326@sandbox-ub","subject":"Re: [RFC/WIP PATCH 3/4] teach add -f option for ignored submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-06T23:10:31Z","receivedAt":"2013-12-06T23:10:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> When the user wants to bypass the ignored status configured by\n> submodule..ignore=all it is now allowed by using the -f option.\n>\n> Signed-off-by: Heiko Voigt \n> ---\n> builtin/add.c | 49 +++++++++++++++++++++++++++++++++++++------------\n> submodule.c | 10 ++++++++++\n> submodule.h | 1 +\n> 3 files changed, 48 insertions(+), 12 deletions(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 2d0d2ef..d6cab7f 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -16,6 +16,7 @@\n> #include \"revision.h\"\n> #include \"bulk-checkin.h\"\n> #include \"submodule.h\"\n> +#include \"string-list.h\"\n> \n> static const char * const builtin_add_usage[] = {\n> \tN_(\"git add [options] [--] ...\"),\n> @@ -37,6 +38,20 @@ struct update_callback_data {\n> static const char *option_with_implicit_dot;\n> static const char *short_option_with_implicit_dot;\n> \n> +static struct lock_file lock_file;\n> +\n> +static const char ignore_error[] =\n> +N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n> +static const char submodule_ignore_error[] =\n> +N_(\"The following paths are ignored submodules:\\n\");\n> +\n> +static int verbose, show_only, ignored_too, refresh_only;\n> +static int ignore_add_errors, intent_to_add, ignore_missing;\n> +\n> +#define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */\n> +static int addremove = ADDREMOVE_DEFAULT;\n> +static int addremove_explicit = -1; /* unspecified */\n> +\n> static void warn_pathless_add(void)\n> {\n> \tstatic int shown;\n> @@ -140,6 +155,9 @@ static void update_callback(struct diff_queue_struct *q,\n> \t\t\twarn_pathless_add();\n> \t\t\tcontinue;\n> \t\t}\n> +\t\tif (is_ignored_submodule(path) && !ignored_too)\n> +\t\t\tcontinue;\n> +\n> \t\tswitch (fix_unmerged_status(p, data)) {\n> \t\tdefault:\n> \t\t\tdie(_(\"unexpected diff status %c\"), p->status);\n> @@ -174,6 +192,7 @@ static void update_files_in_cache(const char *prefix,\n> \tstruct rev_info rev;\n> \n> \tinit_revisions(&rev, prefix);\n> +\tenforce_no_complete_ignore_submodule(&rev.diffopt);\n> \tsetup_revisions(0, NULL, &rev, NULL);\n> \tif (pathspec)\n> \t\tcopy_pathspec(&rev.prune_data, pathspec);\n> @@ -332,18 +351,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n> \treturn 0;\n> }\n> \n> -static struct lock_file lock_file;\n> -\n> -static const char ignore_error[] =\n> -N_(\"The following paths are ignored by one of your .gitignore files:\\n\");\n> -\n> -static int verbose, show_only, ignored_too, refresh_only;\n> -static int ignore_add_errors, intent_to_add, ignore_missing;\n> -\n> -#define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */\n> -static int addremove = ADDREMOVE_DEFAULT;\n> -static int addremove_explicit = -1; /* unspecified */\n> -\n> static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)\n> {\n> \t/* if we are told to ignore, we are not adding removals */\n> @@ -407,6 +414,17 @@ static int add_files(struct dir_struct *dir, int flags)\n> \treturn exit_status;\n> }\n> \n> +static void die_ignored_submodules(struct string_list *ignored_submodules)\n> +{\n> +\tstruct string_list_item *path;\n> +\n> +\tfprintf(stderr, _(submodule_ignore_error));\n> +\tfor_each_string_list_item(path, ignored_submodules)\n> +\t\tfprintf(stderr, \"%s\\n\", path->string);\n> +\tfprintf(stderr, _(\"Use -f if you really want to add them.\\n\"));\n> +\tdie(_(\"no files added\"));\n> +}\n> +\n> int cmd_add(int argc, const char **argv, const char *prefix)\n> {\n> \tint exit_status = 0;\n> @@ -419,6 +437,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \tchar *seen = NULL;\n> \tint implicit_dot = 0;\n> \tstruct update_callback_data update_data;\n> +\tstruct string_list ignored_submodules = STRING_LIST_INIT_NODUP;\n> \n> \tgitmodules_config();\n> \tgit_config(add_config, NULL);\n> @@ -550,6 +569,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \n> \t\tfor (i = 0; i < pathspec.nr; i++) {\n> \t\t\tconst char *path = pathspec.items[i].match;\n> +\t\t\tchar path_copy[PATH_MAX];\n> \t\t\tif (!seen[i] &&\n> \t\t\t ((pathspec.items[i].magic &\n> \t\t\t (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n> @@ -562,6 +582,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n> \t\t\t\t\t pathspec.items[i].original);\n> \t\t\t}\n> +\t\t\tnormalize_path_copy(path_copy, path);\n> +\t\t\tif (is_ignored_submodule(path_copy))\n> +\t\t\t\tstring_list_insert(&ignored_submodules, path);\n> \t\t}\n> \t\tfree(seen);\n> \t}\n> @@ -583,6 +606,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> \tupdate_files_in_cache(prefix, &pathspec, &update_data);\n> \n> \texit_status |= !!update_data.add_errors;\n> +\tif (!ignored_too && ignored_submodules.nr)\n> +\t\tdie_ignored_submodules(&ignored_submodules);\n\nWhy is this done so late in the process? Shouldn't it be done\nimmediately after we have finished iterating over the pathspecs,\nchecking with is_ignored_submodule() and stuffing them into\nignored_submodules string list, not waiting for plugging bulk\ncheckin or updating paths already tracked in the index?\n\n> \tif (add_new_files)\n> \t\texit_status |= add_files(&dir, flags);\n> \n> diff --git a/submodule.c b/submodule.c\n> index e0719b6..c28a926 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -199,6 +199,16 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n> \t}\n> }\n> \n> +int is_ignored_submodule(const char *path)\n> +{\n> +\tstruct diff_options diffopt;\n> +\tmemset(&diffopt, 0, sizeof(diffopt));\n> +\tset_diffopt_flags_from_submodule_config(&diffopt, path);\n> +\tif (DIFF_OPT_TST(&diffopt, IGNORE_SUBMODULES))\n> +\t\treturn 1;\n> +\treturn 0;\n> +}\n> +\n> int submodule_config(const char *var, const char *value, void *cb)\n> {\n> \tif (!prefixcmp(var, \"submodule.\"))\n> diff --git a/submodule.h b/submodule.h\n> index 2c8087e..e067580 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -17,6 +17,7 @@ int remove_path_from_gitmodules(const char *path);\n> void stage_updated_gitmodules(void);\n> void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n> \t\tconst char *path);\n> +int is_ignored_submodule(const char *path);\n> int submodule_config(const char *var, const char *value, void *cb);\n> void gitmodules_config(void);\n> int parse_submodule_config_option(const char *var, const char *value);\n"},{"id":"231829","messageId":"20131209214144.GD9606@sandbox-ub","threadId":"35390","inReplyTo":"52A0E753.5090908@web.de","subject":"Re: Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-09T21:41:44Z","receivedAt":"2013-12-09T21:41:44Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Thu, Dec 05, 2013 at 09:51:31PM +0100, Jens Lehmann wrote:\n> Am 05.12.2013 00:19, schrieb Heiko Voigt:\n> > On Wed, Dec 04, 2013 at 02:32:46PM -0800, Junio C Hamano wrote:\n> > This series tries to achieve the following goals for the\n> > submodule..ignore=all configuration or the --ignore-submodules=all\n> > command line switch.\n> \n> Thanks for the summary.\n> \n> > * Make git status never ignore submodule changes that got somehow in the\n> > index. Currently when ignore=all is specified they are and thus\n> > secretly committed. Basically always show exactly what will be\n> > committed.\n> \n> Yes, what's in the index should always be shown as such even when the\n> user chose to ignore the work tree differences of the submodule.\n> \n> > * Make add ignore submodules that have the ignore=all configuration when\n> > not explicitly naming a certain submodule (i.e. using git add .).\n> > That way ignore=all submodules are not added to the index by default.\n> > That can be overridden by using the -f switch so it behaves the same\n> > as with untracked files specified in one of the ignore files except\n> > that submodules are actually tracked.\n> \n> I think we should do this part in a different series, as everybody\n> seems to agree that this should be fixed that way and it has nothing\n> to do with what is ignored in submodule history.\n\nSo how about I put the two points above into a separate series? IMO, add\nand status belong together in this case.\n\n> > * Let diff always show submodule changes between revisions or\n> > between a revision and the index. Only worktree changes should be\n> > ignored with ignore=all.\n> > \n> > * Generally speaking: Make everything that displays diffs in history,\n> > diffs between revisions or between a revision and the index always\n> > show submodules changes (only the commit ids) even if a submodule is\n> > specified as ignore=all.\n> \n> I'm not so sure about that. Some scripts really want to ignore the\n> history of submodules when comparing a rev to the index:\n> \n> git-filter-branch.sh:\t\t\tgit diff-index -r --name-only --ignore-submodules $commit &&\n> git-pull.sh: git diff-index --cached --name-status -r --ignore-submodules HEAD --\n> git-rebase--merge.sh:\tif ! git diff-index --quiet --ignore-submodules HEAD --\n> git-sh-setup.sh:\tif ! git diff-index --cached --quiet --ignore-submodules HEAD --\n> git-stash.sh:\tgit diff-index --quiet --cached HEAD --ignore-submodules -- &&\n> \n> I didn't check each site in detail, but I suspect each ignore option\n> was added on purpose to fix a problem. That means we still need \"all\"\n> (at least when diffing rev<->index). Unfortunately that area is not\n> covered well in our tests, I only got breakage from the filter-branch\n> tests when teaching \"all\" to only ignore work tree changes (see at the\n> end on how I did that).\n\nWell all hits are on diff-index which is plumbing. If it is required for\nsome (internal) scripts to completely ignore submodules I think it is ok\nto do so just for plumbing with a commandline option like that. But I am\nnot sure whether this should actually be configurable.\n\n> So I'm currently in favor of adding a new \"worktree\"-value which will\n> only ignore the work tree changes of submodules, which seems just what\n> the floating submodule use case needs. But it looks like we need to\n> keep \"all\".\n\nBut that will just add more complexity to the already complex topic of\nsubmodules. There are already enough possibilities to get confused with\nsubmodules. I would like to avoid making it more complex.\n\n>From the feedback we get now from Sergey I take that not many users have\nactually been using the 'all' option. Otherwise there would have been\nmore complaints. So the only thing we have to worry about are scripts\nand those we could cover with plumbing commands.\n\n> > * If ignore=all for a submodule and a diff would usually involve the\n> > worktree we will show the diff of the commit ids between the current\n> > index and the requested revision.\n> \n> I agree if we make that \"ignore=worktree\".\n> \n> >> I do think that it is a good thing to make what \"git add .\" does and\n> >> what \"git status .\" reports consistent, and \"git add .\" that does\n> >> not add everything may be a good step in that direction\n> \n> Yup, as written above I'd propose to start with that too.\n> \n> >> (another\n> >> possible solution may be to admit that ignore=all was a mistake and\n> >> remove that special case altogether, so that \"git status\" will\n> >> always report a submodule that does not match what is in the HEAD\n> >> and/or index).\n> \n> No, looking at the git-scripts that use it together with diff-index it\n> wasn't a mistake. But we might be missing a less drastic option ;-)\n\nWell, as said above diff-index is plumbing and as such allowed to do\nnon-user friendly stuff. For all the other commands I propose to never\nhide stuff from the user. E.g. take update-index there you can actually\nmark any index entry as \"assume unchanged\" which will make git stop\nchecking it for changes (like the submodule..ignore=all setting for\nsubmodules). IMO that is a potentially problematic setting which no\nnormal user should do without really knowing the implications. Still I\nthink for plumbing that is ok since it is not really meant to be\ndirectly used. For porcelain it is a different story and I think we\nshould really support/guard the users here to not hang themselves.\n\n> > I think it was too early to add ignore=all back then when the ignoring\n> > was implemented. We did not think through all implications. Since people\n> > have always been requesting the floating model and as it seems started\n> > using it I am not so sure whether there is not a valid use case. Maybe\n> > Sergey can shed some light on their actual use case and why they do not\n> > care about the precise revision most of the time.\n> \n> You maybe right about not thinking things thoroughly through, but we\n> helped people that rightfully complained when the (then new) submodule\n> awareness broke their scripts.\n\nHere you are confusing 'all' with 'dirty'. Before the submodule\nawareness starting with 1.7.0 the behavior we now get with 'dirty' was\nthe default. So that would be the option that helped them.\n\nWhile talking about I think we should really make 'dirty' the default\ninstead of 'all'. AFAIR, the goal of ignoring submodules was to take the\nruntime penalty that came from recursively scanning submodules for\nchanges. That is already fulfilled with 'dirty'.\n\n> Below is a hack that disables the diffing of rev and index, but not\n> that against the work tree. It breaks t4027-diff-submodule.sh,\n> t7003-filter-branch.sh and t7508-status.sh as expected:\n\nI am not sure what this is for. My patch series already implements that\n(and includes the needed test changes), The testsuite passes for me.\nWith my implementation we have fine grained control over where it is ok\nto completely ignore submodule diffs and where not.\n\nCheers Heiko\n"},{"id":"231830","messageId":"20131209215102.GE9606@sandbox-ub","threadId":"35390","inReplyTo":"xmqqob4t1spk.fsf@gitster.dls.corp.google.com","subject":"Re: Re: [RFC/WIP PATCH 3/4] teach add -f option for ignored submodules","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-12-09T21:51:02Z","receivedAt":"2013-12-09T21:51:02Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Fri, Dec 06, 2013 at 03:10:31PM -0800, Junio C Hamano wrote:\n> Heiko Voigt writes:\n> > diff --git a/builtin/add.c b/builtin/add.c\n> > index 2d0d2ef..d6cab7f 100644\n> > --- a/builtin/add.c\n> > +++ b/builtin/add.c\n> > @@ -550,6 +569,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> > \n> > \t\tfor (i = 0; i < pathspec.nr; i++) {\n> > \t\t\tconst char *path = pathspec.items[i].match;\n> > +\t\t\tchar path_copy[PATH_MAX];\n> > \t\t\tif (!seen[i] &&\n> > \t\t\t ((pathspec.items[i].magic &\n> > \t\t\t (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||\n> > @@ -562,6 +582,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> > \t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\n> > \t\t\t\t\t pathspec.items[i].original);\n> > \t\t\t}\n> > +\t\t\tnormalize_path_copy(path_copy, path);\n> > +\t\t\tif (is_ignored_submodule(path_copy))\n> > +\t\t\t\tstring_list_insert(&ignored_submodules, path);\n> > \t\t}\n> > \t\tfree(seen);\n> > \t}\n> > @@ -583,6 +606,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> > \tupdate_files_in_cache(prefix, &pathspec, &update_data);\n> > \n> > \texit_status |= !!update_data.add_errors;\n> > +\tif (!ignored_too && ignored_submodules.nr)\n> > +\t\tdie_ignored_submodules(&ignored_submodules);\n> \n> Why is this done so late in the process? Shouldn't it be done\n> immediately after we have finished iterating over the pathspecs,\n> checking with is_ignored_submodule() and stuffing them into\n> ignored_submodules string list, not waiting for plugging bulk\n> checkin or updating paths already tracked in the index?\n\nThere was no specific reason. I just imitated the codepath for new\nfiles (which will die in add_files() if they are ignored). This can be\nmoved further up. Will do so.\n\nCheers Heiko\n"},{"id":"231832","messageId":"xmqqy53tvezx.fsf@gitster.dls.corp.google.com","threadId":"35390","inReplyTo":"52A0E753.5090908@web.de","subject":"Re: [RFC/WIP PATCH 0/4] less ignorance of submodules for ignore=all","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-09T22:25:22Z","receivedAt":"2013-12-09T22:25:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> I didn't check each site in detail, but I suspect each ignore option\n> was added on purpose to fix a problem. That means we still need \"all\"\n> (at least when diffing rev<->index). Unfortunately that area is not\n> covered well in our tests, I only got breakage from the filter-branch\n> tests when teaching \"all\" to only ignore work tree changes (see at the\n> end on how I did that).\n>\n> So I'm currently in favor of adding a new \"worktree\"-value which will\n> only ignore the work tree changes of submodules, which seems just what\n> the floating submodule use case needs.\n\nCould you help me clarify what it exactly mean to only ignore the\nwork tree changes? Specifically, if I have a submodule directory\nwhose (1) HEAD points at a commit that is the same as the commit\nthat is recorded by the top-level's gitlink, (2) the index may or\nmay not match HEAD, and (3) the working tree contents may or may not\nmatch the index or the HEAD, does it only have the work tree\nchanges?\n\nIf the HEAD in the submodule directory is different from the commit\nrecorded by the top-level's gitlink, but the index and the working\ntree match that different HEAD, I am guessing that it no longer is\nwith \"only the work tree changes\" and shown as modified.\n\nIf that is the suggestion, it goes back to the very original Git\nsubmodule behavour where we compare $submoduledir/.git/HEAD with the\ncommit object name expected by the top-level and say the submodule\ndoes not have any change when and only when these two object names\nare the same, which sounds like a very sensible default behaviour\n(besides, it is very cheap to check ;-).\n"}]}