Re* [RFC/PATCH] extend meaning of "--root" option to index comparisons

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Fri, Sep 19, 2008 at 01:27:46PM -0700, Junio C Hamano wrote:
> ...
>>  (2) "--root" is about "do we show a creation event as a huge diff from
>>      emptyness?".  Yes, we turn it on for "git log" but it does not have
>>      anything to do with the issue of yet to be born branch, where there
>>      isn't even a big creation event yet.
>
> What about index comparisons? What should an index comparison to a
> branch yet-to-be-born look like? Right now it is an error.

It should be an error, because that is _not_ even an comparison.  At least
at diff-index level.

The diff wrapper UI could do something different, though.  And an obvious
thing to do is to give a fake creation event.

The current output feels perfectly sensible to me.

	$ mkdir d; cd d; tar xf .../t.tar; git init; git add .
	$ git diff --cached
        fatal: No HEAD commit to compare with (yet)

The alternative is no different from "find . -type f | xargs cat" from the
point of view of reviewability.  To make sure you have what you want in
your initial revision, so that you can get things right from the start,
you would want to check things like:

	$ tar df .../t.tar ;# did I change anything?
        $ git ls-files ;# do I have what I want?
        $ git clean -n ;# have I missed anything?

By allowing an auto-fallback to the comparison with an empty tree object, 
you are giving these possibilities:

	$ git diff --cached --stat
	$ git diff --cached --name-only

but the latter is already available from ls-files anyway, and the former
does not feel so interesting.  

In exchange, we lose the reminder to the user that this is a creation
event.  An interactive user (remember, I am not talking about diff-index
here, but diff front-end) may want to treat it specially perhaps by being
extra careful.  If there were no downsides like this in "fall back to
comparing with an empty tree" approach, I wouldn't hesitate to agree it is
a good idea, though.

>> I am reluctant to agree with the opinion that "git log" should be _silent_
>> in a world without any history.
>
> It feels a bit more Unix-y to me. That is, if I am asking for some set
> of commits, and there are _no_ commits in the set, then I expect no
> output.

To this, I am inclined to agree.  We could do something like the attached
patch, but there is a caveat.

There may be commands that

 (1) cannot sanely operate without any positive ref;

 (2) give default "HEAD";

 (3) do not have its own input verification to make sure there is at least
     one positive ref, because they have been relying on revision
     machinery to die() with the existing check.

and this patch is actively breaking them.  If people like this approach
(and I probably will join them), commands that match the above criteria
need to be identified and fixed by setting revs->require_valid_def.

 revision.c |   22 ++++++++++++++++++----
 revision.h |    5 ++++-
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git i/revision.c w/revision.c
index 2f646de..5ce7795 100644
--- i/revision.c
+++ w/revision.c
@@ -1295,12 +1295,26 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		prepare_show_merge(revs);
 	if (revs->def && !revs->pending.nr) {
 		unsigned char sha1[20];
-		struct object *object;
 		unsigned mode;
-		if (get_sha1_with_mode(revs->def, sha1, &mode))
+		int flag;
+		if (!get_sha1_with_mode(revs->def, sha1, &mode)) {
+			struct object *object;
+			object = get_reference(revs, revs->def, sha1, 0);
+			add_pending_object_with_mode(revs, object, revs->def, mode);
+		} else if (!revs->require_valid_def &&
+			   !strcmp(revs->def, "HEAD") &&
+			   resolve_ref(revs->def, sha1, 0, &flag) &&
+			   (flag & REF_ISSYMREF)) {
+			/*
+			 * Most commands can operate on an unborn branch
+			 * without an explicit ref parameter as if no
+			 * positive ref was specified (as opposed to the
+			 * traditional behaviour of barfing on invalid HEAD).
+			 */
+			;
+		} else {
 			die("bad default revision '%s'", revs->def);
-		object = get_reference(revs, revs->def, sha1, 0);
-		add_pending_object_with_mode(revs, object, revs->def, mode);
+		}
 	}
 
 	/* Did the user ask for any diff output? Run the diff! */
diff --git i/revision.h w/revision.h
index 2fdb2dd..7890fb7 100644
--- i/revision.h
+++ w/revision.h
@@ -30,7 +30,10 @@ struct rev_info {
 	const char *prefix;
 	const char *def;
 	void *prune_data;
-	unsigned int early_output;
+
+	/* Miscellaneous */
+	unsigned int	early_output:1,
+			require_valid_def:1;
 
 	/* Traversal flags */
 	unsigned int	dense:1,