{"thread":{"id":"15538","subject":"Diff-tree does not work for initial commit","startedAt":"2008-09-15T20:01:25Z","lastAt":"2008-09-22T13:32:30Z","messageCount":22,"participants":["Anatol Pomozov","Michael J Gruber","Junio C Hamano","Sverre Rabbelier","Jeff King","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"90766","messageId":"3665a1a00809151301p7d8e6387g3cacfb879b45da2f@mail.gmail.com","threadId":"15538","inReplyTo":null,"subject":"Diff-tree does not work for initial commit","fromName":"Anatol Pomozov","fromEmail":"anatol.pomozov@gmail.com","sentAt":"2008-09-15T20:01:25Z","receivedAt":"2008-09-15T20:01:25Z","isPatch":false,"sender":{"key":"anatol.pomozov@gmail.com","avatar":"https://gravatar.com/avatar/71fc20093402ce987148294ec0999d025209e52da6762789ff248cf5c317645f?d=mp&s=160"},"body":"Hi, It looks like I found a bug in git.\n\nThe problem: In my script I need to know what files were modified by\ngiven commit. I use diff-tree for it. Although it works for most\ncases, for initial commit it does not. Here is a sequence of actions.\n\nanatol:~ $ mkdir mkdir initialcommitissue\nanatol:~ $ cd initialcommitissue/\nanatol:initialcommitissue $ git init\nInitialized empty Git repository in /home/anatol/initialcommitissue/.git/\nanatol:initialcommitissue $ echo \"First commit\" > 1.txt\nanatol:initialcommitissue $ git add 1.txt\nanatol:initialcommitissue $ git commit -m \"First commit\"\nCreated initial commit 31ccc6a: First commit\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 1.txt\nanatol:initialcommitissue $ git diff-tree HEAD <<<<< PROBLEM IS HERE\nanatol:initialcommitissue $ echo \"Second commit\" > 2.txt\nanatol:initialcommitissue $ git add 2.txt\nanatol:initialcommitissue $ git commit -m \"Second commit\"\nCreated commit 51e8bcb: Second commit\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 2.txt\nanatol:initialcommitissue $ git diff-tree HEAD\n51e8bcbb739fc8329fc092db7a84b02bbc64feb2\n:000000 100644 0000000000000000000000000000000000000000\nc133ee6afb86d836ae607cc12e7b7b42242aa5fa A\t2.txt\n\n\nso git diff-tree HEAD works fine but git diff-tree HEAD~1 does not. I\nguess in sake of consistency it should show all changed files in\ninitial commit.\n\n-- \nanatol\n"},{"id":"90771","messageId":"48CECA42.1050209@drmicha.warpmail.net","threadId":"15538","inReplyTo":"3665a1a00809151301p7d8e6387g3cacfb879b45da2f@mail.gmail.com","subject":"Re: Diff-tree does not work for initial commit","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2008-09-15T20:49:06Z","receivedAt":"2008-09-15T20:49:06Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Anatol Pomozov venit, vidit, dixit 15.09.2008 22:01:\n> Hi, It looks like I found a bug in git.\n> \n> The problem: In my script I need to know what files were modified by \n> given commit. I use diff-tree for it. Although it works for most \n> cases, for initial commit it does not. Here is a sequence of actions.\n> \n> \n> anatol:~ $ mkdir mkdir initialcommitissue anatol:~ $ cd\n> initialcommitissue/ anatol:initialcommitissue $ git init Initialized\n> empty Git repository in /home/anatol/initialcommitissue/.git/ \n> anatol:initialcommitissue $ echo \"First commit\" > 1.txt \n> anatol:initialcommitissue $ git add 1.txt anatol:initialcommitissue $\n> git commit -m \"First commit\" Created initial commit 31ccc6a: First\n> commit 1 files changed, 1 insertions(+), 0 deletions(-) create mode\n> 100644 1.txt anatol:initialcommitissue $ git diff-tree HEAD <<<<<\n> PROBLEM IS HERE\n\n>From the man page:\n\n Compares the content and mode of the blobs found via two tree\nobjects.\n\n If there is only one given, the commit is compared\nwith its parents (see --stdin below).\n\n Note that git-diff-tree can use the tree encapsulated in a commit\nobject.\n\n\nThe initial commit has no parent, so diff-tree does not know which tree\nto compare to.\n\nYou can do\n\ngit diff-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 HEAD\n\nbut I guess you suggest that diff-tree should do that automatically for\na single parentless treeish: bug -> RFE\n\ndiff-tree is plumbing. Would this change break anything?\n\nMichael\n"},{"id":"90772","messageId":"7vprn59lkd.fsf@gitster.siamese.dyndns.org","threadId":"15538","inReplyTo":"48CECA42.1050209@drmicha.warpmail.net","subject":"Re: Diff-tree does not work for initial commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-15T20:54:42Z","receivedAt":"2008-09-15T20:54:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Anatol Pomozov venit, vidit, dixit 15.09.2008 22:01:\n>> Hi, It looks like I found a bug in git.\n>> \n>> The problem: In my script I need to know what files were modified by \n>> given commit. I use diff-tree for it. Although it works for most \n>> cases, for initial commit it does not. Here is a sequence of actions.\n>> \n>> \n>> anatol:~ $ mkdir mkdir initialcommitissue anatol:~ $ cd\n>> initialcommitissue/ anatol:initialcommitissue $ git init Initialized\n>> empty Git repository in /home/anatol/initialcommitissue/.git/ \n>> anatol:initialcommitissue $ echo \"First commit\" > 1.txt \n>> anatol:initialcommitissue $ git add 1.txt anatol:initialcommitissue $\n>> git commit -m \"First commit\" Created initial commit 31ccc6a: First\n>> commit 1 files changed, 1 insertions(+), 0 deletions(-) create mode\n>> 100644 1.txt anatol:initialcommitissue $ git diff-tree HEAD <<<<<\n>> PROBLEM IS HERE\n>\n> From the man page:\n>\n> Compares the content and mode of the blobs found via two tree\n> objects.\n>\n> If there is only one given, the commit is compared\n> with its parents (see --stdin below).\n>\n> Note that git-diff-tree can use the tree encapsulated in a commit\n> object.\n>\n> The initial commit has no parent, so diff-tree does not know which tree\n> to compare to.\n\n--root?\n"},{"id":"90774","messageId":"48CECEED.3080105@drmicha.warpmail.net","threadId":"15538","inReplyTo":"48CECA42.1050209@drmicha.warpmail.net","subject":"Re: Diff-tree does not work for initial commit","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2008-09-15T21:09:01Z","receivedAt":"2008-09-15T21:09:01Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Michael J Gruber venit, vidit, dixit 15.09.2008 22:49:\n> Anatol Pomozov venit, vidit, dixit 15.09.2008 22:01:\n>> Hi, It looks like I found a bug in git.\n>>\n>> The problem: In my script I need to know what files were modified by \n>> given commit. I use diff-tree for it. Although it works for most \n>> cases, for initial commit it does not. Here is a sequence of actions.\n>>\n>>\n>> anatol:~ $ mkdir mkdir initialcommitissue anatol:~ $ cd\n>> initialcommitissue/ anatol:initialcommitissue $ git init Initialized\n>> empty Git repository in /home/anatol/initialcommitissue/.git/ \n>> anatol:initialcommitissue $ echo \"First commit\" > 1.txt \n>> anatol:initialcommitissue $ git add 1.txt anatol:initialcommitissue $\n>> git commit -m \"First commit\" Created initial commit 31ccc6a: First\n>> commit 1 files changed, 1 insertions(+), 0 deletions(-) create mode\n>> 100644 1.txt anatol:initialcommitissue $ git diff-tree HEAD <<<<<\n>> PROBLEM IS HERE\n> \n> From the man page:\n> \n> Compares the content and mode of the blobs found via two tree\n> objects.\n> \n> If there is only one given, the commit is compared\n> with its parents (see --stdin below).\n> \n> Note that git-diff-tree can use the tree encapsulated in a commit\n> object.\n> \n> \n> The initial commit has no parent, so diff-tree does not know which tree\n> to compare to.\n> \n> You can do\n> \n> git diff-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 HEAD\n> \n> but I guess you suggest that diff-tree should do that automatically for\n> a single parentless treeish: bug -> RFE\n> \n> diff-tree is plumbing. Would this change break anything?\n> \n> Michael\n\nOoops, that man page is just too long. Scrolling way down:\n\n\"git commit-tree --root\" treats the root as a commit with an empty tree.\nSo this does what you want.\n\nBut you may want to look into porcelain like\ngit show --pretty=format: --name-only\n\nMichael\n"},{"id":"90775","messageId":"bd6139dc0809151411p49f5adeaq4beff452574ca980@mail.gmail.com","threadId":"15538","inReplyTo":"48CECA42.1050209@drmicha.warpmail.net","subject":"Re: Diff-tree does not work for initial commit","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-09-15T21:11:30Z","receivedAt":"2008-09-15T21:11:30Z","isPatch":false,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Mon, Sep 15, 2008 at 22:49, Michael J Gruber\n wrote:\n> diff-tree is plumbing. Would this change break anything?\n\nSome of my code uses \"git rev-parse\" on \"HEAD^\" to see if a commit has\na parent; I wouldn't be surprised if someone else has a script that\nuses \"git diff-tree\" or such for that purpose, or at least assumes\nthat for a root commit it will complain. Anyway, as Junio said, for\n\"diff-tree\" you can use the \"--root\" option. A better RFE would\nperhaps be that \"--root\" be supported in more places.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"90778","messageId":"3665a1a00809151448l5a4449e4w3caa9986bc5dd26b@mail.gmail.com","threadId":"15538","inReplyTo":"7vprn59lkd.fsf@gitster.siamese.dyndns.org","subject":"Re: Diff-tree does not work for initial commit","fromName":"Anatol Pomozov","fromEmail":"anatol.pomozov@gmail.com","sentAt":"2008-09-15T21:48:15Z","receivedAt":"2008-09-15T21:48:15Z","isPatch":false,"sender":{"key":"anatol.pomozov@gmail.com","avatar":"https://gravatar.com/avatar/71fc20093402ce987148294ec0999d025209e52da6762789ff248cf5c317645f?d=mp&s=160"},"body":"On Mon, Sep 15, 2008 at 1:54 PM, Junio C Hamano wrote:\n>> The initial commit has no parent, so diff-tree does not know which tree\n>> to compare to.\n>\n> --root?\n\nOops my bad.\n\nI overlooked this part of the manual. Thanks Junio.\n\nTaking back my words about bug.\n\n-- \nanatol\n"},{"id":"90780","messageId":"20080915223442.GD20677@sigill.intra.peff.net","threadId":"15538","inReplyTo":"bd6139dc0809151411p49f5adeaq4beff452574ca980@mail.gmail.com","subject":"Re: Diff-tree does not work for initial commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-15T22:34:43Z","receivedAt":"2008-09-15T22:34:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 15, 2008 at 11:11:30PM +0200, Sverre Rabbelier wrote:\n\n> Some of my code uses \"git rev-parse\" on \"HEAD^\" to see if a commit has\n> a parent; I wouldn't be surprised if someone else has a script that\n> uses \"git diff-tree\" or such for that purpose, or at least assumes\n> that for a root commit it will complain. Anyway, as Junio said, for\n> \"diff-tree\" you can use the \"--root\" option. A better RFE would\n> perhaps be that \"--root\" be supported in more places.\n\nI posted this a week or so ago, but I am sure it is incomplete. If there\nis interest I can clean it up and do a proper submission.\n\n---\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 76651bd..4151900 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -332,8 +332,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\t\tbreak;\n \t\t\telse if (!strcmp(arg, \"--cached\")) {\n \t\t\t\tadd_head_to_pending(&rev);\n-\t\t\t\tif (!rev.pending.nr)\n-\t\t\t\t\tdie(\"No HEAD commit to compare with (yet)\");\n+\t\t\t\tif (!rev.pending.nr) {\n+\t\t\t\t\tif (!rev.show_root_diff)\n+\t\t\t\t\t\tdie(\"No HEAD commit to compare with (yet)\");\n+\t\t\t\t\tadd_empty_to_pending(&rev);\n+\t\t\t\t}\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\ndiff --git a/revision.c b/revision.c\nindex 2f646de..7ec3990 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -145,16 +145,27 @@ void add_pending_object(struct rev_info *revs, struct object *obj, const char *n\n \tadd_pending_object_with_mode(revs, obj, name, S_IFINVALID);\n }\n \n-void add_head_to_pending(struct rev_info *revs)\n+static void add_to_pending_by_name(struct rev_info *revs, const char *name)\n {\n \tunsigned char sha1[20];\n \tstruct object *obj;\n-\tif (get_sha1(\"HEAD\", sha1))\n+\tif (get_sha1(name, sha1))\n \t\treturn;\n \tobj = parse_object(sha1);\n \tif (!obj)\n \t\treturn;\n-\tadd_pending_object(revs, obj, \"HEAD\");\n+\tadd_pending_object(revs, obj, name);\n+}\n+\n+void add_head_to_pending(struct rev_info *revs)\n+{\n+\tadd_to_pending_by_name(revs, \"HEAD\");\n+}\n+\n+void add_empty_to_pending(struct rev_info *revs)\n+{\n+\tadd_to_pending_by_name(revs,\n+\t\t\t\"4b825dc642cb6eb9a060e54bf8d69288fbee4904\");\n }\n \n static struct object *get_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags)\ndiff --git a/revision.h b/revision.h\nindex 2fdb2dd..8c990d5 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -152,6 +152,7 @@ extern void add_object(struct object *obj,\n extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);\n \n extern void add_head_to_pending(struct rev_info *);\n+extern void add_empty_to_pending(struct rev_info *);\n \n enum commit_action {\n \tcommit_ignore,\n"},{"id":"90806","messageId":"bd6139dc0809152319m31a79877h5dc1b701a8210802@mail.gmail.com","threadId":"15538","inReplyTo":"20080915223442.GD20677@sigill.intra.peff.net","subject":"Re: Diff-tree does not work for initial commit","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-09-16T06:19:37Z","receivedAt":"2008-09-16T06:19:37Z","isPatch":false,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Tue, Sep 16, 2008 at 00:34, Jeff King wrote:\n> I posted this a week or so ago, but I am sure it is incomplete. If there\n> is interest I can clean it up and do a proper submission.\n\n\n\nI like it, although I think that if we add broader support for it, we\nshould probably be consequent and add it everywhere where appropriate?\n(That is ofcourse, assuming that does not take too long to implement\netc.)\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"90807","messageId":"20080916062105.GA12708@coredump.intra.peff.net","threadId":"15538","inReplyTo":"bd6139dc0809152319m31a79877h5dc1b701a8210802@mail.gmail.com","subject":"Re: Diff-tree does not work for initial commit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-16T06:21:05Z","receivedAt":"2008-09-16T06:21:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 16, 2008 at 08:19:37AM +0200, Sverre Rabbelier wrote:\n\n> I like it, although I think that if we add broader support for it, we\n> should probably be consequent and add it everywhere where appropriate?\n> (That is ofcourse, assuming that does not take too long to implement\n> etc.)\n\nRight, that was what I meant by \"incomplete\". I think there are several\nother cases where giving \"--root\" would have expected behavior but is\ncurrently ignored. I'll take a closer look, but I probably won't have\ntime for a few days.\n\n-Peff\n"},{"id":"91017","messageId":"20080918092152.GA18732@coredump.intra.peff.net","threadId":"15538","inReplyTo":"20080916062105.GA12708@coredump.intra.peff.net","subject":"[RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-18T09:21:52Z","receivedAt":"2008-09-18T09:21:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"--root\" option generally means \"treat any commits\nwithout parents as a big creation event\". This extends the\nmeaning to make an index comparison against a non-existant\nHEAD into a big creation event. In other words, \"if this\nindex _were_ to become a commit, this is how we would show\nit with --root.\"\n\nSpecifically, we cover the case of\n\n git diff --cached --root\n\nto show either the diff between the index and HEAD, or if\nthere is no HEAD, show the diff against the empty tree.\nThis can simplify calling scripts which must otherwise\nspecial-case the initial commit when showing the index\nstatus.\n\nWe intentionally don't cover:\n\n - git diff --cached --root HEAD\n\n The user has specifically asked for HEAD, which doesn't\n exist.\n\n - git diff-index\n\n The user is required to specify a tree-ish to\n diff-index; if that tree-ish doesn't exist, we should\n report an error.\n\nSigned-off-by: Jeff King \n---\nOn Tue, Sep 16, 2008 at 02:21:05AM -0400, Jeff King wrote:\n\n> Right, that was what I meant by \"incomplete\". I think there are several\n> other cases where giving \"--root\" would have expected behavior but is\n> currently ignored. I'll take a closer look, but I probably won't have\n> time for a few days.\n\nActually, I wasn't able to find any more cases. I don't think it makes\nsense to override the behavior when an explicit tree-ish is given, so\nthat cuts out the two places mentioned above. Though I think\nthat scripts using this might prefer the plumbing\ndiff-index, so maybe there is a better way to support this\n(e.g., if --root is specified without a tree-ish, assume\nHEAD or empty tree).\n\ndiff-tree already handles --root itself. And there is no way to my\nknowledge to provoke the same kind of \"show the diff against its parent\"\nbehavior via git-diff, since a single tree-ish there means \"diff against\nthe working tree\".\n\nAnd of course for diff-files, such an option makes no sense.\n\nCan you think of any other cases?\n\n builtin-diff.c | 7 +++++--\n revision.c | 17 ++++++++++++++---\n revision.h | 1 +\n t/t4030-diff-root.sh | 21 +++++++++++++++++++++\n 4 files changed, 41 insertions(+), 5 deletions(-)\n create mode 100755 t/t4030-diff-root.sh\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 037c303..0a1efb5 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -315,8 +315,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\t\tbreak;\n \t\t\telse if (!strcmp(arg, \"--cached\")) {\n \t\t\t\tadd_head_to_pending(&rev);\n-\t\t\t\tif (!rev.pending.nr)\n-\t\t\t\t\tdie(\"No HEAD commit to compare with (yet)\");\n+\t\t\t\tif (!rev.pending.nr) {\n+\t\t\t\t\tif (!rev.show_root_diff)\n+\t\t\t\t\t\tdie(\"No HEAD commit to compare with (yet)\");\n+\t\t\t\t\tadd_empty_to_pending(&rev);\n+\t\t\t\t}\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\ndiff --git a/revision.c b/revision.c\nindex 499f0e0..de0fd89 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -145,16 +145,27 @@ void add_pending_object(struct rev_info *revs, struct object *obj, const char *n\n \tadd_pending_object_with_mode(revs, obj, name, S_IFINVALID);\n }\n \n-void add_head_to_pending(struct rev_info *revs)\n+static void add_to_pending_by_name(struct rev_info *revs, const char *name)\n {\n \tunsigned char sha1[20];\n \tstruct object *obj;\n-\tif (get_sha1(\"HEAD\", sha1))\n+\tif (get_sha1(name, sha1))\n \t\treturn;\n \tobj = parse_object(sha1);\n \tif (!obj)\n \t\treturn;\n-\tadd_pending_object(revs, obj, \"HEAD\");\n+\tadd_pending_object(revs, obj, name);\n+}\n+\n+void add_head_to_pending(struct rev_info *revs)\n+{\n+\tadd_to_pending_by_name(revs, \"HEAD\");\n+}\n+\n+void add_empty_to_pending(struct rev_info *revs)\n+{\n+\tadd_to_pending_by_name(revs,\n+\t\t\t\"4b825dc642cb6eb9a060e54bf8d69288fbee4904\");\n }\n \n static struct object *get_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags)\ndiff --git a/revision.h b/revision.h\nindex fc23522..108f43d 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -151,6 +151,7 @@ extern void add_object(struct object *obj,\n extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);\n \n extern void add_head_to_pending(struct rev_info *);\n+extern void add_empty_to_pending(struct rev_info *);\n \n enum commit_action {\n \tcommit_ignore,\ndiff --git a/t/t4030-diff-root.sh b/t/t4030-diff-root.sh\nnew file mode 100755\nindex 0000000..e5174b7\n--- /dev/null\n+++ b/t/t4030-diff-root.sh\n@@ -0,0 +1,21 @@\n+#!/bin/sh\n+\n+test_description='diff --root allows comparison between index and root'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo content >file &&\n+\tgit add file\n+'\n+\n+test_expect_success 'diff --cached (without --root)' '\n+\ttest_must_fail git diff --cached --name-only\n+'\n+\n+test_expect_success 'diff --cached (with --root)' '\n+\techo file >expect &&\n+\tgit diff --cached --name-only --root >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.6.0.2.249.g97d7f.dirty\n"},{"id":"91044","messageId":"3665a1a00809180931t191b5a24wd58554cdb761535@mail.gmail.com","threadId":"15538","inReplyTo":"20080918092152.GA18732@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Anatol Pomozov","fromEmail":"anatol.pomozov@gmail.com","sentAt":"2008-09-18T16:31:24Z","receivedAt":"2008-09-18T16:31:24Z","isPatch":true,"sender":{"key":"anatol.pomozov@gmail.com","avatar":"https://gravatar.com/avatar/71fc20093402ce987148294ec0999d025209e52da6762789ff248cf5c317645f?d=mp&s=160"},"body":"Hi, Jeff.\n\nThanks for your patch.\n\nOn Thu, Sep 18, 2008 at 2:21 AM, Jeff King wrote:\n> The \"--root\" option generally means \"treat any commits\n> without parents as a big creation event\". This extends the\n> meaning to make an index comparison against a non-existant\n> HEAD into a big creation event. In other words, \"if this\n> index _were_ to become a commit, this is how we would show\n> it with --root.\"\n>\n> Specifically, we cover the case of\n>\n> git diff --cached --root\n>\n> to show either the diff between the index and HEAD, or if\n> there is no HEAD, show the diff against the empty tree.\n> This can simplify calling scripts which must otherwise\n> special-case the initial commit when showing the index\n> status.\n>\n> We intentionally don't cover:\n>\n> - git diff --cached --root HEAD\n>\n> The user has specifically asked for HEAD, which doesn't\n> exist.\n>\n> - git diff-index\n>\n> The user is required to specify a tree-ish to\n> diff-index; if that tree-ish doesn't exist, we should\n> report an error.\n>\n> Signed-off-by: Jeff King \n> ---\n> On Tue, Sep 16, 2008 at 02:21:05AM -0400, Jeff King wrote:\n>\n>> Right, that was what I meant by \"incomplete\". I think there are several\n>> other cases where giving \"--root\" would have expected behavior but is\n>> currently ignored. I'll take a closer look, but I probably won't have\n>> time for a few days.\n>\n> Actually, I wasn't able to find any more cases. I don't think it makes\n> sense to override the behavior when an explicit tree-ish is given, so\n> that cuts out the two places mentioned above. Though I think\n> that scripts using this might prefer the plumbing\n> diff-index, so maybe there is a better way to support this\n> (e.g., if --root is specified without a tree-ish, assume\n> HEAD or empty tree).\n>\n> diff-tree already handles --root itself. And there is no way to my\n> knowledge to provoke the same kind of \"show the diff against its parent\"\n> behavior via git-diff, since a single tree-ish there means \"diff against\n> the working tree\".\n>\n> And of course for diff-files, such an option makes no sense.\n>\n> Can you think of any other cases?\n\ngit log??\n\ngit log --root for empty repo should not print anything (instead of\nerror message that we have now).\n\n>\n> builtin-diff.c | 7 +++++--\n> revision.c | 17 ++++++++++++++---\n> revision.h | 1 +\n> t/t4030-diff-root.sh | 21 +++++++++++++++++++++\n\nShould documentation (man-pages) reflect your changes as well?\n\n> 4 files changed, 41 insertions(+), 5 deletions(-)\n> create mode 100755 t/t4030-diff-root.sh\n>\n> diff --git a/builtin-diff.c b/builtin-diff.c\n> index 037c303..0a1efb5 100644\n> --- a/builtin-diff.c\n> +++ b/builtin-diff.c\n> @@ -315,8 +315,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n> break;\n> else if (!strcmp(arg, \"--cached\")) {\n> add_head_to_pending(&rev);\n> - if (!rev.pending.nr)\n> - die(\"No HEAD commit to compare with (yet)\");\n> + if (!rev.pending.nr) {\n> + if (!rev.show_root_diff)\n> + die(\"No HEAD commit to compare with (yet)\");\n> + add_empty_to_pending(&rev);\n> + }\n> break;\n> }\n> }\n> diff --git a/revision.c b/revision.c\n> index 499f0e0..de0fd89 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -145,16 +145,27 @@ void add_pending_object(struct rev_info *revs, struct object *obj, const char *n\n> add_pending_object_with_mode(revs, obj, name, S_IFINVALID);\n> }\n>\n> -void add_head_to_pending(struct rev_info *revs)\n> +static void add_to_pending_by_name(struct rev_info *revs, const char *name)\n> {\n> unsigned char sha1[20];\n> struct object *obj;\n> - if (get_sha1(\"HEAD\", sha1))\n> + if (get_sha1(name, sha1))\n> return;\n> obj = parse_object(sha1);\n> if (!obj)\n> return;\n> - add_pending_object(revs, obj, \"HEAD\");\n> + add_pending_object(revs, obj, name);\n> +}\n> +\n> +void add_head_to_pending(struct rev_info *revs)\n> +{\n> + add_to_pending_by_name(revs, \"HEAD\");\n> +}\n> +\n> +void add_empty_to_pending(struct rev_info *revs)\n> +{\n> + add_to_pending_by_name(revs,\n> + \"4b825dc642cb6eb9a060e54bf8d69288fbee4904\");\n> }\n>\n> static struct object *get_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags)\n> diff --git a/revision.h b/revision.h\n> index fc23522..108f43d 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -151,6 +151,7 @@ extern void add_object(struct object *obj,\n> extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);\n>\n> extern void add_head_to_pending(struct rev_info *);\n> +extern void add_empty_to_pending(struct rev_info *);\n>\n> enum commit_action {\n> commit_ignore,\n> diff --git a/t/t4030-diff-root.sh b/t/t4030-diff-root.sh\n> new file mode 100755\n> index 0000000..e5174b7\n> --- /dev/null\n> +++ b/t/t4030-diff-root.sh\n> @@ -0,0 +1,21 @@\n> +#!/bin/sh\n> +\n> +test_description='diff --root allows comparison between index and root'\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> + echo content >file &&\n> + git add file\n> +'\n> +\n> +test_expect_success 'diff --cached (without --root)' '\n> + test_must_fail git diff --cached --name-only\n> +'\n> +\n> +test_expect_success 'diff --cached (with --root)' '\n> + echo file >expect &&\n> + git diff --cached --name-only --root >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_done\n> --\n> 1.6.0.2.249.g97d7f.dirty\n\n-- \nanatol\n"},{"id":"91048","messageId":"bd6139dc0809180951p2380be79mbc43c53052966b1b@mail.gmail.com","threadId":"15538","inReplyTo":"3665a1a00809180931t191b5a24wd58554cdb761535@mail.gmail.com","subject":"Re: [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-09-18T16:51:17Z","receivedAt":"2008-09-18T16:51:17Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"[Please cull the parts of the mail you are not responding to, it is\nhard to find your reply when you don't.]\n\nOn Thu, Sep 18, 2008 at 18:31, Anatol Pomozov wrote:\n> git log??\n>\n> git log --root for empty repo should not print anything (instead of\n> error message that we have now).\n\nI do not agree here, what is the use in having a command that does nothing?\n\n> Should documentation (man-pages) reflect your changes as well?\n\nYes, they should be updated.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"91120","messageId":"20080919142537.GA1287@coredump.intra.peff.net","threadId":"15538","inReplyTo":"3665a1a00809180931t191b5a24wd58554cdb761535@mail.gmail.com","subject":"Re: [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-19T14:25:38Z","receivedAt":"2008-09-19T14:25:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 18, 2008 at 09:31:24AM -0700, Anatol Pomozov wrote:\n\n> > Can you think of any other cases?\n> \n> git log??\n> \n> git log --root for empty repo should not print anything (instead of\n> error message that we have now).\n\nI'm not sure that's the same as \"--root\", though. In existing --root\ncases, we are saying \"pretend that beyond the initial commit, there is a\ncommit that contains the empty tree\". The logical extension of git-log\nhere would be to print out that commit.\n\nNot to mention that \"git log --root\" _already_ has defined semantics\n(you just don't really need it since log.showroot defaults to true).\n\nI wonder if my patch is actually confusing things more, and the right\nsolution is an option that says \"pretend that a non-existant HEAD is a\ncommit with no log and the empty tree.\" But I think that may just be\nconfusing things more, because the semantics of such a null commit\nwouldn't be clear (e.g., git log would actually produce a little bit of\noutput).\n\nMaybe it really is better to just force the caller to check the initial\ncommit condition. It's more work for them, but the semantics are simple\nand unambiguous.\n\n> Should documentation (man-pages) reflect your changes as well?\n\nYes, definitely. However, I'm not sure yet what the changes should _be_\n(if any).\n\n-Peff\n"},{"id":"91131","messageId":"3665a1a00809190954q2473e164u5d80d3653d238a27@mail.gmail.com","threadId":"15538","inReplyTo":"20080919142537.GA1287@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Anatol Pomozov","fromEmail":"anatol.pomozov@gmail.com","sentAt":"2008-09-19T16:54:15Z","receivedAt":"2008-09-19T16:54:15Z","isPatch":true,"sender":{"key":"anatol.pomozov@gmail.com","avatar":"https://gravatar.com/avatar/71fc20093402ce987148294ec0999d025209e52da6762789ff248cf5c317645f?d=mp&s=160"},"body":"Hi, Jeff.\n\nOn Fri, Sep 19, 2008 at 7:25 AM, Jeff King wrote:\n> I'm not sure that's the same as \"--root\", though. In existing --root\n> cases, we are saying \"pretend that beyond the initial commit, there is a\n> commit that contains the empty tree\". The logical extension of git-log\n> here would be to print out that commit.\nWell yeah, agree. My proposal differs from --root meaning and what you\nare doing in your patch. So let's continue this 'git log' discussion\nwithout relation to your changes.\n\n> Not to mention that \"git log --root\" _already_ has defined semantics\n> (you just don't really need it since log.showroot defaults to true).\nHm..\n\nanatol:opensource $ mkdir ex\nanatol:opensource $ cd ex/\nanatol:ex $ git init\nInitialized empty Git repository in /personal/sources/opensource/ex/.git/\nanatol:ex $ git log\nfatal: bad default revision 'HEAD'\nanatol:ex $ git config log.showroot true\nanatol:ex $ git config log.showroot\ntrue\nanatol:ex $ git log\nfatal: bad default revision 'HEAD'\nanatol:ex $ git config core.showroot true\nanatol:ex $ git config core.showroot\ntrue\nanatol:ex $ git log\nfatal: bad default revision 'HEAD'\n\nI dont see how does log.showroot or core.showroot affect 'git log'.\nman git-log says nothing, git-config only mentions that initial commit\nis \"a big creation event\".\n\n> I wonder if my patch is actually confusing things more, and the right\n> solution is an option that says \"pretend that a non-existant HEAD is a\n> commit with no log and the empty tree.\" But I think that may just be\n> confusing things more, because the semantics of such a null commit\n> wouldn't be clear (e.g., git log would actually produce a little bit of\n> output).\nYeap - probably it would confuse even more, that is why I brought it\nfor discussion :)\n\nBut for me as for person that still actively works with Subversion it\nwas quite surprising that I have error messages right after I created\na fresh empty repo. I always thought that \"Empty repo\" -> \"No history\"\n-> \"No log output\"\n\nSubversion in the same situation.\n\nanatol:2 $ svn info | grep Revision\nRevision: 0\nanatol:2 $ svn log\n------------------------------------------------------------------------\n\nSo svn has the same notion of Initial commit which is \"big creation\nevent\" but not visible in \"svn log\"\n\nThe difference from git is that init git repo has no HEAD. HEAD is\nundefined. Would it be better if absence of HEAD would mean the same\nas \"HEAD points to the initial commit\".\n\n> Maybe it really is better to just force the caller to check the initial\n> commit condition. It's more work for them, but the semantics are simple\n> and unambiguous.\n\nWhat is the best way to check that repo has valid HEAD? Check that\nfile .git/HEAD exists?\n\n-- \nanatol\n"},{"id":"91133","messageId":"20080919173947.GA24541@sigill.intra.peff.net","threadId":"15538","inReplyTo":"3665a1a00809190954q2473e164u5d80d3653d238a27@mail.gmail.com","subject":"Re: [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-19T17:39:48Z","receivedAt":"2008-09-19T17:39:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 19, 2008 at 09:54:15AM -0700, Anatol Pomozov wrote:\n\n> I dont see how does log.showroot or core.showroot affect 'git log'.\n> man git-log says nothing, git-config only mentions that initial commit\n> is \"a big creation event\".\n\nTry this:\n\n mkdir repo && cd repo && git init\n echo content >file && git add file && git commit -m initial\n git show ;# you see initial as creation event\n git config log.showroot false\n git show ;# you see commit log, but no diff\n git show --root ;# same as log.showroot=true\n\nwhere of course the same holds for log, as show uses the same display\nlogic.\n\n> But for me as for person that still actively works with Subversion it\n> was quite surprising that I have error messages right after I created\n> a fresh empty repo. I always thought that \"Empty repo\" -> \"No history\"\n> -> \"No log output\"\n\nI agree it is a bit nicer not to get an error in that situation. It is\nreally a result of the way git thinks of history. It is not \"no history\"\nbut rather \"some history may or may not exist, but you have no valid\npointer to any history\".\n\nHowever, I wonder if it might be possible to special-case the \"branch\nyet to be born\" case. That is, if HEAD points to a ref which does not\nexist, can we treat that specially. I suspect it may turn out to be a\nlot of work tracking down all of the spots in the code that would need\nto be special-cased, which may make it not worthwhile.\n\n> > Maybe it really is better to just force the caller to check the initial\n> > commit condition. It's more work for them, but the semantics are simple\n> > and unambiguous.\n> \n> What is the best way to check that repo has valid HEAD? Check that\n> file .git/HEAD exists?\n\nNo, HEAD will exist even in a just-created repo. But you can check\nwhether HEAD points to a valid commit:\n\n git rev-parse --verify HEAD\n\n-Peff\n"},{"id":"91140","messageId":"7vskrvswxp.fsf@gitster.siamese.dyndns.org","threadId":"15538","inReplyTo":"20080919142537.GA1287@coredump.intra.peff.net","subject":"Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-19T20:27:46Z","receivedAt":"2008-09-19T20:27:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Sep 18, 2008 at 09:31:24AM -0700, Anatol Pomozov wrote:\n>\n>> > Can you think of any other cases?\n>> \n>> git log??\n>> \n>> git log --root for empty repo should not print anything (instead of\n>> error message that we have now).\n>\n> I'm not sure that's the same as \"--root\", though. In existing --root\n> cases, we are saying \"pretend that beyond the initial commit, there is a\n> commit that contains the empty tree\". The logical extension of git-log\n> here would be to print out that commit.\n\nI would say:\n\n (1) A user getting an error message from \"git init && git log\" may be\n annoyed, but he very well knows there is no history yet _anyway_.\n This initial annoyance will pass immediately after creating any\n commit, so I do not think it is a big issue.\n\n \"bad default revision 'HEAD'\" is a cryptic way to give that indicaion\n that can be improved but that is a separate issue. Rewording it so\n that it explains the situation better in user's terms would be a\n worthy improvement.\n\n (2) \"--root\" is about \"do we show a creation event as a huge diff from\n emptyness?\". Yes, we turn it on for \"git log\" but it does not have\n anything to do with the issue of yet to be born branch, where there\n isn't even a big creation event yet.\n\nI am reluctant to agree with the opinion that \"git log\" should be _silent_\nin a world without any history.\n\nPerhaps something like this would be a good compromise? I dunno.\n\n builtin-log.c | 9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git c/builtin-log.c w/builtin-log.c\nindex 081e660..881324c 100644\n--- c/builtin-log.c\n+++ w/builtin-log.c\n@@ -42,7 +42,14 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \tif (default_date_mode)\n \t\trev->date_mode = parse_date_format(default_date_mode);\n \n-\targc = setup_revisions(argc, argv, rev, \"HEAD\");\n+\targc = setup_revisions(argc, argv, rev, NULL);\n+\tif (!rev->pending.nr) {\n+\t\tadd_head_to_pending(rev);\n+\t\tif (!rev->pending.nr) {\n+\t\t\tprintf(\"No commits (yet).\\n\");\n+\t\t\texit(0);\n+\t\t}\n+\t}\n \n \tif (rev->diffopt.pickaxe || rev->diffopt.filter)\n \t\trev->always_show_header = 0;\n"},{"id":"91226","messageId":"20080921135616.GA25238@sigill.intra.peff.net","threadId":"15538","inReplyTo":"7vskrvswxp.fsf@gitster.siamese.dyndns.org","subject":"Re: Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-21T13:56:16Z","receivedAt":"2008-09-21T13:56:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 19, 2008 at 01:27:46PM -0700, Junio C Hamano wrote:\n\n> (1) A user getting an error message from \"git init && git log\" may be\n> annoyed, but he very well knows there is no history yet _anyway_.\n> This initial annoyance will pass immediately after creating any\n> commit, so I do not think it is a big issue.\n\nI think there is an additional case of script writers, who want their\nscripts to fail gracefully or otherwise do the right thing with an\ninitial commit. Right now they have to special-case the initial commit.\nI don't know if it is possible to have sane enough behavior that the\nspecial case can be eliminated, or if it will simply make things worse.\n\n> \"bad default revision 'HEAD'\" is a cryptic way to give that indicaion\n> that can be improved but that is a separate issue. Rewording it so\n> that it explains the situation better in user's terms would be a\n> worthy improvement.\n\nI agree that would be an improvement.\n\n> (2) \"--root\" is about \"do we show a creation event as a huge diff from\n> emptyness?\". Yes, we turn it on for \"git log\" but it does not have\n> anything to do with the issue of yet to be born branch, where there\n> isn't even a big creation event yet.\n\nWhat about index comparisons? What should an index comparison to a\nbranch yet-to-be-born look like? Right now it is an error.\n\n> I am reluctant to agree with the opinion that \"git log\" should be _silent_\n> in a world without any history.\n\nIt feels a bit more Unix-y to me. That is, if I am asking for some set\nof commits, and there are _no_ commits in the set, then I expect no\noutput. That makes sense for text processing.\n\n> -\targc = setup_revisions(argc, argv, rev, \"HEAD\");\n> +\targc = setup_revisions(argc, argv, rev, NULL);\n> +\tif (!rev->pending.nr) {\n> +\t\tadd_head_to_pending(rev);\n> +\t\tif (!rev->pending.nr) {\n> +\t\t\tprintf(\"No commits (yet).\\n\");\n> +\t\t\texit(0);\n> +\t\t}\n> +\t}\n\nI like the idea of an improved message, but such a message should\ndefinitely not go to stdout; it would feed nonsense to a command like\n\"git log | my_log_filter\".\n\n-Peff\n"},{"id":"91231","messageId":"3665a1a00809210858r1c494d22p77b5e9964c06424e@mail.gmail.com","threadId":"15538","inReplyTo":"20080921135616.GA25238@sigill.intra.peff.net","subject":"Re: Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Anatol Pomozov","fromEmail":"anatol.pomozov@gmail.com","sentAt":"2008-09-21T15:58:30Z","receivedAt":"2008-09-21T15:58:30Z","isPatch":true,"sender":{"key":"anatol.pomozov@gmail.com","avatar":"https://gravatar.com/avatar/71fc20093402ce987148294ec0999d025209e52da6762789ff248cf5c317645f?d=mp&s=160"},"body":"Hi,\n\nJeff mostly explained what I expect from 'git log' and I agree with\nhim. We ('git log' 'git rev-parse' ...) should separate cases when\nrevision is broken (like we have a junk in the HEAD file) from the\ncase when \"branch is not created yet\" (which is part of normal\nworkflow).\n\nWhat about following algorithm. HEAD points to ref and ref is not\ncreated yet. Additional check could be\n a) there are no other refs\n or/and b) object database is empty\n\n>> - argc = setup_revisions(argc, argv, rev, \"HEAD\");\n>> + argc = setup_revisions(argc, argv, rev, NULL);\n>> + if (!rev->pending.nr) {\n>> + add_head_to_pending(rev);\n>> + if (!rev->pending.nr) {\n>> + printf(\"No commits (yet).\\n\");\n>> + exit(0);\n>> + }\n>> + }\n>\n> I like the idea of an improved message, but such a message should\n> definitely not go to stdout; it would feed nonsense to a command like\n> \"git log | my_log_filter\".\n\n+1 here. By default 'git log' should not output anything in this case,\neven \"No commits yet\". Although such message would be fine for\nsomething like \"git log --verbose\"\n\n-- \nanatol\n"},{"id":"91242","messageId":"gb5uq6$565$1@ger.gmane.org","threadId":"15538","inReplyTo":"3665a1a00809210858r1c494d22p77b5e9964c06424e@mail.gmail.com","subject":"Re: Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-09-21T17:04:08Z","receivedAt":"2008-09-21T17:04:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Anatol Pomozov wrote:\n\n> What about following algorithm. HEAD points to ref and ref is not\n> created yet. Additional check could be\n>  a) there are no other refs\n>  or/and b) object database is empty\n\nWhat about the case when you create _independent_ branch (additional\nroot, i.e. parentless commit), like 'html', 'man' and 'todo' branches\nin git.git repository? Neither a) nor b) applies, and I don't consider\nsuch situation a bug. HEAD should be in proper symref format:\n\"ref: refs/heads/\".\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"91250","messageId":"7v63opz66t.fsf@gitster.siamese.dyndns.org","threadId":"15538","inReplyTo":"20080921135616.GA25238@sigill.intra.peff.net","subject":"Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-21T18:48:10Z","receivedAt":"2008-09-21T18:48:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Sep 19, 2008 at 01:27:46PM -0700, Junio C Hamano wrote:\n> ...\n>> (2) \"--root\" is about \"do we show a creation event as a huge diff from\n>> emptyness?\". Yes, we turn it on for \"git log\" but it does not have\n>> anything to do with the issue of yet to be born branch, where there\n>> isn't even a big creation event yet.\n>\n> What about index comparisons? What should an index comparison to a\n> branch yet-to-be-born look like? Right now it is an error.\n\nIt should be an error, because that is _not_ even an comparison. At least\nat diff-index level.\n\nThe diff wrapper UI could do something different, though. And an obvious\nthing to do is to give a fake creation event.\n\nThe current output feels perfectly sensible to me.\n\n\t$ mkdir d; cd d; tar xf .../t.tar; git init; git add .\n\t$ git diff --cached\n fatal: No HEAD commit to compare with (yet)\n\nThe alternative is no different from \"find . -type f | xargs cat\" from the\npoint of view of reviewability. To make sure you have what you want in\nyour initial revision, so that you can get things right from the start,\nyou would want to check things like:\n\n\t$ tar df .../t.tar ;# did I change anything?\n $ git ls-files ;# do I have what I want?\n $ git clean -n ;# have I missed anything?\n\nBy allowing an auto-fallback to the comparison with an empty tree object, \nyou are giving these possibilities:\n\n\t$ git diff --cached --stat\n\t$ git diff --cached --name-only\n\nbut the latter is already available from ls-files anyway, and the former\ndoes not feel so interesting. \n\nIn exchange, we lose the reminder to the user that this is a creation\nevent. An interactive user (remember, I am not talking about diff-index\nhere, but diff front-end) may want to treat it specially perhaps by being\nextra careful. If there were no downsides like this in \"fall back to\ncomparing with an empty tree\" approach, I wouldn't hesitate to agree it is\na good idea, though.\n\n>> I am reluctant to agree with the opinion that \"git log\" should be _silent_\n>> in a world without any history.\n>\n> It feels a bit more Unix-y to me. That is, if I am asking for some set\n> of commits, and there are _no_ commits in the set, then I expect no\n> output.\n\nTo this, I am inclined to agree. We could do something like the attached\npatch, but there is a caveat.\n\nThere may be commands that\n\n (1) cannot sanely operate without any positive ref;\n\n (2) give default \"HEAD\";\n\n (3) do not have its own input verification to make sure there is at least\n one positive ref, because they have been relying on revision\n machinery to die() with the existing check.\n\nand this patch is actively breaking them. If people like this approach\n(and I probably will join them), commands that match the above criteria\nneed to be identified and fixed by setting revs->require_valid_def.\n\n revision.c | 22 ++++++++++++++++++----\n revision.h | 5 ++++-\n 2 files changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git i/revision.c w/revision.c\nindex 2f646de..5ce7795 100644\n--- i/revision.c\n+++ w/revision.c\n@@ -1295,12 +1295,26 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\tprepare_show_merge(revs);\n \tif (revs->def && !revs->pending.nr) {\n \t\tunsigned char sha1[20];\n-\t\tstruct object *object;\n \t\tunsigned mode;\n-\t\tif (get_sha1_with_mode(revs->def, sha1, &mode))\n+\t\tint flag;\n+\t\tif (!get_sha1_with_mode(revs->def, sha1, &mode)) {\n+\t\t\tstruct object *object;\n+\t\t\tobject = get_reference(revs, revs->def, sha1, 0);\n+\t\t\tadd_pending_object_with_mode(revs, object, revs->def, mode);\n+\t\t} else if (!revs->require_valid_def &&\n+\t\t\t !strcmp(revs->def, \"HEAD\") &&\n+\t\t\t resolve_ref(revs->def, sha1, 0, &flag) &&\n+\t\t\t (flag & REF_ISSYMREF)) {\n+\t\t\t/*\n+\t\t\t * Most commands can operate on an unborn branch\n+\t\t\t * without an explicit ref parameter as if no\n+\t\t\t * positive ref was specified (as opposed to the\n+\t\t\t * traditional behaviour of barfing on invalid HEAD).\n+\t\t\t */\n+\t\t\t;\n+\t\t} else {\n \t\t\tdie(\"bad default revision '%s'\", revs->def);\n-\t\tobject = get_reference(revs, revs->def, sha1, 0);\n-\t\tadd_pending_object_with_mode(revs, object, revs->def, mode);\n+\t\t}\n \t}\n \n \t/* Did the user ask for any diff output? Run the diff! */\ndiff --git i/revision.h w/revision.h\nindex 2fdb2dd..7890fb7 100644\n--- i/revision.h\n+++ w/revision.h\n@@ -30,7 +30,10 @@ struct rev_info {\n \tconst char *prefix;\n \tconst char *def;\n \tvoid *prune_data;\n-\tunsigned int early_output;\n+\n+\t/* Miscellaneous */\n+\tunsigned int\tearly_output:1,\n+\t\t\trequire_valid_def:1;\n \n \t/* Traversal flags */\n \tunsigned int\tdense:1,\n"},{"id":"91309","messageId":"20080922131556.GA7133@sigill.intra.peff.net","threadId":"15538","inReplyTo":"3665a1a00809210858r1c494d22p77b5e9964c06424e@mail.gmail.com","subject":"Re: Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-22T13:15:57Z","receivedAt":"2008-09-22T13:15:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 21, 2008 at 08:58:30AM -0700, Anatol Pomozov wrote:\n\n> revision is broken (like we have a junk in the HEAD file) from the\n> case when \"branch is not created yet\" (which is part of normal\n> workflow).\n> \n> What about following algorithm. HEAD points to ref and ref is not\n> created yet. Additional check could be\n> a) there are no other refs\n> or/and b) object database is empty\n\nI don't think those things have to do with \"branch not created yet\".\nThey are about \"repository has no branches\".\n\nThe only thing that indicates you a branch has not been created is the\nabsence of that ref. So you really have two situations there:\n\n 1. a symref like HEAD points to a branch that does not exist\n\n 2. the user inputs a ref-name that does not exist\n\nIn the former, that means we are on a branch that hasn't been created\nyet (which generally does mean a new repo, but it's possible to reach\nthis state in other ways). Or it means somebody made a mistake when they\nupdated the symref.\n\nIn the latter, it _could_ mean that we are interested in a branch yet to\nbe born, but in general we consider it to mean the user made a typo.\n\n-Peff\n"},{"id":"91310","messageId":"20080922133230.GB7133@sigill.intra.peff.net","threadId":"15538","inReplyTo":"7v63opz66t.fsf@gitster.siamese.dyndns.org","subject":"Re: Re* [RFC/PATCH] extend meaning of \"--root\" option to index comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-09-22T13:32:30Z","receivedAt":"2008-09-22T13:32:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 21, 2008 at 11:48:10AM -0700, Junio C Hamano wrote:\n\n> > What about index comparisons? What should an index comparison to a\n> > branch yet-to-be-born look like? Right now it is an error.\n> \n> It should be an error, because that is _not_ even an comparison. At least\n> at diff-index level.\n> \n> The diff wrapper UI could do something different, though. And an obvious\n> thing to do is to give a fake creation event.\n\nSo is that an implicit endorsement of my \"diff --cached --root\" patch,\nwhich does exactly that?\n\n> The current output feels perfectly sensible to me.\n> \n> \t$ mkdir d; cd d; tar xf .../t.tar; git init; git add .\n> \t$ git diff --cached\n> fatal: No HEAD commit to compare with (yet)\n\nSure. I don't think any behavior should be changed without a \"please\ntreat branch-to-be-born as an empty tree\" flag.\n\n> The alternative is no different from \"find . -type f | xargs cat\" from the\n> point of view of reviewability. To make sure you have what you want in\n\nInteresting comparison. The find example you give has a problem if there\nare no files. But my patch is more akin to adding a --no-run-if-empty\nflag to xargs here.\n\n> By allowing an auto-fallback to the comparison with an empty tree object, \n> you are giving these possibilities:\n> \n> \t$ git diff --cached --stat\n> \t$ git diff --cached --name-only\n> \n> but the latter is already available from ls-files anyway, and the former\n> does not feel so interesting. \n\nI didn't think we are introducing any new possibilities anyway, since\none can always just compare against the empty tree manually (though I\nthink \"git diff --cached --stat\" might be useful for a \"status\"-like\nscript).\n\nThe advantage is saving callers from having to do two _different_ things\nfor the initial and regular commit cases.\n\nAnd for interactive users, seeing the error and saying \"Oh, I really\nwould like to see the diff against the empty tree, but I can't remember\nthe SHA-1 of the empty tree\" (though for that, I have also been running\nwith a fake ref \"EMPTY\" which is just simpler to remember). So instead\nthey can just repeat the command with \"--root\".\n\n> In exchange, we lose the reminder to the user that this is a creation\n> event. An interactive user (remember, I am not talking about diff-index\n> here, but diff front-end) may want to treat it specially perhaps by being\n> extra careful. If there were no downsides like this in \"fall back to\n> comparing with an empty tree\" approach, I wouldn't hesitate to agree it is\n> a good idea, though.\n\nYou seem to be arguing against doing this by default, which I am not\nreally advocating (to be honest, I am not 100% sure I am advocating\n\"diff --cached --root\", but this discussion is helping me sort out the\npositives and negatives). It only makes sense to me with an extra flag\nthat explicitly says \"and if there is no HEAD, do this fallback.\"\n\n> To this, I am inclined to agree. We could do something like the attached\n> patch, but there is a caveat.\n\nI do think this approach makes sense (and it mirrors what I just\nexplained in my last mail to Anatol, which is that we are really talking\nabout identifying non-existent branches in symrefs).\n\nBut I agree there will be a big fallout as we break many of the callers\nwho expect the barfing. I can try to look at some of the implications,\nbut expect a delay there from me due to real life concerns.\n\n> +\t\t} else if (!revs->require_valid_def &&\n> +\t\t\t !strcmp(revs->def, \"HEAD\") &&\n> +\t\t\t resolve_ref(revs->def, sha1, 0, &flag) &&\n> +\t\t\t (flag & REF_ISSYMREF)) {\n\nI wonder if this should be restricted to HEAD and not simply all\nsymrefs. The thing we are really pointing out is that something points\nto a non-existing ref, and because that something is on disk and not\ntyped in by the user, we are assuming it is not just a mistake.\n\nThe only other example I can think of is a refs/remotes/$foo/HEAD, which\nwe might access via \"$foo\", can point to an unborn branch.\n\nBut maybe it is best to be conservative at first and stick with \"HEAD\".\n\n-Peff\n"}]}