Some issues working with empty/bare repositories...

Some issues working with empty/bare repositories...

[Reece Dunn]

Hi,

Some git operations -- only tried clone and log -- require a
repository to contain at least one commit.

    $ git --version
    git version 1.5.3.1

While this is fine when working with existing projects, it can be
confusing/problematic when setting up a new repository.

Consider the following workflow:

    $ mkdir foo
    $ cd foo
    $ git --bare init

    $ git log
    fatal: bad default revision 'HEAD'

This message is confusing for a newbie. Displaying "no commits" would
make more sense here.

    $ cat HEAD
    ref: refs/heads/master
    $ find refs/heads/master
    find: refs/heads/master: No such file or directory

If the user wants to use the bare repository as the main location of
the project, but make changes to it elsewhere (e.g. when setting up a
superproject for adding submodules to), they might want to do
something like:

    $ git clone foo bar
    Initialized empty Git repository in /home/reece/bar/.git/
    $ ls -a /home/reece
    . .. foo

It appears from the git output (with the inconsistently capitalized
git - see --version output above) that the clone succeeded, but it
does not create bar/.git/, even though it said that it did.

- Reece

Re: Some issues working with empty/bare repositories...

[Jeff King]

On Sat, Mar 01, 2008 at 07:40:39PM +0000, Reece Dunn wrote:

> Consider the following workflow:
> 
>     $ mkdir foo
>     $ cd foo
>     $ git --bare init
> 
>     $ git log
>     fatal: bad default revision 'HEAD'
> 
> This message is confusing for a newbie. Displaying "no commits" would
> make more sense here.

The tricky thing here is that "git log" doesn't know we have no
commits in the repo; it only knows that HEAD is bogus. But it may be
that we can just say something like:

  fatal: unable to resolve HEAD; do you have any commits?

>     $ git clone foo bar
>     Initialized empty Git repository in /home/reece/bar/.git/
>     $ ls -a /home/reece
>     . .. foo

As of 1.5.4, this now says "fatal: cannot clone empty repository". There
has been work recently on a C version of clone which tries to match the
sequence of "init && remote add && fetch && checkout" more closely. I
haven't looked closely, but I suspect it may just work (by which I mean
create an empty repo with origin config pointing to the parent repo).

> It appears from the git output (with the inconsistently capitalized
> git - see --version output above) that the clone succeeded, but it
> does not create bar/.git/, even though it said that it did.

It did create it...it just deleted it afterwards without telling you. :)

-Peff

Re: Some issues working with empty/bare repositories...

[Reece Dunn]

On 03/03/2008, Jeff King <peff@peff.net> wrote:
> On Sat, Mar 01, 2008 at 07:40:39PM +0000, Reece Dunn wrote:
>
>  > Consider the following workflow:
>  >
>  >     $ mkdir foo
>  >     $ cd foo
>  >     $ git --bare init
>  >
>  >     $ git log
>  >     fatal: bad default revision 'HEAD'
>  >
>  > This message is confusing for a newbie. Displaying "no commits" would
>  > make more sense here.
>
> The tricky thing here is that "git log" doesn't know we have no
>  commits in the repo; it only knows that HEAD is bogus. But it may be
>  that we can just say something like:
>
>   fatal: unable to resolve HEAD; do you have any commits?

That would be better. The current message would indicate something
like a corrupt repository (or at least the metadata to the
repository).

>  >     $ git clone foo bar
>  >     Initialized empty Git repository in /home/reece/bar/.git/
>  >     $ ls -a /home/reece
>  >     . .. foo
>
> As of 1.5.4, this now says "fatal: cannot clone empty repository". There
>  has been work recently on a C version of clone which tries to match the
>  sequence of "init && remote add && fetch && checkout" more closely. I
>  haven't looked closely, but I suspect it may just work (by which I mean
>  create an empty repo with origin config pointing to the parent repo).

I have verified that behaviour with 1.5.4. Having an empty repository
(possibly with a warning issued about cloning an empty repository)
with the origin pointing to the parent repository is what I would
expect to happen.

>  > It appears from the git output (with the inconsistently capitalized
>  > git - see --version output above) that the clone succeeded, but it
>  > does not create bar/.git/, even though it said that it did.
>
> It did create it...it just deleted it afterwards without telling you. :)

:)

Thanks,
- Reece

[RFC] improve 'bad default revision' message for empty repo

[Jeff King]

On Tue, Mar 04, 2008 at 09:51:02PM +0000, Reece Dunn wrote:

> >  > Consider the following workflow:
> >  >
> >  >     $ mkdir foo
> >  >     $ cd foo
> >  >     $ git --bare init
> >  >
> >  >     $ git log
> >  >     fatal: bad default revision 'HEAD'
> >  >
> >  > This message is confusing for a newbie. Displaying "no commits" would
> >  > make more sense here.

