Jeff King <[email protected]> writes:
> On Fri, Aug 28, 2015 at 02:11:01PM -0700, Junio C Hamano wrote:
>
>> * jk/log-missing-default-HEAD (2015-06-03) 1 commit
>> - log: diagnose empty HEAD more clearly
>>
>> "git init empty && git -C empty log" said "bad default revision 'HEAD'",
>> which was found to be a bit confusing to new users.
>>
>> What's the status of this one?
>
> You and I both provided patches, and you queued mine, but I think there
> was some question of whether the error messages were actually much
> better. Here's a re-roll that tries to improve them by avoiding the word
> "HEAD", which the user did not provide in the first place.
>
> -- >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: your current branch 'master' does not have any commits yet
>
> 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 a generic 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 <[email protected]>
> ---
> Note that this doesn't take us any closer to a world where "git log" on
> an empty HEAD silently exits with success (which your patch did). I
> think it is somewhat orthogonal, though. If we wanted to do that we
> would probably still die for a while (as your patch did), and it would
> make sense to die using this diagnose function.
>
> So I'd be happy if you wanted to resurrect yours on top, or squash them
> together. But I do not really think it is worth dealing with the
> compatibility surprises to make the change.
Thanks, let's take just one baby step and apply this for now.
Others can work on top after the dust settles.
>
> revision.c | 17 ++++++++++++++++-
> t/t4202-log.sh | 14 ++++++++++++++
> 2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index 5350139..af2a18e 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2187,6 +2187,21 @@ 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 (!refname || !(flags & REF_ISSYMREF) || (flags & REF_ISBROKEN))
> + die(_("your current branch appears to be broken"));
> +
> + skip_prefix(refname, "refs/heads/", &refname);
> + die(_("your current branch '%s' does not have any commits yet"),
> + refname);
> +}
> +
> /*
> * Parse revision information, filling in the "rev_info" structure,
> * and removing the used arguments from the argument list.
> @@ -2316,7 +2331,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 35d2d7c..6ede069 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -894,4 +894,18 @@ 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 does.not.have.any.commits stderr &&
> + echo 1234abcd >empty/.git/refs/heads/master &&
> + test_must_fail git -C empty log 2>stderr &&
> + test_i18ngrep broken stderr &&
> + echo "ref: refs/heads/invalid.lock" >empty/.git/HEAD &&
> + 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 broken stderr
> +'
> +
> test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
