{"thread":{"id":"46839","subject":"[PATCH] submodule: correct error message for missing commits.","startedAt":"2017-09-26T18:28:04Z","lastAt":"2017-09-27T00:52:57Z","messageCount":3,"participants":["Stefan Beller","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"328955","messageId":"20170926182756.13319-1-sbeller@google.com","threadId":"46839","inReplyTo":null,"subject":"[PATCH] submodule: correct error message for missing commits.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-26T18:27:56Z","receivedAt":"2017-09-26T18:28:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a submodule diff should be displayed we currently just add the\nsubmodule objects to the main object store and then e.g. walk the\nrevision graph and create a summary for that submodule.\n\nIt is possible that we are missing the submodule either completely or\npartially, which we currently differentiate with different error messages\ndepending on whether (1) the whole submodule object store is missing or\n(2) just the needed for this particular diff. (1) is reported as\n\"not initialized\", and (2) is reported as \"commits not present\".\n\nIf a submodule is deinit'ed its repository data is still around inside\nthe superproject, such that the diff can still be produced. In that way\nthe error message (1) is misleading as we can have a diff despite the\nsubmodule being not initialized.\n\nDowngrade the error message (1) to be the same as (2) and just say\nthe commits are not present, as that is the true reason why the diff\ncannot be shown.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule.c                               | 2 +-\n t/t4059-diff-submodule-not-initialized.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 6531c5d609..280c246477 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -567,7 +567,7 @@ static void show_submodule_header(FILE *f, const char *path,\n \n \tif (add_submodule_odb(path)) {\n \t\tif (!message)\n-\t\t\tmessage = \"(not initialized)\";\n+\t\t\tmessage = \"(commits not present)\";\n \t\tgoto output_header;\n \t}\n \ndiff --git a/t/t4059-diff-submodule-not-initialized.sh b/t/t4059-diff-submodule-not-initialized.sh\nindex cd70fd5192..49bca7b48d 100755\n--- a/t/t4059-diff-submodule-not-initialized.sh\n+++ b/t/t4059-diff-submodule-not-initialized.sh\n@@ -95,7 +95,7 @@ test_expect_success 'submodule not initialized in new clone' '\n \tgit clone . sm3 &&\n \tgit -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n \tcat >expected <<-EOF &&\n-\tSubmodule sm1 $smhead1...$smhead2 (not initialized)\n+\tSubmodule sm1 $smhead1...$smhead2 (commits not present)\n \tEOF\n \ttest_cmp expected actual\n '\n-- \n2.14.0.rc0.3.g6c2e499285\n\n"},{"id":"328973","messageId":"CA+P7+xr6_7BW0q7iZsMdeexFff=A-8rp8Lfds2NTXmvu1tOqCw@mail.gmail.com","threadId":"46839","inReplyTo":"20170926182756.13319-1-sbeller@google.com","subject":"Re: [PATCH] submodule: correct error message for missing commits.","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-09-26T23:37:02Z","receivedAt":"2017-09-26T23:37:29Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Tue, Sep 26, 2017 at 11:27 AM, Stefan Beller <sbeller@google.com> wrote:\n> When a submodule diff should be displayed we currently just add the\n> submodule objects to the main object store and then e.g. walk the\n> revision graph and create a summary for that submodule.\n>\n> It is possible that we are missing the submodule either completely or\n> partially, which we currently differentiate with different error messages\n> depending on whether (1) the whole submodule object store is missing or\n> (2) just the needed for this particular diff. (1) is reported as\n> \"not initialized\", and (2) is reported as \"commits not present\".\n>\n> If a submodule is deinit'ed its repository data is still around inside\n> the superproject, such that the diff can still be produced. In that way\n> the error message (1) is misleading as we can have a diff despite the\n> submodule being not initialized.\n>\n> Downgrade the error message (1) to be the same as (2) and just say\n> the commits are not present, as that is the true reason why the diff\n> cannot be shown.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n\nThis makes sense to me.\n\nThanks,\nJake\n\n> ---\n>  submodule.c                               | 2 +-\n>  t/t4059-diff-submodule-not-initialized.sh | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 6531c5d609..280c246477 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -567,7 +567,7 @@ static void show_submodule_header(FILE *f, const char *path,\n>\n>         if (add_submodule_odb(path)) {\n>                 if (!message)\n> -                       message = \"(not initialized)\";\n> +                       message = \"(commits not present)\";\n>                 goto output_header;\n>         }\n>\n> diff --git a/t/t4059-diff-submodule-not-initialized.sh b/t/t4059-diff-submodule-not-initialized.sh\n> index cd70fd5192..49bca7b48d 100755\n> --- a/t/t4059-diff-submodule-not-initialized.sh\n> +++ b/t/t4059-diff-submodule-not-initialized.sh\n> @@ -95,7 +95,7 @@ test_expect_success 'submodule not initialized in new clone' '\n>         git clone . sm3 &&\n>         git -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&\n>         cat >expected <<-EOF &&\n> -       Submodule sm1 $smhead1...$smhead2 (not initialized)\n> +       Submodule sm1 $smhead1...$smhead2 (commits not present)\n>         EOF\n>         test_cmp expected actual\n>  '\n> --\n> 2.14.0.rc0.3.g6c2e499285\n>\n"},{"id":"328989","messageId":"xmqqlgl17yjj.fsf@gitster.mtv.corp.google.com","threadId":"46839","inReplyTo":"20170926182756.13319-1-sbeller@google.com","subject":"Re: [PATCH] submodule: correct error message for missing commits.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-27T00:52:48Z","receivedAt":"2017-09-27T00:52:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> When a submodule diff should be displayed we currently just add the\n> submodule objects to the main object store and then e.g. walk the\n> revision graph and create a summary for that submodule.\n>\n> It is possible that we are missing the submodule either completely or\n> partially, which we currently differentiate with different error messages\n> depending on whether (1) the whole submodule object store is missing or\n> (2) just the needed for this particular diff. (1) is reported as\n> \"not initialized\", and (2) is reported as \"commits not present\".\n>\n> If a submodule is deinit'ed its repository data is still around inside\n> the superproject, such that the diff can still be produced. In that way\n> the error message (1) is misleading as we can have a diff despite the\n> submodule being not initialized.\n>\n> Downgrade the error message (1) to be the same as (2) and just say\n> the commits are not present, as that is the true reason why the diff\n> cannot be shown.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n\nSounds good.  Thanks for working on it.\n\n"}]}