What do people think of this patch? It feels a little hack-ish to make
guesses as to the reasons for a failure, but in my experience an empty
repo is the cause of this message in 99% of cases.

We could special-case it to HEAD and make a better message, perhaps, but
that feels even more hack-ish.

---
 revision.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/revision.c b/revision.c
index 63bf2c5..847dbc8 100644
--- a/revision.c
+++ b/revision.c
@@ -1324,7 +1324,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		struct object *object;
 		unsigned mode;
 		if (get_sha1_with_mode(def, sha1, &mode))
-			die("bad default revision '%s'", def);
+			die("unable to resolve '%s'; do you have any commits on this branch?", def);
 		object = get_reference(revs, def, sha1, 0);
 		add_pending_object_with_mode(revs, object, def, mode);
 	}
-- 
1.5.4.3.531.ga940.dirty

Re: [RFC] improve 'bad default revision' message for empty repo

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Mar 04, 2008 at 09:51:02PM +0000, Reece Dunn wrote:
>
>> >  > Consider the following workflow:
>> >  >
>> >  >     $ mkdir foo
>> >  >     $ cd foo
>> >  >     $ git --bare init
>> >  >
>> >  >     $ git log
>> >  >     fatal: bad default revision 'HEAD'
>> >  >
>> >  > This message is confusing for a newbie. Displaying "no commits" would
>> >  > make more sense here.
>
> What do people think of this patch? It feels a little hack-ish to make
> guesses as to the reasons for a failure, but in my experience an empty
> repo is the cause of this message in 99% of cases.

>
> We could special-case it to HEAD and make a better message, perhaps, but
> that feels even more hack-ish.

How about doing it this way instead, then?

Saying "fatal: You haven't made a commit?" is like saying "How stupid do
you have to be to realize that you are not allowed to run git-log before
making a commit, dummy?", but that is not the message we need to be
sending.  Asking for log is fine.  If you are on an unborn branch and say
"git log", you deserve to get nothing.  Not even an error message.

Not that I haven't thought through the ramifications of this yet; some
callers may need to be adjusted, but they should be prepared for a case
where no rev was given from the command line anyway.

---

 revision.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/revision.c b/revision.c
index 63bf2c5..cdd5ad2 100644
--- a/revision.c
+++ b/revision.c
@@ -1323,10 +1323,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		unsigned char sha1[20];
 		struct object *object;
 		unsigned mode;
-		if (get_sha1_with_mode(def, sha1, &mode))
-			die("bad default revision '%s'", def);
-		object = get_reference(revs, def, sha1, 0);
-		add_pending_object_with_mode(revs, object, def, mode);
+		if (!get_sha1_with_mode(def, sha1, &mode)) {
+			object = get_reference(revs, def, sha1, 0);
+			add_pending_object_with_mode(revs, object, def, mode);
+		}
 	}
 
 	/* Did the user ask for any diff output? Run the diff! */

Re: [RFC] improve 'bad default revision' message for empty repo

[Jeff King]

On Tue, Mar 04, 2008 at 06:43:25PM -0800, Junio C Hamano wrote:

> Saying "fatal: You haven't made a commit?" is like saying "How stupid do
> you have to be to realize that you are not allowed to run git-log before
> making a commit, dummy?", but that is not the message we need to be
> sending.  Asking for log is fine.  If you are on an unborn branch and say
> "git log", you deserve to get nothing.  Not even an error message.

I like that behavior much better, but...

> diff --git a/revision.c b/revision.c
> index 63bf2c5..cdd5ad2 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1323,10 +1323,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
>  		unsigned char sha1[20];
>  		struct object *object;
>  		unsigned mode;
> -		if (get_sha1_with_mode(def, sha1, &mode))
> -			die("bad default revision '%s'", def);
> -		object = get_reference(revs, def, sha1, 0);
> -		add_pending_object_with_mode(revs, object, def, mode);
> +		if (!get_sha1_with_mode(def, sha1, &mode)) {
> +			object = get_reference(revs, def, sha1, 0);
> +			add_pending_object_with_mode(revs, object, def, mode);
> +		}
>  	}
>  

I'm not sure it's this easy. We're basically just ignoring the error
return from get_sha1_with_mode, but do we really want to ignore _all_
errors? Specifically, should "git log --default foobar" silently produce
no commits?

I think a tighter rule that would accomplish the same thing is "if we
resolve to a ref that is yet-to-be-born, then ignore." But unfortunately
that information is lost deep within the bowels of get_sha1_with_mode.

