{"thread":{"id":"47209","subject":"[PATCHv4 1/7] t6120: fix typo in test name","startedAt":"2017-11-15T00:31:12Z","lastAt":"2017-11-17T01:07:03Z","messageCount":17,"participants":["Stefan Beller","Jonathan Tan","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"332580","messageId":"20171115003043.24080-2-sbeller@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-1-sbeller@google.com","subject":"[PATCHv4 1/7] t6120: fix typo in test name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:37Z","receivedAt":"2017-11-15T00:31:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \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.128.gcadd42da22\n\n"},{"id":"332581","messageId":"20171115003043.24080-1-sbeller@google.com","threadId":"47209","inReplyTo":null,"subject":"[PATCHv4 0/7] describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:36Z","receivedAt":"2017-11-15T00:31:16Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"* omitted the check for options, none of them made sense.\n* use of --reverse\n* an additional test asked for by Jonathan\n\nprevious discussion\nhttps://public-inbox.org/git/20171102194148.2124-1-sbeller@google.com/\ninterdiff to current queued patches below.\n\nThanks,\nStefan\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 | 126 ++++++++++++++++++++++++++++---------\n list-objects.c | 52 +++++++++------\n revision.c | 2 +\n revision.h | 3 +-\n t/t6100-rev-list-in-order.sh | 77 +++++++++++++++++++++++\n t/t6120-describe.sh | 21 ++++++-\n 8 files changed, 249 insertions(+), 50 deletions(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\n-- \n2.15.0.128.gcadd42da22\n\ndiff --git c/builtin/describe.c w/builtin/describe.c\nindex acfd853a30..a2a5fdc48d 100644\n--- c/builtin/describe.c\n+++ w/builtin/describe.c\n@@ -473,6 +473,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)\n \t\t\"--single-worktree\",\n \t\t\"--objects\",\n \t\t\"--in-commit-order\",\n+\t\t\"--reverse\",\n \t\tNULL);\n \n \tinit_revisions(&revs, NULL);\n@@ -501,13 +502,9 @@ static void describe(const char *arg, int last_one)\n \n \tif (cmit)\n \t\tdescribe_commit(&oid, &sb);\n-\telse if (lookup_blob(&oid)) {\n-\t\tif (all || tags || longformat || first_parent ||\n-\t\t patterns.nr || exclude_patterns.nr ||\n-\t\t always || dirty || broken)\n-\t\t\tdie(_(\"options not available for describing blobs\"));\n+\telse if (lookup_blob(&oid))\n \t\tdescribe_blob(oid, &sb);\n-\t} else\n+\telse\n \t\tdie(_(\"%s is neither a commit nor blob\"), arg);\n \n \tputs(sb.buf);\ndiff --git c/t/t6100-rev-list-in-order.sh w/t/t6100-rev-list-in-order.sh\nindex d4d539b0da..b2bb0a7f61 100755\n--- c/t/t6100-rev-list-in-order.sh\n+++ w/t/t6100-rev-list-in-order.sh\n@@ -4,7 +4,7 @@ test_description='rev-list testing in-commit-order'\n \n . ./test-lib.sh\n \n-test_expect_success 'rev-list --in-commit-order' '\n+test_expect_success 'setup a commit history with trees, blobs' '\n \tfor x in one two three four\n \tdo\n \t\techo $x >$x &&\n@@ -17,7 +17,10 @@ test_expect_success 'rev-list --in-commit-order' '\n \t\tgit rm $x &&\n \t\tgit commit -m \"remove $x\" ||\n \t\treturn 1\n-\tdone &&\n+\tdone\n+'\n+\n+test_expect_success 'rev-list --in-commit-order' '\n \tgit rev-list --in-commit-order --objects HEAD >actual.raw &&\n \tcut -c 1-40 >actual actual.raw &&\n+\tcut -c 1-40 >actual expect.raw <<-\\EOF &&\n+\t\tHEAD^{commit}\n+\t\tHEAD~1^{commit}\n+\t\tHEAD~2^{commit}\n+\t\tHEAD~3^{commit}\n+\t\tHEAD~4^{commit}\n+\t\tHEAD~5^{commit}\n+\t\tHEAD^{tree}\n+\t\tHEAD^{tree}:one\n+\t\tHEAD^{tree}:two\n+\t\tHEAD~1^{tree}\n+\t\tHEAD~1^{tree}:three\n+\t\tHEAD~2^{tree}\n+\t\tHEAD~2^{tree}:four\n+\t\t# HEAD~3^{tree} skipped, same as HEAD~1^{tree}\n+\t\t# HEAD~4^{tree} skipped, same as HEAD^{tree}\n+\t\tHEAD~5^{tree}\n+\tEOF\n+\tgrep -v \"#\" >expect actual &&\n-\tgrep unique-file-1-g actual\n+\tgrep unique-file-3-g actual\n '\n \n test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n"},{"id":"332582","messageId":"20171115003043.24080-4-sbeller@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-1-sbeller@google.com","subject":"[PATCHv4 3/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:39Z","receivedAt":"2017-11-15T00:31:18Z","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 one of the next commits,\nwhere we teach `git describe` to describe not only commits, but blobs, too.\n\nThe change in list-objects.c is rather minimal as we'll be re-using\nthe infrastructure put in place of the revision walking machinery. For\nexample one could expect that add_pending_tree is not called, but rather\ncommit->tree is directly passed to the tree traversal function. This\nhowever requires a lot more code than just emptying the queue containing\ntrees after each commit.\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 | 77 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 88 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..b2bb0a7f61\n--- /dev/null\n+++ b/t/t6100-rev-list-in-order.sh\n@@ -0,0 +1,77 @@\n+#!/bin/sh\n+\n+test_description='rev-list testing in-commit-order'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup a commit history with trees, blobs' '\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+'\n+\n+test_expect_success 'rev-list --in-commit-order' '\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 actual.raw &&\n+\tcut -c 1-40 >actual expect.raw <<-\\EOF &&\n+\t\tHEAD^{commit}\n+\t\tHEAD~1^{commit}\n+\t\tHEAD~2^{commit}\n+\t\tHEAD~3^{commit}\n+\t\tHEAD~4^{commit}\n+\t\tHEAD~5^{commit}\n+\t\tHEAD^{tree}\n+\t\tHEAD^{tree}:one\n+\t\tHEAD^{tree}:two\n+\t\tHEAD~1^{tree}\n+\t\tHEAD~1^{tree}:three\n+\t\tHEAD~2^{tree}\n+\t\tHEAD~2^{tree}:four\n+\t\t# HEAD~3^{tree} skipped, same as HEAD~1^{tree}\n+\t\t# HEAD~4^{tree} skipped, same as HEAD^{tree}\n+\t\tHEAD~5^{tree}\n+\tEOF\n+\tgrep -v \"#\" >expect \nSigned-off-by: Junio C Hamano \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.128.gcadd42da22\n\n"},{"id":"332584","messageId":"20171115003043.24080-3-sbeller@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-1-sbeller@google.com","subject":"[PATCHv4 2/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:38Z","receivedAt":"2017-11-15T00:31:21Z","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 \nSigned-off-by: Junio C Hamano \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.128.gcadd42da22\n\n"},{"id":"332585","messageId":"20171115003043.24080-6-sbeller@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-1-sbeller@google.com","subject":"[PATCHv4 5/7] builtin/describe.c: print debug statements earlier","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:41Z","receivedAt":"2017-11-15T00:31:23Z","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\nThis re-arrangement of debug printing is solely done for a better\nrefactoring in the next commit, not to aid debugging git-describe,\nwhich is expected to have the same information readily available\nwith the new prints.\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.128.gcadd42da22\n\n"},{"id":"332586","messageId":"20171115003043.24080-7-sbeller@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-1-sbeller@google.com","subject":"[PATCHv4 6/7] builtin/describe.c: factor out describe_commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:42Z","receivedAt":"2017-11-15T00:31:25Z","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 \nSigned-off-by: Junio C Hamano \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.128.gcadd42da22\n\n"},{"id":"332587","messageId":"20171115003043.24080-8-sbeller@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-1-sbeller@google.com","subject":"[PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-15T00:30:43Z","receivedAt":"2017-11-15T00:31:27Z","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\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\nexample\n\n git describe --tags v0.99:Makefile\n conversion-901-g7672db20c2:Makefile\n\ntells us the Makefile as it was in v0.99 was introduced in commit 7672db20.\n\nThe walking is performed in reverse order to show the introduction of a\nblob rather than its last occurrence.\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 | 66 ++++++++++++++++++++++++++++++++++++++----\n t/t6120-describe.sh | 19 ++++++++++++\n 3 files changed, 92 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex c924c945ba..a25443ca91 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..a2a5fdc48d 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,57 @@ 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\t\"--reverse\",\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 +498,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..ec4f25d009 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -310,6 +310,25 @@ 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 its last introduction' '\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit rm file &&\n+\tgit commit -m \"delete blob\" &&\n+\tgit revert HEAD &&\n+\tgit commit --allow-empty -m \"empty commit\" &&\n+\tgit describe HEAD:file >actual &&\n+\tgrep unique-file-3-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.128.gcadd42da22\n\n"},{"id":"332590","messageId":"20171114173812.9ac53b0cf0aa93db08d6559c@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-4-sbeller@google.com","subject":"Re: [PATCHv4 3/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-15T01:38:12Z","receivedAt":"2017-11-15T01:38:18Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 14 Nov 2017 16:30:39 -0800\nStefan Beller wrote:\n\n> The change in list-objects.c is rather minimal as we'll be re-using\n> the infrastructure put in place of the revision walking machinery. For\n> example one could expect that add_pending_tree is not called, but rather\n> commit->tree is directly passed to the tree traversal function. This\n> however requires a lot more code than just emptying the queue containing\n> trees after each commit.\n\nAh, I see. You're calling add_pending_tree(), then\ntraverse_trees_and_blobs() will immediately flush the list of pending\ntrees.\n\nI'm not sure that passing commit->tree directly to the tree traversal\nfunction will require a lot more code, but even if it does, you should\nat least add a NEEDSWORK - currently, flushing the list of pending trees\nfrees the array containing the list of pending trees, so each invocation\nwill need to reallocate a new array.\n"},{"id":"332591","messageId":"20171114174134.51c5a41788ecb5675b8d93f8@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-6-sbeller@google.com","subject":"Re: [PATCHv4 5/7] builtin/describe.c: print debug statements earlier","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-15T01:41:34Z","receivedAt":"2017-11-15T01:41:42Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 14 Nov 2017 16:30:41 -0800\nStefan Beller wrote:\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 then change the next debug output to describe why we\n> need to search. Conveniently drop the arg from the second line;\n> which will be useful in a follow up commit, that refactors the\n> describe function.\n> \n> This re-arrangement of debug printing is solely done for a better\n> refactoring in the next commit, not to aid debugging git-describe,\n> which is expected to have the same information readily available\n> with the new prints.\n\nThis paragraph (\"not to aid debugging\") contradicts the previous one\n(\"For debuggers aid\").\n\nLooking at this patch and the subsequent patches, I would write the\ncommit message like this:\n\n When debugging, print the received argument at the start of the\n function instead of in the middle. This ensures that the received\n argument is printed in all code paths, and also allows a subsequent\n refactoring to not need to move the \"arg\" parameter.\n\nAlso change the title to \"print arg earlier when debugging\" or something\nlike that.\n"},{"id":"332592","messageId":"20171114174458.48e345602d6cb3b2614941ef@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-7-sbeller@google.com","subject":"Re: [PATCHv4 6/7] builtin/describe.c: factor out describe_commit","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-15T01:44:58Z","receivedAt":"2017-11-15T01:45:06Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 14 Nov 2017 16:30:42 -0800\nStefan Beller wrote:\n\n> In the next patch we'll learn how to describe more than just commits,\n> so factor out describing commits into its own function. That will make\n> the next patches easy as we still need to describe a commit as part of\n> describing blobs.\n> \n> While factoring out the functionality to describe_commit, make sure\n> that any output to stdout is put into a strbuf, which we can print\n> afterwards, using puts which also adds the line ending.\n\nI think the sentences here are a bit jumbled. The aim is to make the\nnext patch easier (your 2nd sentence), and how we accomplish that is by\nfactoring out the description of commits (1st sentence) *and* by\noutputting to a strbuf because we need to postprocess the output further\nwhen describing a commit as part of describing a blob (3rd sentence).\n"},{"id":"332593","messageId":"20171114175207.f23d492045d52b8aa16c00be@google.com","threadId":"47209","inReplyTo":"20171115003043.24080-8-sbeller@google.com","subject":"Re: [PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-15T01:52:07Z","receivedAt":"2017-11-15T01:52:14Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 14 Nov 2017 16:30:43 -0800\nStefan Beller wrote:\n\n> The walking is performed in reverse order to show the introduction of a\n> blob rather than its last occurrence.\n\nThe code as implemented here does not do this - it instead shows the last\noccurrence.\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\nI would write \"Describe a commit or blob using a tag reachable from it\".\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\nI think some of this needs to be updated?\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\nSetting max_count to 0 does not work when reverse is used, because the\ncommits are first buffered into revs->commits (see get_revision() in\nrevision.c). There doesn't seem to be a convenient way to terminate the\ntraversal immediately - I think setting revs->commits to NULL should\nwork (but I didn't check). Remember to free revs->commits (using\nfree_commit_list) first.\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\nThis is probably better named \"describe a blob at a directly tagged\ncommit\". (Should we also test the case where a blob is directly\ntagged?)\n\n> +test_expect_success 'describe a blob with its last introduction' '\n> +\tgit commit --allow-empty -m \"empty commit\" &&\n> +\tgit rm file &&\n> +\tgit commit -m \"delete blob\" &&\n> +\tgit revert HEAD &&\n> +\tgit commit --allow-empty -m \"empty commit\" &&\n> +\tgit describe HEAD:file >actual &&\n> +\tgrep unique-file-3-g actual\n> +'\n\nThe description is not true: firstly, this shows the last occurrence,\nnot the last introduction (you can verify this by adding another commit\nand noticing that the contents of \"actual\" changes), and what we want is\nnot the last introduction anyway, but the first one.\n"},{"id":"332634","messageId":"CAGZ79kaum5py=14kdFy1a+K_0MzfaD5boYStixh=1aY2tUCV-Q@mail.gmail.com","threadId":"47209","inReplyTo":"20171114175207.f23d492045d52b8aa16c00be@google.com","subject":"Re: [PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T01:22:51Z","receivedAt":"2017-11-16T01:22:59Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Nov 14, 2017 at 5:52 PM, Jonathan Tan wrote:\n> On Tue, 14 Nov 2017 16:30:43 -0800\n> Stefan Beller wrote:\n>\n>> The walking is performed in reverse order to show the introduction of a\n>> blob rather than its last occurrence.\n>\n> The code as implemented here does not do this - it instead shows the last\n> occurrence.\n\nfixed to show the first occurrence.\n\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> I would write \"Describe a commit or blob using a tag reachable from it\".\n\nusing a ref, as we also can use refs.\nI think 'the graph' is technically correct here, but may be too confusing.\n\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> I think some of this needs to be updated?\n\nfixed.\n\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>> + pcd->revs->max_count = 0;\n>> + }\n>> +}\n>\n> Setting max_count to 0 does not work when reverse is used, because the\n> commits are first buffered into revs->commits (see get_revision() in\n> revision.c). There doesn't seem to be a convenient way to terminate the\n> traversal immediately - I think setting revs->commits to NULL should\n> work (but I didn't check). Remember to free revs->commits (using\n> free_commit_list) first.\n\nThis does work indeed.\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> This is probably better named \"describe a blob at a directly tagged\n> commit\".\n\nok\n\n> (Should we also test the case where a blob is directly\n> tagged?)\n\nWe do a bad job at describing tags that point at a blob currently:\n\n git tag test-blob HEAD:Makefile\n git describe test-blob\nerror: object cd75985991f4535c45e2589222a9e6a38fb1d613 is a blob, not a commit\nfatal: test-blob is not a valid 'commit' object\n\nThis series changes this to\n\n git describe test-blob\n v2.15.0-rc0-43-g54bd705a95:Makefile\n\nwhich might not be expected (you'd expect \"test-blob\"),\nso I think I can write a test telling that this is suboptimal\nbehavior?\n\n>\n>> +test_expect_success 'describe a blob with its last introduction' '\n>> + git commit --allow-empty -m \"empty commit\" &&\n>> + git rm file &&\n>> + git commit -m \"delete blob\" &&\n>> + git revert HEAD &&\n>> + git commit --allow-empty -m \"empty commit\" &&\n>> + git describe HEAD:file >actual &&\n>> + grep unique-file-3-g actual\n>> +'\n>\n> The description is not true: firstly, this shows the last occurrence,\n> not the last introduction (you can verify this by adding another commit\n> and noticing that the contents of \"actual\" changes), and what we want is\n> not the last introduction anyway, but the first one.\n\nfixed.\n"},{"id":"332636","messageId":"xmqq7eurxak0.fsf@gitster.mtv.corp.google.com","threadId":"47209","inReplyTo":"CAGZ79kaum5py=14kdFy1a+K_0MzfaD5boYStixh=1aY2tUCV-Q@mail.gmail.com","subject":"Re: [PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-16T01:43:43Z","receivedAt":"2017-11-16T01:43:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\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>> I would write \"Describe a commit or blob using a tag reachable from it\".\n>\n> using a ref, as we also can use refs.\n> I think 'the graph' is technically correct here, but may be too confusing.\n\nWithout saying graph over what, \"graph relations\" is not just\nconfusing but an insufficient explanation for a technically correct\nexplanation. Even though we have \"--contains\", we say \"reachable from\"\nand nobody has complained---so perhaps we can keep the white lie to\nkeep the synopsis simpler?\n\nIf I were writing this sentence from scratch, perhaps I wouldn't\neven use the word \"describe\". How about \n\n Give an object a human readable name based on an available ref\n\nor something like that?\n\n>> (Should we also test the case where a blob is directly\n>> tagged?)\n>\n> We do a bad job at describing tags that point at a blob currently:\n>\n> git tag test-blob HEAD:Makefile\n> git describe test-blob\n> error: object cd75985991f4535c45e2589222a9e6a38fb1d613 is a blob, not a commit\n> fatal: test-blob is not a valid 'commit' object\n>\n> This series changes this to\n>\n> git describe test-blob\n> v2.15.0-rc0-43-g54bd705a95:Makefile\n>\n> which might not be expected (you'd expect \"test-blob\"),\n> so I think I can write a test telling that this is suboptimal\n> behavior?\n\nOr a sentence in BUGS section.\n\nA case (or two) I find more interesting is to see how the code\nbehaves against these:\n\n\tgit tag -a -m \"annotated blob\" a-blob HEAD:Makefile\n\tgit tag -a -m \"annotated tree\" a-tree HEAD:t\n\tgit describe a-blob a-tree\n\nThanks.\n"},{"id":"332637","messageId":"CAGZ79kY109yTQESzVg69nsibucAhvP0m+vKUH21xB9m7WTGaFg@mail.gmail.com","threadId":"47209","inReplyTo":"xmqq7eurxak0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T01:49:01Z","receivedAt":"2017-11-16T01:49:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">\n> Give an object a human readable name based on an available ref\n>\n> or something like that?\n\nwill use\n\n> Or a sentence in BUGS section.\n\nwill add.\n\n> A case (or two) I find more interesting is to see how the code\n> behaves against these:\n>\n> git tag -a -m \"annotated blob\" a-blob HEAD:Makefile\n> git tag -a -m \"annotated tree\" a-tree HEAD:t\n> git describe a-blob a-tree\n\nGlad I added a test for exactly this (well only the a-blob case,\nbut a-tree will be the same):\n\ntest_expect_success 'describe tag object' '\n git tag test-blob-1 -a -m msg unique-file:file &&\n test_must_fail git describe test-blob-1 2>actual &&\n grep \"fatal: test-blob-1 is neither a commit nor blob\" actual\n '\n"},{"id":"332646","messageId":"xmqq375fx9ly.fsf@gitster.mtv.corp.google.com","threadId":"47209","inReplyTo":"CAGZ79kY109yTQESzVg69nsibucAhvP0m+vKUH21xB9m7WTGaFg@mail.gmail.com","subject":"Re: [PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-16T02:04:09Z","receivedAt":"2017-11-16T02:04:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> grep \"fatal: test-blob-1 is neither a commit nor blob\" actual\n\nOK, that might be somewhat unsatisfying from end-user's point of\nview (logically \"test-blob-1\" is already a name based on the 'graph\nrelations' that is satisfactory).\n\n\t[side note] should that grep become test_i18ngrep?\n\nIf this machinery is to find an object that is *bad* (in the sense\nin one of your earlier messages), I suspect it may be quite easy to\nuse the new mechanism added by the series to also locate a tree\nobject and report \"that tree appears in this commit at this path\"\nand it would be equally useful as the feature to find a blob.\n\nThanks.\n\n"},{"id":"332729","messageId":"xmqqd14hvhle.fsf@gitster.mtv.corp.google.com","threadId":"47209","inReplyTo":"xmqq375fx9ly.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv4 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-17T01:06:53Z","receivedAt":"2017-11-17T01:07:03Z","isPatch":false,"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>> grep \"fatal: test-blob-1 is neither a commit nor blob\" actual\n>\n> OK, that might be somewhat unsatisfying from end-user's point of\n> view (logically \"test-blob-1\" is already a name based on the 'graph\n> relations' that is satisfactory).\n\n... but that is correct and I think we need any further updates to\naddress it. If a user starts with a tag object, the user already\nhas one usable human-readable name. By \"unsatisfying\", I didn't\nmean that it is something we need to spend more cycles.\n\n"}]}