{"thread":{"id":"39508","subject":"Minor bug report","startedAt":"2015-06-03T05:54:19Z","lastAt":"2015-06-05T20:47:40Z","messageCount":11,"participants":["Tummala Dhanvi","Jeff King","Junio C Hamano","Dennis Kaarsemaker","Stefan Näwe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"262810","messageId":"CAMOUyJ_2snCaKcWNoOgH7aoHrdbUsCucqu_W_J3aH9_QQe1jEw@mail.gmail.com","threadId":"39508","inReplyTo":null,"subject":"Minor bug report","fromName":"Tummala Dhanvi","fromEmail":"dhanvicse@gmail.com","sentAt":"2015-06-03T05:54:19Z","receivedAt":"2015-06-03T05:54:19Z","isPatch":false,"sender":{"key":"dhanvicse@gmail.com","avatar":null},"body":"When we do create a new empty git repo using git init or create a\norphan branch and do a git log then I am getting an error saying that\nfatal: bad default revision 'HEAD'\n\nWell the error should have been something like no commits to show\neither the branch is orphan / you didn't make any commits in the new\nrepo\n\nI would like to fix the trival bug myself can some one point me in the\nright direction to fix it\n"},{"id":"262811","messageId":"20150603062005.GA20580@peff.net","threadId":"39508","inReplyTo":"CAMOUyJ_2snCaKcWNoOgH7aoHrdbUsCucqu_W_J3aH9_QQe1jEw@mail.gmail.com","subject":"Re: Minor bug report","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-03T06:20:05Z","receivedAt":"2015-06-03T06:20:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 03, 2015 at 11:24:19AM +0530, Tummala Dhanvi wrote:\n\n> When we do create a new empty git repo using git init or create a\n> orphan branch and do a git log then I am getting an error saying that\n> fatal: bad default revision 'HEAD'\n> \n> Well the error should have been something like no commits to show\n> either the branch is orphan / you didn't make any commits in the new\n> repo\n> \n> I would like to fix the trival bug myself can some one point me in the\n> right direction to fix it\n\nHere are some prior discussions:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/75692\n\n  http://thread.gmane.org/gmane.comp.version-control.git/200504\n\nI just skimmed through them, but I expect the most desirable solution\nwould involve:\n\n  1. We still die(), but just improve the error message (so we don't\n     have any regressions for people expecting \"git log\" to fail).\n\n  2. We use the message only when pointing to an unborn branch, and not\n     on other errors. The simplest way to do this is probably to make an\n     extra call to resolve_ref() in the error code-path.\n\n-Peff\n"},{"id":"262818","messageId":"CAPc5daU9VdndgkMFNmtiwhVdwyGQgDbFX3rrrsEEEUVewgs-dw@mail.gmail.com","threadId":"39508","inReplyTo":"20150603062005.GA20580@peff.net","subject":"Re: Minor bug report","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-03T06:48:30Z","receivedAt":"2015-06-03T06:48:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, Jun 2, 2015 at 11:20 PM, Jeff King <peff@peff.net> wrote:\n>\n> Here are some prior discussions:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/75692\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/200504\n>\n> I just skimmed through them, but I expect the most desirable solution\n> would involve:\n>\n>   1. We still die(), but just improve the error message (so we don't\n>      have any regressions for people expecting \"git log\" to fail).\n>\n>   2. We use the message only when pointing to an unborn branch, and not\n>      on other errors. The simplest way to do this is probably to make an\n>      extra call to resolve_ref() in the error code-path.\n>\n> -Peff\n\nI am kind of surprised after reading these two threads that my\ntake on this issue has changed over time, as my knee-jerk\nreaction before reading them was the opposite, something\nalong the lines of \"This is only immediately after 'git init'; why\nbother?\". Or depending on my mood, that \"How stupid do you\nhave to be...\" sounds exactly like a message I may advocate\nfor us to send. Perhaps I grew more bitter with age.\n\nJokes aside, I wonder why we did not pursue that \"default to\nHEAD only when HEAD points at a branch that exists\"\napproach, with the definition of \"exists\" being \"either a file\nexists under the $GIT_DIR/refs/heads directory with any\n(including corrupt) contents, or an entry in the packed refs\nfile exists with any (including corrupt) contents\". With or\nwithout the error exit and warning, that sounds like a very\nsensible way to set the default, at least to me.\n"},{"id":"262844","messageId":"20150603081432.GA32000@peff.net","threadId":"39508","inReplyTo":"CAPc5daU9VdndgkMFNmtiwhVdwyGQgDbFX3rrrsEEEUVewgs-dw@mail.gmail.com","subject":"[PATCH] log: diagnose empty HEAD more clearly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-03T08:14:32Z","receivedAt":"2015-06-03T08:14:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 02, 2015 at 11:48:30PM -0700, Junio C Hamano wrote:\n\n> I am kind of surprised after reading these two threads that my\n> take on this issue has changed over time, as my knee-jerk\n> reaction before reading them was the opposite, something\n> along the lines of \"This is only immediately after 'git init'; why\n> bother?\". Or depending on my mood, that \"How stupid do you\n> have to be...\" sounds exactly like a message I may advocate\n> for us to send. Perhaps I grew more bitter with age.\n\nI think it is one of those cases where the message makes sense when you\nknow how git works. But a new user who does not even know what \"HEAD\" or\na ref actually is may find it a bit confusing. Saying \"we can't run\ngit-log, your repository empty!\" may seem redundantly obvious even to\nsuch a new user. But saying \"bad revision 'HEAD'\" may leave them saying\n\"eh, what is HEAD and why is it bad?\".\n\n> Jokes aside, I wonder why we did not pursue that \"default to\n> HEAD only when HEAD points at a branch that exists\"\n> approach, with the definition of \"exists\" being \"either a file\n> exists under the $GIT_DIR/refs/heads directory with any\n> (including corrupt) contents, or an entry in the packed refs\n> file exists with any (including corrupt) contents\". With or\n> without the error exit and warning, that sounds like a very\n> sensible way to set the default, at least to me.\n\nMy concern there would be risk of regression. I.e., that we would take\nsome case which used to error out and turn it into a silent noop. So I'd\nprefer to keep the behavior the same, and just modify the error code\npath. The end result is pretty similar to the user, because we obviously\ncannot produce any actual output either way.\n\nSomething like:\n\n-- >8 --\nSubject: log: diagnose empty HEAD more clearly\n\nIf you init or clone an empty repository, the initial\nmessage from running \"git log\" is not very friendly:\n\n  $ git init\n  Initialized empty Git repository in /home/peff/foo/.git/\n  $ git log\n  fatal: bad default revision 'HEAD'\n\nLet's detect this situation and write a more friendly\nmessage:\n\n  $ git log\n  fatal: default revision 'HEAD' points to an unborn branch 'master'\n\nWe also detect the case that 'HEAD' points to a broken ref;\nthis should be even less common, but is easy to see. Note\nthat we do not diagnose all possible cases. We rely on\nresolve_ref, which means we do not get information about\ncomplex cases. E.g., \"--default master\" would use dwim_ref\nto find \"refs/heads/master\", but we notice only that\n\"master\" does not exist. Similarly, a complex sha1\nexpression like \"--default HEAD^2\" will not resolve as a\nref.\n\nBut that's OK. We fall back to the existing error message in\nthose cases, and they are unlikely to be used anyway.\nCatching an empty or broken \"HEAD\" improves the common case,\nand the other cases are not regressed.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI'm not a big fan of the new error message, either, just because the\nterm \"unborn\" is also likely to be confusing to new users.\n\nWe can also probably get rid of mentioning \"HEAD\" completely. It is\nalways the default unless the user has overridden it explicitly. So we\ncould go with something like:\n\n  fatal: your default branch 'master' does not contain any commits\n\nor something. I dunno. Bikeshed colors welcome. Adding:\n\n  advise(\"try running 'git commit'\");\n\nis probably too pedantic. ;)\n\n revision.c     | 19 ++++++++++++++++++-\n t/t4202-log.sh | 11 +++++++++++\n 2 files changed, 29 insertions(+), 1 deletion(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7ddbaa0..775df8b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2170,6 +2170,23 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \treturn 1;\n }\n \n+static void NORETURN diagnose_missing_default(const char *def)\n+{\n+\tunsigned char sha1[20];\n+\tint flags;\n+\tconst char *refname;\n+\n+\trefname = resolve_ref_unsafe(def, 0, sha1, &flags);\n+\tif (!(flags & REF_ISSYMREF))\n+\t\tdie(_(\"bad default revision '%s'\"), def);\n+\tif (flags & REF_ISBROKEN)\n+\t\tdie(_(\"default revision '%s' points to a broken branch\"), def);\n+\n+\tskip_prefix(refname, \"refs/heads/\", &refname);\n+\tdie(_(\"default revision '%s' points to an unborn branch '%s'\"),\n+\t    def, refname);\n+}\n+\n /*\n  * Parse revision information, filling in the \"rev_info\" structure,\n  * and removing the used arguments from the argument list.\n@@ -2299,7 +2316,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tstruct object *object;\n \t\tstruct object_context oc;\n \t\tif (get_sha1_with_context(revs->def, 0, sha1, &oc))\n-\t\t\tdie(\"bad default revision '%s'\", revs->def);\n+\t\t\tdiagnose_missing_default(revs->def);\n \t\tobject = get_reference(revs, revs->def, sha1, 0);\n \t\tadd_pending_object_with_mode(revs, object, revs->def, oc.mode);\n \t}\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 1b2e981..71d8326 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -871,4 +871,15 @@ test_expect_success 'log --graph --no-walk is forbidden' '\n \ttest_must_fail git log --graph --no-walk\n '\n \n+test_expect_success 'log diagnoses bogus HEAD' '\n+\tgit init empty &&\n+\ttest_must_fail git -C empty log 2>stderr &&\n+\ttest_i18ngrep unborn stderr &&\n+\techo 1234abcd >empty/.git/refs/heads/master &&\n+\ttest_must_fail git -C empty log 2>stderr &&\n+\ttest_i18ngrep broken stderr &&\n+\ttest_must_fail git -C empty log --default totally-bogus 2>stderr &&\n+\ttest_i18ngrep bad.default.revision stderr\n+'\n+\n test_done\n-- \n2.4.2.745.g0aa058d\n"},{"id":"262871","messageId":"1433345954.21083.14.camel@kaarsemaker.net","threadId":"39508","inReplyTo":"CAPc5daU9VdndgkMFNmtiwhVdwyGQgDbFX3rrrsEEEUVewgs-dw@mail.gmail.com","subject":"Re: Minor bug report","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2015-06-03T15:39:14Z","receivedAt":"2015-06-03T15:39:14Z","isPatch":false,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote:\n> \n> I am kind of surprised after reading these two threads that my\n> take on this issue has changed over time, as my knee-jerk\n> reaction before reading them was the opposite, something\n> along the lines of \"This is only immediately after 'git init'; why\n> bother?\". Or depending on my mood, that \"How stupid do you\n> have to be...\" sounds exactly like a message I may advocate\n> for us to send. Perhaps I grew more bitter with age.\n\nThe \"fatal: Failed to resolve 'HEAD' as a valid ref.\" message, closely\nrelated to the \"fatal: bad default revision 'HEAD'\" message that started\nthis thread just came by in #git with the following situation:\n\n$ git init\n$ git add .\n# Oops, didn't want to add foo.xyz\n$ git reset foo.xyz\nfatal: Failed to resolve 'HEAD' as a valid ref.\n\nThe solution there is simple, git rm --cached, but I think git could\nproduce more helpful messages when a repo is empty.\n\nI think you are growing bitter with age ;)\n\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"262884","messageId":"xmqqpp5cyabx.fsf@gitster.dls.corp.google.com","threadId":"39508","inReplyTo":"20150603081432.GA32000@peff.net","subject":"Re: [PATCH] log: diagnose empty HEAD more clearly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-03T17:24:02Z","receivedAt":"2015-06-03T17:24:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> My concern there would be risk of regression. I.e., that we would take\n> some case which used to error out and turn it into a silent noop. So I'd\n> prefer to keep the behavior the same, and just modify the error code\n> path. The end result is pretty similar to the user, because we obviously\n> cannot produce any actual output either way.\n\nOkay.\n\n> Something like:\n>\n> -- >8 --\n> Subject: log: diagnose empty HEAD more clearly\n>\n> If you init or clone an empty repository, the initial\n> message from running \"git log\" is not very friendly:\n>\n>   $ git init\n>   Initialized empty Git repository in /home/peff/foo/.git/\n>   $ git log\n>   fatal: bad default revision 'HEAD'\n>\n> Let's detect this situation and write a more friendly\n> message:\n>\n>   $ git log\n>   fatal: default revision 'HEAD' points to an unborn branch 'master'\n>\n> We also detect the case that 'HEAD' points to a broken ref;\n> this should be even less common, but is easy to see. Note\n> that we do not diagnose all possible cases. We rely on\n> resolve_ref, which means we do not get information about\n> complex cases. E.g., \"--default master\" would use dwim_ref\n> to find \"refs/heads/master\", but we notice only that\n> \"master\" does not exist. Similarly, a complex sha1\n> expression like \"--default HEAD^2\" will not resolve as a\n> ref.\n>\n> But that's OK. We fall back to the existing error message in\n> those cases, and they are unlikely to be used anyway.\n> Catching an empty or broken \"HEAD\" improves the common case,\n> and the other cases are not regressed.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I'm not a big fan of the new error message, either, just because the\n> term \"unborn\" is also likely to be confusing to new users.\n>\n> We can also probably get rid of mentioning \"HEAD\" completely. It is\n> always the default unless the user has overridden it explicitly.\n\nI think that still mentioning \"HEAD\" goes directly against the\nreasoning you made in an earlier part of your message:\n\n> I think it is one of those cases where the message makes sense when you\n> know how git works. But a new user who does not even know what \"HEAD\" or\n> a ref actually is may find it a bit confusing. Saying \"we can't run\n> git-log, your repository empty!\" may seem redundantly obvious even to\n> such a new user. But saying \"bad revision 'HEAD'\" may leave them saying\n> \"eh, what is HEAD and why is it bad?\".\n\nand I agree with that reasoning: the user didn't say anything about\n\"HEAD\", so why complain about it?\n\nWhich is what led me to say \"Why are we defaulting to HEAD before\nchecking if it even exists?  Isn't that the root cause of this\nconfusion?  What happens if we stopped doing it?\"\n\nAnd I think the \"diagnose after finding that pending is empty and we\nwere given 'def'\" approach almost gives us the equivalent, which is\nwhy I said \"Okay\" above.\n\nIf we can make it possible to tell if that \"def\" was given by the\nuser (i.e. --default parameter) or by the machinery (e.g. \"git log\"\nand friends), then we can remove \"almost\" from the above sentence.\n\n> So we\n> could go with something like:\n>\n>   fatal: your default branch 'master' does not contain any commits\n>\n> or something. I dunno. Bikeshed colors welcome. Adding:\n>\n>   advise(\"try running 'git commit'\");\n>\n> is probably too pedantic. ;)\n\n;-)\n\nI am wondering if the attached is better, if only because it is of\nless impact.  I have die() there to avoid the behaviour change, but\nif we are going to have another future version where we are willing\nto take incompatiblity for better end-user experience like we did at\n2.0, I suspect turning it to warning() or even removing it might be\na candidate for such a version.  If you have one commit and ask\n\"log\", you get one commit back. If you have no commit and ask \"log\",\nI think it is OK to say that you should get nothing back without\nfuss.\n\n builtin/log.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 4c4e6be..3b568a1 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\trev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;\n \targc = setup_revisions(argc, argv, rev, opt);\n \n+\tif (!rev->pending.nr && !opt->def)\n+\t\tdie(\"you do not have a commit yet on your branch\");\n+\n \t/* Any arguments at this point are not recognized */\n \tif (argc > 1)\n \t\tdie(_(\"unrecognized argument: %s\"), argv[1]);\n@@ -404,6 +407,14 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \treturn git_diff_ui_config(var, value, cb);\n }\n \n+static void default_to_head_if_exists(struct setup_revision_opt *opt)\n+{\n+\tunsigned char unused[20];\n+\n+\tif (resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, unused, NULL))\n+\t\topt->def = \"HEAD\";\n+}\n+\n int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info rev;\n@@ -416,7 +427,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n \trev.diff = 1;\n \trev.simplify_history = 0;\n \tmemset(&opt, 0, sizeof(opt));\n-\topt.def = \"HEAD\";\n+\tdefault_to_head_if_exists(&opt);\n \topt.revarg_opt = REVARG_COMMITTISH;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n \tif (!rev.diffopt.output_format)\n@@ -530,7 +541,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \trev.diffopt.stat_width = -1; \t/* Scale to real terminal size */\n \n \tmemset(&opt, 0, sizeof(opt));\n-\topt.def = \"HEAD\";\n+\tdefault_to_head_if_exists(&opt);\n \topt.tweak = show_rev_tweak_rev;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n \n@@ -607,7 +618,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \tinit_reflog_walk(&rev.reflog_info);\n \trev.verbose_header = 1;\n \tmemset(&opt, 0, sizeof(opt));\n-\topt.def = \"HEAD\";\n+\tdefault_to_head_if_exists(&opt);\n \tcmd_log_init_defaults(&rev);\n \trev.abbrev_commit = 1;\n \trev.commit_format = CMIT_FMT_ONELINE;\n@@ -629,7 +640,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&rev, prefix);\n \trev.always_show_header = 1;\n \tmemset(&opt, 0, sizeof(opt));\n-\topt.def = \"HEAD\";\n+\tdefault_to_head_if_exists(&opt);\n \topt.revarg_opt = REVARG_COMMITTISH;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n \treturn cmd_log_walk(&rev);\n"},{"id":"262930","messageId":"556FFEB8.4050407@atlas-elektronik.com","threadId":"39508","inReplyTo":"xmqqpp5cyabx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] log: diagnose empty HEAD more clearly","fromName":"Stefan Näwe","fromEmail":"stefan.naewe@atlas-elektronik.com","sentAt":"2015-06-04T07:31:04Z","receivedAt":"2015-06-04T07:31:04Z","isPatch":true,"sender":{"key":"stefan.naewe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4468?v=4"},"body":"Am 03.06.2015 um 19:24 schrieb Junio C Hamano:\n> Jeff King <peff@peff.net> writes:\n> \n>> My concern there would be risk of regression. I.e., that we would take\n>> some case which used to error out and turn it into a silent noop. So I'd\n>> prefer to keep the behavior the same, and just modify the error code\n>> path. The end result is pretty similar to the user, because we obviously\n>> cannot produce any actual output either way.\n> \n> Okay.\n> \n>> Something like:\n>>\n>> -- >8 --\n>> Subject: log: diagnose empty HEAD more clearly\n>>\n>> If you init or clone an empty repository, the initial\n>> message from running \"git log\" is not very friendly:\n>>\n>>   $ git init\n>>   Initialized empty Git repository in /home/peff/foo/.git/\n>>   $ git log\n>>   fatal: bad default revision 'HEAD'\n>>\n>> Let's detect this situation and write a more friendly\n>> message:\n>>\n>>   $ git log\n>>   fatal: default revision 'HEAD' points to an unborn branch 'master'\n>>\n>> We also detect the case that 'HEAD' points to a broken ref;\n>> this should be even less common, but is easy to see. Note\n>> that we do not diagnose all possible cases. We rely on\n>> resolve_ref, which means we do not get information about\n>> complex cases. E.g., \"--default master\" would use dwim_ref\n>> to find \"refs/heads/master\", but we notice only that\n>> \"master\" does not exist. Similarly, a complex sha1\n>> expression like \"--default HEAD^2\" will not resolve as a\n>> ref.\n>>\n>> But that's OK. We fall back to the existing error message in\n>> those cases, and they are unlikely to be used anyway.\n>> Catching an empty or broken \"HEAD\" improves the common case,\n>> and the other cases are not regressed.\n>>\n>> Signed-off-by: Jeff King <peff@peff.net>\n>> ---\n>> I'm not a big fan of the new error message, either, just because the\n>> term \"unborn\" is also likely to be confusing to new users.\n>>\n>> We can also probably get rid of mentioning \"HEAD\" completely. It is\n>> always the default unless the user has overridden it explicitly.\n> \n> I think that still mentioning \"HEAD\" goes directly against the\n> reasoning you made in an earlier part of your message:\n> \n>> I think it is one of those cases where the message makes sense when you\n>> know how git works. But a new user who does not even know what \"HEAD\" or\n>> a ref actually is may find it a bit confusing. Saying \"we can't run\n>> git-log, your repository empty!\" may seem redundantly obvious even to\n>> such a new user. But saying \"bad revision 'HEAD'\" may leave them saying\n>> \"eh, what is HEAD and why is it bad?\".\n> \n> and I agree with that reasoning: the user didn't say anything about\n> \"HEAD\", so why complain about it?\n> \n> Which is what led me to say \"Why are we defaulting to HEAD before\n> checking if it even exists?  Isn't that the root cause of this\n> confusion?  What happens if we stopped doing it?\"\n> \n> And I think the \"diagnose after finding that pending is empty and we\n> were given 'def'\" approach almost gives us the equivalent, which is\n> why I said \"Okay\" above.\n> \n> If we can make it possible to tell if that \"def\" was given by the\n> user (i.e. --default parameter) or by the machinery (e.g. \"git log\"\n> and friends), then we can remove \"almost\" from the above sentence.\n> \n>> So we\n>> could go with something like:\n>>\n>>   fatal: your default branch 'master' does not contain any commits\n>>\n>> or something. I dunno. Bikeshed colors welcome. Adding:\n>>\n>>   advise(\"try running 'git commit'\");\n>>\n>> is probably too pedantic. ;)\n> \n> ;-)\n> \n> I am wondering if the attached is better, if only because it is of\n> less impact.  I have die() there to avoid the behaviour change, but\n> if we are going to have another future version where we are willing\n> to take incompatiblity for better end-user experience like we did at\n> 2.0, I suspect turning it to warning() or even removing it might be\n> a candidate for such a version.  If you have one commit and ask\n> \"log\", you get one commit back. If you have no commit and ask \"log\",\n> I think it is OK to say that you should get nothing back without\n> fuss.\n> \n>  builtin/log.c | 19 +++++++++++++++----\n>  1 file changed, 15 insertions(+), 4 deletions(-)\n> \n> diff --git a/builtin/log.c b/builtin/log.c\n> index 4c4e6be..3b568a1 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>  \t\trev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;\n>  \targc = setup_revisions(argc, argv, rev, opt);\n>  \n> +\tif (!rev->pending.nr && !opt->def)\n> +\t\tdie(\"you do not have a commit yet on your branch\");\n\nI am not a native english speaker but shouldn't this be:\n\n  \"you do not have a commit on your branch yet\"\n\n??\n\n> [...]\n\nStefan\n-- \n----------------------------------------------------------------\n/dev/random says: I bet you I could stop gambling.\npython -c \"print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')\" \nGPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF\n"},{"id":"262938","messageId":"20150604082125.GA5771@peff.net","threadId":"39508","inReplyTo":"1433345954.21083.14.camel@kaarsemaker.net","subject":"Re: Minor bug report","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-04T08:21:25Z","receivedAt":"2015-06-04T08:21:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 03, 2015 at 05:39:14PM +0200, Dennis Kaarsemaker wrote:\n\n> On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote:\n> > \n> > I am kind of surprised after reading these two threads that my\n> > take on this issue has changed over time, as my knee-jerk\n> > reaction before reading them was the opposite, something\n> > along the lines of \"This is only immediately after 'git init'; why\n> > bother?\". Or depending on my mood, that \"How stupid do you\n> > have to be...\" sounds exactly like a message I may advocate\n> > for us to send. Perhaps I grew more bitter with age.\n> \n> The \"fatal: Failed to resolve 'HEAD' as a valid ref.\" message, closely\n> related to the \"fatal: bad default revision 'HEAD'\" message that started\n> this thread just came by in #git with the following situation:\n> \n> $ git init\n> $ git add .\n> # Oops, didn't want to add foo.xyz\n> $ git reset foo.xyz\n> fatal: Failed to resolve 'HEAD' as a valid ref.\n> \n> The solution there is simple, git rm --cached, but I think git could\n> produce more helpful messages when a repo is empty.\n\nYeah, I think there are a lot of places we could handle unborn branches\nbetter. We've slowly been smoothing these rough edges over the years\n(usually by using the empty tree when we wanted HEAD to be a tree-ish).\n\nPatches welcome. :)\n\n-Peff\n"},{"id":"262940","messageId":"20150604083430.GB5771@peff.net","threadId":"39508","inReplyTo":"xmqqpp5cyabx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] log: diagnose empty HEAD more clearly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-04T08:34:30Z","receivedAt":"2015-06-04T08:34:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 03, 2015 at 10:24:02AM -0700, Junio C Hamano wrote:\n\n> Which is what led me to say \"Why are we defaulting to HEAD before\n> checking if it even exists?  Isn't that the root cause of this\n> confusion?  What happens if we stopped doing it?\"\n> \n> And I think the \"diagnose after finding that pending is empty and we\n> were given 'def'\" approach almost gives us the equivalent, which is\n> why I said \"Okay\" above.\n> \n> If we can make it possible to tell if that \"def\" was given by the\n> user (i.e. --default parameter) or by the machinery (e.g. \"git log\"\n> and friends), then we can remove \"almost\" from the above sentence.\n\nMy thought was that we would not stop dying (for compatibility), and\nthat if we keep dying, it is effectively the same as what I proposed\n(perhaps slightly worse, in the sense that we do not diagnose \"--default\nfoo\" as thoroughly, but effectively the same in that basically nobody\nuses \"--default\" in the first place).\n\nBut if you are OK to eventually stop dying, I think this line of\nreasoning is OK.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 4c4e6be..3b568a1 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>  \t\trev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;\n>  \targc = setup_revisions(argc, argv, rev, opt);\n>  \n> +\tif (!rev->pending.nr && !opt->def)\n> +\t\tdie(\"you do not have a commit yet on your branch\");\n\nDo we want to mention the name of the branch here? I guess it does not\nreally matter. Perhaps \"the current branch\" would be better than \"your\nbranch\", though. Maybe:\n\n  fatal: you do not have any commits yet on the current branch\n\nThis message hopefully goes away in the long run, but we'll have it for\na while.\n\n> +static void default_to_head_if_exists(struct setup_revision_opt *opt)\n> +{\n> +\tunsigned char unused[20];\n> +\n> +\tif (resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, unused, NULL))\n> +\t\topt->def = \"HEAD\";\n> +}\n\nThis approach looks sane to me. Want to wrap it up with a commit\nmessage and a test?\n\n-Peff\n"},{"id":"262942","messageId":"20150604084505.GC5771@peff.net","threadId":"39508","inReplyTo":"556FFEB8.4050407@atlas-elektronik.com","subject":"Re: [PATCH] log: diagnose empty HEAD more clearly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-04T08:45:06Z","receivedAt":"2015-06-04T08:45:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 04, 2015 at 09:31:04AM +0200, Stefan Näwe wrote:\n\n> > +\tif (!rev->pending.nr && !opt->def)\n> > +\t\tdie(\"you do not have a commit yet on your branch\");\n> \n> I am not a native english speaker but shouldn't this be:\n> \n>   \"you do not have a commit on your branch yet\"\n\nBoth are fine, as is:\n\n  \"you do not yet have a commit on your branch\"\n\nthough I think yours is slightly more clear.\n\nIf you are wondering at the reason, \"yet\" is an adverb modifying \"have\".\nSo it may come before or after the verb. If we substitute a simpler verb\n(that does not need a direct object \"a commit\") and drop the\nprepositional phrase (\"on your branch\"), we can do either:\n\n  - you do not yet program\n\n  - you do not program yet\n\nIf we add back in the prepositional phrase (which is really acting as an\nadverbial phrase, modifying the verb here), we can do it in either\norder:\n\n  - you do not program yet in the office\n\n  - you do not program in the office yet\n\nBut the latter makes it more clear that the \"yet\" applies to \"in the\noffice\".\n\nYou can also add back in the object of the verb:\n\n  - you do not yet program computers\n\nbut you would not want:\n\n  - you do not program yet computers\n\nbecause it splits the verb from its object.\n\n</grammar rant>\n\n-Peff\n"},{"id":"263071","messageId":"xmqqvbf1rifn.fsf@gitster.dls.corp.google.com","threadId":"39508","inReplyTo":"20150604083430.GB5771@peff.net","subject":"Re: [PATCH] log: diagnose empty HEAD more clearly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-05T20:47:40Z","receivedAt":"2015-06-05T20:47:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But if you are OK to eventually stop dying, I think this line of\n> reasoning is OK.\n>\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 4c4e6be..3b568a1 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n>>  \t\trev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;\n>>  \targc = setup_revisions(argc, argv, rev, opt);\n>>  \n>> +\tif (!rev->pending.nr && !opt->def)\n>> +\t\tdie(\"you do not have a commit yet on your branch\");\n>\n> Do we want to mention the name of the branch here? I guess it does not\n> really matter. Perhaps \"the current branch\" would be better than \"your\n> branch\", though. Maybe:\n>\n>   fatal: you do not have any commits yet on the current branch\n>\n> This message hopefully goes away in the long run, but we'll have it for\n> a while.\n>\n>> +static void default_to_head_if_exists(struct setup_revision_opt *opt)\n>> +{\n>> +\tunsigned char unused[20];\n>> +\n>> +\tif (resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, unused, NULL))\n>> +\t\topt->def = \"HEAD\";\n>> +}\n>\n> This approach looks sane to me. Want to wrap it up with a commit\n> message and a test?\n\nThere is an unmerged attempt jc/log-missing-default-HEAD in my\n\"broken-out\" repository https://github.com/gitster/git; I do agree\nthat we should spend more cycles after deciding to error out to\ngive a more detailed diagnosis, but I didn't have enough energy\nto bring myself do that, so I've tentatively merged your version\ninstead of this one.\n"}]}