{"thread":{"id":"13212","subject":"[regression?] \"git status -a\" reports modified for empty submodule directory","startedAt":"2008-04-22T11:01:40Z","lastAt":"2008-05-04T07:10:05Z","messageCount":28,"participants":["Ping Yin","Johannes Sixt","Roman Shaposhnik","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"74945","messageId":"46dff0320804220401h26d2f2ebg1748a4a310acc0f5@mail.gmail.com","threadId":"13212","inReplyTo":null,"subject":"[regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-22T11:01:40Z","receivedAt":"2008-04-22T11:01:40Z","isPatch":false,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"# create a super project super\n$ mkdir super && cd super && git init\n$ touch foo && git add foo && git commit -m \"add foo\"\n\n# create a sub project sub\n$ mkdir sub && cd sub && git init\n$ touch bar && git add bar && git commit -m \"add bar\"\n\n# add sub project to super project\n$ cd ..\n$ git add sub && git commit -m 'add sub'\n\n# remote contents of subproject\n$ rm -rf sub/* sub/.git\n\n# git status -a regression\n$ git status\n# On branch master\nnothing to commit (working directory clean)\n$ git status -a\n# On branch master\n# Changes to be committed:\n#   (use \"git reset HEAD <file>...\" to unstage)\n#\n#       deleted:    sub\n#\n\n\n-- \nPing Yin\n"},{"id":"74946","messageId":"46dff0320804220404u40dd3351tefacf775d4da19ef@mail.gmail.com","threadId":"13212","inReplyTo":"46dff0320804220401h26d2f2ebg1748a4a310acc0f5@mail.gmail.com","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-22T11:04:05Z","receivedAt":"2008-04-22T11:04:05Z","isPatch":false,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Tue, Apr 22, 2008 at 7:01 PM, Ping Yin <pkufranky@gmail.com> wrote:\n> # create a super project super\n>  $ mkdir super && cd super && git init\n>  $ touch foo && git add foo && git commit -m \"add foo\"\n>\n>  # create a sub project sub\n>  $ mkdir sub && cd sub && git init\n>  $ touch bar && git add bar && git commit -m \"add bar\"\n>\n>  # add sub project to super project\n>  $ cd ..\n>  $ git add sub && git commit -m 'add sub'\n>\n>  # remote contents of subproject\n\ns/remote/remove/\n\n>  --\n>  Ping Yin\n>\n\n\n\n-- \nPing Yin\n"},{"id":"74950","messageId":"480DD6D8.9040900@viscovery.net","threadId":"13212","inReplyTo":"46dff0320804220401h26d2f2ebg1748a4a310acc0f5@mail.gmail.com","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-04-22T12:15:20Z","receivedAt":"2008-04-22T12:15:20Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Ping Yin schrieb:\n> # create a super project super\n> $ mkdir super && cd super && git init\n> $ touch foo && git add foo && git commit -m \"add foo\"\n> \n> # create a sub project sub\n> $ mkdir sub && cd sub && git init\n> $ touch bar && git add bar && git commit -m \"add bar\"\n> \n> # add sub project to super project\n> $ cd ..\n> $ git add sub && git commit -m 'add sub'\n> \n> # remote contents of subproject\n> $ rm -rf sub/* sub/.git\n> \n> # git status -a regression\n> $ git status\n> # On branch master\n> nothing to commit (working directory clean)\n\nThis should have reported:\n\n# On branch master\n# Changed but not updated:\n#   (use \"git add/rm <file>...\" to update what will be committed)\n#\n#       deleted:    sub\nno changes added to commit (use \"git add\" and/or \"git commit -a\")\n\nRight?\n\n> $ git status -a\n> # On branch master\n> # Changes to be committed:\n> #   (use \"git reset HEAD <file>...\" to unstage)\n> #\n> #       deleted:    sub\n> #\n\nThere's nothing wrong with this.\n\n-- Hannes\n"},{"id":"74952","messageId":"46dff0320804220539y51c02dedoe181a0eed8599902@mail.gmail.com","threadId":"13212","inReplyTo":"480DD6D8.9040900@viscovery.net","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-22T12:39:56Z","receivedAt":"2008-04-22T12:39:56Z","isPatch":false,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Tue, Apr 22, 2008 at 8:15 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Ping Yin schrieb:\n>\n> > # create a super project super\n>  > $ mkdir super && cd super && git init\n>  > $ touch foo && git add foo && git commit -m \"add foo\"\n>  >\n>  > # create a sub project sub\n>  > $ mkdir sub && cd sub && git init\n>  > $ touch bar && git add bar && git commit -m \"add bar\"\n>  >\n>  > # add sub project to super project\n>  > $ cd ..\n>  > $ git add sub && git commit -m 'add sub'\n>  >\n>  > # remote contents of subproject\n>  > $ rm -rf sub/* sub/.git\n>  >\n>  > # git status -a regression\n>  > $ git status\n>  > # On branch master\n>  > nothing to commit (working directory clean)\n>\n>  This should have reported:\n>\n>  # On branch master\n>  # Changed but not updated:\n>  #   (use \"git add/rm <file>...\" to update what will be committed)\n>  #\n>  #       deleted:    sub\n>  no changes added to commit (use \"git add\" and/or \"git commit -a\")\n>\n>  Right?\n>\n>\n>  > $ git status -a\n>  > # On branch master\n>  > # Changes to be committed:\n>  > #   (use \"git reset HEAD <file>...\" to unstage)\n>  > #\n>  > #       deleted:    sub\n>  > #\n>\n>  There's nothing wrong with this.\n>\n>  -- Hannes\n\nIt seems that in 1.5.4, both 'git status' and 'git status -a' report\n\"no changes added to commit\". And i think this is the right behaviour.\nBecause when a super project is cloned, all submodule directories are\nempty in the beginning. In this case 'git status' and 'git status -a'\nshould report \" no changes added to commit\".\n\n\n\n-- \nPing Yin\n"},{"id":"74954","messageId":"480DDE2A.3050905@viscovery.net","threadId":"13212","inReplyTo":"46dff0320804220539y51c02dedoe181a0eed8599902@mail.gmail.com","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-04-22T12:46:34Z","receivedAt":"2008-04-22T12:46:34Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Ping Yin schrieb:\n> It seems that in 1.5.4, both 'git status' and 'git status -a' report\n> \"no changes added to commit\". And i think this is the right behaviour.\n> Because when a super project is cloned, all submodule directories are\n> empty in the beginning. In this case 'git status' and 'git status -a'\n> should report \" no changes added to commit\".\n\nThat makes sense.\n\n-- Hannes\n"},{"id":"74996","messageId":"1208887218.25663.449.camel@work.sfbay.sun.com","threadId":"13212","inReplyTo":"46dff0320804220539y51c02dedoe181a0eed8599902@mail.gmail.com","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Roman Shaposhnik","fromEmail":"rvs@sun.com","sentAt":"2008-04-22T18:00:18Z","receivedAt":"2008-04-22T18:00:18Z","isPatch":false,"sender":{"key":"rvs@sun.com","avatar":null},"body":"On Tue, 2008-04-22 at 20:39 +0800, Ping Yin wrote:\n> >  > $ git status -a\n> >  > # On branch master\n> >  > # Changes to be committed:\n> >  > #   (use \"git reset HEAD <file>...\" to unstage)\n> >  > #\n> >  > #       deleted:    sub\n> >  > #\n> >\n> >  There's nothing wrong with this.\n> >\n> >  -- Hannes\n> \n> It seems that in 1.5.4, both 'git status' and 'git status -a' report\n> \"no changes added to commit\". And i think this is the right behaviour.\n> Because when a super project is cloned, all submodule directories are\n> empty in the beginning. In this case 'git status' and 'git status -a'\n> should report \" no changes added to commit\".\n\nAgreed 100%\n\nThanks,\nRoman.\n"},{"id":"75533","messageId":"46dff0320804290831u7ef1a78ag2988d5d12f782bdb@mail.gmail.com","threadId":"13212","inReplyTo":"46dff0320804220401h26d2f2ebg1748a4a310acc0f5@mail.gmail.com","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-29T15:31:59Z","receivedAt":"2008-04-29T15:31:59Z","isPatch":false,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Tue, Apr 22, 2008 at 7:01 PM, Ping Yin <pkufranky@gmail.com> wrote:\n> # create a super project super\n>  $ mkdir super && cd super && git init\n>  $ touch foo && git add foo && git commit -m \"add foo\"\n>\n>  # create a sub project sub\n>  $ mkdir sub && cd sub && git init\n>  $ touch bar && git add bar && git commit -m \"add bar\"\n>\n>  # add sub project to super project\n>  $ cd ..\n>  $ git add sub && git commit -m 'add sub'\n>\n>  # remote contents of subproject\n>  $ rm -rf sub/* sub/.git\n>\n>  # git status -a regression\n>  $ git status\n>  # On branch master\n>  nothing to commit (working directory clean)\n>  $ git status -a\n>  # On branch master\n>  # Changes to be committed:\n>  #   (use \"git reset HEAD <file>...\" to unstage)\n>  #\n>  #       deleted:    sub\n>  #\n>\n\nAnother regression following\n\nIn the super project super with empty submodule directory sub\n$ git diff\ndiff --git a/sub b/sub\ndeleted file mode 160000\nindex f2c0d45..0000000\n--- a/sub\n+++ /dev/null\n@@ -1 +0,0 @@\n-Subproject commit f2c0d4509a3178c...\n\n\n-- \nPing Yin\n"},{"id":"75540","messageId":"1209485240-9003-1-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"46dff0320804290831u7ef1a78ag2988d5d12f782bdb@mail.gmail.com","subject":"[PATCH 0/2] Add tests for submodule with empty directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-29T16:07:18Z","receivedAt":"2008-04-29T16:07:18Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"For regression about empty submodule directory\n\n* \"git status -a\" reports modified\n* \"git diff\" generates diff\n"},{"id":"75538","messageId":"1209485240-9003-2-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"1209485240-9003-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH 1/2] t4027: test diff for submodule with empty directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-29T16:07:19Z","receivedAt":"2008-04-29T16:07:19Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n t/t4027-diff-submodule.sh |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 1fd3fb7..ba6679c 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -50,4 +50,11 @@ test_expect_success 'git diff-files --raw' '\n \ttest_cmp expect actual.files\n '\n \n+test_expect_success 'git diff (empty submodule dir)' '\n+\t: >empty &&\n+\trm -rf sub/* sub/.git &&\n+\tgit diff > actual.empty &&\n+\ttest_cmp empty actual.empty\n+'\n+\n test_done\n-- \n1.5.5.1.96.gef4a.dirty\n"},{"id":"75539","messageId":"1209485240-9003-3-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"1209485240-9003-2-git-send-email-pkufranky@gmail.com","subject":"[PATCH 2/2] Add t7506 to test submodule related functions for git-status","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-29T16:07:20Z","receivedAt":"2008-04-29T16:07:20Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n t/t7506-status-submodule.sh |   38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n create mode 100755 t/t7506-status-submodule.sh\n\ndiff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh\nnew file mode 100755\nindex 0000000..a75130c\n--- /dev/null\n+++ b/t/t7506-status-submodule.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git-status for submodule'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_create_repo sub\n+\tcd sub &&\n+\t: >bar &&\n+\tgit add bar &&\n+\tgit commit -m \" Add bar\" &&\n+\tcd .. &&\n+\tgit add sub &&\n+\tgit commit -m \"Add submodule sub\"\n+'\n+\n+test_expect_success 'status clean' '\n+\tgit status |\n+\tgrep \"nothing to commit\"\n+'\n+test_expect_success 'status -a clean' '\n+\tgit status -a |\n+\tgrep \"nothing to commit\"\n+'\n+test_expect_success 'rm submodule contents' '\n+\trm -rf sub/* sub/.git\n+'\n+test_expect_success 'status clean (empty submodule dir)' '\n+\tgit status |\n+\tgrep \"nothing to commit\"\n+'\n+test_expect_success 'status -a clean (empty submodule dir)' '\n+\tgit status -a |\n+\tgrep \"nothing to commit\"\n+'\n+\n+test_done\n-- \n1.5.5.1.96.gef4a.dirty\n"},{"id":"75589","messageId":"7v4p9k7326.fsf@gitster.siamese.dyndns.org","threadId":"13212","inReplyTo":"46dff0320804290831u7ef1a78ag2988d5d12f782bdb@mail.gmail.com","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-29T21:40:17Z","receivedAt":"2008-04-29T21:40:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n> In the super project super with empty submodule directory sub\n> $ git diff\n> diff --git a/sub b/sub\n> deleted file mode 160000\n> index f2c0d45..0000000\n> --- a/sub\n> +++ /dev/null\n> @@ -1 +0,0 @@\n> -Subproject commit f2c0d4509a3178c...\n\nThe repository used to have a subproject and now it doesn't.  Why\nshouldn't it report the removal?\n"},{"id":"75643","messageId":"4818143C.6050206@viscovery.net","threadId":"13212","inReplyTo":"7v4p9k7326.fsf@gitster.siamese.dyndns.org","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-04-30T06:39:56Z","receivedAt":"2008-04-30T06:39:56Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> \"Ping Yin\" <pkufranky@gmail.com> writes:\n> \n>> In the super project super with empty submodule directory sub\n>> $ git diff\n>> diff --git a/sub b/sub\n>> deleted file mode 160000\n>> index f2c0d45..0000000\n>> --- a/sub\n>> +++ /dev/null\n>> @@ -1 +0,0 @@\n>> -Subproject commit f2c0d4509a3178c...\n> \n> The repository used to have a subproject and now it doesn't.  Why\n> shouldn't it report the removal?\n\nBecause you are not required to have a subproject checked out?\n\n-- Hannes\n"},{"id":"75646","messageId":"7vej8n3imp.fsf@gitster.siamese.dyndns.org","threadId":"13212","inReplyTo":"4818143C.6050206@viscovery.net","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-30T07:29:50Z","receivedAt":"2008-04-30T07:29:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Junio C Hamano schrieb:\n>> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>> \n>>> In the super project super with empty submodule directory sub\n>>> $ git diff\n>>> diff --git a/sub b/sub\n>>> deleted file mode 160000\n>>> index f2c0d45..0000000\n>>> --- a/sub\n>>> +++ /dev/null\n>>> @@ -1 +0,0 @@\n>>> -Subproject commit f2c0d4509a3178c...\n>> \n>> The repository used to have a subproject and now it doesn't.  Why\n>> shouldn't it report the removal?\n>\n> Because you are not required to have a subproject checked out?\n\nYes, but.\n\nThis is a policy issue, which is not very well enforced currently.\n\nI have been scratching my head to figure out where the right balance we\nshould strike at for the past week and a half.\n\nFor example, it has long been known ever since submodules were introduced\nthat if you work inside a sparsely checked out supermodule you have to use\n\"commit -a\" with care, because the command notices missing submodule, and\nthere is no way for it to differenciate between the case you _want to_\nremove it and the case you did not care about it, so you will end up\nremoving unchecked out submodules.\n\nThat quirk was something people could live with while submodule support\nwas merely a newly invented curiosity.  But I do not think a command at\nhigh level (iow Porcelain) such as commit and status should be left\nunaware of the Policy that equate missing submodule and unmodified one\nforever.  We should actively enforce the policy, so that unless you\nexplicitly ask nicely, the command should consider a missing submodule\njust as unmodified, e.g. \"commit -a\" should not remove unchecked out\nsubmodules.\n\nBut then you would need a way to ask nicely.  How?  Perhaps using \"git rm\",\nand low level \"update-index --remove\".  Do we even need \"commit -A\"?  I\ndoubt it --- you do not remove submodules every day.\n\nWe'd like to keep the lowest-level unaware of the Policy, which means that\n\"diff-files\" and \"diff-index\" should report unchecked out submodules.\nOtherwise script writers will be left with no way to differenciate missing\nand removed submodules.\n\nOnce we start doing this, I think \"git diff\" Porcelain should fall into\nPolicy-aware category.\n"},{"id":"75684","messageId":"46dff0320804300856w941d948rbcc1cee06f1b41a9@mail.gmail.com","threadId":"13212","inReplyTo":"7vej8n3imp.fsf@gitster.siamese.dyndns.org","subject":"Re: [regression?] \"git status -a\" reports modified for empty submodule directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-04-30T15:56:29Z","receivedAt":"2008-04-30T15:56:29Z","isPatch":false,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Wed, Apr 30, 2008 at 3:29 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>\n>  For example, it has long been known ever since submodules were introduced\n>  that if you work inside a sparsely checked out supermodule you have to use\n>  \"commit -a\" with care, because the command notices missing submodule, and\n>  there is no way for it to differenciate between the case you _want to_\n>  remove it and the case you did not care about it,\n\nIf i am not misunderstood, there is one way now (although broken now,\nit used this kind of behaviour before git-status became built-in):\nempty directory to denote that the submodule is not checked out and\nunaware, and deleted directory to denote that the submodule is\ndeleted.\n\nActually, i don't like the empty directory trick, since it will leave\nso many empty directories in a repository with many submodules\nunchecked out.\n\n>\n>  That quirk was something people could live with while submodule support\n>  was merely a newly invented curiosity.  But I do not think a command at\n>  high level (iow Porcelain) such as commit and status should be left\n>  unaware of the Policy that equate missing submodule and unmodified one\n>  forever.  We should actively enforce the policy, so that unless you\n>  explicitly ask nicely, the command should consider a missing submodule\n>  just as unmodified, e.g. \"commit -a\" should not remove unchecked out\n>  submodules.\n>\n>  But then you would need a way to ask nicely.  How?  Perhaps using \"git rm\",\n>  and low level \"update-index --remove\".  Do we even need \"commit -A\"?  I\n>  doubt it --- you do not remove submodules every day.\n>\n>  We'd like to keep the lowest-level unaware of the Policy, which means that\n>  \"diff-files\" and \"diff-index\" should report unchecked out submodules.\n>  Otherwise script writers will be left with no way to differenciate missing\n>  and removed submodules.\n\nGood point.\n\n>\n>  Once we start doing this, I think \"git diff\" Porcelain should fall into\n>  Policy-aware category.\n>\n\nAgree this pilicy.\n\nIf this change needs a long way. Should we fix the regression first?\nAnyway, 'git status' and 'git status -a' should behave the same for\nsubmodules unchecked out. I have tried but i failed. I just found this\nregression was introduced on the first day of built-in status\n\n-- \nPing Yin\n"},{"id":"75838","messageId":"1209735336-4690-1-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"46dff0320804300856w941d948rbcc1cee06f1b41a9@mail.gmail.com","subject":"[PATCH 0/4] Fix regression for unchecked out submodules","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T13:35:32Z","receivedAt":"2008-05-02T13:35:32Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"\n> If this change needs a long way. Should we fix the regression first?\n> Anyway, 'git status' and 'git status -a' should behave the same for\n> submodules unchecked out. I have tried but i failed. I just found this\n> regression was introduced on the first day of built-in status\n\nOk, this is my try to fix the regression.\n"},{"id":"75835","messageId":"1209735336-4690-2-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"1209735336-4690-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH 1/4] t4027: test diff for submodule with empty directory","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T13:35:33Z","receivedAt":"2008-05-02T13:35:33Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n t/t4027-diff-submodule.sh |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 1fd3fb7..ba6679c 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -50,4 +50,11 @@ test_expect_success 'git diff-files --raw' '\n \ttest_cmp expect actual.files\n '\n \n+test_expect_success 'git diff (empty submodule dir)' '\n+\t: >empty &&\n+\trm -rf sub/* sub/.git &&\n+\tgit diff > actual.empty &&\n+\ttest_cmp empty actual.empty\n+'\n+\n test_done\n-- \n1.5.5.1.116.ge4b9c.dirty\n"},{"id":"75836","messageId":"1209735336-4690-3-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"1209735336-4690-2-git-send-email-pkufranky@gmail.com","subject":"[PATCH 2/4] Add t7506 to test submodule related functions for git-status","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T13:35:34Z","receivedAt":"2008-05-02T13:35:34Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n t/t7506-status-submodule.sh |   38 ++++++++++++++++++++++++++++++++++++++\n 1 files changed, 38 insertions(+), 0 deletions(-)\n create mode 100755 t/t7506-status-submodule.sh\n\ndiff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh\nnew file mode 100755\nindex 0000000..a75130c\n--- /dev/null\n+++ b/t/t7506-status-submodule.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git-status for submodule'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_create_repo sub\n+\tcd sub &&\n+\t: >bar &&\n+\tgit add bar &&\n+\tgit commit -m \" Add bar\" &&\n+\tcd .. &&\n+\tgit add sub &&\n+\tgit commit -m \"Add submodule sub\"\n+'\n+\n+test_expect_success 'status clean' '\n+\tgit status |\n+\tgrep \"nothing to commit\"\n+'\n+test_expect_success 'status -a clean' '\n+\tgit status -a |\n+\tgrep \"nothing to commit\"\n+'\n+test_expect_success 'rm submodule contents' '\n+\trm -rf sub/* sub/.git\n+'\n+test_expect_success 'status clean (empty submodule dir)' '\n+\tgit status |\n+\tgrep \"nothing to commit\"\n+'\n+test_expect_success 'status -a clean (empty submodule dir)' '\n+\tgit status -a |\n+\tgrep \"nothing to commit\"\n+'\n+\n+test_done\n-- \n1.5.5.1.116.ge4b9c.dirty\n"},{"id":"75834","messageId":"1209735336-4690-4-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"1209735336-4690-3-git-send-email-pkufranky@gmail.com","subject":"[PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T13:35:35Z","receivedAt":"2008-05-02T13:35:35Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"This regression is introduced by f58dbf23c3, which calls\ncheck_work_tree_entity in run_diff_files. While check_work_tree_entity\ntreats submodule not checked out as non stagable which causes that\ndiff-files shows these submodules as deleted.\n\ncheck_work_tree_entity considers a worktree entity having two statuses:\nstagable and inexistent. Actually, there is a 3rd status: a submodule\nentity can be existent but not stagable (for example, empty directory for\nnon-checked-out submodule)\n\nThis patch redesigns the return value of check_work_tree_entity to\nconsider both the 3 statuses.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff-lib.c |   22 +++++++++++++---------\n 1 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex cfd629d..72c2a7b 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -337,25 +337,29 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)\n \t}\n \treturn run_diff_files(revs, options);\n }\n+\n+#define ENT_STAGABLE 1\n+#define ENT_INEXISTENT 2\n+#define ENT_NOTGITDIR 3\t\t/* Existent but not stagable (not a git dir) */\n /*\n- * See if work tree has an entity that can be staged.  Return 0 if so,\n- * return 1 if not and return -1 if error.\n+ * Check the status of a work tree entity\n+ * Return ENT_{STAGABLE,INEXISTENT,NOTGITDIR} or -1 if error\n  */\n static int check_work_tree_entity(const struct cache_entry *ce, struct stat *st, char *symcache)\n {\n \tif (lstat(ce->name, st) < 0) {\n \t\tif (errno != ENOENT && errno != ENOTDIR)\n \t\t\treturn -1;\n-\t\treturn 1;\n+\t\treturn ENT_INEXISTENT;\n \t}\n \tif (has_symlink_leading_path(ce->name, symcache))\n-\t\treturn 1;\n+\t\treturn ENT_INEXISTENT;\n \tif (S_ISDIR(st->st_mode)) {\n \t\tunsigned char sub[20];\n \t\tif (resolve_gitlink_ref(ce->name, \"HEAD\", sub))\n-\t\t\treturn 1;\n+\t\t\treturn ENT_NOTGITDIR;\n \t}\n-\treturn 0;\n+\treturn ENT_STAGABLE;\n }\n \n int run_diff_files(struct rev_info *revs, unsigned int option)\n@@ -403,7 +407,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t       sizeof(struct combine_diff_parent)*5);\n \n \t\t\tchanged = check_work_tree_entity(ce, &st, symcache);\n-\t\t\tif (!changed)\n+\t\t\tif (changed != ENT_INEXISTENT)\n \t\t\t\tdpath->mode = ce_mode_from_stat(ce, st.st_mode);\n \t\t\telse {\n \t\t\t\tif (changed < 0) {\n@@ -467,7 +471,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\tcontinue;\n \n \t\tchanged = check_work_tree_entity(ce, &st, symcache);\n-\t\tif (changed) {\n+\t\tif (changed == ENT_INEXISTENT) {\n \t\t\tif (changed < 0) {\n \t\t\t\tperror(ce->name);\n \t\t\t\tcontinue;\n@@ -527,7 +531,7 @@ static int get_stat_data(struct cache_entry *ce,\n \t\tchanged = check_work_tree_entity(ce, &st, cbdata->symcache);\n \t\tif (changed < 0)\n \t\t\treturn -1;\n-\t\telse if (changed) {\n+\t\telse if (changed == ENT_INEXISTENT) {\n \t\t\tif (match_missing) {\n \t\t\t\t*sha1p = sha1;\n \t\t\t\t*modep = mode;\n-- \n1.5.5.1.116.ge4b9c.dirty\n"},{"id":"75837","messageId":"1209735336-4690-5-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"1209735336-4690-4-git-send-email-pkufranky@gmail.com","subject":"[PATCH 4/4] Fix ie_match_stat for non-checked-out submodule","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T13:35:36Z","receivedAt":"2008-05-02T13:35:36Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"For submodules non checked out, ie_match_stat should always return 0.\nSo in this case avoid calling is_racy_timestamp.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n read-cache.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex a92b25b..8c9c8e2 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -294,7 +294,7 @@ int ie_match_stat(const struct index_state *istate,\n \t * whose mtime are the same as the index file timestamp more\n \t * carefully than others.\n \t */\n-\tif (!changed && is_racy_timestamp(istate, ce)) {\n+\tif (!changed && !S_ISGITLINK(ce->ce_mode) && is_racy_timestamp(istate, ce)) {\n \t\tif (assume_racy_is_modified)\n \t\t\tchanged |= DATA_CHANGED;\n \t\telse\n-- \n1.5.5.1.116.ge4b9c.dirty\n"},{"id":"75880","messageId":"7vod7owerk.fsf@gitster.siamese.dyndns.org","threadId":"13212","inReplyTo":"1209735336-4690-5-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH 4/4] Fix ie_match_stat for non-checked-out submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-02T21:57:19Z","receivedAt":"2008-05-02T21:57:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> For submodules non checked out, ie_match_stat should always return 0.\n> So in this case avoid calling is_racy_timestamp.\n\nIt is conceptually wrong to have this check in that function, I think.\n\nLook at what ce_match_stat_basic() does.  For S_IFGITLINK entries, we do\nnot even compare the timestamps, so is_racy_timestamp() check should not\neven care and should return Ok for them.\n\nPerhaps this patch would be a better, in that it covers the other caller\non the write_index() callpath as well.\n\n---\ndiff --git a/read-cache.c b/read-cache.c\nindex a92b25b..0a0ea3b 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -258,6 +258,7 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n static int is_racy_timestamp(const struct index_state *istate, struct cache_entry *ce)\n {\n \treturn (istate->timestamp &&\n+\t\t!S_ISGITLINK(ce->ce_mode) &&\n \t\t((unsigned int)istate->timestamp) <= ce->ce_mtime);\n }\n \n"},{"id":"75882","messageId":"7vfxt0wdkq.fsf@gitster.siamese.dyndns.org","threadId":"13212","inReplyTo":"1209735336-4690-4-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-02T22:23:01Z","receivedAt":"2008-05-02T22:23:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> This regression is introduced by f58dbf23c3, which calls\n> check_work_tree_entity in run_diff_files.  While check_work_tree_entity\n> treats submodule not checked out as non stagable which causes that\n> diff-files shows these submodules as deleted.\n\n>  int run_diff_files(struct rev_info *revs, unsigned int option)\n> @@ -403,7 +407,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n>  \t\t\t       sizeof(struct combine_diff_parent)*5);\n>  \n>  \t\t\tchanged = check_work_tree_entity(ce, &st, symcache);\n> -\t\t\tif (!changed)\n> +\t\t\tif (changed != ENT_INEXISTENT)\n>  \t\t\t\tdpath->mode = ce_mode_from_stat(ce, st.st_mode);\n\n>  \t\t\telse {\n>  \t\t\t\tif (changed < 0) {\n> @@ -467,7 +471,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n>  \t\t\tcontinue;\n>  \n>  \t\tchanged = check_work_tree_entity(ce, &st, symcache);\n> -\t\tif (changed) {\n> +\t\tif (changed == ENT_INEXISTENT) {\n>  \t\t\tif (changed < 0) {\n>  \t\t\t\tperror(ce->name);\n>  \t\t\t\tcontinue;\n> @@ -527,7 +531,7 @@ static int get_stat_data(struct cache_entry *ce,\n>  \t\tchanged = check_work_tree_entity(ce, &st, cbdata->symcache);\n>  \t\tif (changed < 0)\n>  \t\t\treturn -1;\n> -\t\telse if (changed) {\n> +\t\telse if (changed == ENT_INEXISTENT) {\n>  \t\t\tif (match_missing) {\n>  \t\t\t\t*sha1p = sha1;\n>  \t\t\t\t*modep = mode;\n\nEarier I said we may have to teach the Porcelain layer (status, diff) to\nequate a submodule that is not checked out and a submodule that is not\nmodified while keeping low-level plumbing (diff-files and diff-index)\nstill aware that the submodule is missing from the work tree, but that was\nbecause I incorrectly thought there are only two cases (either the\nsubmodule is fully checked out or the submodule directory itself does not\neven exist) and treating the latter the same as an unmodified case would\nmean there won't be an easy way to remove the submodule from the\nsuperproject for Porcelains that are written in terms of diff-files and\ndiff-index.\n\nBut that was a faulty thinking on my part (heh, why didn't anybody correct\nme?).  Actually, there are three cases:\n\n - submodule directory exists and it is a full fledged repository.  It may\n   or may not be modified, but we can tell by looking at its .git/HEAD.\n\n - submodule directory exists but there is nothing there (no \"init\" nor\n   \"update\" was done, just an empty directory checked out).  This is how a\n   superproject with a submodule is checked out by default.\n\n - submodule directory itself does not even exist.\n\nThe second case is \"not checked out -- treat me as unmodified\", and the\nthird case is \"the user does not want the submodule there\", and the latter\nis still reported as \"removed\".  That is exactly what your patch does.\n\nI like it.\n\nBy the way, \"inexistent\" is a word, but somehow it sounds quite awkward.\nPerhaps one of NONEXISTENT (more common), REMOVED (run_diff_files() takes\na SILENT_ON_REMOVED option) or or MISSING (update-index --refresh takes an\nIGNORE_MISSING option) is better?  I dunno.\n"},{"id":"75890","messageId":"46dff0320805021634oa7500e8hee004a6e328fd0fa@mail.gmail.com","threadId":"13212","inReplyTo":"7vod7owerk.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] Fix ie_match_stat for non-checked-out submodule","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T23:34:00Z","receivedAt":"2008-05-02T23:34:00Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sat, May 3, 2008 at 5:57 AM, Junio C Hamano <gitster@pobox.com> wrote:\n\n>\n> --- a/read-cache.c\n>  +++ b/read-cache.c\n>  @@ -258,6 +258,7 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n>   static int is_racy_timestamp(const struct index_state *istate, struct cache_entry *ce)\n>   {\n>         return (istate->timestamp &&\n>  +               !S_ISGITLINK(ce->ce_mode) &&\n>                 ((unsigned int)istate->timestamp) <= ce->ce_mtime);\n>   }\n>\n\nfine\n\n\n\n-- \nPing Yin\n"},{"id":"75892","messageId":"46dff0320805021655w6235ad75k6d48c8d2ae540026@mail.gmail.com","threadId":"13212","inReplyTo":"7vfxt0wdkq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T23:55:40Z","receivedAt":"2008-05-02T23:55:40Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sat, May 3, 2008 at 6:23 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ping Yin <pkufranky@gmail.com> writes:\n>\n>  > This regression is introduced by f58dbf23c3, which calls\n>  > check_work_tree_entity in run_diff_files.  While check_work_tree_entity\n>  > treats submodule not checked out as non stagable which causes that\n>  > diff-files shows these submodules as deleted.\n>\n>\n>\n> >  int run_diff_files(struct rev_info *revs, unsigned int option)\n>  > @@ -403,7 +407,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n>  >                              sizeof(struct combine_diff_parent)*5);\n>  >\n>  >                       changed = check_work_tree_entity(ce, &st, symcache);\n>  > -                     if (!changed)\n>  > +                     if (changed != ENT_INEXISTENT)\n>  >                               dpath->mode = ce_mode_from_stat(ce, st.st_mode);\n>\n>  >                       else {\n>  >                               if (changed < 0) {\n>  > @@ -467,7 +471,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n>  >                       continue;\n>  >\n>  >               changed = check_work_tree_entity(ce, &st, symcache);\n>  > -             if (changed) {\n>  > +             if (changed == ENT_INEXISTENT) {\n>  >                       if (changed < 0) {\n>  >                               perror(ce->name);\n>  >                               continue;\n>  > @@ -527,7 +531,7 @@ static int get_stat_data(struct cache_entry *ce,\n>  >               changed = check_work_tree_entity(ce, &st, cbdata->symcache);\n>  >               if (changed < 0)\n>  >                       return -1;\n>  > -             else if (changed) {\n>  > +             else if (changed == ENT_INEXISTENT) {\n>  >                       if (match_missing) {\n>  >                               *sha1p = sha1;\n>  >                               *modep = mode;\n>\n>  Earier I said we may have to teach the Porcelain layer (status, diff) to\n>  equate a submodule that is not checked out and a submodule that is not\n>  modified while keeping low-level plumbing (diff-files and diff-index)\n>  still aware that the submodule is missing from the work tree, but that was\n>  because I incorrectly thought there are only two cases (either the\n>  submodule is fully checked out or the submodule directory itself does not\n>  even exist) and treating the latter the same as an unmodified case would\n>  mean there won't be an easy way to remove the submodule from the\n>  superproject for Porcelains that are written in terms of diff-files and\n>  diff-index.\n>\n>  But that was a faulty thinking on my part (heh, why didn't anybody correct\n>  me?).  Actually, there are three cases:\n\nActually, i pointed out in early discussion that we use empty\ndirectory (or directory without .git subdirectory) to represent the\n3rd case. However, i don't like the trick. And i don't think you are\nthinking faultily because I prefer your idea: we treat nonexistent\ndirectory and directory without .git as the same for 'git diff'. \"git\ndiff\" will show \"no modified\" for both case.\n\nOne point i don't agree with you is that i think diff-files should\nalso show \"no modified\" for both case. By doing this, we can avoid the\nempty directory for nonchecked out submodules. For a project with\nhundreds of submodules, it's really really annoying to see so many\nunused empty directories.\n\nSo how we diffrentiate removed and unchecked out? For submodule, we\ndon't differentiate it. If you want to remove a submodule, use \"git\nrm\" or \"git update-index --removed\" instead of \"rm && git commit -a\".\n\n>\n>   - submodule directory exists and it is a full fledged repository.  It may\n>    or may not be modified, but we can tell by looking at its .git/HEAD.\n>\n>   - submodule directory exists but there is nothing there (no \"init\" nor\n>    \"update\" was done, just an empty directory checked out).  This is how a\n>    superproject with a submodule is checked out by default.\n>\n>   - submodule directory itself does not even exist.\n>\n>  The second case is \"not checked out -- treat me as unmodified\", and the\n>  third case is \"the user does not want the submodule there\", and the latter\n>  is still reported as \"removed\".  That is exactly what your patch does.\n\nGreat, you make the 3 cases more clear.\n\n\n\n-- \nPing Yin\n"},{"id":"75893","messageId":"1209773238-25987-1-git-send-email-pkufranky@gmail.com","threadId":"13212","inReplyTo":"7vfxt0wdkq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Rename ENT_INEXISTENT to ENT_NONEXISTENT","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T00:07:18Z","receivedAt":"2008-05-03T00:07:18Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n> By the way, \"inexistent\" is a word, but somehow it sounds quite awkward.\n> Perhaps one of NONEXISTENT (more common), REMOVED (run_diff_files() takes\n> a SILENT_ON_REMOVED option) or or MISSING (update-index --refresh takes an\n> IGNORE_MISSING option) is better? \n\nI prefer nonexistent because removed or missing has the meaning that the\nuser has removed it. However, it may be not this case (althogh it is at\ncurrent time).\n\n\n diff-lib.c |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 72c2a7b..61a1b7c 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -339,7 +339,7 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)\n }\n \n #define ENT_STAGABLE 1\n-#define ENT_INEXISTENT 2\n+#define ENT_NONEXISTENT 2\n #define ENT_NOTGITDIR 3\t\t/* Existent but not stagable (not a git dir) */\n /*\n  * Check the status of a work tree entity\n@@ -350,10 +350,10 @@ static int check_work_tree_entity(const struct cache_entry *ce, struct stat *st,\n \tif (lstat(ce->name, st) < 0) {\n \t\tif (errno != ENOENT && errno != ENOTDIR)\n \t\t\treturn -1;\n-\t\treturn ENT_INEXISTENT;\n+\t\treturn ENT_NONEXISTENT;\n \t}\n \tif (has_symlink_leading_path(ce->name, symcache))\n-\t\treturn ENT_INEXISTENT;\n+\t\treturn ENT_NONEXISTENT;\n \tif (S_ISDIR(st->st_mode)) {\n \t\tunsigned char sub[20];\n \t\tif (resolve_gitlink_ref(ce->name, \"HEAD\", sub))\n@@ -407,7 +407,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t       sizeof(struct combine_diff_parent)*5);\n \n \t\t\tchanged = check_work_tree_entity(ce, &st, symcache);\n-\t\t\tif (changed != ENT_INEXISTENT)\n+\t\t\tif (changed != ENT_NONEXISTENT)\n \t\t\t\tdpath->mode = ce_mode_from_stat(ce, st.st_mode);\n \t\t\telse {\n \t\t\t\tif (changed < 0) {\n@@ -471,7 +471,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\tcontinue;\n \n \t\tchanged = check_work_tree_entity(ce, &st, symcache);\n-\t\tif (changed == ENT_INEXISTENT) {\n+\t\tif (changed == ENT_NONEXISTENT) {\n \t\t\tif (changed < 0) {\n \t\t\t\tperror(ce->name);\n \t\t\t\tcontinue;\n@@ -531,7 +531,7 @@ static int get_stat_data(struct cache_entry *ce,\n \t\tchanged = check_work_tree_entity(ce, &st, cbdata->symcache);\n \t\tif (changed < 0)\n \t\t\treturn -1;\n-\t\telse if (changed == ENT_INEXISTENT) {\n+\t\telse if (changed == ENT_NONEXISTENT) {\n \t\t\tif (match_missing) {\n \t\t\t\t*sha1p = sha1;\n \t\t\t\t*modep = mode;\n-- \n1.5.5.1.117.g73010\n"},{"id":"75917","messageId":"alpine.DEB.1.00.0805031335380.30431@racer","threadId":"13212","inReplyTo":"7vfxt0wdkq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-03T12:36:49Z","receivedAt":"2008-05-03T12:36:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 May 2008, Junio C Hamano wrote:\n\n> By the way, \"inexistent\" is a word, but somehow it sounds quite awkward.\n\nIn the same vein, I had to think about \"stagable\" for some time.  I think \nthe correct term is \"stageable\", but then, I am not sure if there is no \nbetter word anyway.\n\nCiao,\nDscho\n"},{"id":"75944","messageId":"7vy76ruxvc.fsf@gitster.siamese.dyndns.org","threadId":"13212","inReplyTo":"alpine.DEB.1.00.0805031335380.30431@racer","subject":"Re: [PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T16:59:51Z","receivedAt":"2008-05-03T16:59:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> In the same vein, I had to think about \"stagable\" for some time.  I think \n> the correct term is \"stageable\", but then, I am not sure if there is no \n> better word anyway.\n\nOh, I did not even mention that because it quite obviously is an typo of a\nnon-word ;-)\n"},{"id":"75992","messageId":"7vej8ir2ik.fsf@gitster.siamese.dyndns.org","threadId":"13212","inReplyTo":"7vfxt0wdkq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-04T06:45:23Z","receivedAt":"2008-05-04T06:45:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The second case is \"not checked out -- treat me as unmodified\", and the\n> third case is \"the user does not want the submodule there\", and the latter\n> is still reported as \"removed\".  That is exactly what your patch does.\n\nHaving looked at the code a bit more, I do not think we need the\nthree-kind distinction for this part.\n\nThe attached patch would be both sufficient and cleaner.  The real change\nis a single-liner, and everything else is additional comment ;-)  I'd\nfollow it up with s/check_work_tree_entity/check_removed/ for\nclarification.\n\nA cleaned up series is queued near the tip of 'pu' for tonight, but 'pu'\nitself has some uncompiable crap in it (not your series) and needs to be\nrebuilt soon.\n\n---\n[PATCH] diff: a submodule not checked out is not modified\n\n948dd34 (diff-index: careful when inspecting work tree items, 2008-03-30)\nmade the work tree check careful not to be fooled by a new directory that\nexists at a place the index expects a blob.  For such a change to be a\ntypechange from blob to submodule, the new directory has to be a\nrepository.\n\nHowever, if the index expects a submodule there, we should not insist the\nwork tree entity to be a repository --- a simple directory that is not a\nfull fledged repository (even an empty directory would do) should be\nconsidered an unmodified subproject, because that is how a superproject\nwith a submodule is checked out sparsely by default.\n\nThis makes the function check_work_tree_entity() even more careful not to\nreport a submodule that is not checked out as removed.  It fixes the\nrecently added test in t4027.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff-lib.c                |   25 ++++++++++++++++++++++---\n t/t4027-diff-submodule.sh |    2 +-\n 2 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex cfd629d..e0ebcdc 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -337,9 +337,15 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)\n \t}\n \treturn run_diff_files(revs, options);\n }\n+\n /*\n- * See if work tree has an entity that can be staged.  Return 0 if so,\n- * return 1 if not and return -1 if error.\n+ * Has the work tree entity been removed?\n+ *\n+ * Return 1 if it was removed from the work tree, 0 if an entity to be\n+ * compared with the cache entry ce still exists (the latter includes\n+ * the case where a directory that is not a submodule repository\n+ * exists for ce that is a submodule -- it is a submodule that is not\n+ * checked out).  Return negative for an error.\n  */\n static int check_work_tree_entity(const struct cache_entry *ce, struct stat *st, char *symcache)\n {\n@@ -352,7 +358,20 @@ static int check_work_tree_entity(const struct cache_entry *ce, struct stat *st,\n \t\treturn 1;\n \tif (S_ISDIR(st->st_mode)) {\n \t\tunsigned char sub[20];\n-\t\tif (resolve_gitlink_ref(ce->name, \"HEAD\", sub))\n+\n+\t\t/*\n+\t\t * If ce is already a gitlink, we can have a plain\n+\t\t * directory (i.e. the submodule is not checked out),\n+\t\t * or a checked out submodule.  Either case this is not\n+\t\t * a case where something was removed from the work tree,\n+\t\t * so we will return 0.\n+\t\t *\n+\t\t * Otherwise, if the directory is not a submodule\n+\t\t * repository, that means ce which was a blob turned into\n+\t\t * a directory --- the blob was removed!\n+\t\t */\n+\t\tif (!S_ISGITLINK(ce->ce_mode) &&\n+\t\t    resolve_gitlink_ref(ce->name, \"HEAD\", sub))\n \t\t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 61caad0..ba6679c 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -50,7 +50,7 @@ test_expect_success 'git diff-files --raw' '\n \ttest_cmp expect actual.files\n '\n \n-test_expect_failure 'git diff (empty submodule dir)' '\n+test_expect_success 'git diff (empty submodule dir)' '\n \t: >empty &&\n \trm -rf sub/* sub/.git &&\n \tgit diff > actual.empty &&\n-- \n1.5.5.1.219.gb8f92\n"},{"id":"75994","messageId":"46dff0320805040010s2dce0f7cr82548088e08ff54a@mail.gmail.com","threadId":"13212","inReplyTo":"7vej8ir2ik.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] Fix diff regression for submodules not checked out","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T07:10:05Z","receivedAt":"2008-05-04T07:10:05Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>  > The second case is \"not checked out -- treat me as unmodified\", and the\n>  > third case is \"the user does not want the submodule there\", and the latter\n>  > is still reported as \"removed\".  That is exactly what your patch does.\n>\n>  Having looked at the code a bit more, I do not think we need the\n>  three-kind distinction for this part.\n>\n>  The attached patch would be both sufficient and cleaner.  The real change\n>  is a single-liner, and everything else is additional comment ;-)  I'd\n>  follow it up with s/check_work_tree_entity/check_removed/ for\n>  clarification.\n>\n\nFine, it's cleaner.\n\n-- \nPing Yin\n"}]}