* Minor bug report @ 2015-06-03 5:54 Tummala Dhanvi 2015-06-03 6:20 ` Jeff King 0 siblings, 1 reply; 11+ messages in thread From: Tummala Dhanvi @ 2015-06-03 5:54 UTC (permalink / raw) To: git; +Cc: gitster When we do create a new empty git repo using git init or create a orphan branch and do a git log then I am getting an error saying that fatal: bad default revision 'HEAD' Well the error should have been something like no commits to show either the branch is orphan / you didn't make any commits in the new repo I would like to fix the trival bug myself can some one point me in the right direction to fix it ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Minor bug report 2015-06-03 5:54 Minor bug report Tummala Dhanvi @ 2015-06-03 6:20 ` Jeff King 2015-06-03 6:48 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Jeff King @ 2015-06-03 6:20 UTC (permalink / raw) To: Tummala Dhanvi; +Cc: git, gitster On Wed, Jun 03, 2015 at 11:24:19AM +0530, Tummala Dhanvi wrote: > When we do create a new empty git repo using git init or create a > orphan branch and do a git log then I am getting an error saying that > fatal: bad default revision 'HEAD' > > Well the error should have been something like no commits to show > either the branch is orphan / you didn't make any commits in the new > repo > > I would like to fix the trival bug myself can some one point me in the > right direction to fix it Here are some prior discussions: http://thread.gmane.org/gmane.comp.version-control.git/75692 http://thread.gmane.org/gmane.comp.version-control.git/200504 I just skimmed through them, but I expect the most desirable solution would involve: 1. We still die(), but just improve the error message (so we don't have any regressions for people expecting "git log" to fail). 2. We use the message only when pointing to an unborn branch, and not on other errors. The simplest way to do this is probably to make an extra call to resolve_ref() in the error code-path. -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Minor bug report 2015-06-03 6:20 ` Jeff King @ 2015-06-03 6:48 ` Junio C Hamano 2015-06-03 8:14 ` [PATCH] log: diagnose empty HEAD more clearly Jeff King 2015-06-03 15:39 ` Minor bug report Dennis Kaarsemaker 0 siblings, 2 replies; 11+ messages in thread From: Junio C Hamano @ 2015-06-03 6:48 UTC (permalink / raw) To: Jeff King; +Cc: Tummala Dhanvi, Git Mailing List On Tue, Jun 2, 2015 at 11:20 PM, Jeff King <peff@peff.net> wrote: > > Here are some prior discussions: > > http://thread.gmane.org/gmane.comp.version-control.git/75692 > > http://thread.gmane.org/gmane.comp.version-control.git/200504 > > I just skimmed through them, but I expect the most desirable solution > would involve: > > 1. We still die(), but just improve the error message (so we don't > have any regressions for people expecting "git log" to fail). > > 2. We use the message only when pointing to an unborn branch, and not > on other errors. The simplest way to do this is probably to make an > extra call to resolve_ref() in the error code-path. > > -Peff I am kind of surprised after reading these two threads that my take on this issue has changed over time, as my knee-jerk reaction before reading them was the opposite, something along the lines of "This is only immediately after 'git init'; why bother?". Or depending on my mood, that "How stupid do you have to be..." sounds exactly like a message I may advocate for us to send. Perhaps I grew more bitter with age. Jokes aside, I wonder why we did not pursue that "default to HEAD only when HEAD points at a branch that exists" approach, with the definition of "exists" being "either a file exists under the $GIT_DIR/refs/heads directory with any (including corrupt) contents, or an entry in the packed refs file exists with any (including corrupt) contents". With or without the error exit and warning, that sounds like a very sensible way to set the default, at least to me. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] log: diagnose empty HEAD more clearly 2015-06-03 6:48 ` Junio C Hamano @ 2015-06-03 8:14 ` Jeff King 2015-06-03 17:24 ` Junio C Hamano 2015-06-03 15:39 ` Minor bug report Dennis Kaarsemaker 1 sibling, 1 reply; 11+ messages in thread From: Jeff King @ 2015-06-03 8:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tummala Dhanvi, Git Mailing List On Tue, Jun 02, 2015 at 11:48:30PM -0700, Junio C Hamano wrote: > I am kind of surprised after reading these two threads that my > take on this issue has changed over time, as my knee-jerk > reaction before reading them was the opposite, something > along the lines of "This is only immediately after 'git init'; why > bother?". Or depending on my mood, that "How stupid do you > have to be..." sounds exactly like a message I may advocate > for us to send. Perhaps I grew more bitter with age. I think it is one of those cases where the message makes sense when you know how git works. But a new user who does not even know what "HEAD" or a ref actually is may find it a bit confusing. Saying "we can't run git-log, your repository empty!" may seem redundantly obvious even to such a new user. But saying "bad revision 'HEAD'" may leave them saying "eh, what is HEAD and why is it bad?". > Jokes aside, I wonder why we did not pursue that "default to > HEAD only when HEAD points at a branch that exists" > approach, with the definition of "exists" being "either a file > exists under the $GIT_DIR/refs/heads directory with any > (including corrupt) contents, or an entry in the packed refs > file exists with any (including corrupt) contents". With or > without the error exit and warning, that sounds like a very > sensible way to set the default, at least to me. My concern there would be risk of regression. I.e., that we would take some case which used to error out and turn it into a silent noop. So I'd prefer to keep the behavior the same, and just modify the error code path. The end result is pretty similar to the user, because we obviously cannot produce any actual output either way. Something like: -- >8 -- Subject: log: diagnose empty HEAD more clearly If you init or clone an empty repository, the initial message from running "git log" is not very friendly: $ git init Initialized empty Git repository in /home/peff/foo/.git/ $ git log fatal: bad default revision 'HEAD' Let's detect this situation and write a more friendly message: $ git log fatal: default revision 'HEAD' points to an unborn branch 'master' We also detect the case that 'HEAD' points to a broken ref; this should be even less common, but is easy to see. Note that we do not diagnose all possible cases. We rely on resolve_ref, which means we do not get information about complex cases. E.g., "--default master" would use dwim_ref to find "refs/heads/master", but we notice only that "master" does not exist. Similarly, a complex sha1 expression like "--default HEAD^2" will not resolve as a ref. But that's OK. We fall back to the existing error message in those cases, and they are unlikely to be used anyway. Catching an empty or broken "HEAD" improves the common case, and the other cases are not regressed. Signed-off-by: Jeff King <peff@peff.net> --- I'm not a big fan of the new error message, either, just because the term "unborn" is also likely to be confusing to new users. We can also probably get rid of mentioning "HEAD" completely. It is always the default unless the user has overridden it explicitly. So we could go with something like: fatal: your default branch 'master' does not contain any commits or something. I dunno. Bikeshed colors welcome. Adding: advise("try running 'git commit'"); is probably too pedantic. ;) revision.c | 19 ++++++++++++++++++- t/t4202-log.sh | 11 +++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/revision.c b/revision.c index 7ddbaa0..775df8b 100644 --- a/revision.c +++ b/revision.c @@ -2170,6 +2170,23 @@ static int handle_revision_pseudo_opt(const char *submodule, return 1; } +static void NORETURN diagnose_missing_default(const char *def) +{ + unsigned char sha1[20]; + int flags; + const char *refname; + + refname = resolve_ref_unsafe(def, 0, sha1, &flags); + if (!(flags & REF_ISSYMREF)) + die(_("bad default revision '%s'"), def); + if (flags & REF_ISBROKEN) + die(_("default revision '%s' points to a broken branch"), def); + + skip_prefix(refname, "refs/heads/", &refname); + die(_("default revision '%s' points to an unborn branch '%s'"), + def, refname); +} + /* * Parse revision information, filling in the "rev_info" structure, * and removing the used arguments from the argument list. @@ -2299,7 +2316,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s struct object *object; struct object_context oc; if (get_sha1_with_context(revs->def, 0, sha1, &oc)) - die("bad default revision '%s'", revs->def); + diagnose_missing_default(revs->def); object = get_reference(revs, revs->def, sha1, 0); add_pending_object_with_mode(revs, object, revs->def, oc.mode); } diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 1b2e981..71d8326 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -871,4 +871,15 @@ test_expect_success 'log --graph --no-walk is forbidden' ' test_must_fail git log --graph --no-walk ' +test_expect_success 'log diagnoses bogus HEAD' ' + git init empty && + test_must_fail git -C empty log 2>stderr && + test_i18ngrep unborn stderr && + echo 1234abcd >empty/.git/refs/heads/master && + test_must_fail git -C empty log 2>stderr && + test_i18ngrep broken stderr && + test_must_fail git -C empty log --default totally-bogus 2>stderr && + test_i18ngrep bad.default.revision stderr +' + test_done -- 2.4.2.745.g0aa058d ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] log: diagnose empty HEAD more clearly 2015-06-03 8:14 ` [PATCH] log: diagnose empty HEAD more clearly Jeff King @ 2015-06-03 17:24 ` Junio C Hamano 2015-06-04 7:31 ` Stefan Näwe 2015-06-04 8:34 ` Jeff King 0 siblings, 2 replies; 11+ messages in thread From: Junio C Hamano @ 2015-06-03 17:24 UTC (permalink / raw) To: Jeff King; +Cc: Tummala Dhanvi, Git Mailing List Jeff King <peff@peff.net> writes: > My concern there would be risk of regression. I.e., that we would take > some case which used to error out and turn it into a silent noop. So I'd > prefer to keep the behavior the same, and just modify the error code > path. The end result is pretty similar to the user, because we obviously > cannot produce any actual output either way. Okay. > Something like: > > -- >8 -- > Subject: log: diagnose empty HEAD more clearly > > If you init or clone an empty repository, the initial > message from running "git log" is not very friendly: > > $ git init > Initialized empty Git repository in /home/peff/foo/.git/ > $ git log > fatal: bad default revision 'HEAD' > > Let's detect this situation and write a more friendly > message: > > $ git log > fatal: default revision 'HEAD' points to an unborn branch 'master' > > We also detect the case that 'HEAD' points to a broken ref; > this should be even less common, but is easy to see. Note > that we do not diagnose all possible cases. We rely on > resolve_ref, which means we do not get information about > complex cases. E.g., "--default master" would use dwim_ref > to find "refs/heads/master", but we notice only that > "master" does not exist. Similarly, a complex sha1 > expression like "--default HEAD^2" will not resolve as a > ref. > > But that's OK. We fall back to the existing error message in > those cases, and they are unlikely to be used anyway. > Catching an empty or broken "HEAD" improves the common case, > and the other cases are not regressed. > > Signed-off-by: Jeff King <peff@peff.net> > --- > I'm not a big fan of the new error message, either, just because the > term "unborn" is also likely to be confusing to new users. > > We can also probably get rid of mentioning "HEAD" completely. It is > always the default unless the user has overridden it explicitly. I think that still mentioning "HEAD" goes directly against the reasoning you made in an earlier part of your message: > I think it is one of those cases where the message makes sense when you > know how git works. But a new user who does not even know what "HEAD" or > a ref actually is may find it a bit confusing. Saying "we can't run > git-log, your repository empty!" may seem redundantly obvious even to > such a new user. But saying "bad revision 'HEAD'" may leave them saying > "eh, what is HEAD and why is it bad?". and I agree with that reasoning: the user didn't say anything about "HEAD", so why complain about it? Which is what led me to say "Why are we defaulting to HEAD before checking if it even exists? Isn't that the root cause of this confusion? What happens if we stopped doing it?" And I think the "diagnose after finding that pending is empty and we were given 'def'" approach almost gives us the equivalent, which is why I said "Okay" above. If we can make it possible to tell if that "def" was given by the user (i.e. --default parameter) or by the machinery (e.g. "git log" and friends), then we can remove "almost" from the above sentence. > So we > could go with something like: > > fatal: your default branch 'master' does not contain any commits > > or something. I dunno. Bikeshed colors welcome. Adding: > > advise("try running 'git commit'"); > > is probably too pedantic. ;) ;-) I am wondering if the attached is better, if only because it is of less impact. I have die() there to avoid the behaviour change, but if we are going to have another future version where we are willing to take incompatiblity for better end-user experience like we did at 2.0, I suspect turning it to warning() or even removing it might be a candidate for such a version. If you have one commit and ask "log", you get one commit back. If you have no commit and ask "log", I think it is OK to say that you should get nothing back without fuss. builtin/log.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 4c4e6be..3b568a1 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT; argc = setup_revisions(argc, argv, rev, opt); + if (!rev->pending.nr && !opt->def) + die("you do not have a commit yet on your branch"); + /* Any arguments at this point are not recognized */ if (argc > 1) die(_("unrecognized argument: %s"), argv[1]); @@ -404,6 +407,14 @@ static int git_log_config(const char *var, const char *value, void *cb) return git_diff_ui_config(var, value, cb); } +static void default_to_head_if_exists(struct setup_revision_opt *opt) +{ + unsigned char unused[20]; + + if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL)) + opt->def = "HEAD"; +} + int cmd_whatchanged(int argc, const char **argv, const char *prefix) { struct rev_info rev; @@ -416,7 +427,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix) rev.diff = 1; rev.simplify_history = 0; memset(&opt, 0, sizeof(opt)); - opt.def = "HEAD"; + default_to_head_if_exists(&opt); opt.revarg_opt = REVARG_COMMITTISH; cmd_log_init(argc, argv, prefix, &rev, &opt); if (!rev.diffopt.output_format) @@ -530,7 +541,7 @@ int cmd_show(int argc, const char **argv, const char *prefix) rev.diffopt.stat_width = -1; /* Scale to real terminal size */ memset(&opt, 0, sizeof(opt)); - opt.def = "HEAD"; + default_to_head_if_exists(&opt); opt.tweak = show_rev_tweak_rev; cmd_log_init(argc, argv, prefix, &rev, &opt); @@ -607,7 +618,7 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix) init_reflog_walk(&rev.reflog_info); rev.verbose_header = 1; memset(&opt, 0, sizeof(opt)); - opt.def = "HEAD"; + default_to_head_if_exists(&opt); cmd_log_init_defaults(&rev); rev.abbrev_commit = 1; rev.commit_format = CMIT_FMT_ONELINE; @@ -629,7 +640,7 @@ int cmd_log(int argc, const char **argv, const char *prefix) init_revisions(&rev, prefix); rev.always_show_header = 1; memset(&opt, 0, sizeof(opt)); - opt.def = "HEAD"; + default_to_head_if_exists(&opt); opt.revarg_opt = REVARG_COMMITTISH; cmd_log_init(argc, argv, prefix, &rev, &opt); return cmd_log_walk(&rev); ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] log: diagnose empty HEAD more clearly 2015-06-03 17:24 ` Junio C Hamano @ 2015-06-04 7:31 ` Stefan Näwe 2015-06-04 8:45 ` Jeff King 2015-06-04 8:34 ` Jeff King 1 sibling, 1 reply; 11+ messages in thread From: Stefan Näwe @ 2015-06-04 7:31 UTC (permalink / raw) To: Junio C Hamano, Jeff King; +Cc: Tummala Dhanvi, Git Mailing List Am 03.06.2015 um 19:24 schrieb Junio C Hamano: > Jeff King <peff@peff.net> writes: > >> My concern there would be risk of regression. I.e., that we would take >> some case which used to error out and turn it into a silent noop. So I'd >> prefer to keep the behavior the same, and just modify the error code >> path. The end result is pretty similar to the user, because we obviously >> cannot produce any actual output either way. > > Okay. > >> Something like: >> >> -- >8 -- >> Subject: log: diagnose empty HEAD more clearly >> >> If you init or clone an empty repository, the initial >> message from running "git log" is not very friendly: >> >> $ git init >> Initialized empty Git repository in /home/peff/foo/.git/ >> $ git log >> fatal: bad default revision 'HEAD' >> >> Let's detect this situation and write a more friendly >> message: >> >> $ git log >> fatal: default revision 'HEAD' points to an unborn branch 'master' >> >> We also detect the case that 'HEAD' points to a broken ref; >> this should be even less common, but is easy to see. Note >> that we do not diagnose all possible cases. We rely on >> resolve_ref, which means we do not get information about >> complex cases. E.g., "--default master" would use dwim_ref >> to find "refs/heads/master", but we notice only that >> "master" does not exist. Similarly, a complex sha1 >> expression like "--default HEAD^2" will not resolve as a >> ref. >> >> But that's OK. We fall back to the existing error message in >> those cases, and they are unlikely to be used anyway. >> Catching an empty or broken "HEAD" improves the common case, >> and the other cases are not regressed. >> >> Signed-off-by: Jeff King <peff@peff.net> >> --- >> I'm not a big fan of the new error message, either, just because the >> term "unborn" is also likely to be confusing to new users. >> >> We can also probably get rid of mentioning "HEAD" completely. It is >> always the default unless the user has overridden it explicitly. > > I think that still mentioning "HEAD" goes directly against the > reasoning you made in an earlier part of your message: > >> I think it is one of those cases where the message makes sense when you >> know how git works. But a new user who does not even know what "HEAD" or >> a ref actually is may find it a bit confusing. Saying "we can't run >> git-log, your repository empty!" may seem redundantly obvious even to >> such a new user. But saying "bad revision 'HEAD'" may leave them saying >> "eh, what is HEAD and why is it bad?". > > and I agree with that reasoning: the user didn't say anything about > "HEAD", so why complain about it? > > Which is what led me to say "Why are we defaulting to HEAD before > checking if it even exists? Isn't that the root cause of this > confusion? What happens if we stopped doing it?" > > And I think the "diagnose after finding that pending is empty and we > were given 'def'" approach almost gives us the equivalent, which is > why I said "Okay" above. > > If we can make it possible to tell if that "def" was given by the > user (i.e. --default parameter) or by the machinery (e.g. "git log" > and friends), then we can remove "almost" from the above sentence. > >> So we >> could go with something like: >> >> fatal: your default branch 'master' does not contain any commits >> >> or something. I dunno. Bikeshed colors welcome. Adding: >> >> advise("try running 'git commit'"); >> >> is probably too pedantic. ;) > > ;-) > > I am wondering if the attached is better, if only because it is of > less impact. I have die() there to avoid the behaviour change, but > if we are going to have another future version where we are willing > to take incompatiblity for better end-user experience like we did at > 2.0, I suspect turning it to warning() or even removing it might be > a candidate for such a version. If you have one commit and ask > "log", you get one commit back. If you have no commit and ask "log", > I think it is OK to say that you should get nothing back without > fuss. > > builtin/log.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/builtin/log.c b/builtin/log.c > index 4c4e6be..3b568a1 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, > rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT; > argc = setup_revisions(argc, argv, rev, opt); > > + if (!rev->pending.nr && !opt->def) > + die("you do not have a commit yet on your branch"); I am not a native english speaker but shouldn't this be: "you do not have a commit on your branch yet" ?? > [...] Stefan -- ---------------------------------------------------------------- /dev/random says: I bet you I could stop gambling. python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] log: diagnose empty HEAD more clearly 2015-06-04 7:31 ` Stefan Näwe @ 2015-06-04 8:45 ` Jeff King 0 siblings, 0 replies; 11+ messages in thread From: Jeff King @ 2015-06-04 8:45 UTC (permalink / raw) To: Stefan Näwe; +Cc: Junio C Hamano, Tummala Dhanvi, Git Mailing List On Thu, Jun 04, 2015 at 09:31:04AM +0200, Stefan Näwe wrote: > > + if (!rev->pending.nr && !opt->def) > > + die("you do not have a commit yet on your branch"); > > I am not a native english speaker but shouldn't this be: > > "you do not have a commit on your branch yet" Both are fine, as is: "you do not yet have a commit on your branch" though I think yours is slightly more clear. If you are wondering at the reason, "yet" is an adverb modifying "have". So it may come before or after the verb. If we substitute a simpler verb (that does not need a direct object "a commit") and drop the prepositional phrase ("on your branch"), we can do either: - you do not yet program - you do not program yet If we add back in the prepositional phrase (which is really acting as an adverbial phrase, modifying the verb here), we can do it in either order: - you do not program yet in the office - you do not program in the office yet But the latter makes it more clear that the "yet" applies to "in the office". You can also add back in the object of the verb: - you do not yet program computers but you would not want: - you do not program yet computers because it splits the verb from its object. </grammar rant> -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] log: diagnose empty HEAD more clearly 2015-06-03 17:24 ` Junio C Hamano 2015-06-04 7:31 ` Stefan Näwe @ 2015-06-04 8:34 ` Jeff King 2015-06-05 20:47 ` Junio C Hamano 1 sibling, 1 reply; 11+ messages in thread From: Jeff King @ 2015-06-04 8:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tummala Dhanvi, Git Mailing List On Wed, Jun 03, 2015 at 10:24:02AM -0700, Junio C Hamano wrote: > Which is what led me to say "Why are we defaulting to HEAD before > checking if it even exists? Isn't that the root cause of this > confusion? What happens if we stopped doing it?" > > And I think the "diagnose after finding that pending is empty and we > were given 'def'" approach almost gives us the equivalent, which is > why I said "Okay" above. > > If we can make it possible to tell if that "def" was given by the > user (i.e. --default parameter) or by the machinery (e.g. "git log" > and friends), then we can remove "almost" from the above sentence. My thought was that we would not stop dying (for compatibility), and that if we keep dying, it is effectively the same as what I proposed (perhaps slightly worse, in the sense that we do not diagnose "--default foo" as thoroughly, but effectively the same in that basically nobody uses "--default" in the first place). But if you are OK to eventually stop dying, I think this line of reasoning is OK. > diff --git a/builtin/log.c b/builtin/log.c > index 4c4e6be..3b568a1 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, > rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT; > argc = setup_revisions(argc, argv, rev, opt); > > + if (!rev->pending.nr && !opt->def) > + die("you do not have a commit yet on your branch"); Do we want to mention the name of the branch here? I guess it does not really matter. Perhaps "the current branch" would be better than "your branch", though. Maybe: fatal: you do not have any commits yet on the current branch This message hopefully goes away in the long run, but we'll have it for a while. > +static void default_to_head_if_exists(struct setup_revision_opt *opt) > +{ > + unsigned char unused[20]; > + > + if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL)) > + opt->def = "HEAD"; > +} This approach looks sane to me. Want to wrap it up with a commit message and a test? -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] log: diagnose empty HEAD more clearly 2015-06-04 8:34 ` Jeff King @ 2015-06-05 20:47 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2015-06-05 20:47 UTC (permalink / raw) To: Jeff King; +Cc: Tummala Dhanvi, Git Mailing List Jeff King <peff@peff.net> writes: > But if you are OK to eventually stop dying, I think this line of > reasoning is OK. > >> diff --git a/builtin/log.c b/builtin/log.c >> index 4c4e6be..3b568a1 100644 >> --- a/builtin/log.c >> +++ b/builtin/log.c >> @@ -148,6 +148,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, >> rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT; >> argc = setup_revisions(argc, argv, rev, opt); >> >> + if (!rev->pending.nr && !opt->def) >> + die("you do not have a commit yet on your branch"); > > Do we want to mention the name of the branch here? I guess it does not > really matter. Perhaps "the current branch" would be better than "your > branch", though. Maybe: > > fatal: you do not have any commits yet on the current branch > > This message hopefully goes away in the long run, but we'll have it for > a while. > >> +static void default_to_head_if_exists(struct setup_revision_opt *opt) >> +{ >> + unsigned char unused[20]; >> + >> + if (resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, unused, NULL)) >> + opt->def = "HEAD"; >> +} > > This approach looks sane to me. Want to wrap it up with a commit > message and a test? There is an unmerged attempt jc/log-missing-default-HEAD in my "broken-out" repository https://github.com/gitster/git; I do agree that we should spend more cycles after deciding to error out to give a more detailed diagnosis, but I didn't have enough energy to bring myself do that, so I've tentatively merged your version instead of this one. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Minor bug report 2015-06-03 6:48 ` Junio C Hamano 2015-06-03 8:14 ` [PATCH] log: diagnose empty HEAD more clearly Jeff King @ 2015-06-03 15:39 ` Dennis Kaarsemaker 2015-06-04 8:21 ` Jeff King 1 sibling, 1 reply; 11+ messages in thread From: Dennis Kaarsemaker @ 2015-06-03 15:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Tummala Dhanvi, Git Mailing List On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote: > > I am kind of surprised after reading these two threads that my > take on this issue has changed over time, as my knee-jerk > reaction before reading them was the opposite, something > along the lines of "This is only immediately after 'git init'; why > bother?". Or depending on my mood, that "How stupid do you > have to be..." sounds exactly like a message I may advocate > for us to send. Perhaps I grew more bitter with age. The "fatal: Failed to resolve 'HEAD' as a valid ref." message, closely related to the "fatal: bad default revision 'HEAD'" message that started this thread just came by in #git with the following situation: $ git init $ git add . # Oops, didn't want to add foo.xyz $ git reset foo.xyz fatal: Failed to resolve 'HEAD' as a valid ref. The solution there is simple, git rm --cached, but I think git could produce more helpful messages when a repo is empty. I think you are growing bitter with age ;) -- Dennis Kaarsemaker www.kaarsemaker.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Minor bug report 2015-06-03 15:39 ` Minor bug report Dennis Kaarsemaker @ 2015-06-04 8:21 ` Jeff King 0 siblings, 0 replies; 11+ messages in thread From: Jeff King @ 2015-06-04 8:21 UTC (permalink / raw) To: Dennis Kaarsemaker; +Cc: Junio C Hamano, Tummala Dhanvi, Git Mailing List On Wed, Jun 03, 2015 at 05:39:14PM +0200, Dennis Kaarsemaker wrote: > On di, 2015-06-02 at 23:48 -0700, Junio C Hamano wrote: > > > > I am kind of surprised after reading these two threads that my > > take on this issue has changed over time, as my knee-jerk > > reaction before reading them was the opposite, something > > along the lines of "This is only immediately after 'git init'; why > > bother?". Or depending on my mood, that "How stupid do you > > have to be..." sounds exactly like a message I may advocate > > for us to send. Perhaps I grew more bitter with age. > > The "fatal: Failed to resolve 'HEAD' as a valid ref." message, closely > related to the "fatal: bad default revision 'HEAD'" message that started > this thread just came by in #git with the following situation: > > $ git init > $ git add . > # Oops, didn't want to add foo.xyz > $ git reset foo.xyz > fatal: Failed to resolve 'HEAD' as a valid ref. > > The solution there is simple, git rm --cached, but I think git could > produce more helpful messages when a repo is empty. Yeah, I think there are a lot of places we could handle unborn branches better. We've slowly been smoothing these rough edges over the years (usually by using the empty tree when we wanted HEAD to be a tree-ish). Patches welcome. :) -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-06-05 20:47 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-03 5:54 Minor bug report Tummala Dhanvi 2015-06-03 6:20 ` Jeff King 2015-06-03 6:48 ` Junio C Hamano 2015-06-03 8:14 ` [PATCH] log: diagnose empty HEAD more clearly Jeff King 2015-06-03 17:24 ` Junio C Hamano 2015-06-04 7:31 ` Stefan Näwe 2015-06-04 8:45 ` Jeff King 2015-06-04 8:34 ` Jeff King 2015-06-05 20:47 ` Junio C Hamano 2015-06-03 15:39 ` Minor bug report Dennis Kaarsemaker 2015-06-04 8:21 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).