But maybe this is good enough. It is, after all, just the "default" so
perhaps it makes sense to treat all errors as soft errors. And it will
almost always be HEAD, I think. The only script which seems to use
--default is git-svn. I'm not clear on why you would want to use
--default rather than simply specifying the refs you want.

-Peff

Re: [RFC] improve 'bad default revision' message for empty repo

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'm not sure it's this easy. We're basically just ignoring the error
> return from get_sha1_with_mode, but do we really want to ignore _all_
> errors? Specifically, should "git log --default foobar" silently produce
> no commits?

Sure.

The thing is, nobody uses "--default" with random crap (i.e. risk of typo
running from the command line).  It is really about scripted use, and I can
guarantee majority of --default argument is HEAD, and in people's custom
scripts that are specially tailored for specific workflows, they would
use concrete commit object names that their workflow is built around as
convention (e.g. "alias.recent = git log --since=1.day --default master").

We could enhance the --default mechanism to say that its argument is
optional when it begins with a '?', and change our internal callers to
pass the equivalent of "--default ?HEAD", and keep the traditional die()
behaviour for non-optional defaults to catch breakages in end-user
scripts.

> I think a tighter rule that would accomplish the same thing is "if we
> resolve to a ref that is yet-to-be-born, then ignore." But unfortunately
> that information is lost deep within the bowels of get_sha1_with_mode.

Yes and no.  It is in the error path, so you can afford to redo resolving
the symref _after_ seeing get_sha1_with_mode() fail.  

Re: [RFC] improve 'bad default revision' message for empty repo

[Jeff King]

On Wed, Mar 05, 2008 at 12:48:39AM -0800, Junio C Hamano wrote:

> The thing is, nobody uses "--default" with random crap (i.e. risk of typo
> running from the command line).  It is really about scripted use, and I can
> guarantee majority of --default argument is HEAD, and in people's custom
> scripts that are specially tailored for specific workflows, they would
> use concrete commit object names that their workflow is built around as
> convention (e.g. "alias.recent = git log --since=1.day --default master").

Well, if you are comfortable, then I have no real objection. I just
wanted to raise the point since I didn't really know how widely and in
what way --default was being used.

> We could enhance the --default mechanism to say that its argument is
> optional when it begins with a '?', and change our internal callers to
> pass the equivalent of "--default ?HEAD", and keep the traditional die()
> behaviour for non-optional defaults to catch breakages in end-user
> scripts.

Sure, that is reasonable. The syntax is a bit ugly. Maybe just set an
ignore_default_errors flag to '1', and then if --default is specified on
the commandline, set it to '0'. The behavior should be identical for all
external scripts, and we can audit the internal uses.

But given your 'master' example above, I think you would actually want
it to be treated in the same way; if master doesn't exist, it is empty.
IOW, it is the "defaultness" which makes one want to ignore errors.

> > I think a tighter rule that would accomplish the same thing is "if we
> > resolve to a ref that is yet-to-be-born, then ignore." But unfortunately
> > that information is lost deep within the bowels of get_sha1_with_mode.
> 
> Yes and no.  It is in the error path, so you can afford to redo resolving
> the symref _after_ seeing get_sha1_with_mode() fail.

True. Although considering your 'master' example above, it is not just
"symref whose target doesn't exist", but "ref does not exist". In either
case (internal HEAD or explicit --default) I think something like "ref
exists but is corrupted" is something you would expect git to note.

Hrm. Thinking about it a bit more, what should be done with a --default
like "HEAD^^"? It currently works fine, but parsing it down to "HEAD"
requires the magic of get_sha1_with_mode. I think anyone using anything
but an unadorned refname for --default is probably insane, though. Would
it be acceptable to formally disallow it?

-Peff

Re: [RFC] improve 'bad default revision' message for empty repo

[Jeff King]

On Wed, Mar 05, 2008 at 04:10:51AM -0500, Jeff King wrote:

> Hrm. Thinking about it a bit more, what should be done with a --default
> like "HEAD^^"? It currently works fine, but parsing it down to "HEAD"
> requires the magic of get_sha1_with_mode. I think anyone using anything
> but an unadorned refname for --default is probably insane, though. Would
> it be acceptable to formally disallow it?

Hrm. Even if we took this down to the level of resolve_ref(), there
still is no distinction being made between "does not exist" and other
errors. So restricting what is allowed in --default doesn't solve
anything anyway (though it does make the call stack much smaller, which
makes a modifiction to pass an error/not-found condition back a less
painful change).

But given that no distinction is made currently for something like "git
show foobar" between "you don't have foobar" and "foobar is somehow
corrupt", I am inclined to just say "any errors in default lookup are
ignored."

-Peff