{"thread":{"id":"47050","subject":"[RFC PATCH 0/3] git-describe ?","startedAt":"2017-10-28T00:44:26Z","lastAt":"2017-11-20T18:19:15Z","messageCount":110,"participants":["Stefan Beller","Johannes Schindelin","Jacob Keller","Junio C Hamano","Kevin Daudt","Eric Sunshine","Jeff King","Andreas Schwab","Philip Oakley","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"331200","messageId":"20171028004419.10139-1-sbeller@google.com","threadId":"47050","inReplyTo":null,"subject":"[RFC PATCH 0/3] git-describe ?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-28T00:44:15Z","receivedAt":"2017-10-28T00:44:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Occasionally a user is given an object hash from a blob as an error message\nor other output (e.g. [1]).\n\nIt would be useful to get a further description of such a blob, such as\nthe (commit, path) tuple where this blob was introduced.\n\nThis implements the answer in builtin/describe, but I am not sure if that\nis the right place. (One office mate argued it could be a \"reverse-blame\"\nthat tells you all the trees/commits where the blob is referenced from).\n\nThis is RFC for other reasons as well: tests, docs missing.\n\nAny feedback welcome,\n\nthanks,\nStefan\n\n[1] https://stackoverflow.com/questions/10622179/how-to-find-identify-large-files-commits-in-git-history\n\nStefan Beller (3):\n list-objects.c: factor out traverse_trees_and_blobs\n revision.h: introduce blob/tree walking in order of the commits\n builtin/describe: describe blobs\n\n builtin/describe.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++----\n list-objects.c | 48 +++++++++++++++++++++++++++-----------------\n revision.h | 3 ++-\n 3 files changed, 86 insertions(+), 23 deletions(-)\n\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331201","messageId":"20171028004506.10253-1-sbeller@google.com","threadId":"47050","inReplyTo":"20171028004419.10139-1-sbeller@google.com","subject":"[PATCH 1/3] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-28T00:45:04Z","receivedAt":"2017-10-28T00:45:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"With traverse_trees_and_blobs factored out of the main traverse function,\nthe next patch can introduce an in-order revision walking with ease.\n\nSigned-off-by: Stefan Beller \n---\n list-objects.c | 46 ++++++++++++++++++++++++++++------------------\n 1 file changed, 28 insertions(+), 18 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex b3931fa434..0ee0551604 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -183,25 +183,13 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-void traverse_commit_list(struct rev_info *revs,\n-\t\t\t show_commit_fn show_commit,\n-\t\t\t show_object_fn show_object,\n-\t\t\t void *data)\n+static void traverse_trees_and_blobs(struct rev_info *revs,\n+\t\t\t\t struct strbuf *base,\n+\t\t\t\t show_object_fn show_object,\n+\t\t\t\t void *data)\n {\n \tint i;\n-\tstruct commit *commit;\n-\tstruct strbuf base;\n \n-\tstrbuf_init(&base, PATH_MAX);\n-\twhile ((commit = get_revision(revs)) != NULL) {\n-\t\t/*\n-\t\t * an uninteresting boundary commit may not have its tree\n-\t\t * parsed yet, but we are not going to show them anyway\n-\t\t */\n-\t\tif (commit->tree)\n-\t\t\tadd_pending_tree(revs, commit->tree);\n-\t\tshow_commit(commit, data);\n-\t}\n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n \t\tstruct object *obj = pending->item;\n@@ -218,17 +206,39 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t\tpath = \"\";\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\n \t\t oid_to_hex(&obj->oid), name);\n \t}\n \tobject_array_clear(&revs->pending);\n+}\n+\n+void traverse_commit_list(struct rev_info *revs,\n+\t\t\t show_commit_fn show_commit,\n+\t\t\t show_object_fn show_object,\n+\t\t\t void *data)\n+{\n+\tstruct commit *commit;\n+\tstruct strbuf base;\n+\tstrbuf_init(&base, PATH_MAX);\n+\n+\twhile ((commit = get_revision(revs)) != NULL) {\n+\t\t/*\n+\t\t * an uninteresting boundary commit may not have its tree\n+\t\t * parsed yet, but we are not going to show them anyway\n+\t\t */\n+\t\tif (commit->tree)\n+\t\t\tadd_pending_tree(revs, commit->tree);\n+\t\tshow_commit(commit, data);\n+\t}\n+\ttraverse_trees_and_blobs(revs, &base, show_object, data);\n+\n \tstrbuf_release(&base);\n }\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331202","messageId":"20171028004506.10253-2-sbeller@google.com","threadId":"47050","inReplyTo":"20171028004506.10253-1-sbeller@google.com","subject":"[PATCH 2/3] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-28T00:45:05Z","receivedAt":"2017-10-28T00:45:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This will be useful shortly.\n\nSigned-off-by: Stefan Beller \n---\n list-objects.c | 2 ++\n revision.h | 3 ++-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 0ee0551604..ef64a237d3 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -237,6 +237,8 @@ void traverse_commit_list(struct rev_info *revs,\n \t\tif (commit->tree)\n \t\t\tadd_pending_tree(revs, commit->tree);\n \t\tshow_commit(commit, data);\n+\t\tif (revs->tree_blobs_in_commit_order)\n+\t\t\ttraverse_trees_and_blobs(revs, &base, show_object, data);\n \t}\n \ttraverse_trees_and_blobs(revs, &base, show_object, data);\n \ndiff --git a/revision.h b/revision.h\nindex 54761200ad..86985d68aa 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,7 +121,8 @@ struct rev_info {\n \t\t\tbisect:1,\n \t\t\tancestry_path:1,\n \t\t\tfirst_parent_only:1,\n-\t\t\tline_level_traverse:1;\n+\t\t\tline_level_traverse:1,\n+\t\t\ttree_blobs_in_commit_order:1;\n \n \t/* Diff flags */\n \tunsigned int\tdiff:1,\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331203","messageId":"20171028004506.10253-3-sbeller@google.com","threadId":"47050","inReplyTo":"20171028004506.10253-1-sbeller@google.com","subject":"[PATCH 3/3] builtin/describe: describe blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-28T00:45:06Z","receivedAt":"2017-10-28T00:45:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Sometimes users are given a hash of an object and they want to\nidentify it further (ex.: Use verify-pack to find the largest blobs,\nbut what are these? or [1])\n\nThe best identification of a blob hash is done via a its path at a\ngiven commit, which this implements.\n\n[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 54 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 29075dbd0f..752de5843b 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -11,8 +11,9 @@\n #include \"hashmap.h\"\n #include \"argv-array.h\"\n #include \"run-command.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n \n-#define SEEN\t\t(1u << 0)\n #define MAX_TAGS\t(FLAG_BITS - 1)\n \n static const char * const describe_usage[] = {\n@@ -282,6 +283,50 @@ static void show_suffix(int depth, const struct object_id *oid)\n \tprintf(\"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n }\n \n+struct blob_descriptor {\n+\tstruct object_id current_commit;\n+\tstruct object_id looking_for;\n+};\n+\n+static void process_commit(struct commit *commit, void *data)\n+{\n+\tstruct blob_descriptor *bd = data;\n+\n+\tbd->current_commit = commit->object.oid;\n+}\n+\n+static void process_object(struct object *obj, const char *name, void *data)\n+{\n+\tstruct blob_descriptor *bd = data;\n+\n+\tif (!oidcmp(&bd->looking_for, &obj->oid))\n+\t\tprintf(_(\"blob %s present at path %s in commit %s\\n\"),\n+\t\t\toid_to_hex(&bd->looking_for), name,\n+\t\t\toid_to_hex(&bd->current_commit));\n+}\n+\n+static void describe_blob(struct object_id oid)\n+{\n+\tstruct rev_info revs;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct blob_descriptor bd = { null_oid, oid };\n+\n+\targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n+\t\t\"--all\", \"--single-worktree\", \"--objects\", NULL);\n+\n+\trevs.tree_blobs_in_commit_order = 1;\n+\n+\tinit_revisions(&revs, NULL);\n+\n+\tif (setup_revisions(args.argc, args.argv, &revs, NULL) > 1)\n+\t\tBUG(\"setup_revisions could not handle all args?\");\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\ttraverse_commit_list(&revs, process_commit, process_object, &bd);\n+}\n+\n static void describe(const char *arg, int last_one)\n {\n \tstruct object_id oid;\n@@ -295,9 +340,14 @@ static void describe(const char *arg, int last_one)\n \n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference_gently(&oid, 1);\n+\tif (!cmit) {\n+\t\tif (lookup_blob(&oid))\n+\t\t\tdescribe_blob(oid);\n+\t\telse\n+\t\t\tdie(_(\"%s is not a commit nor blob\"), arg);\n+\t\treturn;\n+\t}\n \n \tn = find_commit_name(&cmit->object.oid);\n \tif (n && (tags || all || n->prio == 2)) {\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331225","messageId":"alpine.DEB.2.21.1.1710281802530.6482@virtualbox","threadId":"47050","inReplyTo":"20171028004419.10139-1-sbeller@google.com","subject":"Re: [RFC PATCH 0/3] git-describe ?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-28T16:04:19Z","receivedAt":"2017-10-28T16:04:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 27 Oct 2017, Stefan Beller wrote:\n\n> Occasionally a user is given an object hash from a blob as an error message\n> or other output (e.g. [1]).\n> \n> It would be useful to get a further description of such a blob, such as\n> the (commit, path) tuple where this blob was introduced.\n> \n> This implements the answer in builtin/describe, but I am not sure if that\n> is the right place. (One office mate argued it could be a \"reverse-blame\"\n> that tells you all the trees/commits where the blob is referenced from).\n> \n> This is RFC for other reasons as well: tests, docs missing.\n> \n> Any feedback welcome,\n\nAs you ask so nicely, I'll just cheer (I do not have time to review it\nright now, but I really, really wanted this, I even started working on the\nname-rev side of things some time ago but eventually had to let things\nslide).\n\nCiao,\nDscho\n"},{"id":"331231","messageId":"alpine.DEB.2.21.1.1710281913060.6482@virtualbox","threadId":"47050","inReplyTo":"20171028004506.10253-1-sbeller@google.com","subject":"Re: [PATCH 1/3] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-28T17:15:39Z","receivedAt":"2017-10-28T17:15:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\n[I was intrigued enough by your work to postpone to later this coming week\nreading the What's cooking email in favor of reviewing your patch series.]\n\nOn Fri, 27 Oct 2017, Stefan Beller wrote:\n\n> With traverse_trees_and_blobs factored out of the main traverse function,\n> the next patch can introduce an in-order revision walking with ease.\n\nMakes sense.\n\n> diff --git a/list-objects.c b/list-objects.c\n> index b3931fa434..0ee0551604 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -183,25 +183,13 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n> \tadd_pending_object(revs, &tree->object, \"\");\n> }\n> \n> -void traverse_commit_list(struct rev_info *revs,\n> -\t\t\t show_commit_fn show_commit,\n> -\t\t\t show_object_fn show_object,\n> -\t\t\t void *data)\n> +static void traverse_trees_and_blobs(struct rev_info *revs,\n> +\t\t\t\t struct strbuf *base,\n\nIn the context of one function, it was obvious what `base` meant. Maybe we\ncan call it `base_path` now?\n\nThanks,\nDscho\n"},{"id":"331233","messageId":"alpine.DEB.2.21.1.1710281917260.6482@virtualbox","threadId":"47050","inReplyTo":"20171028004506.10253-2-sbeller@google.com","subject":"Re: [PATCH 2/3] revision.h: introduce blob/tree walking in order of the commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-28T17:20:44Z","receivedAt":"2017-10-28T17:20:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 27 Oct 2017, Stefan Beller wrote:\n\n> This will be useful shortly.\n\nSomething tells me that I will hate you in the future when I read this\ncommit message and lack the context (e.g. when blaming, where I cannot see\nthe child commits let alone the comment in the Merge commit).\n\nHow about:\n\n\tThe functionality to list tree objects in the order they were seen\n\twhile traversing the commits will be used in the next commit,\n\twhere we teach `git describe` to describe not only commits, but\n\ttrees and blobs, too.\n\nThe diff itself is amazingly easy to review, and obviously correct.\n\nCiao,\nDscho\n"},{"id":"331234","messageId":"alpine.DEB.2.21.1.1710281926020.6482@virtualbox","threadId":"47050","inReplyTo":"20171028004506.10253-3-sbeller@google.com","subject":"Re: [PATCH 3/3] builtin/describe: describe blobs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-28T17:32:06Z","receivedAt":"2017-10-28T17:32:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 27 Oct 2017, Stefan Beller wrote:\n\n> Sometimes users are given a hash of an object and they want to identify\n> it further (ex.: Use verify-pack to find the largest blobs, but what are\n> these? or [1])\n> \n> The best identification of a blob hash is done via a its path at a\n> given commit, which this implements.\n> \n> [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n\nI also came up with a script to do that:\nhttps://github.com/msysgit/msysgit/blob/master/bin/what-made-this-repo-so-large.sh\n\nYour method is much more elegant, of course (it describes the commit in the\nsame run as it finds the object, and it does not output tons of stuff only\nto be filtered).\n\n> @@ -282,6 +283,50 @@ static void show_suffix(int depth, const struct object_id *oid)\n> \tprintf(\"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n> }\n> \n> +struct blob_descriptor {\n> +\tstruct object_id current_commit;\n> +\tstruct object_id looking_for;\n> +};\n\nPersonally, I would call this process_commit_data, but I do not mind too\nmuch about the name.\n\n> +static void process_object(struct object *obj, const char *name, void *data)\n> +{\n> +\tstruct blob_descriptor *bd = data;\n> +\n> +\tif (!oidcmp(&bd->looking_for, &obj->oid))\n> +\t\tprintf(_(\"blob %s present at path %s in commit %s\\n\"),\n> +\t\t\toid_to_hex(&bd->looking_for), name,\n> +\t\t\toid_to_hex(&bd->current_commit));\n> +}\n\ns/name/path/\n\n> @@ -295,9 +340,14 @@ static void describe(const char *arg, int last_one)\n> \n> \tif (get_oid(arg, &oid))\n> \t\tdie(_(\"Not a valid object name %s\"), arg);\n> -\tcmit = lookup_commit_reference(&oid);\n> -\tif (!cmit)\n> -\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n> +\tcmit = lookup_commit_reference_gently(&oid, 1);\n> +\tif (!cmit) {\n> +\t\tif (lookup_blob(&oid))\n> +\t\t\tdescribe_blob(oid);\n> +\t\telse\n> +\t\t\tdie(_(\"%s is not a commit nor blob\"), arg);\n\ns/not/neither/\n\nNicely done, sir!\n\nI wonder whether it would make sense to extend this to tree objects while\nwe are at it, but maybe that's an easy up-for-grabs.\n\nThank you very much!\nDscho\n"},{"id":"331246","messageId":"CA+P7+xrtJrN8t=TWMuKCq3OC_YJJkkO66QkerAja=hCp6Vh8sA@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1710281926020.6482@virtualbox","subject":"Re: [PATCH 3/3] builtin/describe: describe blobs","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-28T22:47:08Z","receivedAt":"2017-10-28T22:47:35Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sat, Oct 28, 2017 at 10:32 AM, Johannes Schindelin\n wrote:\n> Hi Stefan,\n>\n>\n> Nicely done, sir!\n>\n> I wonder whether it would make sense to extend this to tree objects while\n> we are at it, but maybe that's an easy up-for-grabs.\n>\n> Thank you very much!\n> Dscho\n\nI'd very much like the same ability for trees and gitlinks as well!\nBut it does seem it might be fairly easy.\n\nThanks,\nJake\n"},{"id":"331257","messageId":"CAGZ79kaYdgf809z=DebzuUzuWbxgc5RFwz96yeh5gQb2JV0LoQ@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1710281913060.6482@virtualbox","subject":"Re: [PATCH 1/3] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-29T03:13:12Z","receivedAt":"2017-10-29T03:13:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Oct 28, 2017 at 10:15 AM, Johannes Schindelin\n wrote:\n> Hi Stefan,\n>\n> [I was intrigued enough by your work to postpone to later this coming week\n> reading the What's cooking email in favor of reviewing your patch series.]\n>\n> On Fri, 27 Oct 2017, Stefan Beller wrote:\n>\n>> With traverse_trees_and_blobs factored out of the main traverse function,\n>> the next patch can introduce an in-order revision walking with ease.\n>\n> Makes sense.\n>\n>> diff --git a/list-objects.c b/list-objects.c\n>> index b3931fa434..0ee0551604 100644\n>> --- a/list-objects.c\n>> +++ b/list-objects.c\n>> @@ -183,25 +183,13 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n>> add_pending_object(revs, &tree->object, \"\");\n>> }\n>>\n>> -void traverse_commit_list(struct rev_info *revs,\n>> - show_commit_fn show_commit,\n>> - show_object_fn show_object,\n>> - void *data)\n>> +static void traverse_trees_and_blobs(struct rev_info *revs,\n>> + struct strbuf *base,\n>\n> In the context of one function, it was obvious what `base` meant. Maybe we\n> can call it `base_path` now?\n\nI was intrigued to keep the base local to the factored out function, but that\nwould mean, we'd have to allocate memory for it in every call. That I wanted\nto avoid, so the only reason to pass it in, is memory management.\n\nbase_path sounds good, will rename.\n\nThanks for the review!\nStefan\n\n>\n> Thanks,\n> Dscho\n"},{"id":"331258","messageId":"CAGZ79kaVqSzzPebHN4=extkAPpyi=VagmjFU8mh_A_tYoJgU8g@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1710281917260.6482@virtualbox","subject":"Re: [PATCH 2/3] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-29T03:22:39Z","receivedAt":"2017-10-29T03:22:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Oct 28, 2017 at 10:20 AM, Johannes Schindelin\n wrote:\n> Hi Stefan,\n>\n> On Fri, 27 Oct 2017, Stefan Beller wrote:\n>\n>> This will be useful shortly.\n>\n> Something tells me that I will hate you in the future when I read this\n> commit message and lack the context (e.g. when blaming, where I cannot see\n> the child commits let alone the comment in the Merge commit).\n\nThanks for calling me out.\n\n(Not making excuses, but...) I remembered some other senior members\nhaving such commit messages, so I felt like I had to step up my game.\n\nhttps://public-inbox.org/git/70fbcd573f5c8a78a19a08ffc255437c36e7f49d.1495014840.git.mhagger@alum.mit.edu/\n\n>\n> How about:\n>\n> The functionality to list tree objects in the order they were seen\n> while traversing the commits will be used in the next commit,\n> where we teach `git describe` to describe not only commits, but\n> trees and blobs, too.\n\nSounds good. If only we had git-when-merged[1] as some easy upstream\nfunction, then the original commit message would provoke less hate as\nfuture us would have an easier time finding the next commit.\n\n[1] https://github.com/mhagger/git-when-merged\n\nWhen writing the patch I wondered if this could be useful for other\nuse cases as well, i.e. do we want to have a command line argument\nto trigger this behavior?\n\nThanks,\nStefan\n"},{"id":"331259","messageId":"CAGZ79kYEfFkUBZ18s=tDo+Dzo1KEA-MVNTZjLim_HHdZmeMa+A@mail.gmail.com","threadId":"47050","inReplyTo":"CAGZ79kaVqSzzPebHN4=extkAPpyi=VagmjFU8mh_A_tYoJgU8g@mail.gmail.com","subject":"Re: [PATCH 2/3] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-29T03:23:19Z","receivedAt":"2017-10-29T03:23:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Oct 28, 2017 at 8:22 PM, Stefan Beller wrote:\n\n>\n> (Not making excuses, but...) I remembered some other senior members\n> having such commit messages, so I felt like I had to step up my game.\n>\n> https://public-inbox.org/git/70fbcd573f5c8a78a19a08ffc255437c36e7f49d.1495014840.git.mhagger@alum.mit.edu/\n\nI forgot to note though, that this never made it into the official tree, though.\n"},{"id":"331260","messageId":"CAGZ79kYpPga-aWmJYXCXT1WyvfeEKnwXEXxq2E0YNYanhUw-Xw@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1710281926020.6482@virtualbox","subject":"Re: [PATCH 3/3] builtin/describe: describe blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-29T03:28:54Z","receivedAt":"2017-10-29T03:29:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Oct 28, 2017 at 10:32 AM, Johannes Schindelin\n wrote:\n> Hi Stefan,\n>\n> On Fri, 27 Oct 2017, Stefan Beller wrote:\n>\n>> Sometimes users are given a hash of an object and they want to identify\n>> it further (ex.: Use verify-pack to find the largest blobs, but what are\n>> these? or [1])\n>>\n>> The best identification of a blob hash is done via a its path at a\n>> given commit, which this implements.\n>>\n>> [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n>\n> I also came up with a script to do that:\n> https://github.com/msysgit/msysgit/blob/master/bin/what-made-this-repo-so-large.sh\n>\n> Your method is much more elegant, of course (it describes the commit in the\n> same run as it finds the object, and it does not output tons of stuff only\n> to be filtered).\n\nThat was the task I was given. Discussion here turned out that users\nmostly only ever care about commit object names, trees and blobs are\nonly useful when specifically given.\n\n\n>\n>> @@ -282,6 +283,50 @@ static void show_suffix(int depth, const struct object_id *oid)\n>> printf(\"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n>> }\n>>\n>> +struct blob_descriptor {\n>> + struct object_id current_commit;\n>> + struct object_id looking_for;\n>> +};\n>\n> Personally, I would call this process_commit_data, but I do not mind too\n> much about the name.\n\nI'll take any naming suggestion, as this code was assembled in a hurry using\ncopy/paste and trial&error as the high-tech methods used.\n\n>> +static void process_object(struct object *obj, const char *name, void *data)\n>> +{\n>> + struct blob_descriptor *bd = data;\n>> +\n>> + if (!oidcmp(&bd->looking_for, &obj->oid))\n>> + printf(_(\"blob %s present at path %s in commit %s\\n\"),\n>> + oid_to_hex(&bd->looking_for), name,\n>> + oid_to_hex(&bd->current_commit));\n>> +}\n>\n> s/name/path/\n>\n>> @@ -295,9 +340,14 @@ static void describe(const char *arg, int last_one)\n>>\n>> if (get_oid(arg, &oid))\n>> die(_(\"Not a valid object name %s\"), arg);\n>> - cmit = lookup_commit_reference(&oid);\n>> - if (!cmit)\n>> - die(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n>> + cmit = lookup_commit_reference_gently(&oid, 1);\n>> + if (!cmit) {\n>> + if (lookup_blob(&oid))\n>> + describe_blob(oid);\n>> + else\n>> + die(_(\"%s is not a commit nor blob\"), arg);\n>\n> s/not/neither/\n>\n> Nicely done, sir!\n>\n> I wonder whether it would make sense to extend this to tree objects while\n> we are at it, but maybe that's an easy up-for-grabs.\n\nI can look into incorporating that, too. What is the use case though?\n(Is there any error message, common enough that users want to\nidentify trees?)\n\nThanks for the review,\nStefan\n"},{"id":"331261","messageId":"xmqqfua2aab1.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CAGZ79kYEfFkUBZ18s=tDo+Dzo1KEA-MVNTZjLim_HHdZmeMa+A@mail.gmail.com","subject":"Re: [PATCH 2/3] revision.h: introduce blob/tree walking in order of the commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-29T03:43:14Z","receivedAt":"2017-10-29T03:43:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Sat, Oct 28, 2017 at 8:22 PM, Stefan Beller wrote:\n>\n>>\n>> (Not making excuses, but...) I remembered some other senior members\n>> having such commit messages, so I felt like I had to step up my game.\n>>\n>> https://public-inbox.org/git/70fbcd573f5c8a78a19a08ffc255437c36e7f49d.1495014840.git.mhagger@alum.mit.edu/\n>\n> I forgot to note though, that this never made it into the official tree, though.\n\nBesides, leading by setting a better example would be the right\napproach to gain seniority (if that is your goal, that is) ;-).\n\n"},{"id":"331264","messageId":"20171029120228.GA6065@alpha.vpn.ikke.info","threadId":"47050","inReplyTo":"CAGZ79kYpPga-aWmJYXCXT1WyvfeEKnwXEXxq2E0YNYanhUw-Xw@mail.gmail.com","subject":"Re: [PATCH 3/3] builtin/describe: describe blobs","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-10-29T12:02:28Z","receivedAt":"2017-10-29T12:02:34Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Sat, Oct 28, 2017 at 08:28:54PM -0700, Stefan Beller wrote:\n> On Sat, Oct 28, 2017 at 10:32 AM, Johannes Schindelin\n> wrote:\n> > \n> > [..]\n> > I wonder whether it would make sense to extend this to tree objects while\n> > we are at it, but maybe that's an easy up-for-grabs.\n> \n> I can look into incorporating that, too. What is the use case though?\n> (Is there any error message, common enough that users want to\n> identify trees?)\n> \n> Thanks for the review,\n> Stefan\n\nNot sure if it's really helpfulp, but sometimes with corrup repos, git\nwould complain about a certain object missing or corrupt, where it might\nbe usefull to find out how it's referenced. Not sure though if this\nwould work in that case, because the repo is already corrupt.\n\nKevin\n"},{"id":"331265","messageId":"alpine.DEB.2.21.1.1710291305190.6482@virtualbox","threadId":"47050","inReplyTo":"CAGZ79kYpPga-aWmJYXCXT1WyvfeEKnwXEXxq2E0YNYanhUw-Xw@mail.gmail.com","subject":"Re: [PATCH 3/3] builtin/describe: describe blobs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-29T12:07:29Z","receivedAt":"2017-10-29T12:07:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\n\nOn Sat, 28 Oct 2017, Stefan Beller wrote:\n\n> On Sat, Oct 28, 2017 at 10:32 AM, Johannes Schindelin\n> wrote:\n>\n> > I wonder whether it would make sense to extend this to tree objects\n> > while we are at it, but maybe that's an easy up-for-grabs.\n> \n> I can look into incorporating that, too. What is the use case though?\n> (Is there any error message, common enough that users want to identify\n> trees?)\n\nI remember that I wanted to know in the past where that tree object came\nfrom that is now missing (back in the days when worktrees gc'ed away\ndetached worktree HEADs and their reflogs), but on second thought, I guess\n`git describe` would go belly up in that case (die()ing due to the very\nsame missing tree object)...\n\nCiao,\nDscho\n"},{"id":"331427","messageId":"20171031003351.22341-1-sbeller@google.com","threadId":"47050","inReplyTo":"20171028004419.10139-1-sbeller@google.com","subject":"[PATCH 0/7] git-describe ","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:44Z","receivedAt":"2017-10-31T00:34:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is not an RFC any more, but a serious series.\n\nOccasionally a user is given an object hash from a blob as an error message\nor other output (e.g. [1]).\n\nIt would be useful to get a further description of such a blob, such as\nthe (commit, path) tuple where this blob was introduced.\n\nThis implements the answer in builtin/describe,\nhowever the heuristics are weak. See patch 6 for details.\n\nAny feedback welcome,\n\nThanks,\nStefan\n\n[1] https://stackoverflow.com/questions/10622179/how-to-find-identify-large-files-commits-in-git-history\n\nStefan Beller (7):\n list-objects.c: factor out traverse_trees_and_blobs\n revision.h: introduce blob/tree walking in order of the commits\n builtin/describe.c: rename `oid` to avoid variable shadowing\n builtin/describe.c: print debug statements earlier\n builtin/describe.c: factor out describe_commit\n builtin/describe.c: describe a blob\n t6120: fix typo in test name\n\n Documentation/git-describe.txt | 12 +++-\n builtin/describe.c | 125 ++++++++++++++++++++++++++++++++---------\n list-objects.c | 50 ++++++++++-------\n revision.c | 2 +\n revision.h | 3 +-\n t/t6100-rev-list-in-order.sh | 44 +++++++++++++++\n t/t6120-describe.sh | 17 +++++-\n 7 files changed, 203 insertions(+), 50 deletions(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331428","messageId":"20171031003351.22341-2-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 1/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:45Z","receivedAt":"2017-10-31T00:34:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"With traverse_trees_and_blobs factored out of the main traverse function,\nthe next patch can introduce an in-order revision walking with ease.\n\nThe variable holding the base path is only used in the newly factored out\nfunction `traverse_trees_and_blobs`, however we keep its scope to\n`traverse_commit_list` to keep the number of invocations for memory\nallocations and release to one per commit traversal. Rename the variable\nto `base_path` for clarity.\n\nSigned-off-by: Stefan Beller \n---\n list-objects.c | 48 +++++++++++++++++++++++++++++-------------------\n 1 file changed, 29 insertions(+), 19 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex b3931fa434..bf46f80dff 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -183,25 +183,13 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-void traverse_commit_list(struct rev_info *revs,\n-\t\t\t show_commit_fn show_commit,\n-\t\t\t show_object_fn show_object,\n-\t\t\t void *data)\n+static void traverse_trees_and_blobs(struct rev_info *revs,\n+\t\t\t\t struct strbuf *base_path,\n+\t\t\t\t show_object_fn show_object,\n+\t\t\t\t void *data)\n {\n \tint i;\n-\tstruct commit *commit;\n-\tstruct strbuf base;\n \n-\tstrbuf_init(&base, PATH_MAX);\n-\twhile ((commit = get_revision(revs)) != NULL) {\n-\t\t/*\n-\t\t * an uninteresting boundary commit may not have its tree\n-\t\t * parsed yet, but we are not going to show them anyway\n-\t\t */\n-\t\tif (commit->tree)\n-\t\t\tadd_pending_tree(revs, commit->tree);\n-\t\tshow_commit(commit, data);\n-\t}\n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n \t\tstruct object *obj = pending->item;\n@@ -218,17 +206,39 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t\tpath = \"\";\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base_path, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base_path, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\n \t\t oid_to_hex(&obj->oid), name);\n \t}\n \tobject_array_clear(&revs->pending);\n-\tstrbuf_release(&base);\n+}\n+\n+void traverse_commit_list(struct rev_info *revs,\n+\t\t\t show_commit_fn show_commit,\n+\t\t\t show_object_fn show_object,\n+\t\t\t void *data)\n+{\n+\tstruct commit *commit;\n+\tstruct strbuf base_path;\n+\tstrbuf_init(&base_path, PATH_MAX);\n+\n+\twhile ((commit = get_revision(revs)) != NULL) {\n+\t\t/*\n+\t\t * an uninteresting boundary commit may not have its tree\n+\t\t * parsed yet, but we are not going to show them anyway\n+\t\t */\n+\t\tif (commit->tree)\n+\t\t\tadd_pending_tree(revs, commit->tree);\n+\t\tshow_commit(commit, data);\n+\t}\n+\ttraverse_trees_and_blobs(revs, &base_path, show_object, data);\n+\n+\tstrbuf_release(&base_path);\n }\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331429","messageId":"20171031003351.22341-6-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 5/7] builtin/describe.c: factor out describe_commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:49Z","receivedAt":"2017-10-31T00:34:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the next patch we'll learn how to describe more than just commits,\nso factor out describing commits into its own function. That will make\nthe next patches easy as we still need to describe a commit as part of\ndescribing blobs.\n\nWhile factoring out the functionality to describe_commit, make sure\nthat any output to stdout is put into a strbuf, which we can print\nafterwards, using puts which also adds the line ending.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 63 ++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 37 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 3136efde31..9e9a5ed5d4 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -256,7 +256,7 @@ static unsigned long finish_depth_computation(\n \treturn seen_commits;\n }\n \n-static void display_name(struct commit_name *n)\n+static void append_name(struct commit_name *n, struct strbuf *dst)\n {\n \tif (n->prio == 2 && !n->tag) {\n \t\tn->tag = lookup_tag(&n->oid);\n@@ -272,19 +272,18 @@ static void display_name(struct commit_name *n)\n \t}\n \n \tif (n->tag)\n-\t\tprintf(\"%s\", n->tag->tag);\n+\t\tstrbuf_addstr(dst, n->tag->tag);\n \telse\n-\t\tprintf(\"%s\", n->path);\n+\t\tstrbuf_addstr(dst, n->path);\n }\n \n-static void show_suffix(int depth, const struct object_id *oid)\n+static void append_suffix(int depth, const struct object_id *oid, struct strbuf *dst)\n {\n-\tprintf(\"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n+\tstrbuf_addf(dst, \"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n }\n \n-static void describe(const char *arg, int last_one)\n+static void describe_commit(struct object_id *oid, struct strbuf *dst)\n {\n-\tstruct object_id oid;\n \tstruct commit *cmit, *gave_up_on = NULL;\n \tstruct commit_list *list;\n \tstruct commit_name *n;\n@@ -293,26 +292,18 @@ static void describe(const char *arg, int last_one)\n \tunsigned long seen_commits = 0;\n \tunsigned int unannotated_cnt = 0;\n \n-\tif (debug)\n-\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n-\n-\tif (get_oid(arg, &oid))\n-\t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference(oid);\n \n \tn = find_commit_name(&cmit->object.oid);\n \tif (n && (tags || all || n->prio == 2)) {\n \t\t/*\n \t\t * Exact match to an existing ref.\n \t\t */\n-\t\tdisplay_name(n);\n+\t\tappend_name(n, dst);\n \t\tif (longformat)\n-\t\t\tshow_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);\n+\t\t\tappend_suffix(0, n->tag ? &n->tag->tagged->oid : oid, dst);\n \t\tif (suffix)\n-\t\t\tprintf(\"%s\", suffix);\n-\t\tprintf(\"\\n\");\n+\t\t\tstrbuf_addstr(dst, suffix);\n \t\treturn;\n \t}\n \n@@ -386,10 +377,9 @@ static void describe(const char *arg, int last_one)\n \tif (!match_cnt) {\n \t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n+\t\t\tstrbuf_addstr(dst, find_unique_abbrev(cmit_oid->hash, abbrev));\n \t\t\tif (suffix)\n-\t\t\t\tprintf(\"%s\", suffix);\n-\t\t\tprintf(\"\\n\");\n+\t\t\t\tstrbuf_addstr(dst, suffix);\n \t\t\treturn;\n \t\t}\n \t\tif (unannotated_cnt)\n@@ -437,15 +427,36 @@ static void describe(const char *arg, int last_one)\n \t\t}\n \t}\n \n-\tdisplay_name(all_matches[0].name);\n+\tappend_name(all_matches[0].name, dst);\n \tif (abbrev)\n-\t\tshow_suffix(all_matches[0].depth, &cmit->object.oid);\n+\t\tappend_suffix(all_matches[0].depth, &cmit->object.oid, dst);\n \tif (suffix)\n-\t\tprintf(\"%s\", suffix);\n-\tprintf(\"\\n\");\n+\t\tstrbuf_addstr(dst, suffix);\n+}\n+\n+static void describe(const char *arg, int last_one)\n+{\n+\tstruct object_id oid;\n+\tstruct commit *cmit;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (debug)\n+\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n+\n+\tif (get_oid(arg, &oid))\n+\t\tdie(_(\"Not a valid object name %s\"), arg);\n+\tcmit = lookup_commit_reference(&oid);\n+\tif (!cmit)\n+\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\n+\tdescribe_commit(&oid, &sb);\n+\n+\tputs(sb.buf);\n \n \tif (!last_one)\n \t\tclear_commit_marks(cmit, -1);\n+\n+\tstrbuf_release(&sb);\n }\n \n int cmd_describe(int argc, const char **argv, const char *prefix)\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331430","messageId":"20171031003351.22341-5-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 4/7] builtin/describe.c: print debug statements earlier","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:48Z","receivedAt":"2017-10-31T00:34:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"For debuggers aid we'd want to print debug statements early, so\nintroduce a new line in the debug output that describes the whole\nfunction, and the change the next debug output to describe why we need\nto search. Conveniently drop the arg from the second line; which will\nbe useful in a follow up commit, that refactors the describe function.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex fd61f463cf..3136efde31 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)\n \tunsigned long seen_commits = 0;\n \tunsigned int unannotated_cnt = 0;\n \n+\tif (debug)\n+\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n+\n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n \tcmit = lookup_commit_reference(&oid);\n@@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)\n \tif (!max_candidates)\n \t\tdie(_(\"no tag exactly matches '%s'\"), oid_to_hex(&cmit->object.oid));\n \tif (debug)\n-\t\tfprintf(stderr, _(\"searching to describe %s\\n\"), arg);\n+\t\tfprintf(stderr, _(\"No exact match on refs or tags, searching to describe\\n\"));\n \n \tif (!have_util) {\n \t\tstruct hashmap_iter iter;\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331431","messageId":"20171031003351.22341-7-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:50Z","receivedAt":"2017-10-31T00:34:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Sometimes users are given a hash of an object and they want to\nidentify it further (ex.: Use verify-pack to find the largest blobs,\nbut what are these? or [1])\n\n\"This is an interesting endeavor, because describing things is hard.\"\n -- me, upon writing this patch.\n\nWhen describing commits, we try to anchor them to tags or refs, as these\nare conceptually on a higher level than the commit. And if there is no ref\nor tag that matches exactly, we're out of luck. So we employ a heuristic\nto make up a name for the commit. These names are ambivalent, there might\nbe different tags or refs to anchor to, and there might be different\npath in the DAG to travel to arrive at the commit precisely.\n\nWhen describing a blob, we want to describe the blob from a higher layer\nas well, which is a tuple of (commit, deep/path) as the tree objects\ninvolved are rather uninteresting. The same blob can be referenced by\nmultiple commits, so how we decide which commit to use? This patch\nimplements a rather naive approach on this: As there are no back pointers\nfrom blobs to commits in which the blob occurs, we'll start walking from\nany tips available, listing the blobs in-order of the commit and once we\nfound the blob, we'll take the first commit that listed the blob. For\nsource code this is likely not the first commit that introduced the blob,\nbut rather the latest commit that contained the blob. For example:\n\n git describe v0.99:Makefile\n v0.99-5-gab6625e06a:Makefile\n\ntells us the latest commit that contained the Makefile as it was in tag\nv0.99 is commit v0.99-5-gab6625e06a (and at the same path), as the next\ncommit on top v0.99-6-gb1de9de2b9 ([PATCH] Bootstrap \"make dist\",\n2005-07-11) touches the Makefile.\n\nLet's see how this description turns out, if it is useful in day-to-day\nuse as I have the intuition that we'd rather want to see the *first*\ncommit that this blob was introduced to the repository (which can be\nachieved easily by giving the `--reverse` flag in the describe_blob rev\nwalk).\n\n[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\nSigned-off-by: Stefan Beller \n---\n Documentation/git-describe.txt | 12 +++++++-\n builtin/describe.c | 65 ++++++++++++++++++++++++++++++++++++++----\n t/t6120-describe.sh | 15 ++++++++++\n 3 files changed, 86 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex c924c945ba..3d618b2445 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -3,7 +3,7 @@ git-describe(1)\n \n NAME\n ----\n-git-describe - Describe a commit using the most recent tag reachable from it\n+git-describe - Describe a commit or blob using the most recent tag reachable from it\n \n \n SYNOPSIS\n@@ -24,6 +24,16 @@ By default (without --all or --tags) `git describe` only shows\n annotated tags. For more information about creating annotated tags\n see the -a and -s options to linkgit:git-tag[1].\n \n+If the given `` refers to a blob, it will be described\n+as `:`, such that the blob can be found\n+at `` in the ` -- `.\n+As blobs do not point at the commits they are contained in,\n+describing blobs is slow as we have to walk the whole graph.\n+\n OPTIONS\n -------\n ...::\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 9e9a5ed5d4..382cbae908 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -3,6 +3,7 @@\n #include \"lockfile.h\"\n #include \"commit.h\"\n #include \"tag.h\"\n+#include \"blob.h\"\n #include \"refs.h\"\n #include \"builtin.h\"\n #include \"exec_cmd.h\"\n@@ -11,8 +12,9 @@\n #include \"hashmap.h\"\n #include \"argv-array.h\"\n #include \"run-command.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n \n-#define SEEN\t\t(1u << 0)\n #define MAX_TAGS\t(FLAG_BITS - 1)\n \n static const char * const describe_usage[] = {\n@@ -434,6 +436,54 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)\n \t\tstrbuf_addstr(dst, suffix);\n }\n \n+struct process_commit_data {\n+\tstruct object_id current_commit;\n+\tstruct object_id looking_for;\n+\tstruct strbuf *dst;\n+};\n+\n+static void process_commit(struct commit *commit, void *data)\n+{\n+\tstruct process_commit_data *pcd = data;\n+\tpcd->current_commit = commit->object.oid;\n+}\n+\n+static void process_object(struct object *obj, const char *path, void *data)\n+{\n+\tstruct process_commit_data *pcd = data;\n+\n+\tif (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n+\t\treset_revision_walk();\n+\t\tdescribe_commit(&pcd->current_commit, pcd->dst);\n+\t\tstrbuf_addf(pcd->dst, \":%s\", path);\n+\t}\n+}\n+\n+static void describe_blob(struct object_id oid, struct strbuf *dst)\n+{\n+\tstruct rev_info revs;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct process_commit_data pcd = { null_oid, oid, dst};\n+\n+\targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n+\t\t\"--all\", \"--reflog\", /* as many starting points as possible */\n+\t\t/* NEEDSWORK: --all is incompatible with worktrees for now: */\n+\t\t\"--single-worktree\",\n+\t\t\"--objects\",\n+\t\t\"--in-commit-order\",\n+\t\tNULL);\n+\n+\tinit_revisions(&revs, NULL);\n+\tif (setup_revisions(args.argc, args.argv, &revs, NULL) > 1)\n+\t\tBUG(\"setup_revisions could not handle all args?\");\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\ttraverse_commit_list(&revs, process_commit, process_object, &pcd);\n+\treset_revision_walk();\n+}\n+\n static void describe(const char *arg, int last_one)\n {\n \tstruct object_id oid;\n@@ -445,11 +495,16 @@ static void describe(const char *arg, int last_one)\n \n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference_gently(&oid, 1);\n \n-\tdescribe_commit(&oid, &sb);\n+\tif (cmit) {\n+\t\tdescribe_commit(&oid, &sb);\n+\t} else {\n+\t\tif (lookup_blob(&oid))\n+\t\t\tdescribe_blob(oid, &sb);\n+\t\telse\n+\t\t\tdie(_(\"%s is neither a commit nor blob\"), arg);\n+\t}\n \n \tputs(sb.buf);\n \ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 1c0e8659d9..3be01316e8 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -310,6 +310,21 @@ test_expect_success 'describe ignoring a borken submodule' '\n \tgrep broken out\n '\n \n+test_expect_success 'describe a blob at a tag' '\n+\techo \"make it a unique blob\" >file &&\n+\tgit add file && git commit -m \"content in file\" &&\n+\tgit tag -a -m \"latest annotated tag\" unique-file &&\n+\tgit describe HEAD:file >actual &&\n+\techo \"unique-file:file\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe a surviving blob' '\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit describe HEAD:file >actual &&\n+\tgrep unique-file-1-g actual\n+'\n+\n test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n \ti=1 &&\n \twhile test $i -lt 8000\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331432","messageId":"20171031003351.22341-8-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 7/7] t6120: fix typo in test name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:51Z","receivedAt":"2017-10-31T00:34:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n t/t6120-describe.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 3be01316e8..fd329f173a 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -304,7 +304,7 @@ test_expect_success 'describe chokes on severely broken submodules' '\n \tmv .git/modules/sub1/ .git/modules/sub_moved &&\n \ttest_must_fail git describe --dirty\n '\n-test_expect_success 'describe ignoring a borken submodule' '\n+test_expect_success 'describe ignoring a broken submodule' '\n \tgit describe --broken >out &&\n \ttest_when_finished \"mv .git/modules/sub_moved .git/modules/sub1\" &&\n \tgrep broken out\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331433","messageId":"20171031003351.22341-4-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 3/7] builtin/describe.c: rename `oid` to avoid variable shadowing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:47Z","receivedAt":"2017-10-31T00:34:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The function `describe` has already a variable named `oid` declared at\nthe beginning of the function for an object id. Do now shadow that\nvariable with a pointer to an object id.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 29075dbd0f..fd61f463cf 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -381,9 +381,9 @@ static void describe(const char *arg, int last_one)\n \t}\n \n \tif (!match_cnt) {\n-\t\tstruct object_id *oid = &cmit->object.oid;\n+\t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tprintf(\"%s\", find_unique_abbrev(oid->hash, abbrev));\n+\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n \t\t\tif (suffix)\n \t\t\t\tprintf(\"%s\", suffix);\n \t\t\tprintf(\"\\n\");\n@@ -392,11 +392,11 @@ static void describe(const char *arg, int last_one)\n \t\tif (unannotated_cnt)\n \t\t\tdie(_(\"No annotated tags can describe '%s'.\\n\"\n \t\t\t \"However, there were unannotated tags: try --tags.\"),\n-\t\t\t oid_to_hex(oid));\n+\t\t\t oid_to_hex(cmit_oid));\n \t\telse\n \t\t\tdie(_(\"No tags can describe '%s'.\\n\"\n \t\t\t \"Try --always, or create some tags.\"),\n-\t\t\t oid_to_hex(oid));\n+\t\t\t oid_to_hex(cmit_oid));\n \t}\n \n \tQSORT(all_matches, match_cnt, compare_pt);\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331434","messageId":"20171031003351.22341-3-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCH 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T00:33:46Z","receivedAt":"2017-10-31T00:34:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The functionality to list tree objects in the order they were seen\nwhile traversing the commits will be used in the next commit,\nwhere we teach `git describe` to describe not only commits, but\ntrees and blobs, too.\n\nHelped-by: Johannes Schindelin \nSigned-off-by: Stefan Beller \n---\n list-objects.c | 2 ++\n revision.c | 2 ++\n revision.h | 3 ++-\n t/t6100-rev-list-in-order.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 50 insertions(+), 1 deletion(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\ndiff --git a/list-objects.c b/list-objects.c\nindex bf46f80dff..5e114c9a8a 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -237,6 +237,8 @@ void traverse_commit_list(struct rev_info *revs,\n \t\tif (commit->tree)\n \t\t\tadd_pending_tree(revs, commit->tree);\n \t\tshow_commit(commit, data);\n+\t\tif (revs->tree_blobs_in_commit_order)\n+\t\t\ttraverse_trees_and_blobs(revs, &base_path, show_object, data);\n \t}\n \ttraverse_trees_and_blobs(revs, &base_path, show_object, data);\n \ndiff --git a/revision.c b/revision.c\nindex d167223e69..9329d4ebbf 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1845,6 +1845,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->dense = 0;\n \t} else if (!strcmp(arg, \"--show-all\")) {\n \t\trevs->show_all = 1;\n+\t} else if (!strcmp(arg, \"--in-commit-order\")) {\n+\t\trevs->tree_blobs_in_commit_order = 1;\n \t} else if (!strcmp(arg, \"--remove-empty\")) {\n \t\trevs->remove_empty_trees = 1;\n \t} else if (!strcmp(arg, \"--merges\")) {\ndiff --git a/revision.h b/revision.h\nindex 54761200ad..86985d68aa 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,7 +121,8 @@ struct rev_info {\n \t\t\tbisect:1,\n \t\t\tancestry_path:1,\n \t\t\tfirst_parent_only:1,\n-\t\t\tline_level_traverse:1;\n+\t\t\tline_level_traverse:1,\n+\t\t\ttree_blobs_in_commit_order:1;\n \n \t/* Diff flags */\n \tunsigned int\tdiff:1,\ndiff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh\nnew file mode 100755\nindex 0000000000..67ebe815d2\n--- /dev/null\n+++ b/t/t6100-rev-list-in-order.sh\n@@ -0,0 +1,44 @@\n+#!/bin/sh\n+\n+test_description='miscellaneous rev-list tests'\n+\n+. ./test-lib.sh\n+\n+\n+test_expect_success 'setup' '\n+\tfor x in one two three four\n+\tdo\n+\t\techo $x >$x &&\n+\t\tgit add $x &&\n+\t\tgit commit -m \"add file $x\"\n+\tdone &&\n+\tfor x in four three\n+\tdo\n+\t\tgit rm $x\n+\t\tgit commit -m \"remove $x\"\n+\tdone &&\n+\tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n+\tcut -c 1-40 > actual < actual.raw &&\n+\n+\t>expect &&\n+\tgit rev-parse HEAD^{commit} >>expect &&\n+\tgit rev-parse HEAD^{tree} >>expect &&\n+\tgit rev-parse HEAD^{tree}:one >>expect &&\n+\tgit rev-parse HEAD^{tree}:two >>expect &&\n+\tgit rev-parse HEAD~1^{commit} >>expect &&\n+\tgit rev-parse HEAD~1^{tree} >>expect &&\n+\tgit rev-parse HEAD~1^{tree}:three >>expect &&\n+\tgit rev-parse HEAD~2^{commit} >>expect &&\n+\tgit rev-parse HEAD~2^{tree} >>expect &&\n+\tgit rev-parse HEAD~2^{tree}:four >>expect &&\n+\tgit rev-parse HEAD~3^{commit} >>expect &&\n+\t# skip HEAD~3^{tree}\n+\tgit rev-parse HEAD~4^{commit} >>expect &&\n+\t# skip HEAD~4^{tree}\n+\tgit rev-parse HEAD~5^{commit} >>expect &&\n+\tgit rev-parse HEAD~5^{tree} >>expect &&\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331457","messageId":"xmqqshdzzw7o.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031003351.22341-2-sbeller@google.com","subject":"Re: [PATCH 1/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-31T06:07:39Z","receivedAt":"2017-10-31T06:07:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> With traverse_trees_and_blobs factored out of the main traverse function,\n> the next patch can introduce an in-order revision walking with ease.\n>\n> The variable holding the base path is only used in the newly factored out\n> function `traverse_trees_and_blobs`, however we keep its scope to\n> `traverse_commit_list` to keep the number of invocations for memory\n> allocations and release to one per commit traversal.\n\nIn this step, the caller is calling traverse_trees_and_blobs() only\nonce per traversal anyway, so the paragraph does not quite justify\nthis somewhat strange calling convention, unless the reader knows\nmore calls will be made to the same function by the caller in later\nsteps, potentially doubling the chance the buffer can be reused with\nthis arrangement.\n\nEven after this function gains more callers, wouldn't the same\ninvariant that \"path\" recorded for trees and blobs are relative to\nthe root of tree recorded in the object in revs->pending.objects[]?\nIOW, should base_path->len be always 0 upon entry to this function?\n\nWe may want to have an explicit _reset() or assert(->len == 0) at\nthe beginning of the function to ensure that.\n\n> Rename the variable to `base_path` for clarity.\n\nHmph. Inside traverse_commit_list(), which never looks at the\ncontents of the buffer, it could just be called with a name that\ndoes not have any meaning (e.g. \"scratch area for the callee to\nuse\"), and such a change would make it clear why it is defined one\nlevel above its use and the caller never looks at it.\n\nIn the callee (i.e. inside traverse_trees_and_blobs()), however, I\ndo not see a reason why base_path is any clearer than base.\n\nOther than that, this looks like a straight-forward code movement\nand looks good.\n"},{"id":"331458","messageId":"xmqqinevzvel.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031003351.22341-7-sbeller@google.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-31T06:25:06Z","receivedAt":"2017-10-31T06:25:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\n> index c924c945ba..3d618b2445 100644\n> --- a/Documentation/git-describe.txt\n> +++ b/Documentation/git-describe.txt\n> @@ -3,7 +3,7 @@ git-describe(1)\n> \n> NAME\n> ----\n> -git-describe - Describe a commit using the most recent tag reachable from it\n> +git-describe - Describe a commit or blob using the most recent tag reachable from it\n\nIf I am not mistaken, this series is about describing a blob as a\ntuple of a recent commit-ish and a path in the tree in it. Blob\nnever reaches anything, so \"desribing blob using X reachable from\nit\" is a mere nonsense.\n\nThe original is not great in that it ignores the \"--contains\" mode\nand referring to \"tagged commit\" merely as \"tag\" for brevity, but\nat least it made some sense.\n\n> @@ -24,6 +24,16 @@ By default (without --all or --tags) `git describe` only shows\n> annotated tags. For more information about creating annotated tags\n> see the -a and -s options to linkgit:git-tag[1].\n> \n> +If the given `` refers to a blob, it will be described\n\nPerhaps this step should update the SYNOPSIS so that the command\ntakes not just commit-ish but a blob too. Given the difficulty in\ncoming up with the single-liner description of what it does we saw\nabove, I suspect that splitting SYNOPSIS out into two very distinct\noperating mode might make it easier to read.\n\n SYNOPSIS\n --------\n [verse]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n +'git describe' [...] ...\n\nThen this additional paragraph can say \"When describin a \",\nwithout using a (technically nonsense) phrase \"if \nrefers to a blob\", which is never true.\n\n"},{"id":"331459","messageId":"xmqqa807ztx4.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031003351.22341-3-sbeller@google.com","subject":"Re: [PATCH 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-31T06:57:11Z","receivedAt":"2017-10-31T06:57:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/list-objects.c b/list-objects.c\n> index bf46f80dff..5e114c9a8a 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -237,6 +237,8 @@ void traverse_commit_list(struct rev_info *revs,\n> \t\tif (commit->tree)\n> \t\t\tadd_pending_tree(revs, commit->tree);\n> \t\tshow_commit(commit, data);\n> +\t\tif (revs->tree_blobs_in_commit_order)\n> +\t\t\ttraverse_trees_and_blobs(revs, &base_path, show_object, data);\n> \t}\n> \ttraverse_trees_and_blobs(revs, &base_path, show_object, data);\n\nThis one is interesting. While we walk the ancestry chain, we may\nadd the tree for each commit that we discover to the \"pending. We\nused to sweep the pending array at the end after we run out of the\ncommits, but now we do the sweeping as we process each commit.\n\nWould this make the last call to traverse_trees_and_blobs() always a\nno-op? I am not suggesting to add a new conditional in front of it;\nI am just asking for curiosity's sake.\n\n> diff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh\n> new file mode 100755\n> index 0000000000..67ebe815d2\n> --- /dev/null\n> +++ b/t/t6100-rev-list-in-order.sh\n> @@ -0,0 +1,44 @@\n> +#!/bin/sh\n> +\n> +test_description='miscellaneous rev-list tests'\n> +\n> +. ./test-lib.sh\n> +\n> +\n> +test_expect_success 'setup' '\n> +\tfor x in one two three four\n> +\tdo\n> +\t\techo $x >$x &&\n> +\t\tgit add $x &&\n> +\t\tgit commit -m \"add file $x\"\n> +\tdone &&\n> +\tfor x in four three\n> +\tdo\n> +\t\tgit rm $x\n> +\t\tgit commit -m \"remove $x\"\n> +\tdone &&\n\nThere is break in &&-chain in the second loop, but in any case, for\nloops and &&-chains do not mix very well unless done carefully.\nImagine what happens if \"git commit\" in the first loop failed when\nx is two; it won't stop and keep going to create three and four and\nnobody would noice any error.\n\n> +\tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n> +\tcut -c 1-40 > actual < actual.raw &&\n> +\n> +\t>expect &&\n> +\tgit rev-parse HEAD^{commit} >>expect &&\n> +\tgit rev-parse HEAD^{tree} >>expect &&\n> +...\n> +\tgit rev-parse HEAD~5^{tree} >>expect &&\n\nOoooh, ugly ;-). I wish we had --stdin interface or something.\nShort of that, I'd probably write this more like...\n\n\t\tgit rev-parse >expect \\\n\t\t\tA B C D \\\n\t\t\tE F\n\nEven though we do not have --stdin for rev-parse, you can still do:\n\n\tgit cat-file --batch-check='%(objectname)' >expect <<-\\EOF &&\n\tHEAD^{commit}\n\tHEAD^{tree}\n\tHEAD:one\n\tHEAD:two\n\t...\n\tEOF\n\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n"},{"id":"331460","messageId":"xmqq60avztm4.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031003351.22341-5-sbeller@google.com","subject":"Re: [PATCH 4/7] builtin/describe.c: print debug statements earlier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-31T07:03:47Z","receivedAt":"2017-10-31T07:03:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> For debuggers aid we'd want to print debug statements early, so\n> introduce a new line in the debug output that describes the whole\n> function, and the change the next debug output to describe why we need\n> to search. Conveniently drop the arg from the second line; which will\n> be useful in a follow up commit, that refactors the describe function.\n>\n> Signed-off-by: Stefan Beller \n> ---\n\nAdding an eary \"entry\" log may indeed be a good idea. This is not a\nnew issue, but I do not think it is a good trade-off to burden i18n\nteams to translate debug-only messages, though.\n\n> builtin/describe.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index fd61f463cf..3136efde31 100644\n> --- a/builtin/describe.c\n> +++ b/builtin/describe.c\n> @@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)\n> \tunsigned long seen_commits = 0;\n> \tunsigned int unannotated_cnt = 0;\n> \n> +\tif (debug)\n> +\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n> +\n> \tif (get_oid(arg, &oid))\n> \t\tdie(_(\"Not a valid object name %s\"), arg);\n> \tcmit = lookup_commit_reference(&oid);\n> @@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)\n> \tif (!max_candidates)\n> \t\tdie(_(\"no tag exactly matches '%s'\"), oid_to_hex(&cmit->object.oid));\n> \tif (debug)\n> -\t\tfprintf(stderr, _(\"searching to describe %s\\n\"), arg);\n> +\t\tfprintf(stderr, _(\"No exact match on refs or tags, searching to describe\\n\"));\n> \n> \tif (!have_util) {\n> \t\tstruct hashmap_iter iter;\n"},{"id":"331465","messageId":"DD1C140E-E5AE-4BD9-86CF-46B828BF4289@gmail.com","threadId":"47050","inReplyTo":"20171031003351.22341-4-sbeller@google.com","subject":"Re: [PATCH 3/7] builtin/describe.c: rename `oid` to avoid variable shadowing","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-31T08:15:22Z","receivedAt":"2017-10-31T08:15:35Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn October 30, 2017 5:33:47 PM PDT, Stefan Beller wrote:\n>The function `describe` has already a variable named `oid` declared at\n>the beginning of the function for an object id. Do now shadow that\n\nNit, s/now/not/\n\n>variable with a pointer to an object id.\n>\n>Signed-off-by: Stefan Beller \n>---\n> builtin/describe.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n>diff --git a/builtin/describe.c b/builtin/describe.c\n>index 29075dbd0f..fd61f463cf 100644\n>--- a/builtin/describe.c\n>+++ b/builtin/describe.c\n>@@ -381,9 +381,9 @@ static void describe(const char *arg, int last_one)\n> \t}\n> \n> \tif (!match_cnt) {\n>-\t\tstruct object_id *oid = &cmit->object.oid;\n>+\t\tstruct object_id *cmit_oid = &cmit->object.oid;\n> \t\tif (always) {\n>-\t\t\tprintf(\"%s\", find_unique_abbrev(oid->hash, abbrev));\n>+\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n> \t\t\tif (suffix)\n> \t\t\t\tprintf(\"%s\", suffix);\n> \t\t\tprintf(\"\\n\");\n>@@ -392,11 +392,11 @@ static void describe(const char *arg, int\n>last_one)\n> \t\tif (unannotated_cnt)\n> \t\t\tdie(_(\"No annotated tags can describe '%s'.\\n\"\n> \t\t\t \"However, there were unannotated tags: try --tags.\"),\n>-\t\t\t oid_to_hex(oid));\n>+\t\t\t oid_to_hex(cmit_oid));\n> \t\telse\n> \t\t\tdie(_(\"No tags can describe '%s'.\\n\"\n> \t\t\t \"Try --always, or create some tags.\"),\n>-\t\t\t oid_to_hex(oid));\n>+\t\t\t oid_to_hex(cmit_oid));\n> \t}\n> \n> \tQSORT(all_matches, match_cnt, compare_pt);\n\n-- \nSent from my Android device with K-9 Mail. Please excuse my brevity.\n"},{"id":"331496","messageId":"CAGZ79kayAgLc7Wju0h39ee7eB37ZgHR8OotX3Uima6QGZ+5n1Q@mail.gmail.com","threadId":"47050","inReplyTo":"xmqqa807ztx4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T18:12:50Z","receivedAt":"2017-10-31T18:12:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Oct 30, 2017 at 11:57 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> diff --git a/list-objects.c b/list-objects.c\n>> index bf46f80dff..5e114c9a8a 100644\n>> --- a/list-objects.c\n>> +++ b/list-objects.c\n>> @@ -237,6 +237,8 @@ void traverse_commit_list(struct rev_info *revs,\n>> if (commit->tree)\n>> add_pending_tree(revs, commit->tree);\n>> show_commit(commit, data);\n>> + if (revs->tree_blobs_in_commit_order)\n>> + traverse_trees_and_blobs(revs, &base_path, show_object, data);\n>> }\n>> traverse_trees_and_blobs(revs, &base_path, show_object, data);\n>\n> This one is interesting. While we walk the ancestry chain, we may\n> add the tree for each commit that we discover to the \"pending. We\n> used to sweep the pending array at the end after we run out of the\n> commits, but now we do the sweeping as we process each commit.\n>\n> Would this make the last call to traverse_trees_and_blobs() always a\n> no-op?\n\nThat is my understanding of the code, the important part of the previous patch\nwas the move of 'object_array_clear(&revs->pending);' into the\n`traverse_trees_and_blobs` function.\n\n> I am not suggesting to add a new conditional in front of it;\n> I am just asking for curiosity's sake.\n\nThanks for being clear on that. I have the same taste for this.\n\n(Though for a split second I wondered if we can pull some esoteric trick\nto skip this, but I could not find any that is sane as well as readable.)\n\n>> +test_expect_success 'setup' '\n>> + for x in one two three four\n>> + do\n>> + echo $x >$x &&\n>> + git add $x &&\n>> + git commit -m \"add file $x\"\n>> + done &&\n>> + for x in four three\n>> + do\n>> + git rm $x\n>> + git commit -m \"remove $x\"\n>> + done &&\n>\n> There is break in &&-chain in the second loop, but in any case, for\n> loops and &&-chains do not mix very well unless done carefully.\n> Imagine what happens if \"git commit\" in the first loop failed when\n> x is two; it won't stop and keep going to create three and four and\n> nobody would noice any error.\n>\n\nI'll fix that.\n\n\n> Even though we do not have --stdin for rev-parse, you can still do:\n>\n> git cat-file --batch-check='%(objectname)' >expect <<-\\EOF &&\n> HEAD^{commit}\n> HEAD^{tree}\n> HEAD:one\n> HEAD:two\n> ...\n> EOF\n\nsounds better than the current implementation.\n"},{"id":"331508","messageId":"CAGZ79kYgeShgCfgm0-D6D0r8ndZFBuxKBXcaVPEo0T5eoPBQEQ@mail.gmail.com","threadId":"47050","inReplyTo":"xmqq60avztm4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/7] builtin/describe.c: print debug statements earlier","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T19:05:58Z","receivedAt":"2017-10-31T19:06:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 31, 2017 at 12:03 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> For debuggers aid we'd want to print debug statements early, so\n>> introduce a new line in the debug output that describes the whole\n>> function, and the change the next debug output to describe why we need\n>> to search. Conveniently drop the arg from the second line; which will\n>> be useful in a follow up commit, that refactors the describe function.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>\n> Adding an eary \"entry\" log may indeed be a good idea. This is not a\n> new issue, but I do not think it is a good trade-off to burden i18n\n> teams to translate debug-only messages, though.\n\nThe original motivation here is to use the 'arg' argument so early in the\nfunction that the next patch can omit passing in 'arg' as the lines\nusing 'arg' are not in the factored out function.\n\n8713ab3079 (Improve git-describe performance by reducing\nrevision listing., 2007-01-13) added the --debug flag, and also\nexposed it to the man page, so it is user visible, which is why we cannot\nkeep these messages untranslated.\n\n>\n>> builtin/describe.c | 5 ++++-\n>> 1 file changed, 4 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/describe.c b/builtin/describe.c\n>> index fd61f463cf..3136efde31 100644\n>> --- a/builtin/describe.c\n>> +++ b/builtin/describe.c\n>> @@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)\n>> unsigned long seen_commits = 0;\n>> unsigned int unannotated_cnt = 0;\n>>\n>> + if (debug)\n>> + fprintf(stderr, _(\"describe %s\\n\"), arg);\n>> +\n>> if (get_oid(arg, &oid))\n>> die(_(\"Not a valid object name %s\"), arg);\n>> cmit = lookup_commit_reference(&oid);\n>> @@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)\n>> if (!max_candidates)\n>> die(_(\"no tag exactly matches '%s'\"), oid_to_hex(&cmit->object.oid));\n>> if (debug)\n>> - fprintf(stderr, _(\"searching to describe %s\\n\"), arg);\n>> + fprintf(stderr, _(\"No exact match on refs or tags, searching to describe\\n\"));\n>>\n>> if (!have_util) {\n>> struct hashmap_iter iter;\n"},{"id":"331509","messageId":"CAGZ79kYO=4SWzfKY6bU8Spn5Ubw39ghOH6wanFhFEsKD8q9vrA@mail.gmail.com","threadId":"47050","inReplyTo":"xmqqinevzvel.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T19:16:30Z","receivedAt":"2017-10-31T19:16:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Oct 30, 2017 at 11:25 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\n>> index c924c945ba..3d618b2445 100644\n>> --- a/Documentation/git-describe.txt\n>> +++ b/Documentation/git-describe.txt\n>> @@ -3,7 +3,7 @@ git-describe(1)\n>>\n>> NAME\n>> ----\n>> -git-describe - Describe a commit using the most recent tag reachable from it\n>> +git-describe - Describe a commit or blob using the most recent tag reachable from it\n>\n> If I am not mistaken, this series is about describing a blob as a\n> tuple of a recent commit-ish and a path in the tree in it. Blob\n> never reaches anything, so \"desribing blob using X reachable from\n> it\" is a mere nonsense.\n\nAgreed, though the commitish used for the blob description may refer to a tag.\n\n> The original is not great in that it ignores the \"--contains\" mode\n> and referring to \"tagged commit\" merely as \"tag\" for brevity, but\n> at least it made some sense.\n\nMaybe\n\n \"git-describe - Describe a blob or commit using graph relations\"\n\nthough that sounds too generic, but it is accurate as all we do is\na heuristic for graph walk ways.\n\n\n>> @@ -24,6 +24,16 @@ By default (without --all or --tags) `git describe` only shows\n>> annotated tags. For more information about creating annotated tags\n>> see the -a and -s options to linkgit:git-tag[1].\n>>\n>> +If the given `` refers to a blob, it will be described\n>\n> Perhaps this step should update the SYNOPSIS so that the command\n> takes not just commit-ish but a blob too.\n\nThat is a good idea, as the --contains would not work for blobs currently.\n\n> Given the difficulty in\n> coming up with the single-liner description of what it does we saw\n> above, I suspect that splitting SYNOPSIS out into two very distinct\n> operating mode might make it easier to read.\n>\n> SYNOPSIS\n> --------\n> [verse]\n> 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n> +'git describe' [...] ...\n>\n> Then this additional paragraph can say \"When describin a \",\n> without using a (technically nonsense) phrase \"if \n> refers to a blob\", which is never true.\n>\n\nok, do we know about 'blob-ish' as a term?\n"},{"id":"331510","messageId":"20171031211852.13001-1-sbeller@google.com","threadId":"47050","inReplyTo":"20171031003351.22341-1-sbeller@google.com","subject":"[PATCHv2 0/7] git describe blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:45Z","receivedAt":"2017-10-31T21:19:00Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\n\n* other variable names in patch v1, the commit message explains the\n unusual strategy for the scratch pad variable, + assert\n* less ugly test in p2\n* typofix in p3 commit msg\n* patch 4 (debug printing) unchanged, awaiting discussion to start/settle.\n* rephrased the man page in p6.\n\nThanks,\nStefan\n\nv1:\nThis is not an RFC any more, but a serious series.\n\nOccasionally a user is given an object hash from a blob as an error message\nor other output (e.g. [1]).\n\nIt would be useful to get a further description of such a blob, such as\nthe (commit, path) tuple where this blob was introduced.\n\nThis implements the answer in builtin/describe,\nhowever the heuristics are weak. See patch 6 for details.\n\nAny feedback welcome,\n\nThanks,\nStefan\n\nStefan Beller (7):\n list-objects.c: factor out traverse_trees_and_blobs\n revision.h: introduce blob/tree walking in order of the commits\n builtin/describe.c: rename `oid` to avoid variable shadowing\n builtin/describe.c: print debug statements earlier\n builtin/describe.c: factor out describe_commit\n builtin/describe.c: describe a blob\n t6120: fix typo in test name\n\n Documentation/git-describe.txt | 13 +++-\n Documentation/rev-list-options.txt | 5 ++\n builtin/describe.c | 125 ++++++++++++++++++++++++++++---------\n list-objects.c | 52 +++++++++------\n revision.c | 2 +\n revision.h | 3 +-\n t/t6100-rev-list-in-order.sh | 46 ++++++++++++++\n t/t6120-describe.sh | 17 ++++-\n 8 files changed, 213 insertions(+), 50 deletions(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331511","messageId":"20171031211852.13001-2-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv2 1/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:46Z","receivedAt":"2017-10-31T21:19:02Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"With traverse_trees_and_blobs factored out of the main traverse function,\nthe next patch can introduce an in-order revision walking with ease.\n\nIn the next patch we'll call `traverse_trees_and_blobs` from within the\nloop walking the commits, such that we'll have one invocation of that\nfunction per commit. That is why we do not want to have memory allocations\nin that function, such as we'd have if we were to use a strbuf locally.\nPass a strbuf from traverse_commit_list into the blob and tree traversing\nfunction as a scratch pad that only needs to be allocated once.\n\nSigned-off-by: Stefan Beller \n---\n list-objects.c | 50 +++++++++++++++++++++++++++++++-------------------\n 1 file changed, 31 insertions(+), 19 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex b3931fa434..ef9dbe8f92 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -183,25 +183,15 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-void traverse_commit_list(struct rev_info *revs,\n-\t\t\t show_commit_fn show_commit,\n-\t\t\t show_object_fn show_object,\n-\t\t\t void *data)\n+static void traverse_trees_and_blobs(struct rev_info *revs,\n+\t\t\t\t struct strbuf *base_path,\n+\t\t\t\t show_object_fn show_object,\n+\t\t\t\t void *data)\n {\n \tint i;\n-\tstruct commit *commit;\n-\tstruct strbuf base;\n \n-\tstrbuf_init(&base, PATH_MAX);\n-\twhile ((commit = get_revision(revs)) != NULL) {\n-\t\t/*\n-\t\t * an uninteresting boundary commit may not have its tree\n-\t\t * parsed yet, but we are not going to show them anyway\n-\t\t */\n-\t\tif (commit->tree)\n-\t\t\tadd_pending_tree(revs, commit->tree);\n-\t\tshow_commit(commit, data);\n-\t}\n+\tassert(base_path->len == 0);\n+\n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n \t\tstruct object *obj = pending->item;\n@@ -218,17 +208,39 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t\tpath = \"\";\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base_path, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base_path, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\n \t\t oid_to_hex(&obj->oid), name);\n \t}\n \tobject_array_clear(&revs->pending);\n-\tstrbuf_release(&base);\n+}\n+\n+void traverse_commit_list(struct rev_info *revs,\n+\t\t\t show_commit_fn show_commit,\n+\t\t\t show_object_fn show_object,\n+\t\t\t void *data)\n+{\n+\tstruct commit *commit;\n+\tstruct strbuf csp; /* callee's scratch pad */\n+\tstrbuf_init(&csp, PATH_MAX);\n+\n+\twhile ((commit = get_revision(revs)) != NULL) {\n+\t\t/*\n+\t\t * an uninteresting boundary commit may not have its tree\n+\t\t * parsed yet, but we are not going to show them anyway\n+\t\t */\n+\t\tif (commit->tree)\n+\t\t\tadd_pending_tree(revs, commit->tree);\n+\t\tshow_commit(commit, data);\n+\t}\n+\ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n+\n+\tstrbuf_release(&csp);\n }\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331512","messageId":"20171031211852.13001-5-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv2 4/7] builtin/describe.c: print debug statements earlier","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:49Z","receivedAt":"2017-10-31T21:19:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"For debuggers aid we'd want to print debug statements early, so\nintroduce a new line in the debug output that describes the whole\nfunction, and the change the next debug output to describe why we need\nto search. Conveniently drop the arg from the second line; which will\nbe useful in a follow up commit, that refactors the describe function.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex fd61f463cf..3136efde31 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)\n \tunsigned long seen_commits = 0;\n \tunsigned int unannotated_cnt = 0;\n \n+\tif (debug)\n+\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n+\n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n \tcmit = lookup_commit_reference(&oid);\n@@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)\n \tif (!max_candidates)\n \t\tdie(_(\"no tag exactly matches '%s'\"), oid_to_hex(&cmit->object.oid));\n \tif (debug)\n-\t\tfprintf(stderr, _(\"searching to describe %s\\n\"), arg);\n+\t\tfprintf(stderr, _(\"No exact match on refs or tags, searching to describe\\n\"));\n \n \tif (!have_util) {\n \t\tstruct hashmap_iter iter;\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331513","messageId":"20171031211852.13001-8-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv2 7/7] t6120: fix typo in test name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:52Z","receivedAt":"2017-10-31T21:19:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n t/t6120-describe.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 3be01316e8..fd329f173a 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -304,7 +304,7 @@ test_expect_success 'describe chokes on severely broken submodules' '\n \tmv .git/modules/sub1/ .git/modules/sub_moved &&\n \ttest_must_fail git describe --dirty\n '\n-test_expect_success 'describe ignoring a borken submodule' '\n+test_expect_success 'describe ignoring a broken submodule' '\n \tgit describe --broken >out &&\n \ttest_when_finished \"mv .git/modules/sub_moved .git/modules/sub1\" &&\n \tgrep broken out\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331514","messageId":"20171031211852.13001-3-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:47Z","receivedAt":"2017-10-31T21:19:14Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The functionality to list tree objects in the order they were seen\nwhile traversing the commits will be used in the next commit,\nwhere we teach `git describe` to describe not only commits, but\ntrees and blobs, too.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/rev-list-options.txt | 5 +++++\n list-objects.c | 2 ++\n revision.c | 2 ++\n revision.h | 3 ++-\n t/t6100-rev-list-in-order.sh | 46 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 57 insertions(+), 1 deletion(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 13501e1556..9066e0c777 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -686,6 +686,11 @@ ifdef::git-rev-list[]\n \tall object IDs which I need to download if I have the commit\n \tobject _bar_ but not _foo_''.\n \n+--in-commit-order::\n+\tPrint tree and blob ids in order of the commits. The tree\n+\tand blob ids are printed after they are first referenced\n+\tby a commit.\n+\n --objects-edge::\n \tSimilar to `--objects`, but also print the IDs of excluded\n \tcommits prefixed with a ``-'' character. This is used by\ndiff --git a/list-objects.c b/list-objects.c\nindex ef9dbe8f92..03438e5a8b 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -239,6 +239,8 @@ void traverse_commit_list(struct rev_info *revs,\n \t\tif (commit->tree)\n \t\t\tadd_pending_tree(revs, commit->tree);\n \t\tshow_commit(commit, data);\n+\t\tif (revs->tree_blobs_in_commit_order)\n+\t\t\ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n \t}\n \ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n \ndiff --git a/revision.c b/revision.c\nindex d167223e69..9329d4ebbf 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1845,6 +1845,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->dense = 0;\n \t} else if (!strcmp(arg, \"--show-all\")) {\n \t\trevs->show_all = 1;\n+\t} else if (!strcmp(arg, \"--in-commit-order\")) {\n+\t\trevs->tree_blobs_in_commit_order = 1;\n \t} else if (!strcmp(arg, \"--remove-empty\")) {\n \t\trevs->remove_empty_trees = 1;\n \t} else if (!strcmp(arg, \"--merges\")) {\ndiff --git a/revision.h b/revision.h\nindex 54761200ad..86985d68aa 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,7 +121,8 @@ struct rev_info {\n \t\t\tbisect:1,\n \t\t\tancestry_path:1,\n \t\t\tfirst_parent_only:1,\n-\t\t\tline_level_traverse:1;\n+\t\t\tline_level_traverse:1,\n+\t\t\ttree_blobs_in_commit_order:1;\n \n \t/* Diff flags */\n \tunsigned int\tdiff:1,\ndiff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh\nnew file mode 100755\nindex 0000000000..651666979b\n--- /dev/null\n+++ b/t/t6100-rev-list-in-order.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='miscellaneous rev-list tests'\n+\n+. ./test-lib.sh\n+\n+\n+test_expect_success 'setup' '\n+\tfor x in one two three four\n+\tdo\n+\t\techo $x >$x &&\n+\t\tgit add $x &&\n+\t\tgit commit -m \"add file $x\"\n+\tdone &&\n+\tfor x in four three\n+\tdo\n+\t\tgit rm $x &&\n+\t\tgit commit -m \"remove $x\"\n+\tdone &&\n+\tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n+\tcut -c 1-40 >actual expect.raw <<-\\EOF &&\n+\t\tHEAD^{commit}\n+\t\tHEAD^{tree}\n+\t\tHEAD^{tree}:one\n+\t\tHEAD^{tree}:two\n+\t\tHEAD~1^{commit}\n+\t\tHEAD~1^{tree}\n+\t\tHEAD~1^{tree}:three\n+\t\tHEAD~2^{commit}\n+\t\tHEAD~2^{tree}\n+\t\tHEAD~2^{tree}:four\n+\t\tHEAD~3^{commit}\n+\t\t# HEAD~3^{tree} skipped\n+\t\tHEAD~4^{commit}\n+\t\t# HEAD~4^{tree} skipped\n+\t\tHEAD~5^{commit}\n+\t\tHEAD~5^{tree}\n+\tEOF\n+\tgrep -v \"#\" >expect \n---\n Documentation/git-describe.txt | 13 ++++++++-\n builtin/describe.c | 65 ++++++++++++++++++++++++++++++++++++++----\n t/t6120-describe.sh | 15 ++++++++++\n 3 files changed, 87 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex c924c945ba..077c3c2193 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -3,7 +3,7 @@ git-describe(1)\n \n NAME\n ----\n-git-describe - Describe a commit using the most recent tag reachable from it\n+git-describe - Describe a commit or blob using the graph relations\n \n \n SYNOPSIS\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]\n+'git describe' [] \n \n DESCRIPTION\n -----------\n@@ -24,6 +25,16 @@ By default (without --all or --tags) `git describe` only shows\n annotated tags. For more information about creating annotated tags\n see the -a and -s options to linkgit:git-tag[1].\n \n+If the given object refers to a blob, it will be described\n+as `:`, such that the blob can be found\n+at `` in the ``. Note, that the commit is likely\n+not the commit that introduced the blob, but the one that was found\n+first; to find the commit that introduced the blob, you need to find\n+the commit that last touched the path, e.g.\n+`git log -- `.\n+As blobs do not point at the commits they are contained in,\n+describing blobs is slow as we have to walk the whole graph.\n+\n OPTIONS\n -------\n ...::\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 9e9a5ed5d4..382cbae908 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -3,6 +3,7 @@\n #include \"lockfile.h\"\n #include \"commit.h\"\n #include \"tag.h\"\n+#include \"blob.h\"\n #include \"refs.h\"\n #include \"builtin.h\"\n #include \"exec_cmd.h\"\n@@ -11,8 +12,9 @@\n #include \"hashmap.h\"\n #include \"argv-array.h\"\n #include \"run-command.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n \n-#define SEEN\t\t(1u << 0)\n #define MAX_TAGS\t(FLAG_BITS - 1)\n \n static const char * const describe_usage[] = {\n@@ -434,6 +436,54 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)\n \t\tstrbuf_addstr(dst, suffix);\n }\n \n+struct process_commit_data {\n+\tstruct object_id current_commit;\n+\tstruct object_id looking_for;\n+\tstruct strbuf *dst;\n+};\n+\n+static void process_commit(struct commit *commit, void *data)\n+{\n+\tstruct process_commit_data *pcd = data;\n+\tpcd->current_commit = commit->object.oid;\n+}\n+\n+static void process_object(struct object *obj, const char *path, void *data)\n+{\n+\tstruct process_commit_data *pcd = data;\n+\n+\tif (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n+\t\treset_revision_walk();\n+\t\tdescribe_commit(&pcd->current_commit, pcd->dst);\n+\t\tstrbuf_addf(pcd->dst, \":%s\", path);\n+\t}\n+}\n+\n+static void describe_blob(struct object_id oid, struct strbuf *dst)\n+{\n+\tstruct rev_info revs;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct process_commit_data pcd = { null_oid, oid, dst};\n+\n+\targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n+\t\t\"--all\", \"--reflog\", /* as many starting points as possible */\n+\t\t/* NEEDSWORK: --all is incompatible with worktrees for now: */\n+\t\t\"--single-worktree\",\n+\t\t\"--objects\",\n+\t\t\"--in-commit-order\",\n+\t\tNULL);\n+\n+\tinit_revisions(&revs, NULL);\n+\tif (setup_revisions(args.argc, args.argv, &revs, NULL) > 1)\n+\t\tBUG(\"setup_revisions could not handle all args?\");\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\ttraverse_commit_list(&revs, process_commit, process_object, &pcd);\n+\treset_revision_walk();\n+}\n+\n static void describe(const char *arg, int last_one)\n {\n \tstruct object_id oid;\n@@ -445,11 +495,16 @@ static void describe(const char *arg, int last_one)\n \n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference_gently(&oid, 1);\n \n-\tdescribe_commit(&oid, &sb);\n+\tif (cmit) {\n+\t\tdescribe_commit(&oid, &sb);\n+\t} else {\n+\t\tif (lookup_blob(&oid))\n+\t\t\tdescribe_blob(oid, &sb);\n+\t\telse\n+\t\t\tdie(_(\"%s is neither a commit nor blob\"), arg);\n+\t}\n \n \tputs(sb.buf);\n \ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 1c0e8659d9..3be01316e8 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -310,6 +310,21 @@ test_expect_success 'describe ignoring a borken submodule' '\n \tgrep broken out\n '\n \n+test_expect_success 'describe a blob at a tag' '\n+\techo \"make it a unique blob\" >file &&\n+\tgit add file && git commit -m \"content in file\" &&\n+\tgit tag -a -m \"latest annotated tag\" unique-file &&\n+\tgit describe HEAD:file >actual &&\n+\techo \"unique-file:file\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe a surviving blob' '\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit describe HEAD:file >actual &&\n+\tgrep unique-file-1-g actual\n+'\n+\n test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n \ti=1 &&\n \twhile test $i -lt 8000\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331516","messageId":"20171031211852.13001-4-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv2 3/7] builtin/describe.c: rename `oid` to avoid variable shadowing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:48Z","receivedAt":"2017-10-31T21:19:19Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The function `describe` has already a variable named `oid` declared at\nthe beginning of the function for an object id. Do not shadow that\nvariable with a pointer to an object id.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 29075dbd0f..fd61f463cf 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -381,9 +381,9 @@ static void describe(const char *arg, int last_one)\n \t}\n \n \tif (!match_cnt) {\n-\t\tstruct object_id *oid = &cmit->object.oid;\n+\t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tprintf(\"%s\", find_unique_abbrev(oid->hash, abbrev));\n+\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n \t\t\tif (suffix)\n \t\t\t\tprintf(\"%s\", suffix);\n \t\t\tprintf(\"\\n\");\n@@ -392,11 +392,11 @@ static void describe(const char *arg, int last_one)\n \t\tif (unannotated_cnt)\n \t\t\tdie(_(\"No annotated tags can describe '%s'.\\n\"\n \t\t\t \"However, there were unannotated tags: try --tags.\"),\n-\t\t\t oid_to_hex(oid));\n+\t\t\t oid_to_hex(cmit_oid));\n \t\telse\n \t\t\tdie(_(\"No tags can describe '%s'.\\n\"\n \t\t\t \"Try --always, or create some tags.\"),\n-\t\t\t oid_to_hex(oid));\n+\t\t\t oid_to_hex(cmit_oid));\n \t}\n \n \tQSORT(all_matches, match_cnt, compare_pt);\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331517","messageId":"20171031211852.13001-6-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv2 5/7] builtin/describe.c: factor out describe_commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-31T21:18:50Z","receivedAt":"2017-10-31T21:19:21Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the next patch we'll learn how to describe more than just commits,\nso factor out describing commits into its own function. That will make\nthe next patches easy as we still need to describe a commit as part of\ndescribing blobs.\n\nWhile factoring out the functionality to describe_commit, make sure\nthat any output to stdout is put into a strbuf, which we can print\nafterwards, using puts which also adds the line ending.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 63 ++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 37 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 3136efde31..9e9a5ed5d4 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -256,7 +256,7 @@ static unsigned long finish_depth_computation(\n \treturn seen_commits;\n }\n \n-static void display_name(struct commit_name *n)\n+static void append_name(struct commit_name *n, struct strbuf *dst)\n {\n \tif (n->prio == 2 && !n->tag) {\n \t\tn->tag = lookup_tag(&n->oid);\n@@ -272,19 +272,18 @@ static void display_name(struct commit_name *n)\n \t}\n \n \tif (n->tag)\n-\t\tprintf(\"%s\", n->tag->tag);\n+\t\tstrbuf_addstr(dst, n->tag->tag);\n \telse\n-\t\tprintf(\"%s\", n->path);\n+\t\tstrbuf_addstr(dst, n->path);\n }\n \n-static void show_suffix(int depth, const struct object_id *oid)\n+static void append_suffix(int depth, const struct object_id *oid, struct strbuf *dst)\n {\n-\tprintf(\"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n+\tstrbuf_addf(dst, \"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n }\n \n-static void describe(const char *arg, int last_one)\n+static void describe_commit(struct object_id *oid, struct strbuf *dst)\n {\n-\tstruct object_id oid;\n \tstruct commit *cmit, *gave_up_on = NULL;\n \tstruct commit_list *list;\n \tstruct commit_name *n;\n@@ -293,26 +292,18 @@ static void describe(const char *arg, int last_one)\n \tunsigned long seen_commits = 0;\n \tunsigned int unannotated_cnt = 0;\n \n-\tif (debug)\n-\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n-\n-\tif (get_oid(arg, &oid))\n-\t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference(oid);\n \n \tn = find_commit_name(&cmit->object.oid);\n \tif (n && (tags || all || n->prio == 2)) {\n \t\t/*\n \t\t * Exact match to an existing ref.\n \t\t */\n-\t\tdisplay_name(n);\n+\t\tappend_name(n, dst);\n \t\tif (longformat)\n-\t\t\tshow_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);\n+\t\t\tappend_suffix(0, n->tag ? &n->tag->tagged->oid : oid, dst);\n \t\tif (suffix)\n-\t\t\tprintf(\"%s\", suffix);\n-\t\tprintf(\"\\n\");\n+\t\t\tstrbuf_addstr(dst, suffix);\n \t\treturn;\n \t}\n \n@@ -386,10 +377,9 @@ static void describe(const char *arg, int last_one)\n \tif (!match_cnt) {\n \t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n+\t\t\tstrbuf_addstr(dst, find_unique_abbrev(cmit_oid->hash, abbrev));\n \t\t\tif (suffix)\n-\t\t\t\tprintf(\"%s\", suffix);\n-\t\t\tprintf(\"\\n\");\n+\t\t\t\tstrbuf_addstr(dst, suffix);\n \t\t\treturn;\n \t\t}\n \t\tif (unannotated_cnt)\n@@ -437,15 +427,36 @@ static void describe(const char *arg, int last_one)\n \t\t}\n \t}\n \n-\tdisplay_name(all_matches[0].name);\n+\tappend_name(all_matches[0].name, dst);\n \tif (abbrev)\n-\t\tshow_suffix(all_matches[0].depth, &cmit->object.oid);\n+\t\tappend_suffix(all_matches[0].depth, &cmit->object.oid, dst);\n \tif (suffix)\n-\t\tprintf(\"%s\", suffix);\n-\tprintf(\"\\n\");\n+\t\tstrbuf_addstr(dst, suffix);\n+}\n+\n+static void describe(const char *arg, int last_one)\n+{\n+\tstruct object_id oid;\n+\tstruct commit *cmit;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (debug)\n+\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n+\n+\tif (get_oid(arg, &oid))\n+\t\tdie(_(\"Not a valid object name %s\"), arg);\n+\tcmit = lookup_commit_reference(&oid);\n+\tif (!cmit)\n+\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\n+\tdescribe_commit(&oid, &sb);\n+\n+\tputs(sb.buf);\n \n \tif (!last_one)\n \t\tclear_commit_marks(cmit, -1);\n+\n+\tstrbuf_release(&sb);\n }\n \n int cmd_describe(int argc, const char **argv, const char *prefix)\n-- \n2.15.0.rc2.443.gfcc3b81c0a\n\n"},{"id":"331519","messageId":"CAPig+cRZhm-s7eAU_d29=pOPrKRA4DnCARyJ7JsyQ_wqKgPXVg@mail.gmail.com","threadId":"47050","inReplyTo":"20171031211852.13001-5-sbeller@google.com","subject":"Re: [PATCHv2 4/7] builtin/describe.c: print debug statements earlier","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-10-31T21:31:12Z","receivedAt":"2017-10-31T21:31:18Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Oct 31, 2017 at 5:18 PM, Stefan Beller wrote:\n> For debuggers aid we'd want to print debug statements early, so\n> introduce a new line in the debug output that describes the whole\n> function, and the change the next debug output to describe why we need\n\ns/and the/and/\n...or...\ns/and the/and then/\n\n> to search. Conveniently drop the arg from the second line; which will\n> be useful in a follow up commit, that refactors the describe function.\n>\n> Signed-off-by: Stefan Beller \n"},{"id":"331523","messageId":"CAPig+cRz6EjdEGUjdMiB8979VybKN8Xza9uUDKNuxKNn+pY8jA@mail.gmail.com","threadId":"47050","inReplyTo":"20171031211852.13001-7-sbeller@google.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-10-31T21:49:05Z","receivedAt":"2017-10-31T21:49:10Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Oct 31, 2017 at 5:18 PM, Stefan Beller wrote:\n> When describing commits, we try to anchor them to tags or refs, as these\n> are conceptually on a higher level than the commit. And if there is no ref\n> or tag that matches exactly, we're out of luck. So we employ a heuristic\n> to make up a name for the commit. These names are ambivalent, there might\n\nI guess you meant s/ambivalent/ambiguous/ ?\n\n> be different tags or refs to anchor to, and there might be different\n> path in the DAG to travel to arrive at the commit precisely.\n>\n> [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n> Signed-off-by: Stefan Beller \n\nBlank line before sign-off.\n\n> ---\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> @@ -445,11 +495,16 @@ static void describe(const char *arg, int last_one)\n>\n> if (get_oid(arg, &oid))\n> die(_(\"Not a valid object name %s\"), arg);\n> - cmit = lookup_commit_reference(&oid);\n> - if (!cmit)\n> - die(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n> + cmit = lookup_commit_reference_gently(&oid, 1);\n>\n> - describe_commit(&oid, &sb);\n> + if (cmit) {\n> + describe_commit(&oid, &sb);\n> + } else {\n> + if (lookup_blob(&oid))\n> + describe_blob(oid, &sb);\n> + else\n> + die(_(\"%s is neither a commit nor blob\"), arg);\n> + }\n\nNot at all worth a re-roll, but less nesting and a bit less noisy:\n\n if (cmt)\n describe_commit(...);\n else if (lookup_blob(...))\n describe_blob(...);\n else\n die(...);\n"},{"id":"331528","messageId":"xmqqh8ueyet8.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031003351.22341-8-sbeller@google.com","subject":"Re: [PATCH 7/7] t6120: fix typo in test name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T01:21:07Z","receivedAt":"2017-11-01T01:21:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Signed-off-by: Stefan Beller \n> ---\n> t/t6120-describe.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nGood. I am guessing that you are sending this as the last/optional\none because this was found _after_ you worked on other parts of the\nseries, but I think it is easier to reason about if this were marked\nas a preliminary clean-up and moved to the front of the series.\n\nThanks.\n\n>\n> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\n> index 3be01316e8..fd329f173a 100755\n> --- a/t/t6120-describe.sh\n> +++ b/t/t6120-describe.sh\n> @@ -304,7 +304,7 @@ test_expect_success 'describe chokes on severely broken submodules' '\n> \tmv .git/modules/sub1/ .git/modules/sub_moved &&\n> \ttest_must_fail git describe --dirty\n> '\n> -test_expect_success 'describe ignoring a borken submodule' '\n> +test_expect_success 'describe ignoring a broken submodule' '\n> \tgit describe --broken >out &&\n> \ttest_when_finished \"mv .git/modules/sub_moved .git/modules/sub1\" &&\n> \tgrep broken out\n"},{"id":"331536","messageId":"xmqq4lqewu2b.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CAGZ79kYO=4SWzfKY6bU8Spn5Ubw39ghOH6wanFhFEsKD8q9vrA@mail.gmail.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T03:34:36Z","receivedAt":"2017-11-01T03:34:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> Given the difficulty in\n>> coming up with the single-liner description of what it does we saw\n>> above, I suspect that splitting SYNOPSIS out into two very distinct\n>> operating mode might make it easier to read.\n>>\n>> SYNOPSIS\n>> --------\n>> [verse]\n>> 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n>> +'git describe' [...] ...\n>>\n>> Then this additional paragraph can say \"When describin a \",\n>> without using a (technically nonsense) phrase \"if \n>> refers to a blob\", which is never true.\n>\n> ok, do we know about 'blob-ish' as a term?\n\nNo, and I do not think there is any need to say -ish at all for this\nuse case.\n\nAfter all, when we accept a when a is called\nfor, that is only because there is only one way to use the commit in\nplace of the wanted ; we take the top-level tree contained in\nit. You cannot say you take and take a , as it is\nunclear which entry in the can act as the substitute for the\nwanted .\n\nYou accept blob object name in this mode, so just saying is\nsufficient.\n"},{"id":"331537","messageId":"xmqqzi86vf1m.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CAGZ79kYO=4SWzfKY6bU8Spn5Ubw39ghOH6wanFhFEsKD8q9vrA@mail.gmail.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T03:44:21Z","receivedAt":"2017-11-01T03:44:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Maybe\n>\n> \"git-describe - Describe a blob or commit using graph relations\"\n>\n> though that sounds too generic, but it is accurate as all we do is\n> a heuristic for graph walk ways.\n\nWe used to describe commit using commit ancestry (i.e. finding the\nplace where a \"wanted\" commit sits in the commit ancestry graph).\nNow we are extending it to describe an object by finding the place\nwhere it sits in the graph that shows object reachability relation,\nbut saying \"graph\" without saying \"graph over what\" is probably not\ndescriptive enough for most readers.\n"},{"id":"331538","messageId":"xmqqvaiuvey8.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031211852.13001-2-sbeller@google.com","subject":"Re: [PATCHv2 1/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T03:46:23Z","receivedAt":"2017-11-01T03:46:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> With traverse_trees_and_blobs factored out of the main traverse function,\n> the next patch can introduce an in-order revision walking with ease.\n>\n> In the next patch we'll call `traverse_trees_and_blobs` from within the\n> loop walking the commits, such that we'll have one invocation of that\n> function per commit. That is why we do not want to have memory allocations\n> in that function, such as we'd have if we were to use a strbuf locally.\n> Pass a strbuf from traverse_commit_list into the blob and tree traversing\n> function as a scratch pad that only needs to be allocated once.\n\nMakes sense.\n\nI still don't think base_path is any clearer than base that was used\nin the original, though.\n\n"},{"id":"331539","messageId":"xmqqr2tiveqp.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031211852.13001-3-sbeller@google.com","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T03:50:54Z","receivedAt":"2017-11-01T03:51:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh\n> new file mode 100755\n> index 0000000000..651666979b\n> --- /dev/null\n> +++ b/t/t6100-rev-list-in-order.sh\n> @@ -0,0 +1,46 @@\n> +#!/bin/sh\n> +\n> +test_description='miscellaneous rev-list tests'\n> +\n> +. ./test-lib.sh\n> +\n> +\n> +test_expect_success 'setup' '\n> +\tfor x in one two three four\n> +\tdo\n> +\t\techo $x >$x &&\n> +\t\tgit add $x &&\n> +\t\tgit commit -m \"add file $x\"\n> +\tdone &&\n> +\tfor x in four three\n> +\tdo\n> +\t\tgit rm $x &&\n> +\t\tgit commit -m \"remove $x\"\n> +\tdone &&\n\nWhen \"git commit -m 'remove four'\" fails, this loop would still\ncontinue, so the &&-chain in \"done &&\" would still be rendered\nineffetive.\n\n> +\tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n> +\tcut -c 1-40 >actual +\n> +\tgit cat-file --batch-check=\"%(objectname)\" >expect.raw <<-\\EOF &&\n> +\t\tHEAD^{commit}\n> +\t\tHEAD^{tree}\n> +\t\tHEAD^{tree}:one\n> +\t\tHEAD^{tree}:two\n> +\t\tHEAD~1^{commit}\n> +\t\tHEAD~1^{tree}\n> +\t\tHEAD~1^{tree}:three\n> +\t\tHEAD~2^{commit}\n> +\t\tHEAD~2^{tree}\n> +\t\tHEAD~2^{tree}:four\n> +\t\tHEAD~3^{commit}\n> +\t\t# HEAD~3^{tree} skipped\n> +\t\tHEAD~4^{commit}\n> +\t\t# HEAD~4^{tree} skipped\n> +\t\tHEAD~5^{commit}\n> +\t\tHEAD~5^{tree}\n> +\tEOF\n> +\tgrep -v \"#\" >expect +\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n"},{"id":"331540","messageId":"xmqqbmkmvdrq.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031211852.13001-7-sbeller@google.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T04:11:53Z","receivedAt":"2017-11-01T04:12:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +If the given object refers to a blob, it will be described\n> +as `:`, such that the blob can be found\n> +at `` in the ``. Note, that the commit is likely\n\nDoes the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or\nwould it always be :?\n\n> +not the commit that introduced the blob, but the one that was found\n> +first; to find the commit that introduced the blob, you need to find\n\nBecause we do not want to descend into the same tree object we saw\nearlier, this \"we show the name we happened to find first without\nattempting to refine it for a better name\" is a rather fundamental\nlimitation of this approach. Hopefully we can later improve it with\nmore thought, but for now it is better than nothing (and much better\nthan \"git log --raw | grep\").\n\n> +the commit that last touched the path, e.g.\n> +`git log -- `.\n> +As blobs do not point at the commits they are contained in,\n> +describing blobs is slow as we have to walk the whole graph.\n\nIs it true that we walk the \"whole graph\", or do we stop immediately\nwe find a path to the blob? The former makes it sound like\ncompletely useless.\n\n> -#define SEEN\t\t(1u << 0)\n\nInteresting. Now we include revision.h this becomes redundant.\n\n> +static void describe_blob(struct object_id oid, struct strbuf *dst)\n> +{\n> +\tstruct rev_info revs;\n> +\tstruct argv_array args = ARGV_ARRAY_INIT;\n> +\tstruct process_commit_data pcd = { null_oid, oid, dst};\n> +\n> +\targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n> +\t\t\"--all\", \"--reflog\", /* as many starting points as possible */\n\nInteresting. \n\nDo we also search in the reflog in the normal \"describe\" operation?\nIf not, perhaps we should? I wonder what's the performance\nimplications would be.\n\nThat tangent aside, as \"describe blob\" is most likely a \"what\nreaches and is holding onto this blob?\" query, finding something\nthat can only be reached from a reflog entry would make it more\nhelpful than without the option.\n\n> +\t\t/* NEEDSWORK: --all is incompatible with worktrees for now: */\n\nWhat's that last colon about?\n\n> +\t\t\"--single-worktree\",\n> +\t\t\"--objects\",\n> +\t\t\"--in-commit-order\",\n> +\t\tNULL);\n> +\n> +\tinit_revisions(&revs, NULL);\n> +\tif (setup_revisions(args.argc, args.argv, &revs, NULL) > 1)\n> +\t\tBUG(\"setup_revisions could not handle all args?\");\n> +\n> +\tif (prepare_revision_walk(&revs))\n> +\t\tdie(\"revision walk setup failed\");\n> +\n> +\ttraverse_commit_list(&revs, process_commit, process_object, &pcd);\n> +\treset_revision_walk();\n> +}\n> +\n\nOK.\n\n> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\n> index 1c0e8659d9..3be01316e8 100755\n> --- a/t/t6120-describe.sh\n> +++ b/t/t6120-describe.sh\n> @@ -310,6 +310,21 @@ test_expect_success 'describe ignoring a borken submodule' '\n> \tgrep broken out\n> '\n> \n> +test_expect_success 'describe a blob at a tag' '\n> +\techo \"make it a unique blob\" >file &&\n> +\tgit add file && git commit -m \"content in file\" &&\n> +\tgit tag -a -m \"latest annotated tag\" unique-file &&\n> +\tgit describe HEAD:file >actual &&\n> +\techo \"unique-file:file\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'describe a surviving blob' '\n> +\tgit commit --allow-empty -m \"empty commit\" &&\n> +\tgit describe HEAD:file >actual &&\n> +\tgrep unique-file-1-g actual\n> +'\n> +\n\nI am not sure what \"surviving\" means in this context. The word\nhinted that the test would be finding a blob that only appears in a\ncommit that only appears as a reflog entry, but that wasn't the\ncase, which was a bit disappointing.\n\n\n\n"},{"id":"331542","messageId":"xmqqzi86twam.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"Re: [PATCHv2 0/7] git describe blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T05:14:41Z","receivedAt":"2017-11-01T05:14:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Occasionally a user is given an object hash from a blob as an error message\n> or other output (e.g. [1]).\n>\n> It would be useful to get a further description of such a blob, such as\n> the (commit, path) tuple where this blob was introduced.\n>\n> This implements the answer in builtin/describe,\n> however the heuristics are weak. See patch 6 for details.\n\nOverall this was a relatively pleasant read. Will queue.\n\nThanks.\n"},{"id":"331562","messageId":"alpine.DEB.2.21.1.1711011316590.6482@virtualbox","threadId":"47050","inReplyTo":"xmqqr2tiveqp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T12:26:57Z","receivedAt":"2017-11-01T12:27:12Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 1 Nov 2017, Junio C Hamano wrote:\n\n> Stefan Beller writes:\n> \n> > diff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh\n> > new file mode 100755\n> > index 0000000000..651666979b\n> > --- /dev/null\n> > +++ b/t/t6100-rev-list-in-order.sh\n> > @@ -0,0 +1,46 @@\n> > +#!/bin/sh\n> > +\n> > +test_description='miscellaneous rev-list tests'\n> > +\n> > +. ./test-lib.sh\n> > +\n> > +\n> > +test_expect_success 'setup' '\n> > +\tfor x in one two three four\n> > +\tdo\n> > +\t\techo $x >$x &&\n> > +\t\tgit add $x &&\n> > +\t\tgit commit -m \"add file $x\"\n> > +\tdone &&\n> > +\tfor x in four three\n> > +\tdo\n> > +\t\tgit rm $x &&\n> > +\t\tgit commit -m \"remove $x\"\n> > +\tdone &&\n> \n> When \"git commit -m 'remove four'\" fails, this loop would still\n> continue, so the &&-chain in \"done &&\" would still be rendered\n> ineffetive.\n\n... so the proper fix is to append an \"|| break\", as in:\n\n\t...\n\tfor x in four three\n\tdo\n\t\tgit rm $x &&\n\t\tgit commit -m \"remote $x\" ||\n\t\tbreak\n\tdone &&\n\t...\n\nBut that is still incorrect, as the \"break\" statement will succeed,\nbreaking (pun intended) the && chain in a different way. The canonical way\nto do it is to wrap the loop in a subshell, *and also* test $? (because\n`(exit 1) && echo something` still prints `something`):\n\n\t...\n\t(\n\t\tfor x in four three\n\t\tdo\n\t\t\tgit rm $x &&\n\t\t\tgit commit -m \"remote $x\" ||\n\t\t\texit\n\t\tdone\n\t) &&\n\ttest 0 -eq $? &&\n\t...\n\nUgly? Yes. Correct? Also yes.\n\nCiao,\nDscho\n"},{"id":"331563","messageId":"alpine.DEB.2.21.1.1711011329300.6482@virtualbox","threadId":"47050","inReplyTo":"xmqqbmkmvdrq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T12:32:45Z","receivedAt":"2017-11-01T12:33:02Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 1 Nov 2017, Junio C Hamano wrote:\n\n> Stefan Beller writes:\n> \n> > +If the given object refers to a blob, it will be described\n> > +as `:`, such that the blob can be found\n> > +at `` in the ``. Note, that the commit is likely\n> \n> Does the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or\n> would it always be :?\n\nAs the blob is described using this function:\n\nstatic void process_object(struct object *obj, const char *path, void *data)\n{\n struct process_commit_data *pcd = data;\n\n if (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n reset_revision_walk();\n describe_commit(&pcd->current_commit, pcd->dst);\n strbuf_addf(pcd->dst, \":%s\", path);\n }\n}\n\ni.e. as `describe_commit()` is used on the commit part, the answer to your\nquestion is: the former. I guess that is why Stefan wrote `commit-ish`\ninstead of `commit` ;-)\n\n> > +not the commit that introduced the blob, but the one that was found\n> > +first; to find the commit that introduced the blob, you need to find\n> \n> Because we do not want to descend into the same tree object we saw\n> earlier, this \"we show the name we happened to find first without\n> attempting to refine it for a better name\" is a rather fundamental\n> limitation of this approach. Hopefully we can later improve it with\n> more thought, but for now it is better than nothing (and much better\n> than \"git log --raw | grep\").\n\nIndeed.\n\nCiao,\nDscho\n"},{"id":"331564","messageId":"CAPc5daWpUMOCYyK=S8JiAV5DHF6BYDff8wH0N6QDeR=nvOXnLA@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711011316590.6482@virtualbox","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T12:37:18Z","receivedAt":"2017-11-01T12:37:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Wed, Nov 1, 2017 at 9:26 PM, Johannes Schindelin\n wrote:\n>\n> ...\n> (\n> for x in four three\n> do\n> git rm $x &&\n> git commit -m \"remote $x\" ||\n> exit\n> done\n> ) &&\n> test 0 -eq $? &&\n> ...\n>\n> Ugly? Yes. Correct? Also yes.\n\nI think returning non-zero with \"return\" is how other tests avoid an\nextra level of subshell.\nUgly? Yes. Correct? Questionable but it seems to work for those who\nwrote them ;-)\n"},{"id":"331586","messageId":"CAGZ79kZPNiNnSJd6CNYb7XkTVsT2ONLQLhwAQxt6_SPFTSwMcw@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711011329300.6482@virtualbox","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T17:59:08Z","receivedAt":"2017-11-01T17:59:13Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> Does the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or\n>> would it always be :?\n>\n> As the blob is described using this function:\n>\n> static void process_object(struct object *obj, const char *path, void *data)\n> {\n> struct process_commit_data *pcd = data;\n>\n> if (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n> reset_revision_walk();\n> describe_commit(&pcd->current_commit, pcd->dst);\n> strbuf_addf(pcd->dst, \":%s\", path);\n> }\n> }\n>\n> i.e. as `describe_commit()` is used on the commit part, the answer to your\n> question is: the former. I guess that is why Stefan wrote `commit-ish`\n> instead of `commit` ;-)\n\n$ ./git describe a9dbc3f12c\nwarning: reflog of 'HEAD' references pruned commits\nv2.15.0-7-g980e40477f:GIT-VERSION-GEN\n\nSo as noted below, this output is less than ideal, but technically correct as\nv2.15.0-7-g980e40477f contains that blob as well (you don't have these;\nit is this very series consisting of 7 patches on top of 2.15, none of them\ntouching GIT-VERSION-GEN, hence that blob stays intact.)\n\nThe way Junio asked, we actually may prefer the commit-ish to give\nthat commit that introduced the blob for the first time, i.e. add the\n--reverse to the graph walking.\n"},{"id":"331588","messageId":"CAGZ79kY5ye-mXy--V9VfUR_zRTrL21VKt3HKghOAP7vHJy7LHw@mail.gmail.com","threadId":"47050","inReplyTo":"xmqqh8ueyet8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/7] t6120: fix typo in test name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T18:13:41Z","receivedAt":"2017-11-01T18:13:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 31, 2017 at 6:21 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>> t/t6120-describe.sh | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> Good. I am guessing that you are sending this as the last/optional\n> one because this was found _after_ you worked on other parts of the\n> series, but I think it is easier to reason about if this were marked\n> as a preliminary clean-up and moved to the front of the series.\n>\n> Thanks.\n\nIt can be independent even. I did not bother to put it on its own flight,\nbut I can shift it to the front of this series.\n\nThanks,\nStefan\n"},{"id":"331594","messageId":"CAGZ79kY-yooE4cZK0Sxh3AL32wi66PRh1LzB8e3djq=bnLDHFg@mail.gmail.com","threadId":"47050","inReplyTo":"CAPc5daWpUMOCYyK=S8JiAV5DHF6BYDff8wH0N6QDeR=nvOXnLA@mail.gmail.com","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T19:37:59Z","receivedAt":"2017-11-01T19:38:07Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 1, 2017 at 5:37 AM, Junio C Hamano wrote:\n> On Wed, Nov 1, 2017 at 9:26 PM, Johannes Schindelin\n> wrote:\n>>\n>> ...\n>> (\n>> for x in four three\n>> do\n>> git rm $x &&\n>> git commit -m \"remote $x\" ||\n>> exit\n>> done\n>> ) &&\n>> test 0 -eq $? &&\n>> ...\n>>\n>> Ugly? Yes. Correct? Also yes.\n>\n> I think returning non-zero with \"return\" is how other tests avoid an\n> extra level of subshell.\n> Ugly? Yes. Correct? Questionable but it seems to work for those who\n> wrote them ;-)\n\nI think I'll go without the extra subshell and with s/return 1/false/ as\nthe exact value doesn't matter.\n\nThanks for hinting at correct implementations\n"},{"id":"331596","messageId":"CAGZ79kZ86fEvtF4rdPiAMW_x040AmwTWUnY4g1ixMjooFjU6Fw@mail.gmail.com","threadId":"47050","inReplyTo":"CAPig+cRz6EjdEGUjdMiB8979VybKN8Xza9uUDKNuxKNn+pY8jA@mail.gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T19:51:34Z","receivedAt":"2017-11-01T19:51:41Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 31, 2017 at 2:49 PM, Eric Sunshine wrote:\n\n>> to make up a name for the commit. These names are ambivalent, there might\n>\n> I guess you meant s/ambivalent/ambiguous/ ?\n\nIndeed!\n\n ambivalent\n early 20th century: from ambivalence (from German Ambivalenz ),\n on the pattern of equivalent.\n\nIn German ambivalent (~synonym to mehrdeutig, \"multi meaning\")\nactually means ambiguous, one of the false friends!\n\nThanks for pointing out.\n\n>> [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n>> Signed-off-by: Stefan Beller \n>\n> Blank line before sign-off.\n\nfixes.\n\n\n>\n> Not at all worth a re-roll, but less nesting and a bit less noisy:\n\nfixed.\n"},{"id":"331608","messageId":"CAGZ79kYC4=rDSWC7WCGVL4n4OC8BpvMJFwxx+LV9GqoGnSHFSA@mail.gmail.com","threadId":"47050","inReplyTo":"xmqq4lqewu2b.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T20:58:54Z","receivedAt":"2017-11-01T20:59:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 31, 2017 at 8:34 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>> Given the difficulty in\n>>> coming up with the single-liner description of what it does we saw\n>>> above, I suspect that splitting SYNOPSIS out into two very distinct\n>>> operating mode might make it easier to read.\n>>>\n>>> SYNOPSIS\n>>> --------\n>>> [verse]\n>>> 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n>>> +'git describe' [...] ...\n>>>\n>>> Then this additional paragraph can say \"When describin a \",\n>>> without using a (technically nonsense) phrase \"if \n>>> refers to a blob\", which is never true.\n>>\n>> ok, do we know about 'blob-ish' as a term?\n>\n> No, and I do not think there is any need to say -ish at all for this\n> use case.\n>\n> After all, when we accept a when a is called\n> for, that is only because there is only one way to use the commit in\n> place of the wanted ; we take the top-level tree contained in\n> it. You cannot say you take and take a , as it is\n> unclear which entry in the can act as the substitute for the\n> wanted .\n\nBut now we have a path as well, the notation of\n COLON \nis not a unique description of the blob, because\n* there can be multiple s depending on the tags and walking\n* in boilerplate code cases, we might even have the blob at different\n places (e.g. pristine copies of a license file in subdirectories)\n\nWhen calling for a tree-ish, we also accept commits and tags\nplus walking directions.\n\nSo I find it hard to think we have to take in exact blob names, but\nwe would also accept blob-ishs (i.e. commit+path)\n"},{"id":"331609","messageId":"EF573E5E-EA72-4DEE-822C-B44265FD581B@gmail.com","threadId":"47050","inReplyTo":"CAGZ79kZPNiNnSJd6CNYb7XkTVsT2ONLQLhwAQxt6_SPFTSwMcw@mail.gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-11-01T21:05:41Z","receivedAt":"2017-11-01T21:06:00Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn November 1, 2017 10:59:08 AM PDT, Stefan Beller wrote:\n>>> Does the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or\n>>> would it always be :?\n>>\n>> As the blob is described using this function:\n>>\n>> static void process_object(struct object *obj, const char *path, void\n>*data)\n>> {\n>> struct process_commit_data *pcd = data;\n>>\n>> if (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n>> reset_revision_walk();\n>> describe_commit(&pcd->current_commit, pcd->dst);\n>> strbuf_addf(pcd->dst, \":%s\", path);\n>> }\n>> }\n>>\n>> i.e. as `describe_commit()` is used on the commit part, the answer to\n>your\n>> question is: the former. I guess that is why Stefan wrote\n>`commit-ish`\n>> instead of `commit` ;-)\n>\n>$ ./git describe a9dbc3f12c\n>warning: reflog of 'HEAD' references pruned commits\n>v2.15.0-7-g980e40477f:GIT-VERSION-GEN\n>\n>So as noted below, this output is less than ideal, but technically\n>correct as\n>v2.15.0-7-g980e40477f contains that blob as well (you don't have these;\n>it is this very series consisting of 7 patches on top of 2.15, none of\n>them\n>touching GIT-VERSION-GEN, hence that blob stays intact.)\n>\n>The way Junio asked, we actually may prefer the commit-ish to give\n>that commit that introduced the blob for the first time, i.e. add the\n>--reverse to the graph walking.\n\nI know id prefer the first commit that introduced the blob. That's what describing a commit does, it finds the oldest tag prior to the commit, while --contains finds the first tag that contains the commit as an ancestor.\n\nNeither of these things is a perfect match for blobs since we want to only find an exact commit that still has that blob. I think finding the first commit to introduce a blob is generally more useful. Finding the last commit from some branch before the blob was changed also might be useful but I don't know if that's exactly what you implemented here...\n\nIn either respect, unless it's super easy to implement, going with what you have now is better than nothing and we can improve it in the future should someone wish to take up the work.\n\n-Jake\n-- \nSent from my Android device with K-9 Mail. Please excuse my brevity.\n"},{"id":"331612","messageId":"alpine.DEB.2.21.1.1711012235070.6482@virtualbox","threadId":"47050","inReplyTo":"CAPc5daWpUMOCYyK=S8JiAV5DHF6BYDff8wH0N6QDeR=nvOXnLA@mail.gmail.com","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T21:36:02Z","receivedAt":"2017-11-01T21:36:25Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 1 Nov 2017, Junio C Hamano wrote:\n\n> On Wed, Nov 1, 2017 at 9:26 PM, Johannes Schindelin\n> wrote:\n> >\n> > ...\n> > (\n> > for x in four three\n> > do\n> > git rm $x &&\n> > git commit -m \"remote $x\" ||\n> > exit\n> > done\n> > ) &&\n> > test 0 -eq $? &&\n> > ...\n> >\n> > Ugly? Yes. Correct? Also yes.\n> \n> I think returning non-zero with \"return\" is how other tests avoid an\n> extra level of subshell.\n> Ugly? Yes. Correct? Questionable but it seems to work for those who\n> wrote them ;-)\n\nGiven that the test_expect_* functions evaluate the code, it makes me\nwonder whether those `return` statements really return appropriately, or\none call level too low.\n\nCiao,\nDscho\n"},{"id":"331613","messageId":"20171101213911.aokzrnchr6s2r2uq@sigill.intra.peff.net","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012235070.6482@virtualbox","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-01T21:39:11Z","receivedAt":"2017-11-01T21:39:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 01, 2017 at 10:36:02PM +0100, Johannes Schindelin wrote:\n\n> Hi Junio,\n> \n> On Wed, 1 Nov 2017, Junio C Hamano wrote:\n> \n> > On Wed, Nov 1, 2017 at 9:26 PM, Johannes Schindelin\n> > wrote:\n> > >\n> > > ...\n> > > (\n> > > for x in four three\n> > > do\n> > > git rm $x &&\n> > > git commit -m \"remote $x\" ||\n> > > exit\n> > > done\n> > > ) &&\n> > > test 0 -eq $? &&\n> > > ...\n> > >\n> > > Ugly? Yes. Correct? Also yes.\n> > \n> > I think returning non-zero with \"return\" is how other tests avoid an\n> > extra level of subshell.\n> > Ugly? Yes. Correct? Questionable but it seems to work for those who\n> > wrote them ;-)\n> \n> Given that the test_expect_* functions evaluate the code, it makes me\n> wonder whether those `return` statements really return appropriately, or\n> one call level too low.\n\nThe test_expect functions eval the actual snippets inside a dummy\nfunction. This is intentional exactly to allow them to call \"return\" at\nwill.\n\n-Peff\n"},{"id":"331614","messageId":"CAGZ79kbVWWo4YAK74hrJHAtwfMxYMixe1ypdcyaeWEMs8SHQpA@mail.gmail.com","threadId":"47050","inReplyTo":"xmqqbmkmvdrq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T21:28:27Z","receivedAt":"2017-11-01T21:43:33Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 31, 2017 at 9:11 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> +If the given object refers to a blob, it will be described\n>> +as `:`, such that the blob can be found\n>> +at `` in the ``. Note, that the commit is likely\n>\n> Does the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or\n> would it always be :?\n>\n>> +not the commit that introduced the blob, but the one that was found\n>> +first; to find the commit that introduced the blob, you need to find\n>\n> Because we do not want to descend into the same tree object we saw\n> earlier, this \"we show the name we happened to find first without\n> attempting to refine it for a better name\" is a rather fundamental\n> limitation of this approach. Hopefully we can later improve it with\n> more thought, but for now it is better than nothing (and much better\n> than \"git log --raw | grep\").\n\nok.\n\n\n>\n>> +the commit that last touched the path, e.g.\n>> +`git log -- `.\n>> +As blobs do not point at the commits they are contained in,\n>> +describing blobs is slow as we have to walk the whole graph.\n>\n> Is it true that we walk the \"whole graph\", or do we stop immediately\n> we find a path to the blob? The former makes it sound like\n> completely useless.\n\nUnfortunately we walk the whole graph, as I have not figured out how to\nstop the walking from the callback in 'traverse_commit_list'.\n\nI assume I have to modify the struct rev_info that we operate on,\nclearing any pending commits?\n\n>\n>> -#define SEEN (1u << 0)\n>\n> Interesting. Now we include revision.h this becomes redundant.\n>\n\nCorrect. In a way this small part is a revert of 8713ab3079\n(Improve git-describe performance by reducing revision listing., 2007-01-13)\n\n\n>> + argv_array_pushl(&args, \"internal: The first arg is not parsed\",\n>> + \"--all\", \"--reflog\", /* as many starting points as possible */\n>\n> Interesting.\n>\n> Do we also search in the reflog in the normal \"describe\" operation?\n> If not, perhaps we should? I wonder what's the performance\n> implications would be.\n\n\"normal\" git describe doesn't need to walk the whole graph\nas we only walk down from the given commit-ish until a land mark\nis found.\n\nFor --contains, this might be an interesting though, as there we\nalso have to \"walk backwards without pointers to follow\".\n\n>\n> That tangent aside, as \"describe blob\" is most likely a \"what\n> reaches and is holding onto this blob?\" query, finding something\n> that can only be reached from a reflog entry would make it more\n> helpful than without the option.\n\nYeah that is my reasoning as well.\n\n>\n>> + /* NEEDSWORK: --all is incompatible with worktrees for now: */\n>\n> What's that last colon about?\n\nwill replace with a dot, it ought to hint at the line that follows,\nthe --single-worktree flag.\n\n>\n>> + \"--single-worktree\",\n>> + \"--objects\",\n>> + \"--in-commit-order\",\n>> + NULL);\n>> +\n>> + init_revisions(&revs, NULL);\n>> + if (setup_revisions(args.argc, args.argv, &revs, NULL) > 1)\n>> + BUG(\"setup_revisions could not handle all args?\");\n>> +\n>> + if (prepare_revision_walk(&revs))\n>> + die(\"revision walk setup failed\");\n>> +\n>> + traverse_commit_list(&revs, process_commit, process_object, &pcd);\n>> + reset_revision_walk();\n>> +}\n>> +\n>\n> OK.\n>\n>> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\n>> index 1c0e8659d9..3be01316e8 100755\n>> --- a/t/t6120-describe.sh\n>> +++ b/t/t6120-describe.sh\n>> @@ -310,6 +310,21 @@ test_expect_success 'describe ignoring a borken submodule' '\n>> grep broken out\n>> '\n>>\n>> +test_expect_success 'describe a blob at a tag' '\n>> + echo \"make it a unique blob\" >file &&\n>> + git add file && git commit -m \"content in file\" &&\n>> + git tag -a -m \"latest annotated tag\" unique-file &&\n>> + git describe HEAD:file >actual &&\n>> + echo \"unique-file:file\" >expect &&\n>> + test_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'describe a surviving blob' '\n>> + git commit --allow-empty -m \"empty commit\" &&\n>> + git describe HEAD:file >actual &&\n>> + grep unique-file-1-g actual\n>> +'\n>> +\n>\n> I am not sure what \"surviving\" means in this context.\n\nMaybe \"unchanged\", \"still kept around\" ?\n\n\n> The word\n> hinted that the test would be finding a blob that only appears in a\n> commit that only appears as a reflog entry, but that wasn't the\n> case, which was a bit disappointing.\n\noh!\n"},{"id":"331618","messageId":"alpine.DEB.2.21.1.1711012307060.6482@virtualbox","threadId":"47050","inReplyTo":"CAGZ79kY-yooE4cZK0Sxh3AL32wi66PRh1LzB8e3djq=bnLDHFg@mail.gmail.com","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T22:08:59Z","receivedAt":"2017-11-01T22:09:13Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Wed, 1 Nov 2017, Stefan Beller wrote:\n\n> On Wed, Nov 1, 2017 at 5:37 AM, Junio C Hamano wrote:\n> > On Wed, Nov 1, 2017 at 9:26 PM, Johannes Schindelin\n> > wrote:\n> >>\n> >> ...\n> >> (\n> >> for x in four three\n> >> do\n> >> git rm $x &&\n> >> git commit -m \"remote $x\" ||\n> >> exit\n> >> done\n> >> ) &&\n> >> test 0 -eq $? &&\n> >> ...\n> >>\n> >> Ugly? Yes. Correct? Also yes.\n> >\n> > I think returning non-zero with \"return\" is how other tests avoid an\n> > extra level of subshell.\n> > Ugly? Yes. Correct? Questionable but it seems to work for those who\n> > wrote them ;-)\n> \n> I think I'll go without the extra subshell and with s/return 1/false/ as\n> the exact value doesn't matter.\n\nYou mean\n\n\tfor ...\n\tdo\n\t\txyz ||\n\t\tfalse\n\tdone\n\n? That does not work. Try this:\n\n\tfor a in 1 2 3; do echo $a && false; done && echo done\n\nWhile it does not print `done`, it will print all of 1, 2 and 3.\n\nCiao,\nDscho\n"},{"id":"331619","messageId":"alpine.DEB.2.21.1.1711012310250.6482@virtualbox","threadId":"47050","inReplyTo":"EF573E5E-EA72-4DEE-822C-B44265FD581B@gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T22:12:40Z","receivedAt":"2017-11-01T22:12:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 1 Nov 2017, Jacob Keller wrote:\n\n> On November 1, 2017 10:59:08 AM PDT, Stefan Beller wrote:\n> >>> Does the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or\n> >>> would it always be :?\n> >>\n> >> As the blob is described using this function:\n> >>\n> >> static void process_object(struct object *obj, const char *path, void\n> >*data)\n> >> {\n> >> struct process_commit_data *pcd = data;\n> >>\n> >> if (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n> >> reset_revision_walk();\n> >> describe_commit(&pcd->current_commit, pcd->dst);\n> >> strbuf_addf(pcd->dst, \":%s\", path);\n> >> }\n> >> }\n> >>\n> >> i.e. as `describe_commit()` is used on the commit part, the answer to\n> >your\n> >> question is: the former. I guess that is why Stefan wrote\n> >`commit-ish`\n> >> instead of `commit` ;-)\n> >\n> >$ ./git describe a9dbc3f12c\n> >warning: reflog of 'HEAD' references pruned commits\n> >v2.15.0-7-g980e40477f:GIT-VERSION-GEN\n> >\n> >So as noted below, this output is less than ideal, but technically\n> >correct as\n> >v2.15.0-7-g980e40477f contains that blob as well (you don't have these;\n> >it is this very series consisting of 7 patches on top of 2.15, none of\n> >them\n> >touching GIT-VERSION-GEN, hence that blob stays intact.)\n> >\n> >The way Junio asked, we actually may prefer the commit-ish to give\n> >that commit that introduced the blob for the first time, i.e. add the\n> >--reverse to the graph walking.\n> \n> I know id prefer the first commit that introduced the blob. That's what\n> describing a commit does, it finds the oldest tag prior to the commit,\n> while --contains finds the first tag that contains the commit as an\n> ancestor.\n\nIt is very easy to wish for \"the oldest commit introducing a blob\". But\nsince we're in a DAG, this question is not necessarily easy to answer:\n\n- A - B - C\n \\ /\n D\n\nLet's assume that all commits except A have the blob, and that B and D\n(for some reason) have the same author/committer dates.\n\nDo you identify B or D as the commit introducing the blob?\n\nCiao,\nDscho\n"},{"id":"331623","messageId":"CAGZ79kbYshVjUh2XhbLHtYmnRuFaCmuWZJZd7HifKAqJbNBPfw@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012307060.6482@virtualbox","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T22:19:00Z","receivedAt":"2017-11-01T22:19:06Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> I think I'll go without the extra subshell and with s/return 1/false/ as\n>> the exact value doesn't matter.\n>\n> You mean\n>\n> for ...\n> do\n> xyz ||\n> false\n> done\n\nYes, I do.\n\n> ? That does not work. Try this:\n>\n> for a in 1 2 3; do echo $a && false; done && echo done\n\n... && echo $?\n 1\n\n> While it does not print `done`, it will print all of 1, 2 and 3.\n\nWe do not care about the internal state, aborting early, we rather\ncare only if the whole loop body was executed. Running the test\n\n test_expect_success 'witty title' '\n for a in 1 2 3; do echo $a && false; done && echo done\n '\n\nnot ok 1 - witty title\n\nThat is all we want to care about here?\nOtherwise I may still have a misunderstanding.\n\nThanks,\nStefan\n"},{"id":"331624","messageId":"CAGZ79kbOEM_W65Rym4yiDNHpFGTNWMYdh=aVPjThNWjEHPQong@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012310250.6482@virtualbox","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T22:21:36Z","receivedAt":"2017-11-01T22:21:42Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> I know id prefer the first commit that introduced the blob. That's what\n>> describing a commit does, it finds the oldest tag prior to the commit,\n>> while --contains finds the first tag that contains the commit as an\n>> ancestor.\n>\n> It is very easy to wish for \"the oldest commit introducing a blob\". But\n> since we're in a DAG, this question is not necessarily easy to answer:\n>\n> - A - B - C\n> \\ /\n> D\n>\n> Let's assume that all commits except A have the blob, and that B and D\n> (for some reason) have the same author/committer dates.\n>\n> Do you identify B or D as the commit introducing the blob?\n\nThe current implementation gives C, though.\n(assuming C is HEAD, and A is ancient)\n\nWith the --reverse flag one of B or D is given (not sure which,\ndepends on the exact order).\n"},{"id":"331627","messageId":"alpine.DEB.2.21.1.1711012333060.6482@virtualbox","threadId":"47050","inReplyTo":"20171101213911.aokzrnchr6s2r2uq@sigill.intra.peff.net","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T22:33:51Z","receivedAt":"2017-11-01T22:35:31Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 1 Nov 2017, Jeff King wrote:\n\n> On Wed, Nov 01, 2017 at 10:36:02PM +0100, Johannes Schindelin wrote:\n> \n> > On Wed, 1 Nov 2017, Junio C Hamano wrote:\n> > \n> > > On Wed, Nov 1, 2017 at 9:26 PM, Johannes Schindelin\n> > > wrote:\n> > > >\n> > > > ...\n> > > > (\n> > > > for x in four three\n> > > > do\n> > > > git rm $x &&\n> > > > git commit -m \"remote $x\" ||\n> > > > exit\n> > > > done\n> > > > ) &&\n> > > > test 0 -eq $? &&\n> > > > ...\n> > > >\n> > > > Ugly? Yes. Correct? Also yes.\n> > > \n> > > I think returning non-zero with \"return\" is how other tests avoid an\n> > > extra level of subshell.\n> > > Ugly? Yes. Correct? Questionable but it seems to work for those who\n> > > wrote them ;-)\n> > \n> > Given that the test_expect_* functions evaluate the code, it makes me\n> > wonder whether those `return` statements really return appropriately, or\n> > one call level too low.\n> \n> The test_expect functions eval the actual snippets inside a dummy\n> function. This is intentional exactly to allow them to call \"return\" at\n> will.\n\nTricksy. And a bit unintuitive for script kings such as myself, but okay.\n\nCiao,\nDscho\n"},{"id":"331628","messageId":"alpine.DEB.2.21.1.1711012337270.6482@virtualbox","threadId":"47050","inReplyTo":"CAGZ79kbYshVjUh2XhbLHtYmnRuFaCmuWZJZd7HifKAqJbNBPfw@mail.gmail.com","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T22:39:37Z","receivedAt":"2017-11-01T22:40:07Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Wed, 1 Nov 2017, Stefan Beller wrote:\n\n> We do not care about the internal state, aborting early, we rather\n> care only if the whole loop body was executed. Running the test\n> \n> test_expect_success 'witty title' '\n> for a in 1 2 3; do echo $a && false; done && echo done\n> '\n> \n> not ok 1 - witty title\n> \n> That is all we want to care about here?\n\nWe care about the loop body being executed successfully *each time*. A\nbetter counter example:\n\n$ for a in 1 2 3; do echo $a && test 2 != $a || false; done && echo done\n1\n2\n3\ndone\n\n(even if the second of three runs failed)\n\nCiao,\nDscho\n"},{"id":"331629","messageId":"alpine.DEB.2.21.1.1711012340290.6482@virtualbox","threadId":"47050","inReplyTo":"CAGZ79kbOEM_W65Rym4yiDNHpFGTNWMYdh=aVPjThNWjEHPQong@mail.gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-01T22:41:45Z","receivedAt":"2017-11-01T22:41:59Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Wed, 1 Nov 2017, Stefan Beller wrote:\n\n> >> I know id prefer the first commit that introduced the blob. That's\n> >> what describing a commit does, it finds the oldest tag prior to the\n> >> commit, while --contains finds the first tag that contains the commit\n> >> as an ancestor.\n> >\n> > It is very easy to wish for \"the oldest commit introducing a blob\". But\n> > since we're in a DAG, this question is not necessarily easy to answer:\n> >\n> > - A - B - C\n> > \\ /\n> > D\n> >\n> > Let's assume that all commits except A have the blob, and that B and D\n> > (for some reason) have the same author/committer dates.\n> >\n> > Do you identify B or D as the commit introducing the blob?\n> \n> The current implementation gives C, though.\n> (assuming C is HEAD, and A is ancient)\n> \n> With the --reverse flag one of B or D is given (not sure which,\n> depends on the exact order).\n\nSure, but it is still a tricky thing. Imagine\n\n- A1 - B1 - A2 - B2 - B3\n\nwhere all the B* commits have the blob. Do you really want to report B1\nrather than B2 as the commit introducing the blob? (I would prefer B2...)\n\nCiao,\nDscho\n"},{"id":"331630","messageId":"CAGZ79ka+EO8U2dDxquH0ypBvOQHy-nDy0Fcdw6j_iTGNWDmsJQ@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012337270.6482@virtualbox","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T22:46:52Z","receivedAt":"2017-11-01T22:46:58Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 1, 2017 at 3:39 PM, Johannes Schindelin\n wrote:\n>> not ok 1 - witty title\n>>\n>> That is all we want to care about here?\n>\n> We care about the loop body being executed successfully *each time*. A\n> better counter example:\n\nGood point. I'll use return in that case. Thanks!\n\nStefan\n"},{"id":"331631","messageId":"CAGZ79ka2Taie+SVjw6poi=ejFMRV5MSBUGBUnCtn+KTdXRzGbg@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012340290.6482@virtualbox","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T22:53:22Z","receivedAt":"2017-11-01T22:53:28Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 1, 2017 at 3:41 PM, Johannes Schindelin\n wrote:\n\n>> The current implementation gives C, though.\n>> (assuming C is HEAD, and A is ancient)\n>>\n>> With the --reverse flag one of B or D is given (not sure which,\n>> depends on the exact order).\n>\n> Sure, but it is still a tricky thing. Imagine\n>\n> - A1 - B1 - A2 - B2 - B3\n>\n> where all the B* commits have the blob. Do you really want to report B1\n> rather than B2 as the commit introducing the blob? (I would prefer B2...)\n\nYou are correct, that B2 is also important to find.\n\nIdeally you want a list of all adjacent commit groups, but that is too\ntricky to implement for now, deferring it to another contribution in the future.\n\nThe current proposal would give you B3, such that `git log B3 -- path` will find\nyou B2.\n"},{"id":"331634","messageId":"xmqqzi85schf.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012333060.6482@virtualbox","subject":"Re: [PATCHv2 2/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-02T01:20:12Z","receivedAt":"2017-11-02T01:20:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> > Given that the test_expect_* functions evaluate the code, it makes me\n>> > wonder whether those `return` statements really return appropriately, or\n>> > one call level too low.\n>> \n>> The test_expect functions eval the actual snippets inside a dummy\n>> function. This is intentional exactly to allow them to call \"return\" at\n>> will.\n>\n> Tricksy. And a bit unintuitive for script kings such as myself, but okay.\n\nExactly.\n\nThe hidden assumption on the way how \"return\" interacts with\nthe way we use \"eval\" is the reason why I said \"Ugly? Yes, Correct?\nQuestionable\" in the first place.\n"},{"id":"331637","messageId":"xmqqmv45sbpq.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"xmqqh8ueyet8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 7/7] t6120: fix typo in test name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-02T01:36:49Z","receivedAt":"2017-11-02T01:36:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Stefan Beller writes:\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>> t/t6120-describe.sh | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> Good. I am guessing that you are sending this as the last/optional\n> one because this was found _after_ you worked on other parts of the\n> series, but I think it is easier to reason about if this were marked\n> as a preliminary clean-up and moved to the front of the series.\n>\n> Thanks.\n\nHaving said that, if you already have v2 that keeps it at the end,\nthat is fine. \n\nIt's just leaving such an \"By the way I found this unrelated one and\nfixed it while at it\" at the end when submitting will give an\nimpression that the series is not as well proof-read as it could be,\nbecause a good proof-reader would notice something out-of-place like\nthis fairly quickly and would move it to the front or even in a\nseparate preliminary clean-up series.\n\n"},{"id":"331638","messageId":"xmqqinetsayr.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CAGZ79kYC4=rDSWC7WCGVL4n4OC8BpvMJFwxx+LV9GqoGnSHFSA@mail.gmail.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-02T01:53:00Z","receivedAt":"2017-11-02T01:53:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> But now we have a path as well, the notation of\n> COLON \n> is not a unique description of the blob, because\n> * there can be multiple s depending on the tags and walking\n> * in boilerplate code cases, we might even have the blob at different\n> places (e.g. pristine copies of a license file in subdirectories)\n>\n> When calling for a tree-ish, we also accept commits and tags\n> plus walking directions.\n\nI think you are confused --- that is not what \"-ish\" suffix is used\nin our conversation on objects.\n\nThe reason why we say \"-ish\" is \"Yes we know v2.15.0 is *NOT* a\ncommit object, we very well know it is a tag object, but because we\nallow it to be used in a context that calls for a commit object, we\nmark that use context as 'this accepts commit-ish, not just\ncommit'\".\n\nBut what you call \"walking direction\", e.g. \"HEAD:Makefile\", names a\n\"BLOB\". It is not \"this is something like BLOB, no, we very well\nknow it is not a blob but the context allows such a non-blob to be\nfed in place of a blob, so we take that even though it is not a\nblob\". Because it is.\n\n"},{"id":"331641","messageId":"xmqq375xs3zr.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"xmqqinetsayr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-02T04:23:36Z","receivedAt":"2017-11-02T04:23:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The reason why we say \"-ish\" is \"Yes we know v2.15.0 is *NOT* a\n> commit object, we very well know it is a tag object, but because we\n> allow it to be used in a context that calls for a commit object, we\n> mark that use context as 'this accepts commit-ish, not just\n> commit'\".\n\nHaving said all that, there is a valid case in which we might want\nto say \"blob-ish\".\n\nTo review, X-ish is the word we use when the command wants to take\nan X, but tolerates a lazy user who gives a Y, which is *NOT* X,\nwithout bothering to add ^{X} suffix, i.e. Y^{X}. In such a case,\nthe command takes not just X but takes X-ish because it takes a Y\nand converts it internally to an X to be extra nice.\n\nWhen the command wants to take a blob, but tolerates something else\nand does \"^{blob}\" internally, we can say it takes \"blob-ish\".\nTechnically that \"something else\" could be an annotated tag that\npoints at a blob object, without any intervening commit or tree (I\ndid not check if the \"describe \" code in this thread handles\nthis, though).\n\nBut because it is not usually done to tag a blob directly, it would\nprobably be not worth to say \"blob-ish\" in the document and cause\nreaders to wonder in what situation something that is not a blob can\nbe treated as if it were a blob. It does feel like we would be\npursuing technical correctness too much and sacrificing the readability\nof the document, at least to me, and a bad trade-off.\n\n\n"},{"id":"331646","messageId":"CA+P7+xqw9zKNUkn9P-qA57ADSB5G_7Sd0JCc6SaK6bBf9-Jhtg@mail.gmail.com","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012340290.6482@virtualbox","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-11-02T06:05:00Z","receivedAt":"2017-11-02T06:05:27Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Nov 1, 2017 at 3:41 PM, Johannes Schindelin\n wrote:\n> Hi Stefan,\n>\n> On Wed, 1 Nov 2017, Stefan Beller wrote:\n>\n>> >> I know id prefer the first commit that introduced the blob. That's\n>> >> what describing a commit does, it finds the oldest tag prior to the\n>> >> commit, while --contains finds the first tag that contains the commit\n>> >> as an ancestor.\n>> >\n>> > It is very easy to wish for \"the oldest commit introducing a blob\". But\n>> > since we're in a DAG, this question is not necessarily easy to answer:\n>> >\n>> > - A - B - C\n>> > \\ /\n>> > D\n>> >\n>> > Let's assume that all commits except A have the blob, and that B and D\n>> > (for some reason) have the same author/committer dates.\n>> >\n>> > Do you identify B or D as the commit introducing the blob?\n>>\n>> The current implementation gives C, though.\n>> (assuming C is HEAD, and A is ancient)\n>>\n>> With the --reverse flag one of B or D is given (not sure which,\n>> depends on the exact order).\n>\n> Sure, but it is still a tricky thing. Imagine\n>\n> - A1 - B1 - A2 - B2 - B3\n>\n> where all the B* commits have the blob. Do you really want to report B1\n> rather than B2 as the commit introducing the blob? (I would prefer B2...)\n>\n> Ciao,\n> Dscho\n\nYes, but I'd rather we find either B1 or B2, rather than B3, or in the\nfirst case, I'd rather we *at least* provide B or D, rather than C.\n\nOr:\n\nA - B1 - C -E - ... - Z\n \\ B2 /\n\nIn this example, if everything except a had the blob, we'd find z,\nwhich I think isn't as useful as finding one of B or D.\n\nSure we can't find a perfect answer, but I'd rather find a commit\nwhich introduced as blob as one that adds the blob, rather than one\nthat happens to point to it because it has history which includes it.\n\nI think both questions are valuable, the first is \"which commit last\nhad this blob\", the second is \"which commit first introduced this\nblob\", neither of which can always give a definitive answer. It really\ndepends on what question you're asking up front.\n\nThanks,\nJake\n"},{"id":"331659","messageId":"871slhm9dm.fsf@linux-m68k.org","threadId":"47050","inReplyTo":"alpine.DEB.2.21.1.1711012340290.6482@virtualbox","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-11-02T07:23:49Z","receivedAt":"2017-11-02T07:23:59Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Nov 01 2017, Johannes Schindelin wrote:\n\n> Sure, but it is still a tricky thing. Imagine\n>\n> - A1 - B1 - A2 - B2 - B3\n>\n> where all the B* commits have the blob. Do you really want to report B1\n> rather than B2 as the commit introducing the blob? (I would prefer B2...)\n\nWhat if B3 renames or copies the blob?\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"331683","messageId":"CAGZ79kZXLpwCyWs3_mfj7kL1qmM965V7vZZ2UH4-hqrfF5f+8g@mail.gmail.com","threadId":"47050","inReplyTo":"871slhm9dm.fsf@linux-m68k.org","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T18:18:24Z","receivedAt":"2017-11-02T18:18:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Nov 2, 2017 at 12:23 AM, Andreas Schwab wrote:\n> On Nov 01 2017, Johannes Schindelin wrote:\n>\n>> Sure, but it is still a tricky thing. Imagine\n>>\n>> - A1 - B1 - A2 - B2 - B3\n>>\n>> where all the B* commits have the blob. Do you really want to report B1\n>> rather than B2 as the commit introducing the blob? (I would prefer B2...)\n>\n> What if B3 renames or copies the blob?\n>\n> Andreas.\n\nWith the current proposed patch you'd find B3, and then use the diff machinery\nto digg deeper from there (renames/copies ought to be easy to detect already?)\n\nSo with a copy B3 might be a better start than B1, as starting from B1 you\nwould not find B3 easily.\n\nFor a rename, I would think a reverse log/blame on B1:path may help.\n\nWith that said, I think I'll just reroll the series with the current logic\nfixing the other minor issues that were brought up as B3 seems to\nbe the most versatile (though not optimal) answer for many use cases.\n\nThanks for that thought,\nStefan\n"},{"id":"331690","messageId":"20171102194148.2124-1-sbeller@google.com","threadId":"47050","inReplyTo":"20171031211852.13001-1-sbeller@google.com","subject":"[PATCHv3 0/7] git describe blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:41Z","receivedAt":"2017-11-02T19:42:02Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Thanks for the discussion on v2[1].\n\nInterdiff is below, just fixing minor things.\n\nWe'll keep the original algorithm for now, deferring an improvement on \nthe algorithm front towards a future developer.\n\nThanks,\nStefan\n\n[1] https://public-inbox.org/git/20171031211852.13001-1-sbeller@google.com/\n\nStefan Beller (7):\n t6120: fix typo in test name\n list-objects.c: factor out traverse_trees_and_blobs\n revision.h: introduce blob/tree walking in order of the commits\n builtin/describe.c: rename `oid` to avoid variable shadowing\n builtin/describe.c: print debug statements earlier\n builtin/describe.c: factor out describe_commit\n builtin/describe.c: describe a blob\n\n Documentation/git-describe.txt | 13 +++-\n Documentation/rev-list-options.txt | 5 ++\n builtin/describe.c | 125 ++++++++++++++++++++++++++++---------\n list-objects.c | 52 +++++++++------\n revision.c | 2 +\n revision.h | 3 +-\n t/t6100-rev-list-in-order.sh | 47 ++++++++++++++\n t/t6120-describe.sh | 17 ++++-\n 8 files changed, 214 insertions(+), 50 deletions(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\ndiff --git c/Documentation/git-describe.txt w/Documentation/git-describe.txt\nindex 077c3c2193..79ec0be62a 100644\n--- c/Documentation/git-describe.txt\n+++ w/Documentation/git-describe.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n [verse]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]\n-'git describe' [] \n+'git describe' [] \n \n DESCRIPTION\n -----------\ndiff --git c/builtin/describe.c w/builtin/describe.c\nindex 382cbae908..cf08bef344 100644\n--- c/builtin/describe.c\n+++ w/builtin/describe.c\n@@ -440,6 +440,7 @@ struct process_commit_data {\n \tstruct object_id current_commit;\n \tstruct object_id looking_for;\n \tstruct strbuf *dst;\n+\tstruct rev_info *revs;\n };\n \n static void process_commit(struct commit *commit, void *data)\n@@ -456,6 +457,7 @@ static void process_object(struct object *obj, const char *path, void *data)\n \t\treset_revision_walk();\n \t\tdescribe_commit(&pcd->current_commit, pcd->dst);\n \t\tstrbuf_addf(pcd->dst, \":%s\", path);\n+\t\tpcd->revs->max_count = 0;\n \t}\n }\n \n@@ -463,7 +465,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)\n {\n \tstruct rev_info revs;\n \tstruct argv_array args = ARGV_ARRAY_INIT;\n-\tstruct process_commit_data pcd = { null_oid, oid, dst};\n+\tstruct process_commit_data pcd = { null_oid, oid, dst, &revs};\n \n \targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n \t\t\"--all\", \"--reflog\", /* as many starting points as possible */\n@@ -497,14 +499,12 @@ static void describe(const char *arg, int last_one)\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n \tcmit = lookup_commit_reference_gently(&oid, 1);\n \n-\tif (cmit) {\n+\tif (cmit)\n \t\tdescribe_commit(&oid, &sb);\n-\t} else {\n-\t\tif (lookup_blob(&oid))\n-\t\t\tdescribe_blob(oid, &sb);\n-\t\telse\n-\t\t\tdie(_(\"%s is neither a commit nor blob\"), arg);\n-\t}\n+\telse if (lookup_blob(&oid))\n+\t\tdescribe_blob(oid, &sb);\n+\telse\n+\t\tdie(_(\"%s is neither a commit nor blob\"), arg);\n \n \tputs(sb.buf);\n \ndiff --git c/list-objects.c w/list-objects.c\nindex 03438e5a8b..07a92f35fe 100644\n--- c/list-objects.c\n+++ w/list-objects.c\n@@ -184,13 +184,13 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n }\n \n static void traverse_trees_and_blobs(struct rev_info *revs,\n-\t\t\t\t struct strbuf *base_path,\n+\t\t\t\t struct strbuf *base,\n \t\t\t\t show_object_fn show_object,\n \t\t\t\t void *data)\n {\n \tint i;\n \n-\tassert(base_path->len == 0);\n+\tassert(base->len == 0);\n \n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n@@ -208,12 +208,12 @@ static void traverse_trees_and_blobs(struct rev_info *revs,\n \t\t\tpath = \"\";\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t base_path, path, data);\n+\t\t\t\t base, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t base_path, path, data);\n+\t\t\t\t base, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\ndiff --git c/t/t6100-rev-list-in-order.sh w/t/t6100-rev-list-in-order.sh\nindex 651666979b..d4d539b0da 100755\n--- c/t/t6100-rev-list-in-order.sh\n+++ w/t/t6100-rev-list-in-order.sh\n@@ -1,21 +1,22 @@\n #!/bin/sh\n \n-test_description='miscellaneous rev-list tests'\n+test_description='rev-list testing in-commit-order'\n \n . ./test-lib.sh\n \n-\n-test_expect_success 'setup' '\n+test_expect_success 'rev-list --in-commit-order' '\n \tfor x in one two three four\n \tdo\n \t\techo $x >$x &&\n \t\tgit add $x &&\n-\t\tgit commit -m \"add file $x\"\n+\t\tgit commit -m \"add file $x\" ||\n+\t\treturn 1\n \tdone &&\n \tfor x in four three\n \tdo\n \t\tgit rm $x &&\n-\t\tgit commit -m \"remove $x\"\n+\t\tgit commit -m \"remove $x\" ||\n+\t\treturn 1\n \tdone &&\n \tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n \tcut -c 1-40 >actual actual &&\n \tgrep unique-file-1-g actual\n"},{"id":"331691","messageId":"20171102194148.2124-2-sbeller@google.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"[PATCHv3 1/7] t6120: fix typo in test name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:42Z","receivedAt":"2017-11-02T19:42:05Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n t/t6120-describe.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 1c0e8659d9..c8b7ed82d9 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -304,7 +304,7 @@ test_expect_success 'describe chokes on severely broken submodules' '\n \tmv .git/modules/sub1/ .git/modules/sub_moved &&\n \ttest_must_fail git describe --dirty\n '\n-test_expect_success 'describe ignoring a borken submodule' '\n+test_expect_success 'describe ignoring a broken submodule' '\n \tgit describe --broken >out &&\n \ttest_when_finished \"mv .git/modules/sub_moved .git/modules/sub1\" &&\n \tgrep broken out\n-- \n2.15.0.7.g980e40477f\n\n"},{"id":"331692","messageId":"20171102194148.2124-3-sbeller@google.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"[PATCHv3 2/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:43Z","receivedAt":"2017-11-02T19:42:07Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"With traverse_trees_and_blobs factored out of the main traverse function,\nthe next patch can introduce an in-order revision walking with ease.\n\nIn the next patch we'll call `traverse_trees_and_blobs` from within the\nloop walking the commits, such that we'll have one invocation of that\nfunction per commit. That is why we do not want to have memory allocations\nin that function, such as we'd have if we were to use a strbuf locally.\nPass a strbuf from traverse_commit_list into the blob and tree traversing\nfunction as a scratch pad that only needs to be allocated once.\n\nSigned-off-by: Stefan Beller \n---\n list-objects.c | 50 +++++++++++++++++++++++++++++++-------------------\n 1 file changed, 31 insertions(+), 19 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex b3931fa434..7c2ce9c4bd 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -183,25 +183,15 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n \tadd_pending_object(revs, &tree->object, \"\");\n }\n \n-void traverse_commit_list(struct rev_info *revs,\n-\t\t\t show_commit_fn show_commit,\n-\t\t\t show_object_fn show_object,\n-\t\t\t void *data)\n+static void traverse_trees_and_blobs(struct rev_info *revs,\n+\t\t\t\t struct strbuf *base,\n+\t\t\t\t show_object_fn show_object,\n+\t\t\t\t void *data)\n {\n \tint i;\n-\tstruct commit *commit;\n-\tstruct strbuf base;\n \n-\tstrbuf_init(&base, PATH_MAX);\n-\twhile ((commit = get_revision(revs)) != NULL) {\n-\t\t/*\n-\t\t * an uninteresting boundary commit may not have its tree\n-\t\t * parsed yet, but we are not going to show them anyway\n-\t\t */\n-\t\tif (commit->tree)\n-\t\t\tadd_pending_tree(revs, commit->tree);\n-\t\tshow_commit(commit, data);\n-\t}\n+\tassert(base->len == 0);\n+\n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *pending = revs->pending.objects + i;\n \t\tstruct object *obj = pending->item;\n@@ -218,17 +208,39 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t\tpath = \"\";\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t &base, path, data);\n+\t\t\t\t base, path, data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\n \t\t oid_to_hex(&obj->oid), name);\n \t}\n \tobject_array_clear(&revs->pending);\n-\tstrbuf_release(&base);\n+}\n+\n+void traverse_commit_list(struct rev_info *revs,\n+\t\t\t show_commit_fn show_commit,\n+\t\t\t show_object_fn show_object,\n+\t\t\t void *data)\n+{\n+\tstruct commit *commit;\n+\tstruct strbuf csp; /* callee's scratch pad */\n+\tstrbuf_init(&csp, PATH_MAX);\n+\n+\twhile ((commit = get_revision(revs)) != NULL) {\n+\t\t/*\n+\t\t * an uninteresting boundary commit may not have its tree\n+\t\t * parsed yet, but we are not going to show them anyway\n+\t\t */\n+\t\tif (commit->tree)\n+\t\t\tadd_pending_tree(revs, commit->tree);\n+\t\tshow_commit(commit, data);\n+\t}\n+\ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n+\n+\tstrbuf_release(&csp);\n }\n-- \n2.15.0.7.g980e40477f\n\n"},{"id":"331693","messageId":"20171102194148.2124-4-sbeller@google.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"[PATCHv3 3/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:44Z","receivedAt":"2017-11-02T19:42:09Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The functionality to list tree objects in the order they were seen\nwhile traversing the commits will be used in the next commit,\nwhere we teach `git describe` to describe not only commits, but\ntrees and blobs, too.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/rev-list-options.txt | 5 ++++\n list-objects.c | 2 ++\n revision.c | 2 ++\n revision.h | 3 ++-\n t/t6100-rev-list-in-order.sh | 47 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 58 insertions(+), 1 deletion(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 13501e1556..9066e0c777 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -686,6 +686,11 @@ ifdef::git-rev-list[]\n \tall object IDs which I need to download if I have the commit\n \tobject _bar_ but not _foo_''.\n \n+--in-commit-order::\n+\tPrint tree and blob ids in order of the commits. The tree\n+\tand blob ids are printed after they are first referenced\n+\tby a commit.\n+\n --objects-edge::\n \tSimilar to `--objects`, but also print the IDs of excluded\n \tcommits prefixed with a ``-'' character. This is used by\ndiff --git a/list-objects.c b/list-objects.c\nindex 7c2ce9c4bd..07a92f35fe 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -239,6 +239,8 @@ void traverse_commit_list(struct rev_info *revs,\n \t\tif (commit->tree)\n \t\t\tadd_pending_tree(revs, commit->tree);\n \t\tshow_commit(commit, data);\n+\t\tif (revs->tree_blobs_in_commit_order)\n+\t\t\ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n \t}\n \ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n \ndiff --git a/revision.c b/revision.c\nindex d167223e69..9329d4ebbf 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1845,6 +1845,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->dense = 0;\n \t} else if (!strcmp(arg, \"--show-all\")) {\n \t\trevs->show_all = 1;\n+\t} else if (!strcmp(arg, \"--in-commit-order\")) {\n+\t\trevs->tree_blobs_in_commit_order = 1;\n \t} else if (!strcmp(arg, \"--remove-empty\")) {\n \t\trevs->remove_empty_trees = 1;\n \t} else if (!strcmp(arg, \"--merges\")) {\ndiff --git a/revision.h b/revision.h\nindex 54761200ad..86985d68aa 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,7 +121,8 @@ struct rev_info {\n \t\t\tbisect:1,\n \t\t\tancestry_path:1,\n \t\t\tfirst_parent_only:1,\n-\t\t\tline_level_traverse:1;\n+\t\t\tline_level_traverse:1,\n+\t\t\ttree_blobs_in_commit_order:1;\n \n \t/* Diff flags */\n \tunsigned int\tdiff:1,\ndiff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh\nnew file mode 100755\nindex 0000000000..d4d539b0da\n--- /dev/null\n+++ b/t/t6100-rev-list-in-order.sh\n@@ -0,0 +1,47 @@\n+#!/bin/sh\n+\n+test_description='rev-list testing in-commit-order'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'rev-list --in-commit-order' '\n+\tfor x in one two three four\n+\tdo\n+\t\techo $x >$x &&\n+\t\tgit add $x &&\n+\t\tgit commit -m \"add file $x\" ||\n+\t\treturn 1\n+\tdone &&\n+\tfor x in four three\n+\tdo\n+\t\tgit rm $x &&\n+\t\tgit commit -m \"remove $x\" ||\n+\t\treturn 1\n+\tdone &&\n+\tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n+\tcut -c 1-40 >actual expect.raw <<-\\EOF &&\n+\t\tHEAD^{commit}\n+\t\tHEAD^{tree}\n+\t\tHEAD^{tree}:one\n+\t\tHEAD^{tree}:two\n+\t\tHEAD~1^{commit}\n+\t\tHEAD~1^{tree}\n+\t\tHEAD~1^{tree}:three\n+\t\tHEAD~2^{commit}\n+\t\tHEAD~2^{tree}\n+\t\tHEAD~2^{tree}:four\n+\t\tHEAD~3^{commit}\n+\t\t# HEAD~3^{tree} skipped, same as HEAD~1^{tree}\n+\t\tHEAD~4^{commit}\n+\t\t# HEAD~4^{tree} skipped, same as HEAD^{tree}\n+\t\tHEAD~5^{commit}\n+\t\tHEAD~5^{tree}\n+\tEOF\n+\tgrep -v \"#\" >expect \n---\n builtin/describe.c | 63 ++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 37 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 3136efde31..9e9a5ed5d4 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -256,7 +256,7 @@ static unsigned long finish_depth_computation(\n \treturn seen_commits;\n }\n \n-static void display_name(struct commit_name *n)\n+static void append_name(struct commit_name *n, struct strbuf *dst)\n {\n \tif (n->prio == 2 && !n->tag) {\n \t\tn->tag = lookup_tag(&n->oid);\n@@ -272,19 +272,18 @@ static void display_name(struct commit_name *n)\n \t}\n \n \tif (n->tag)\n-\t\tprintf(\"%s\", n->tag->tag);\n+\t\tstrbuf_addstr(dst, n->tag->tag);\n \telse\n-\t\tprintf(\"%s\", n->path);\n+\t\tstrbuf_addstr(dst, n->path);\n }\n \n-static void show_suffix(int depth, const struct object_id *oid)\n+static void append_suffix(int depth, const struct object_id *oid, struct strbuf *dst)\n {\n-\tprintf(\"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n+\tstrbuf_addf(dst, \"-%d-g%s\", depth, find_unique_abbrev(oid->hash, abbrev));\n }\n \n-static void describe(const char *arg, int last_one)\n+static void describe_commit(struct object_id *oid, struct strbuf *dst)\n {\n-\tstruct object_id oid;\n \tstruct commit *cmit, *gave_up_on = NULL;\n \tstruct commit_list *list;\n \tstruct commit_name *n;\n@@ -293,26 +292,18 @@ static void describe(const char *arg, int last_one)\n \tunsigned long seen_commits = 0;\n \tunsigned int unannotated_cnt = 0;\n \n-\tif (debug)\n-\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n-\n-\tif (get_oid(arg, &oid))\n-\t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference(oid);\n \n \tn = find_commit_name(&cmit->object.oid);\n \tif (n && (tags || all || n->prio == 2)) {\n \t\t/*\n \t\t * Exact match to an existing ref.\n \t\t */\n-\t\tdisplay_name(n);\n+\t\tappend_name(n, dst);\n \t\tif (longformat)\n-\t\t\tshow_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);\n+\t\t\tappend_suffix(0, n->tag ? &n->tag->tagged->oid : oid, dst);\n \t\tif (suffix)\n-\t\t\tprintf(\"%s\", suffix);\n-\t\tprintf(\"\\n\");\n+\t\t\tstrbuf_addstr(dst, suffix);\n \t\treturn;\n \t}\n \n@@ -386,10 +377,9 @@ static void describe(const char *arg, int last_one)\n \tif (!match_cnt) {\n \t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n+\t\t\tstrbuf_addstr(dst, find_unique_abbrev(cmit_oid->hash, abbrev));\n \t\t\tif (suffix)\n-\t\t\t\tprintf(\"%s\", suffix);\n-\t\t\tprintf(\"\\n\");\n+\t\t\t\tstrbuf_addstr(dst, suffix);\n \t\t\treturn;\n \t\t}\n \t\tif (unannotated_cnt)\n@@ -437,15 +427,36 @@ static void describe(const char *arg, int last_one)\n \t\t}\n \t}\n \n-\tdisplay_name(all_matches[0].name);\n+\tappend_name(all_matches[0].name, dst);\n \tif (abbrev)\n-\t\tshow_suffix(all_matches[0].depth, &cmit->object.oid);\n+\t\tappend_suffix(all_matches[0].depth, &cmit->object.oid, dst);\n \tif (suffix)\n-\t\tprintf(\"%s\", suffix);\n-\tprintf(\"\\n\");\n+\t\tstrbuf_addstr(dst, suffix);\n+}\n+\n+static void describe(const char *arg, int last_one)\n+{\n+\tstruct object_id oid;\n+\tstruct commit *cmit;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (debug)\n+\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n+\n+\tif (get_oid(arg, &oid))\n+\t\tdie(_(\"Not a valid object name %s\"), arg);\n+\tcmit = lookup_commit_reference(&oid);\n+\tif (!cmit)\n+\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\n+\tdescribe_commit(&oid, &sb);\n+\n+\tputs(sb.buf);\n \n \tif (!last_one)\n \t\tclear_commit_marks(cmit, -1);\n+\n+\tstrbuf_release(&sb);\n }\n \n int cmd_describe(int argc, const char **argv, const char *prefix)\n-- \n2.15.0.7.g980e40477f\n\n"},{"id":"331695","messageId":"20171102194148.2124-6-sbeller@google.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"[PATCHv3 5/7] builtin/describe.c: print debug statements earlier","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:46Z","receivedAt":"2017-11-02T19:42:15Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"For debuggers aid we'd want to print debug statements early, so\nintroduce a new line in the debug output that describes the whole\nfunction, and then change the next debug output to describe why we\nneed to search. Conveniently drop the arg from the second line;\nwhich will be useful in a follow up commit, that refactors the\\\ndescribe function.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex fd61f463cf..3136efde31 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)\n \tunsigned long seen_commits = 0;\n \tunsigned int unannotated_cnt = 0;\n \n+\tif (debug)\n+\t\tfprintf(stderr, _(\"describe %s\\n\"), arg);\n+\n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n \tcmit = lookup_commit_reference(&oid);\n@@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)\n \tif (!max_candidates)\n \t\tdie(_(\"no tag exactly matches '%s'\"), oid_to_hex(&cmit->object.oid));\n \tif (debug)\n-\t\tfprintf(stderr, _(\"searching to describe %s\\n\"), arg);\n+\t\tfprintf(stderr, _(\"No exact match on refs or tags, searching to describe\\n\"));\n \n \tif (!have_util) {\n \t\tstruct hashmap_iter iter;\n-- \n2.15.0.7.g980e40477f\n\n"},{"id":"331696","messageId":"20171102194148.2124-8-sbeller@google.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"[PATCHv3 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:48Z","receivedAt":"2017-11-02T19:42:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Sometimes users are given a hash of an object and they want to\nidentify it further (ex.: Use verify-pack to find the largest blobs,\nbut what are these? or [1])\n\n\"This is an interesting endeavor, because describing things is hard.\"\n -- me, upon writing this patch.\n\nWhen describing commits, we try to anchor them to tags or refs, as these\nare conceptually on a higher level than the commit. And if there is no ref\nor tag that matches exactly, we're out of luck. So we employ a heuristic\nto make up a name for the commit. These names are ambiguous, there might\nbe different tags or refs to anchor to, and there might be different\npath in the DAG to travel to arrive at the commit precisely.\n\nWhen describing a blob, we want to describe the blob from a higher layer\nas well, which is a tuple of (commit, deep/path) as the tree objects\ninvolved are rather uninteresting. The same blob can be referenced by\nmultiple commits, so how we decide which commit to use? This patch\nimplements a rather naive approach on this: As there are no back pointers\nfrom blobs to commits in which the blob occurs, we'll start walking from\nany tips available, listing the blobs in-order of the commit and once we\nfound the blob, we'll take the first commit that listed the blob. For\nsource code this is likely not the first commit that introduced the blob,\nbut rather the latest commit that contained the blob. For example:\n\n git describe v0.99:Makefile\n v0.99-5-gab6625e06a:Makefile\n\ntells us the latest commit that contained the Makefile as it was in tag\nv0.99 is commit v0.99-5-gab6625e06a (and at the same path), as the next\ncommit on top v0.99-6-gb1de9de2b9 ([PATCH] Bootstrap \"make dist\",\n2005-07-11) touches the Makefile.\n\nLet's see how this description turns out, if it is useful in day-to-day\nuse as I have the intuition that we'd rather want to see the *first*\ncommit that this blob was introduced to the repository (which can be\nachieved easily by giving the `--reverse` flag in the describe_blob rev\nwalk).\n\n[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-describe.txt | 13 ++++++++-\n builtin/describe.c | 65 ++++++++++++++++++++++++++++++++++++++----\n t/t6120-describe.sh | 15 ++++++++++\n 3 files changed, 87 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex c924c945ba..79ec0be62a 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -3,7 +3,7 @@ git-describe(1)\n \n NAME\n ----\n-git-describe - Describe a commit using the most recent tag reachable from it\n+git-describe - Describe a commit or blob using the graph relations\n \n \n SYNOPSIS\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n 'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]\n+'git describe' [] \n \n DESCRIPTION\n -----------\n@@ -24,6 +25,16 @@ By default (without --all or --tags) `git describe` only shows\n annotated tags. For more information about creating annotated tags\n see the -a and -s options to linkgit:git-tag[1].\n \n+If the given object refers to a blob, it will be described\n+as `:`, such that the blob can be found\n+at `` in the ``. Note, that the commit is likely\n+not the commit that introduced the blob, but the one that was found\n+first; to find the commit that introduced the blob, you need to find\n+the commit that last touched the path, e.g.\n+`git log -- `.\n+As blobs do not point at the commits they are contained in,\n+describing blobs is slow as we have to walk the whole graph.\n+\n OPTIONS\n -------\n ...::\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 9e9a5ed5d4..cf08bef344 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -3,6 +3,7 @@\n #include \"lockfile.h\"\n #include \"commit.h\"\n #include \"tag.h\"\n+#include \"blob.h\"\n #include \"refs.h\"\n #include \"builtin.h\"\n #include \"exec_cmd.h\"\n@@ -11,8 +12,9 @@\n #include \"hashmap.h\"\n #include \"argv-array.h\"\n #include \"run-command.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n \n-#define SEEN\t\t(1u << 0)\n #define MAX_TAGS\t(FLAG_BITS - 1)\n \n static const char * const describe_usage[] = {\n@@ -434,6 +436,56 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)\n \t\tstrbuf_addstr(dst, suffix);\n }\n \n+struct process_commit_data {\n+\tstruct object_id current_commit;\n+\tstruct object_id looking_for;\n+\tstruct strbuf *dst;\n+\tstruct rev_info *revs;\n+};\n+\n+static void process_commit(struct commit *commit, void *data)\n+{\n+\tstruct process_commit_data *pcd = data;\n+\tpcd->current_commit = commit->object.oid;\n+}\n+\n+static void process_object(struct object *obj, const char *path, void *data)\n+{\n+\tstruct process_commit_data *pcd = data;\n+\n+\tif (!oidcmp(&pcd->looking_for, &obj->oid) && !pcd->dst->len) {\n+\t\treset_revision_walk();\n+\t\tdescribe_commit(&pcd->current_commit, pcd->dst);\n+\t\tstrbuf_addf(pcd->dst, \":%s\", path);\n+\t\tpcd->revs->max_count = 0;\n+\t}\n+}\n+\n+static void describe_blob(struct object_id oid, struct strbuf *dst)\n+{\n+\tstruct rev_info revs;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct process_commit_data pcd = { null_oid, oid, dst, &revs};\n+\n+\targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n+\t\t\"--all\", \"--reflog\", /* as many starting points as possible */\n+\t\t/* NEEDSWORK: --all is incompatible with worktrees for now: */\n+\t\t\"--single-worktree\",\n+\t\t\"--objects\",\n+\t\t\"--in-commit-order\",\n+\t\tNULL);\n+\n+\tinit_revisions(&revs, NULL);\n+\tif (setup_revisions(args.argc, args.argv, &revs, NULL) > 1)\n+\t\tBUG(\"setup_revisions could not handle all args?\");\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\n+\ttraverse_commit_list(&revs, process_commit, process_object, &pcd);\n+\treset_revision_walk();\n+}\n+\n static void describe(const char *arg, int last_one)\n {\n \tstruct object_id oid;\n@@ -445,11 +497,14 @@ static void describe(const char *arg, int last_one)\n \n \tif (get_oid(arg, &oid))\n \t\tdie(_(\"Not a valid object name %s\"), arg);\n-\tcmit = lookup_commit_reference(&oid);\n-\tif (!cmit)\n-\t\tdie(_(\"%s is not a valid '%s' object\"), arg, commit_type);\n+\tcmit = lookup_commit_reference_gently(&oid, 1);\n \n-\tdescribe_commit(&oid, &sb);\n+\tif (cmit)\n+\t\tdescribe_commit(&oid, &sb);\n+\telse if (lookup_blob(&oid))\n+\t\tdescribe_blob(oid, &sb);\n+\telse\n+\t\tdie(_(\"%s is neither a commit nor blob\"), arg);\n \n \tputs(sb.buf);\n \ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex c8b7ed82d9..aec6ed192d 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -310,6 +310,21 @@ test_expect_success 'describe ignoring a broken submodule' '\n \tgrep broken out\n '\n \n+test_expect_success 'describe a blob at a tag' '\n+\techo \"make it a unique blob\" >file &&\n+\tgit add file && git commit -m \"content in file\" &&\n+\tgit tag -a -m \"latest annotated tag\" unique-file &&\n+\tgit describe HEAD:file >actual &&\n+\techo \"unique-file:file\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe a blob with commit-ish' '\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit describe HEAD:file >actual &&\n+\tgrep unique-file-1-g actual\n+'\n+\n test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n \ti=1 &&\n \twhile test $i -lt 8000\n-- \n2.15.0.7.g980e40477f\n\n"},{"id":"331697","messageId":"20171102194148.2124-5-sbeller@google.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"[PATCHv3 4/7] builtin/describe.c: rename `oid` to avoid variable shadowing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-02T19:41:45Z","receivedAt":"2017-11-02T19:42:20Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The function `describe` has already a variable named `oid` declared at\nthe beginning of the function for an object id. Do not shadow that\nvariable with a pointer to an object id.\n\nSigned-off-by: Stefan Beller \n---\n builtin/describe.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 29075dbd0f..fd61f463cf 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -381,9 +381,9 @@ static void describe(const char *arg, int last_one)\n \t}\n \n \tif (!match_cnt) {\n-\t\tstruct object_id *oid = &cmit->object.oid;\n+\t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tprintf(\"%s\", find_unique_abbrev(oid->hash, abbrev));\n+\t\t\tprintf(\"%s\", find_unique_abbrev(cmit_oid->hash, abbrev));\n \t\t\tif (suffix)\n \t\t\t\tprintf(\"%s\", suffix);\n \t\t\tprintf(\"\\n\");\n@@ -392,11 +392,11 @@ static void describe(const char *arg, int last_one)\n \t\tif (unannotated_cnt)\n \t\t\tdie(_(\"No annotated tags can describe '%s'.\\n\"\n \t\t\t \"However, there were unannotated tags: try --tags.\"),\n-\t\t\t oid_to_hex(oid));\n+\t\t\t oid_to_hex(cmit_oid));\n \t\telse\n \t\t\tdie(_(\"No tags can describe '%s'.\\n\"\n \t\t\t \"Try --always, or create some tags.\"),\n-\t\t\t oid_to_hex(oid));\n+\t\t\t oid_to_hex(cmit_oid));\n \t}\n \n \tQSORT(all_matches, match_cnt, compare_pt);\n-- \n2.15.0.7.g980e40477f\n\n"},{"id":"331751","messageId":"CA+P7+xr0owJt4qvWeNkWhEionJ4468h0pTdB+h-FrEaBqi375w@mail.gmail.com","threadId":"47050","inReplyTo":"20171102194148.2124-1-sbeller@google.com","subject":"Re: [PATCHv3 0/7] git describe blob","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-11-03T00:23:51Z","receivedAt":"2017-11-03T00:24:21Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Nov 2, 2017 at 12:41 PM, Stefan Beller wrote:\n> Thanks for the discussion on v2[1].\n>\n> Interdiff is below, just fixing minor things.\n>\n> We'll keep the original algorithm for now, deferring an improvement on\n> the algorithm front towards a future developer.\n>\n\nI agree, \"something\" is better than \"nothing\", and we can work to\nimprove \"something\" in the future, especially after we get more real\nuse and see what people think. Only question would be how much do we\nneed to document the current behavior might be open for improvement?\n\n> Thanks,\n> Stefan\n>\n> [1] https://public-inbox.org/git/20171031211852.13001-1-sbeller@google.com/\n>\n> Stefan Beller (7):\n> t6120: fix typo in test name\n> list-objects.c: factor out traverse_trees_and_blobs\n> revision.h: introduce blob/tree walking in order of the commits\n> builtin/describe.c: rename `oid` to avoid variable shadowing\n> builtin/describe.c: print debug statements earlier\n> builtin/describe.c: factor out describe_commit\n> builtin/describe.c: describe a blob\n>\n> Documentation/git-describe.txt | 13 +++-\n> Documentation/rev-list-options.txt | 5 ++\n> builtin/describe.c | 125 ++++++++++++++++++++++++++++---------\n> list-objects.c | 52 +++++++++------\n> revision.c | 2 +\n> revision.h | 3 +-\n> t/t6100-rev-list-in-order.sh | 47 ++++++++++++++\n> t/t6120-describe.sh | 17 ++++-\n> 8 files changed, 214 insertions(+), 50 deletions(-)\n> create mode 100755 t/t6100-rev-list-in-order.sh\n> diff --git c/Documentation/git-describe.txt w/Documentation/git-describe.txt\n> index 077c3c2193..79ec0be62a 100644\n> --- c/Documentation/git-describe.txt\n> +++ w/Documentation/git-describe.txt\n> @@ -11,7 +11,7 @@ SYNOPSIS\n> [verse]\n> 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]\n> 'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]\n> -'git describe' [] \n> +'git describe' [] \n>\n> DESCRIPTION\n> -----------\n> diff --git c/builtin/describe.c w/builtin/describe.c\n> index 382cbae908..cf08bef344 100644\n> --- c/builtin/describe.c\n> +++ w/builtin/describe.c\n> @@ -440,6 +440,7 @@ struct process_commit_data {\n> struct object_id current_commit;\n> struct object_id looking_for;\n> struct strbuf *dst;\n> + struct rev_info *revs;\n> };\n>\n> static void process_commit(struct commit *commit, void *data)\n> @@ -456,6 +457,7 @@ static void process_object(struct object *obj, const char *path, void *data)\n> reset_revision_walk();\n> describe_commit(&pcd->current_commit, pcd->dst);\n> strbuf_addf(pcd->dst, \":%s\", path);\n> + pcd->revs->max_count = 0;\n> }\n> }\n>\n> @@ -463,7 +465,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)\n> {\n> struct rev_info revs;\n> struct argv_array args = ARGV_ARRAY_INIT;\n> - struct process_commit_data pcd = { null_oid, oid, dst};\n> + struct process_commit_data pcd = { null_oid, oid, dst, &revs};\n>\n> argv_array_pushl(&args, \"internal: The first arg is not parsed\",\n> \"--all\", \"--reflog\", /* as many starting points as possible */\n> @@ -497,14 +499,12 @@ static void describe(const char *arg, int last_one)\n> die(_(\"Not a valid object name %s\"), arg);\n> cmit = lookup_commit_reference_gently(&oid, 1);\n>\n> - if (cmit) {\n> + if (cmit)\n> describe_commit(&oid, &sb);\n> - } else {\n> - if (lookup_blob(&oid))\n> - describe_blob(oid, &sb);\n> - else\n> - die(_(\"%s is neither a commit nor blob\"), arg);\n> - }\n> + else if (lookup_blob(&oid))\n> + describe_blob(oid, &sb);\n> + else\n> + die(_(\"%s is neither a commit nor blob\"), arg);\n>\n> puts(sb.buf);\n>\n> diff --git c/list-objects.c w/list-objects.c\n> index 03438e5a8b..07a92f35fe 100644\n> --- c/list-objects.c\n> +++ w/list-objects.c\n> @@ -184,13 +184,13 @@ static void add_pending_tree(struct rev_info *revs, struct tree *tree)\n> }\n>\n> static void traverse_trees_and_blobs(struct rev_info *revs,\n> - struct strbuf *base_path,\n> + struct strbuf *base,\n> show_object_fn show_object,\n> void *data)\n> {\n> int i;\n>\n> - assert(base_path->len == 0);\n> + assert(base->len == 0);\n>\n> for (i = 0; i < revs->pending.nr; i++) {\n> struct object_array_entry *pending = revs->pending.objects + i;\n> @@ -208,12 +208,12 @@ static void traverse_trees_and_blobs(struct rev_info *revs,\n> path = \"\";\n> if (obj->type == OBJ_TREE) {\n> process_tree(revs, (struct tree *)obj, show_object,\n> - base_path, path, data);\n> + base, path, data);\n> continue;\n> }\n> if (obj->type == OBJ_BLOB) {\n> process_blob(revs, (struct blob *)obj, show_object,\n> - base_path, path, data);\n> + base, path, data);\n> continue;\n> }\n> die(\"unknown pending object %s (%s)\",\n> diff --git c/t/t6100-rev-list-in-order.sh w/t/t6100-rev-list-in-order.sh\n> index 651666979b..d4d539b0da 100755\n> --- c/t/t6100-rev-list-in-order.sh\n> +++ w/t/t6100-rev-list-in-order.sh\n> @@ -1,21 +1,22 @@\n> #!/bin/sh\n>\n> -test_description='miscellaneous rev-list tests'\n> +test_description='rev-list testing in-commit-order'\n>\n> . ./test-lib.sh\n>\n> -\n> -test_expect_success 'setup' '\n> +test_expect_success 'rev-list --in-commit-order' '\n> for x in one two three four\n> do\n> echo $x >$x &&\n> git add $x &&\n> - git commit -m \"add file $x\"\n> + git commit -m \"add file $x\" ||\n> + return 1\n> done &&\n> for x in four three\n> do\n> git rm $x &&\n> - git commit -m \"remove $x\"\n> + git commit -m \"remove $x\" ||\n> + return 1\n> done &&\n> git rev-list --in-commit-order --objects HEAD >actual.raw &&\n> cut -c 1-40 >actual @@ -32,9 +33,9 @@ test_expect_success 'setup' '\n> HEAD~2^{tree}\n> HEAD~2^{tree}:four\n> HEAD~3^{commit}\n> - # HEAD~3^{tree} skipped\n> + # HEAD~3^{tree} skipped, same as HEAD~1^{tree}\n> HEAD~4^{commit}\n> - # HEAD~4^{tree} skipped\n> + # HEAD~4^{tree} skipped, same as HEAD^{tree}\n> HEAD~5^{commit}\n> HEAD~5^{tree}\n> EOF\n> diff --git c/t/t6120-describe.sh w/t/t6120-describe.sh\n> index fd329f173a..aec6ed192d 100755\n> --- c/t/t6120-describe.sh\n> +++ w/t/t6120-describe.sh\n> @@ -319,7 +319,7 @@ test_expect_success 'describe a blob at a tag' '\n> test_cmp expect actual\n> '\n>\n> -test_expect_success 'describe a surviving blob' '\n> +test_expect_success 'describe a blob with commit-ish' '\n> git commit --allow-empty -m \"empty commit\" &&\n> git describe HEAD:file >actual &&\n> grep unique-file-1-g actual\n"},{"id":"331755","messageId":"xmqq8tfoqgle.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CA+P7+xr0owJt4qvWeNkWhEionJ4468h0pTdB+h-FrEaBqi375w@mail.gmail.com","subject":"Re: [PATCHv3 0/7] git describe blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-03T01:46:37Z","receivedAt":"2017-11-03T01:46:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> I agree, \"something\" is better than \"nothing\", and we can work to\n> improve \"something\" in the future, especially after we get more real\n> use and see what people think. Only question would be how much do we\n> need to document the current behavior might be open for improvement?\n\nIf \n\n - it always digs to the root of the history no matter what; and/or\n - it almost always yields correct but suboptimal result\n\nthen that fact must be documented in BUGS section, possibly a brief\ndescrition of why that limitation is there, with a hint to invite\npeople to look into fixing it.\n\nWe should mark it prominently as experimental and advertise it as\nsuch. \"It's too slow in real project to be usable\" and \"Its output\nbases the answer on an irrelevant commit\" are not something we want\nour users to experience, except for those with inclination to (or\nability and time to) help improve the feature.\n"},{"id":"331759","messageId":"CAGZ79kYhV7pjbCAiJir_CJQNVrBOTA3ZoCRn6Svz80Xi5O3LWA@mail.gmail.com","threadId":"47050","inReplyTo":"xmqq8tfoqgle.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3 0/7] git describe blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-03T02:29:37Z","receivedAt":"2017-11-03T02:29:42Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Nov 2, 2017 at 6:46 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> I agree, \"something\" is better than \"nothing\", and we can work to\n>> improve \"something\" in the future, especially after we get more real\n>> use and see what people think. Only question would be how much do we\n>> need to document the current behavior might be open for improvement?\n>\n> If\n>\n> - it always digs to the root of the history no matter what; and/or\n\nthis is fixed.\n\n> - it almost always yields correct but suboptimal result\n\n\nthis is not, for the lack of knowing what the optimal result is.\n\n>\n> then that fact must be documented in BUGS section, possibly a brief\n> descrition of why that limitation is there, with a hint to invite\n> people to look into fixing it.\n>\n> We should mark it prominently as experimental and advertise it as\n> such. \"It's too slow in real project to be usable\"\n\nI found it quite fast after fixing the history walking, but still.\n\n> and \"Its output\n> bases the answer on an irrelevant commit\" are not something we want\n> our users to experience, except for those with inclination to (or\n> ability and time to) help improve the feature.\n\nI think the current documentation states exactly this.\n\nThanks.\n"},{"id":"331762","messageId":"xmqqtvycos7z.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CA+P7+xqw9zKNUkn9P-qA57ADSB5G_7Sd0JCc6SaK6bBf9-Jhtg@mail.gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-03T05:18:24Z","receivedAt":"2017-11-03T05:18:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> I think both questions are valuable, the first is \"which commit last\n> had this blob\", the second is \"which commit first introduced this\n> blob\", neither of which can always give a definitive answer. It really\n> depends on what question you're asking up front.\n\nGiven that \"describe\" is about giving an object _a_ name that is\nplausibly useful to refer to it, it is not a good match for the\nabove query that wants to know where it came from, how long it\nremains in the tree and where it ceases to be relevant. In order to\nsupport that use case, a totally different and possibly more useful\navenue would be to think how this can be hooked into \"git log\"\nmachinery.\n\nA refresher for how \"git log [--options] \" works may be\nbeneficial. We walk history and compare the tree of the commit we\nare looking at with the tree of its parent commits. If everything\nwithin is the same, we mark the transition between the\nparent and our commit TREESAME (other commits, i.e. the ones that\nhave meaningful change within , are !TREESAME). Then the\noutput routine presents the set of commits that includes commits\nthat are !TREESAME, within the constraints of the --options given\n(e.g. with --full-history, sides of a merge that is TREESAME may\nstill be shown to preserve connectedness of the resulting graph).\n\nIt is easy to imagine that we can restrict \"git log\" traversal with\na \"--blobchange=\" option instead of (or in addition to) the\nlimitation gives us. Instead of treating a commit whose\ndiff against its parent commit has any filepair that is different\nwithin as \"!TREESAME\", we can treat a commit whose diff\nagainst its parent commit has a filepair that has the on\neither side of the filepair as \"!TREESAME\" (in other words, we\nignore a transition that is not about introducing or forgetting the\n we are looking for as an \"interesting change\"). That would\ngive you a commit history graph in which only (and all) such commits\nthat either adds or removes the in it.\n\nHmm?\n"},{"id":"331764","messageId":"CA+P7+xrRgzGLFMZ+iaomQifOUXqANKD0v9XGoR+==0kVmyVL5Q@mail.gmail.com","threadId":"47050","inReplyTo":"xmqqtvycos7z.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-11-03T06:55:32Z","receivedAt":"2017-11-03T06:56:00Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Nov 2, 2017 at 10:18 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>> I think both questions are valuable, the first is \"which commit last\n>> had this blob\", the second is \"which commit first introduced this\n>> blob\", neither of which can always give a definitive answer. It really\n>> depends on what question you're asking up front.\n>\n> Given that \"describe\" is about giving an object _a_ name that is\n> plausibly useful to refer to it, it is not a good match for the\n> above query that wants to know where it came from, how long it\n> remains in the tree and where it ceases to be relevant. In order to\n> support that use case, a totally different and possibly more useful\n> avenue would be to think how this can be hooked into \"git log\"\n> machinery.\n>\n> A refresher for how \"git log [--options] \" works may be\n> beneficial. We walk history and compare the tree of the commit we\n> are looking at with the tree of its parent commits. If everything\n> within is the same, we mark the transition between the\n> parent and our commit TREESAME (other commits, i.e. the ones that\n> have meaningful change within , are !TREESAME). Then the\n> output routine presents the set of commits that includes commits\n> that are !TREESAME, within the constraints of the --options given\n> (e.g. with --full-history, sides of a merge that is TREESAME may\n> still be shown to preserve connectedness of the resulting graph).\n>\n> It is easy to imagine that we can restrict \"git log\" traversal with\n> a \"--blobchange=\" option instead of (or in addition to) the\n> limitation gives us. Instead of treating a commit whose\n> diff against its parent commit has any filepair that is different\n> within as \"!TREESAME\", we can treat a commit whose diff\n> against its parent commit has a filepair that has the on\n> either side of the filepair as \"!TREESAME\" (in other words, we\n> ignore a transition that is not about introducing or forgetting the\n> we are looking for as an \"interesting change\"). That would\n> give you a commit history graph in which only (and all) such commits\n> that either adds or removes the in it.\n>\n> Hmm?\n\nThis seems quite useful in the context of figuring out how a file got\nto such a state. This is useful to me, since if I know the state of a\nfile (ie: it's exact contents) I can determine the blob name, and then\nuse that to lookup where it was introduced.\n\nThanks,\nJake\n"},{"id":"331772","messageId":"alpine.DEB.2.21.1.1711031300010.6482@virtualbox","threadId":"47050","inReplyTo":"CAGZ79kZXLpwCyWs3_mfj7kL1qmM965V7vZZ2UH4-hqrfF5f+8g@mail.gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-03T12:05:27Z","receivedAt":"2017-11-03T12:05:43Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Thu, 2 Nov 2017, Stefan Beller wrote:\n\n> On Thu, Nov 2, 2017 at 12:23 AM, Andreas Schwab wrote:\n> > On Nov 01 2017, Johannes Schindelin wrote:\n> >\n> >> Sure, but it is still a tricky thing. Imagine\n> >>\n> >> - A1 - B1 - A2 - B2 - B3\n> >>\n> >> where all the B* commits have the blob. Do you really want to report B1\n> >> rather than B2 as the commit introducing the blob? (I would prefer B2...)\n> >\n> > What if B3 renames or copies the blob?\n> >\n> > Andreas.\n> \n> With the current proposed patch you'd find B3, and then use the diff machinery\n> to digg deeper from there (renames/copies ought to be easy to detect already?)\n> \n> So with a copy B3 might be a better start than B1, as starting from B1 you\n> would not find B3 easily.\n> \n> For a rename, I would think a reverse log/blame on B1:path may help.\n> \n> With that said, I think I'll just reroll the series with the current logic\n> fixing the other minor issues that were brought up as B3 seems to\n> be the most versatile (though not optimal) answer for many use cases.\n\nI know this is a bit of semantics, but I disagree that B3 is the most\nversatile. For my use cases, `git describe` comes in handy relatively\nrarely, and usually only when I want to know what version some work was\nbased on. In that respect, B2 would be the most versatile answer.\n\nHowever, if you say that B3 is the easiest answer to explain, I\nwhole-heartedly agree. Any other answer would be necessarily more\ncomplicated to reason about, given that we're operating on a DAG, i.e. not\nalways on a linear history.\n\nCiao,\nDscho\n"},{"id":"331783","messageId":"xmqqbmkjpfrh.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"CA+P7+xrRgzGLFMZ+iaomQifOUXqANKD0v9XGoR+==0kVmyVL5Q@mail.gmail.com","subject":"Re: [PATCHv2 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-03T15:02:10Z","receivedAt":"2017-11-03T15:02:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> On Thu, Nov 2, 2017 at 10:18 PM, Junio C Hamano wrote:\n>> ...\n>> It is easy to imagine that we can restrict \"git log\" traversal with\n>> a \"--blobchange=\" option instead of (or in addition to) the\n>> limitation gives us. Instead of treating a commit whose\n>> diff against its parent commit has any filepair that is different\n>> within as \"!TREESAME\", we can treat a commit whose diff\n>> against its parent commit has a filepair that has the on\n>> either side of the filepair as \"!TREESAME\" (in other words, we\n>> ignore a transition that is not about introducing or forgetting the\n>> we are looking for as an \"interesting change\"). That would\n>> give you a commit history graph in which only (and all) such commits\n>> that either adds or removes the in it.\n>>\n>> Hmm?\n>\n> This seems quite useful in the context of figuring out how a file got\n> to such a state. This is useful to me, since if I know the state of a\n> file (ie: it's exact contents) I can determine the blob name, and then\n> use that to lookup where it was introduced.\n\nThis is probably a bit harder than an average #leftoverbits, but if\nsomebody wants to get their hands dirty, it should be reasonably\nstraightforward. The needed changes would roughly go like so:\n\n - Add \"struct oid *blob_change;\" to diff_options, initialized to\n NULL.\n\n - Teach diff_opt_parse() a new option \"--blobchange=\".\n Allocate a struct oid when you parse this option and point at it\n with the blob_change field above.\n\n - Write diffcore-blobchange.c, modeled after diffcore-pickaxe.c,\n but this should be a lot simpler (as there is no need for an\n equivalent for \"pickaxe-all\" option). It would\n\n - prepare an empty diff_queue_struct \"outq\";\n - iterate over the given diff_queue \"q\", and \n - a filepair p whose p->one is blob_change and p->two is not,\n (or the other way around) is added to outq with diff_q()\n - a filepair whose p->one/p->two do not involve blob_change\n is freed with diff_free_filepair().\n - replace \"q\" with \"outq\".\n\n - Add a call to diffcore_blobchange() early in diffcore_std(),\n probably immediately after skip-stat-unmatch, when blob_change\n field is not NULL.\n\n - Arrange that blob_change is propagated to revs->pruning in\n setup_revisions().\n\n"},{"id":"331831","messageId":"BFE987312194406AAAEA8DAC7EC5BFF5@PhilipOakley","threadId":"47050","inReplyTo":"xmqq375xs3zr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-11-04T21:15:33Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \nSent: Thursday, November 02, 2017 4:23 AM\n> Junio C Hamano writes:\n>\n>> The reason why we say \"-ish\" is \"Yes we know v2.15.0 is *NOT* a\n>> commit object, we very well know it is a tag object, but because we\n>> allow it to be used in a context that calls for a commit object, we\n>> mark that use context as 'this accepts commit-ish, not just\n>> commit'\".\n>\n> Having said all that, there is a valid case in which we might want\n> to say \"blob-ish\".\n\nIs this not also an alternative case, relative to the user, for the scenario \nwhere the user has an oid/sha1 value but does not know what it is, and would \nlike to find its source and type relative to the `describe` command.\n\nIIUC the existing `describe` command only accepts values, and \nhere we are extending that to be even more inclusive, but at the same time \nthe options become more restricted. Thus the synopsis terminology would be \nmore about suggesting the range of options available (search style/start \npoints) that are applicable to blobs, than being exactly about the \n'allow-blobs' parameter.\n\nOr have I misunderstood how the fast commit search and the slower \npotentially-a-blob searching are disambiguated?\n--\nPhilip\n\n>\n> To review, X-ish is the word we use when the command wants to take\n> an X, but tolerates a lazy user who gives a Y, which is *NOT* X,\n> without bothering to add ^{X} suffix, i.e. Y^{X}. In such a case,\n> the command takes not just X but takes X-ish because it takes a Y\n> and converts it internally to an X to be extra nice.\n>\n> When the command wants to take a blob, but tolerates something else\n> and does \"^{blob}\" internally, we can say it takes \"blob-ish\".\n> Technically that \"something else\" could be an annotated tag that\n> points at a blob object, without any intervening commit or tree (I\n> did not check if the \"describe \" code in this thread handles\n> this, though).\n>\n> But because it is not usually done to tag a blob directly, it would\n> probably be not worth to say \"blob-ish\" in the document and cause\n> readers to wonder in what situation something that is not a blob can\n> be treated as if it were a blob. It does feel like we would be\n> pursuing technical correctness too much and sacrificing the readability\n> of the document, at least to me, and a bad trade-off.\n>\n> \n\n"},{"id":"331843","messageId":"xmqqlgjlkzmm.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"BFE987312194406AAAEA8DAC7EC5BFF5@PhilipOakley","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-05T06:28:49Z","receivedAt":"2017-11-05T06:28:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> Is this not also an alternative case, relative to the user, for the\n> scenario where the user has an oid/sha1 value but does not know what\n> it is, and would like to find its source and type relative to the\n> `describe` command.\n\nI am not sure what you wanted to say with \"source and type RELATIVE TO\nthe describe command\".\n\nThe first thing the combination of the user and the describe command\nwould do when the user has a 40-hex string would be to do the\nequivalent of \"cat-file -t\" to learn if it even exists and what its\ntype is. With Stefan's patch, that is what describe command does in\norder to choose quite a different codeflow from the traditional mode\nwhen it learns that it was given a blob.\n\n> IIUC the existing `describe` command only accepts values,\n> and here we are extending that to be even more inclusive, but at the\n> same time the options become more restricted.\n\nDo you mean that the command should check if it was given an option\nthat would not be applicable to the \"find a commit that has the\nblob\" mode, once it learns that it was given a blob and needs to go\nin that codepath? I think that would make sense.\n\n> Or have I misunderstood how the fast commit search and the slower\n> potentially-a-blob searching are disambiguated?\n\nI do not think so. We used to barf when we got anything but\ncommit-ish, but Stefan's new code kicks in if the object turns out\nto be a blob---I think that is what you mean by the disambiguation.\n"},{"id":"331950","messageId":"D199FB1260C4462ABFC1F0F77D26EF06@PhilipOakley","threadId":"47050","inReplyTo":"xmqqlgjlkzmm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-11-06T23:50:13Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \nSent: Sunday, November 05, 2017 6:28 AM\n> \"Philip Oakley\" writes:\n>\n>> Is this not also an alternative case, relative to the user, for the\n>> scenario where the user has an oid/sha1 value but does not know what\n>> it is, and would like to find its source and type relative to the\n>> `describe` command.\n>\n> I am not sure what you wanted to say with \"source and type RELATIVE TO\n> the describe command\".\n\nThe 'relative to' was meaning the user's expectation about this particular \ncommand. For a non-expert user, who may not have come across cat-file yet, \ntheir world view may not extend beyond 'Git describe ' for me.\n\n>\n> The first thing the combination of the user and the describe command\n> would do when the user has a 40-hex string would be to do the\n> equivalent of \"cat-file -t\" to learn if it even exists and what its\n> type is. With Stefan's patch, that is what describe command does in\n> order to choose quite a different codeflow from the traditional mode\n> when it learns that it was given a blob.\n\nI realised, after sending, that this was probably the method for \nnon-ambiguous shortened oid's. Thanks for the reminder.\n>\n>> IIUC the existing `describe` command only accepts values,\n>> and here we are extending that to be even more inclusive, but at the\n>> same time the options become more restricted.\n>\n> Do you mean that the command should check if it was given an option\n> that would not be applicable to the \"find a commit that has the\n> blob\" mode, once it learns that it was given a blob and needs to go\n> in that codepath? I think that would make sense.\n\nCorrect, it was the option selection aspect.\n>\n>> Or have I misunderstood how the fast commit search and the slower\n>> potentially-a-blob searching are disambiguated?\n>\n> I do not think so. We used to barf when we got anything but\n> commit-ish, but Stefan's new code kicks in if the object turns out\n> to be a blob---I think that is what you mean by the disambiguation.\n\nCorrect. We ask to describe an object, but then the option choices may vary \nby type.\n\nThe new [blob] synopys only lists , while the old [commit-ish] \nshows specifics. It wasn't clear if the options are the same for both. I \nquess they are the same once the cat-file -t has done its bit. Its only the \nspeed that's affected.\n\nAs a side note, the commit message example don't show any pathspec that is \nnot in the top level directory.\n\n--\nPhilip \n\n"},{"id":"332133","messageId":"CAGZ79kYRunzS9vDO=Zk1jQk1BvwJomN5+=1gQi0aDau_qbamew@mail.gmail.com","threadId":"47050","inReplyTo":"D199FB1260C4462ABFC1F0F77D26EF06@PhilipOakley","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-09T20:30:36Z","receivedAt":"2017-11-09T20:30:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Rereading this discussion, there is currently no urgent thing to address?\nThen the state as announced by the last cooking email, to just cook it, seems\nabout right and we'll wait for further feedback.\n\nThanks,\nStefan\n"},{"id":"332137","messageId":"54073F7B0C9E49C4BA55CA97A605A724@PhilipOakley","threadId":"47050","inReplyTo":"CAGZ79kYRunzS9vDO=Zk1jQk1BvwJomN5+=1gQi0aDau_qbamew@mail.gmail.com","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-11-10T00:25:36Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Stefan Beller\" \n> Rereading this discussion, there is currently no urgent thing to address?\n\nTrue.\n\n> Then the state as announced by the last cooking email, to just cook it, \n> seems\n> about right and we'll wait for further feedback.\n\nPossibly only checking the documenation aspects, so folks don't fall into \nthe same trap as me.. ;-)\n--\nPhilip \n\n"},{"id":"332144","messageId":"xmqqo9obhqo0.fsf@gitster.mtv.corp.google.com","threadId":"47050","inReplyTo":"54073F7B0C9E49C4BA55CA97A605A724@PhilipOakley","subject":"Re: [PATCH 6/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-10T01:24:15Z","receivedAt":"2017-11-10T01:24:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> From: \"Stefan Beller\" \n>> Rereading this discussion, there is currently no urgent thing to address?\n>\n> True.\n>\n>> Then the state as announced by the last cooking email, to just cook\n>> it, seems\n>> about right and we'll wait for further feedback.\n\nA shiny new toy that is not a fix for a grave bug is rarely urgent,\nso with that criterion, we'd end up with hundreds of topics not in\n'next' but in 'pu' waiting for the original contributor to get out\nof his or her procrastination, which certainly is not what I want to\nsee, as I'd have to throw them into the Stalled bin and then\neventually discard them, while having to worry about possible\nmismerges with remaining good topics caused by these topics\nappearing and disappearing from 'pu'.\n\nI'd rather see any topic that consumed reviewers' time to be\npolished enough to get into 'next' while we all recall the issues\nraised during previous reviews. I consider the process to further\nincrementally polish it after that happens a true \"cooking\".\n\nFor this topic, aside from \"known issues\" that we decided to punt\nfor now, my impression was that the code is in good enough shape,\nand we need a bit of documentation polishes before I can mark it\nas \"Will merge to 'next'\".\n\n> Possibly only checking the documenation aspects, so folks don't fall\n> into the same trap as me.. ;-)\n\nYup, so let's resolve that documentation thing while we remember\nthat the topic has that issue, and what part of the documentation\nwe find needs improvement.\n\nI am not sure what \"trap: you fell into, though. Are you saying\nthat giving\n\n\tgit describe [