{"thread":{"id":"47216","subject":"[PATCHv5 0/7] describe blob","startedAt":"2017-11-16T02:00:54Z","lastAt":"2017-12-22T23:10:30Z","messageCount":19,"participants":["Stefan Beller","Junio C Hamano","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"332638","messageId":"20171116020039.17810-1-sbeller@google.com","threadId":"47216","inReplyTo":null,"subject":"[PATCHv5 0/7] describe blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:32Z","receivedAt":"2017-11-16T02:00:54Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Thanks Jonathan and Junio for the patient review!\n\n* fixed issues brought up in the last patch, see interdiff below.\n (I found that walking from so many refs as starting points was\n the source of confusion, hence we only want to walk from HEAD\n* reworded commit messages from earlier patches\n* added a BUGS section to the man page\n* took Junios suggestion for the NAME section.\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 | 18 +++++-\n Documentation/rev-list-options.txt | 5 ++\n builtin/describe.c | 122 ++++++++++++++++++++++++++++---------\n list-objects.c | 58 ++++++++++++------\n revision.c | 2 +\n revision.h | 3 +-\n t/t6100-rev-list-in-order.sh | 77 +++++++++++++++++++++++\n t/t6120-describe.sh | 36 ++++++++++-\n 8 files changed, 270 insertions(+), 51 deletions(-)\n create mode 100755 t/t6100-rev-list-in-order.sh\n\n-- \n2.15.0.128.gcadd42da22\n\ndiff --git c/Documentation/git-describe.txt w/Documentation/git-describe.txt\nindex a25443ca91..e027fb8c4b 100644\n--- c/Documentation/git-describe.txt\n+++ w/Documentation/git-describe.txt\n@@ -3,8 +3,7 @@ git-describe(1)\n \n NAME\n ----\n-git-describe - Describe a commit or blob using the graph relations\n-\n+git-describe - Give an object a human readable name based on an available ref\n \n SYNOPSIS\n --------\n@@ -27,13 +26,9 @@ 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+at `` in the ``, which itself describes the\n+first commit in which this blob occurs in a reverse revision walk\n+from HEAD.\n \n OPTIONS\n -------\n@@ -197,6 +192,14 @@ selected and output. Here fewest commits different is defined as\n the number of commits which would be shown by `git log tag..input`\n will be the smallest number of commits possible.\n \n+BUGS\n+----\n+\n+Tree objects as well as tag objects not pointing at commits, cannot be described.\n+When describing blobs, the lightweight tags pointing at blobs are ignored,\n+but the blob is still described as : despite the lightweight\n+tag being favorable.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git c/builtin/describe.c w/builtin/describe.c\nindex a2a5fdc48d..5b4bfaba3f 100644\n--- c/builtin/describe.c\n+++ w/builtin/describe.c\n@@ -457,7 +457,8 @@ 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\tfree_commit_list(pcd->revs->commits);\n+\t\tpcd->revs->commits = NULL;\n \t}\n }\n \n@@ -468,12 +469,7 @@ static void describe_blob(struct object_id oid, struct strbuf *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-\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\t\"--objects\", \"--in-commit-order\", \"--reverse\", \"HEAD\",\n \t\tNULL);\n \n \tinit_revisions(&revs, NULL);\ndiff --git c/list-objects.c w/list-objects.c\nindex 07a92f35fe..4caa6fcb77 100644\n--- c/list-objects.c\n+++ w/list-objects.c\n@@ -239,7 +239,13 @@ 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+\n \t\tif (revs->tree_blobs_in_commit_order)\n+\t\t\t/*\n+\t\t\t * NEEDSWORK: Adding the tree and then flushing it here\n+\t\t\t * needs a reallocation for each commit. Can we pass the\n+\t\t\t * tree directory without allocation churn?\n+\t\t\t */\n \t\t\ttraverse_trees_and_blobs(revs, &csp, show_object, data);\n \t}\n \ttraverse_trees_and_blobs(revs, &csp, show_object, data);\ndiff --git c/t/t6120-describe.sh w/t/t6120-describe.sh\nindex ec4f25d009..4668f0058e 100755\n--- c/t/t6120-describe.sh\n+++ w/t/t6120-describe.sh\n@@ -310,7 +310,7 @@ 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+test_expect_success 'describe a blob at a directly tagged commit' '\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@@ -319,14 +319,29 @@ test_expect_success 'describe a blob at a tag' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'describe a blob with its last introduction' '\n+test_expect_success 'describe a blob with its first 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+\techo \"unique-file:file\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe directly tagged blob' '\n+\tgit tag test-blob unique-file:file &&\n+\tgit describe test-blob >actual &&\n+\techo \"unique-file:file\" >expect &&\n+\t# suboptimal: we rather want to see \"test-blob\"\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe tag object' '\n+\tgit tag test-blob-1 -a -m msg unique-file:file &&\n+\ttest_must_fail git describe test-blob-1 2>actual &&\n+\tgrep \"fatal: test-blob-1 is neither a commit nor blob\" actual\n '\n \n test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n"},{"id":"332639","messageId":"20171116020039.17810-2-sbeller@google.com","threadId":"47216","inReplyTo":"20171116020039.17810-1-sbeller@google.com","subject":"[PATCHv5 1/7] t6120: fix typo in test name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:33Z","receivedAt":"2017-11-16T02:00:57Z","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":"332640","messageId":"20171116020039.17810-3-sbeller@google.com","threadId":"47216","inReplyTo":"20171116020039.17810-1-sbeller@google.com","subject":"[PATCHv5 2/7] list-objects.c: factor out traverse_trees_and_blobs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:34Z","receivedAt":"2017-11-16T02:01:00Z","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":"332641","messageId":"20171116020039.17810-4-sbeller@google.com","threadId":"47216","inReplyTo":"20171116020039.17810-1-sbeller@google.com","subject":"[PATCHv5 3/7] revision.h: introduce blob/tree walking in order of the commits","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:35Z","receivedAt":"2017-11-16T02:01:04Z","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 | 8 ++++\n revision.c | 2 +\n revision.h | 3 +-\n t/t6100-rev-list-in-order.sh | 77 ++++++++++++++++++++++++++++++++++++++\n 5 files changed, 94 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..4caa6fcb77 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -239,6 +239,14 @@ 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+\n+\t\tif (revs->tree_blobs_in_commit_order)\n+\t\t\t/*\n+\t\t\t * NEEDSWORK: Adding the tree and then flushing it here\n+\t\t\t * needs a reallocation for each commit. Can we pass the\n+\t\t\t * tree directory without allocation churn?\n+\t\t\t */\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":"332643","messageId":"20171116020039.17810-6-sbeller@google.com","threadId":"47216","inReplyTo":"20171116020039.17810-1-sbeller@google.com","subject":"[PATCHv5 5/7] builtin/describe.c: print debug statements earlier","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:37Z","receivedAt":"2017-11-16T02:01:09Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When debugging, print the received argument at the start of the\nfunction instead of in the middle. This ensures that the received\nargument is printed in all code paths, and also allows a subsequent\nrefactoring to not need to move the \"arg\" parameter.\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":"332644","messageId":"20171116020039.17810-7-sbeller@google.com","threadId":"47216","inReplyTo":"20171116020039.17810-1-sbeller@google.com","subject":"[PATCHv5 6/7] builtin/describe.c: factor out describe_commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:38Z","receivedAt":"2017-11-16T02:01:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Factor out describing commits into its own function `describe_commit`,\nwhich will put any output to stdout into a strbuf, to be printed\nafterwards.\n\nAs the next patch will teach Git to describe blobs using a commit and path,\nthis refactor will make it easy to reuse the code describing commits.\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.128.gcadd42da22\n\n"},{"id":"332645","messageId":"20171116020039.17810-8-sbeller@google.com","threadId":"47216","inReplyTo":"20171116020039.17810-1-sbeller@google.com","subject":"[PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T02:00:39Z","receivedAt":"2017-11-16T02:01:15Z","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 | 18 ++++++++++--\n builtin/describe.c | 62 ++++++++++++++++++++++++++++++++++++++----\n t/t6120-describe.sh | 34 +++++++++++++++++++++++\n 3 files changed, 107 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex c924c945ba..e027fb8c4b 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -3,14 +3,14 @@ git-describe(1)\n \n NAME\n ----\n-git-describe - Describe a commit using the most recent tag reachable from it\n-\n+git-describe - Give an object a human readable name based on an available ref\n \n SYNOPSIS\n --------\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 +24,12 @@ 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 ``, which itself describes the\n+first commit in which this blob occurs in a reverse revision walk\n+from HEAD.\n+\n OPTIONS\n -------\n ...::\n@@ -186,6 +192,14 @@ selected and output. Here fewest commits different is defined as\n the number of commits which would be shown by `git log tag..input`\n will be the smallest number of commits possible.\n \n+BUGS\n+----\n+\n+Tree objects as well as tag objects not pointing at commits, cannot be described.\n+When describing blobs, the lightweight tags pointing at blobs are ignored,\n+but the blob is still described as : despite the lightweight\n+tag being favorable.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 9e9a5ed5d4..5b4bfaba3f 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,53 @@ 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\tfree_commit_list(pcd->revs->commits);\n+\t\tpcd->revs->commits = NULL;\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\"--objects\", \"--in-commit-order\", \"--reverse\", \"HEAD\",\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 +494,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..4668f0058e 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -310,6 +310,40 @@ test_expect_success 'describe ignoring a broken submodule' '\n \tgrep broken out\n '\n \n+test_expect_success 'describe a blob at a directly tagged commit' '\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 first 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+\techo \"unique-file:file\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe directly tagged blob' '\n+\tgit tag test-blob unique-file:file &&\n+\tgit describe test-blob >actual &&\n+\techo \"unique-file:file\" >expect &&\n+\t# suboptimal: we rather want to see \"test-blob\"\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe tag object' '\n+\tgit tag test-blob-1 -a -m msg unique-file:file &&\n+\ttest_must_fail git describe test-blob-1 2>actual &&\n+\tgrep \"fatal: test-blob-1 is neither a commit nor blob\" 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":"332647","messageId":"xmqqwp2qx5w6.fsf@gitster.mtv.corp.google.com","threadId":"47216","inReplyTo":"20171116020039.17810-8-sbeller@google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-16T03:24:25Z","receivedAt":"2017-11-16T03:24:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\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 ambiguous, there might\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\nI am not sure if \"And if there is ...\" is adding much value here (I\ndo not think it is even technically correct for that matter). If\nthere are more than one tag that point at the commit the user is\ninterested in, we use one of the tags, as tags conceptually sit at a\nhigher level. And we use a heuristic to use one or the other tag to\nmake up a name for the commit, so the same commit can have two valid\nnames. ---So what? Neither of these two valid names is \"ambigous\";\nthe commit object the user wanted to name _is_ correctly identified\n(I would assume that we are not discussing a hash collision).\n\nLucikly, if we remove \"And if...precisely\", the logic still flows\nnicely, if not more, to the next paragraph.\n\n> When describing a blob, we want to describe the blob from a higher layer\n> as well, which is a tuple of (commit, deep/path) as the tree objects\n> involved are rather uninteresting. The same blob can be referenced by\n> multiple commits, so how we decide which commit to use? This patch\n> implements a rather naive approach on this: As there are no back pointers\n> from blobs to commits in which the blob occurs, we'll start walking from\n> any tips available, listing the blobs in-order of the commit and once we\n\nIs \"any tips\" still the case? I was wondering why you start your\ntraversal at HEAD and nothing else in this iteration. There seems\nto be no mention of this design decision in the documentation and no\njustification in the log.\n\n> found the blob, we'll take the first commit that listed the blob. For\n> example\n>\n> git describe --tags v0.99:Makefile\n> conversion-901-g7672db20c2:Makefile\n>\n> tells us the Makefile as it was in v0.99 was introduced in commit 7672db20.\n>\n> The walking is performed in reverse order to show the introduction of a\n> blob rather than its last occurrence.\n\nThe reversing may improve the chance of an older commit to be chosen\nrather than the newer one, but it does not even guarantee to show the\n\"introduction\".\n\nWhat this guarantees is that a long history will be traversed fully\nbefore we start considering which commit has the blob of interest, I\nam afraid. Is this a sensible trade-off?\n\n> +\targv_array_pushl(&args, \"internal: The first arg is not parsed\",\n> +\t\t\"--objects\", \"--in-commit-order\", \"--reverse\", \"HEAD\",\n> +\t\tNULL);\n\n"},{"id":"332718","messageId":"CAGZ79kaGGUJSGG6OdfaTepDrvGBGFd17paBNNYuQt7t8XnDfHw@mail.gmail.com","threadId":"47216","inReplyTo":"xmqqwp2qx5w6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-16T19:34:12Z","receivedAt":"2017-11-16T19:34:24Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 15, 2017 at 7:24 PM, Junio C Hamano wrote:\n\n> I am not sure if \"And if there is ...\" is adding much value here (I\n> do not think it is even technically correct for that matter). If\n> there are more than one tag that point at the commit the user is\n> interested in, we use one of the tags, as tags conceptually sit at a\n> higher level. And we use a heuristic to use one or the other tag to\n> make up a name for the commit, so the same commit can have two valid\n> names. ---So what? Neither of these two valid names is \"ambigous\";\n> the commit object the user wanted to name _is_ correctly identified\n> (I would assume that we are not discussing a hash collision).\n>\n> Lucikly, if we remove \"And if...precisely\", the logic still flows\n> nicely, if not more, to the next paragraph.\n\nfixed.\n\n>> When describing a blob, we want to describe the blob from a higher layer\n>> as well, which is a tuple of (commit, deep/path) as the tree objects\n>> involved are rather uninteresting. The same blob can be referenced by\n>> multiple commits, so how we decide which commit to use? This patch\n>> implements a rather naive approach on this: As there are no back pointers\n>> from blobs to commits in which the blob occurs, we'll start walking from\n>> any tips available, listing the blobs in-order of the commit and once we\n>\n> Is \"any tips\" still the case? I was wondering why you start your\n> traversal at HEAD and nothing else in this iteration. There seems\n> to be no mention of this design decision in the documentation and no\n> justification in the log.\n\nfixed the text. The design decision to reverse walk HEAD is tied to\nyour observation below:\n\n> The reversing may improve the chance of an older commit to be chosen\n> rather than the newer one, but it does not even guarantee to show the\n> \"introduction\".\n\nThis is what I realized when we started walking all refs including reflog.\nThe introduction can only be found when we take the commit-parents\ninto account and look at the diffs. I noticed you started\norigin/jc/diff-blobfind\nwhich may be helpful to find the introduction correctly, which I'll play around\nwith that and see if I can get that working.\n\n> What this guarantees is that a long history will be traversed fully\n> before we start considering which commit has the blob of interest, I\n> am afraid. Is this a sensible trade-off?\n\nI am not fully convinced all descriptions are in recent history, but I\ntend to agree that most are, so probably the trade off is a wash.\n"},{"id":"332919","messageId":"B03A23EA75104E99BADBB420B6BEA614@PhilipOakley","threadId":"47216","inReplyTo":"20171116020039.17810-8-sbeller@google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-11-20T17:26:48Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Stefan Beller\" \nSent: Thursday, November 16, 2017 2:00 AM\n\n[in catch up mode..]\n\n> Sometimes users are given a hash of an object and they want to\n> identify it further (ex.: Use verify-pack to find the largest blobs,\n> but what are these? or [1])\n>\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 ambiguous, there might\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> When describing a blob, we want to describe the blob from a higher layer\n> as well, which is a tuple of (commit, deep/path) as the tree objects\n> involved are rather uninteresting. The same blob can be referenced by\n> multiple commits, so how we decide which commit to use? This patch\n> implements a rather naive approach on this: As there are no back pointers\n> from blobs to commits in which the blob occurs, we'll start walking from\n> any tips available, listing the blobs in-order of the commit and once we\n> found the blob, we'll take the first commit that listed the blob. For\n> example\n>\n> git describe --tags v0.99:Makefile\n> conversion-901-g7672db20c2:Makefile\n>\n> tells us the Makefile as it was in v0.99 was introduced in commit \n> 7672db20.\n>\n> The walking is performed in reverse order to show the introduction of a\n> blob rather than its last occurrence.\n>\n> [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob\n>\n> Signed-off-by: Stefan Beller \n> ---\n> Documentation/git-describe.txt | 18 ++++++++++--\n> builtin/describe.c | 62 \n> ++++++++++++++++++++++++++++++++++++++----\n> t/t6120-describe.sh | 34 +++++++++++++++++++++++\n> 3 files changed, 107 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/git-describe.txt \n> b/Documentation/git-describe.txt\n> index c924c945ba..e027fb8c4b 100644\n> --- a/Documentation/git-describe.txt\n> +++ b/Documentation/git-describe.txt\n> @@ -3,14 +3,14 @@ git-describe(1)\n>\n> NAME\n> ----\n> -git-describe - Describe a commit using the most recent tag reachable from \n> it\n> -\n> +git-describe - Give an object a human readable name based on an available \n> ref\n>\n> SYNOPSIS\n> --------\n> [verse]\n> 'git describe' [--all] [--tags] [--contains] [--abbrev=] \n> [...]\n> 'git describe' [--all] [--tags] [--contains] \n> [--abbrev=] --dirty[=]\n> +'git describe' \n>\n> DESCRIPTION\n> -----------\n> @@ -24,6 +24,12 @@ By default (without --all or --tags) `git describe` \n> 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 ``, which itself describes the\n> +first commit in which this blob occurs in a reverse revision walk\n> +from HEAD.\n> +\n> OPTIONS\n> -------\n> ...::\n> @@ -186,6 +192,14 @@ selected and output. Here fewest commits different \n> is defined as\n> the number of commits which would be shown by `git log tag..input`\n> will be the smallest number of commits possible.\n>\n> +BUGS\n> +----\n> +\n> +Tree objects as well as tag objects not pointing at commits, cannot be \n> described.\n\nIs this true? Is it stand alone from the describing of a blob? If so should \nit be its own patchlet. - I thought I'd read that within the series there is \nnow a tree / tag (of blob/trees) description capability.\n\nI'd prefer that we don't start with the \"can't\" view (relative to the \nsubsequent sentences of the paragraph). It puts off the reader - we are \nabout to say what can be described but in a limited way - the limitation \nbeing the bug. Maybe just swap the line to form a second paragraph.\n\n> +When describing blobs, the lightweight tags pointing at blobs are \n> ignored,\n> +but the blob is still described as : despite the \n> lightweight\n> +tag being favorable.\n> +\n--\nPhilip\n\n> GIT\n> ---\n> Part of the linkgit:git[1] suite\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index 9e9a5ed5d4..5b4bfaba3f 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 (1u << 0)\n> #define MAX_TAGS (FLAG_BITS - 1)\n>\n> static const char * const describe_usage[] = {\n> @@ -434,6 +436,53 @@ static void describe_commit(struct object_id *oid, \n> struct strbuf *dst)\n> strbuf_addstr(dst, suffix);\n> }\n>\n> +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> +{\n> + struct process_commit_data *pcd = data;\n> + pcd->current_commit = commit->object.oid;\n> +}\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> + free_commit_list(pcd->revs->commits);\n> + pcd->revs->commits = NULL;\n> + }\n> +}\n> +\n> +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, &revs};\n> +\n> + argv_array_pushl(&args, \"internal: The first arg is not parsed\",\n> + \"--objects\", \"--in-commit-order\", \"--reverse\", \"HEAD\",\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> static void describe(const char *arg, int last_one)\n> {\n> struct object_id oid;\n> @@ -445,11 +494,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>\n> - describe_commit(&oid, &sb);\n> + if (cmit)\n> + describe_commit(&oid, &sb);\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 a/t/t6120-describe.sh b/t/t6120-describe.sh\n> index c8b7ed82d9..4668f0058e 100755\n> --- a/t/t6120-describe.sh\n> +++ b/t/t6120-describe.sh\n> @@ -310,6 +310,40 @@ test_expect_success 'describe ignoring a broken \n> submodule' '\n> grep broken out\n> '\n>\n> +test_expect_success 'describe a blob at a directly tagged commit' '\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 blob with its first 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> + echo \"unique-file:file\" >expect &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'describe directly tagged blob' '\n> + git tag test-blob unique-file:file &&\n> + git describe test-blob >actual &&\n> + echo \"unique-file:file\" >expect &&\n> + # suboptimal: we rather want to see \"test-blob\"\n> + test_cmp expect actual\n> +'\n> +\n> +test_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> +\n> test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n> i=1 &&\n> while test $i -lt 8000\n> -- \n> 2.15.0.128.gcadd42da22\n> \n\n"},{"id":"333254","messageId":"xmqq7euiu4r8.fsf@gitster.mtv.corp.google.com","threadId":"47216","inReplyTo":"CAGZ79kaGGUJSGG6OdfaTepDrvGBGFd17paBNNYuQt7t8XnDfHw@mail.gmail.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-22T07:55:23Z","receivedAt":"2017-11-22T07:55:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> ...\n> fixed.\n> ...\n> fixed the text...\n> ...\n> I am not fully convinced all descriptions are in recent history, but I\n> tend to agree that most are, so probably the trade off is a wash.\n\nSo what do we want with this topic? I think the \"teach 'git log' to\nhighlight commits whose changes involve the given blob\" is a more or\nless an orthogonal thing, and I suspect that it is something users\nmay (although I personally do not) find valuable to have a related\nbut different feature in \"git describe\".\n\n\n"},{"id":"333296","messageId":"CAGZ79kZajrugviv60kQA6UepYrQGpJK447BK1fBFd3N5Wnn5rQ@mail.gmail.com","threadId":"47216","inReplyTo":"xmqq7euiu4r8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-22T17:00:43Z","receivedAt":"2017-11-22T17:00:50Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Nov 21, 2017 at 11:55 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> ...\n>> fixed.\n>> ...\n>> fixed the text...\n>> ...\n>> I am not fully convinced all descriptions are in recent history, but I\n>> tend to agree that most are, so probably the trade off is a wash.\n>\n> So what do we want with this topic? I think the \"teach 'git log' to\n> highlight commits whose changes involve the given blob\" is a more or\n> less an orthogonal thing,\n\nWell, both of them solve our immediate needs, so I'd be fine with pursuing\njust one of them, but I do not oppose taking both.\n\n> and I suspect that it is something users\n> may (although I personally do not) find valuable to have a related\n> but different feature in \"git describe\".\n\nagreed.\n"},{"id":"333420","messageId":"xmqqtvxkm9eq.fsf@gitster.mtv.corp.google.com","threadId":"47216","inReplyTo":"CAGZ79kZajrugviv60kQA6UepYrQGpJK447BK1fBFd3N5Wnn5rQ@mail.gmail.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-24T07:18:53Z","receivedAt":"2017-11-24T07:19:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Nov 21, 2017 at 11:55 PM, Junio C Hamano wrote:\n>> Stefan Beller writes:\n>>\n>>> ...\n>>> fixed.\n>>> ...\n>>> fixed the text...\n>>> ...\n>>> I am not fully convinced all descriptions are in recent history, but I\n>>> tend to agree that most are, so probably the trade off is a wash.\n>>\n>> So what do we want with this topic? I think the \"teach 'git log' to\n>> highlight commits whose changes involve the given blob\" is a more or\n>> less an orthogonal thing,\n>\n> Well, both of them solve our immediate needs, so I'd be fine with pursuing\n> just one of them, but I do not oppose taking both.\n>\n>> and I suspect that it is something users\n>> may (although I personally do not) find valuable to have a related\n>> but different feature in \"git describe\".\n>\n> agreed.\n\nI was reacting to your \"fixed\". So will we see a rerolled series or\nnot?\n"},{"id":"333626","messageId":"CAGZ79kYbvdov_0cR1W9zOzkbHra6qk6gE0eAPGkzGWL_4WwcaA@mail.gmail.com","threadId":"47216","inReplyTo":"xmqqtvxkm9eq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-27T19:38:40Z","receivedAt":"2017-11-27T19:38:47Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Nov 23, 2017 at 11:18 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> On Tue, Nov 21, 2017 at 11:55 PM, Junio C Hamano wrote:\n>>> Stefan Beller writes:\n>>>\n>>>> ...\n>>>> fixed.\n>>>> ...\n>>>> fixed the text...\n>>>> ...\n>>>> I am not fully convinced all descriptions are in recent history, but I\n>>>> tend to agree that most are, so probably the trade off is a wash.\n>>>\n>>> So what do we want with this topic? I think the \"teach 'git log' to\n>>> highlight commits whose changes involve the given blob\" is a more or\n>>> less an orthogonal thing,\n>>\n>> Well, both of them solve our immediate needs, so I'd be fine with pursuing\n>> just one of them, but I do not oppose taking both.\n>>\n>>> and I suspect that it is something users\n>>> may (although I personally do not) find valuable to have a related\n>>> but different feature in \"git describe\".\n>>\n>> agreed.\n>\n> I was reacting to your \"fixed\". So will we see a rerolled series or\n> not?\n\nI was not planning on rerolling it.\n\nFYI: I am on vacation until next Monday, so if there is anything\nto be fixed here, I'd do it in a week, which may be enough time\nto warrant incremental fixes?\n"},{"id":"333652","messageId":"xmqqwp2b9ttw.fsf@gitster.mtv.corp.google.com","threadId":"47216","inReplyTo":"CAGZ79kYbvdov_0cR1W9zOzkbHra6qk6gE0eAPGkzGWL_4WwcaA@mail.gmail.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T23:37:31Z","receivedAt":"2017-11-27T23:37:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> I was reacting to your \"fixed\". So will we see a rerolled series or\n>> not?\n>\n> I was not planning on rerolling it.\n\nOK. Then I do not have to worry about this one and drop it perhaps.\nOne less topic on 'pu' is always good, if it is not active.\n\nEnjoy your vacation.\n\nThanks.\n"},{"id":"335036","messageId":"xmqqy3lyv7yt.fsf@gitster.mtv.corp.google.com","threadId":"47216","inReplyTo":"20171116020039.17810-8-sbeller@google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-19T19:22:18Z","receivedAt":"2017-12-19T19:22:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I had to squash in the following to make 'pu' pass under\ngettext-poison build. Is this ready for 'next' otherwise?\n\nWith the \"log --find-object\" thing, it may be that this no longer is\nneeded, but then again we haven't done anything with the other\nJonathan's idea to unify the --find-object thing into the --pickaxe\nframework.\n\nIt seems that we tend to open and then abandon new interests without\nseeing them through completion, leaving too many loose ends untied.\nThis has to stop.\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 4668f0058e..3e3fb462a0 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -341,7 +341,7 @@ test_expect_success 'describe directly tagged blob' '\n test_expect_success 'describe tag object' '\n \tgit tag test-blob-1 -a -m msg unique-file:file &&\n \ttest_must_fail git describe test-blob-1 2>actual &&\n-\tgrep \"fatal: test-blob-1 is neither a commit nor blob\" actual\n+\ttest_i18ngrep \"fatal: test-blob-1 is neither a commit nor blob\" actual\n '\n \n test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '\n"},{"id":"335037","messageId":"CAGZ79ka8PRbiHggTADNDwwO_=8rysK3sDP7tV4qerbc8C5yaSQ@mail.gmail.com","threadId":"47216","inReplyTo":"xmqqy3lyv7yt.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-12-19T19:39:47Z","receivedAt":"2017-12-19T19:40:01Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Dec 19, 2017 at 11:22 AM, Junio C Hamano wrote:\n> I had to squash in the following to make 'pu' pass under\n> gettext-poison build. Is this ready for 'next' otherwise?\n\nI saw that in pu, thanks for squashing. I should have spoken\nup earlier confirming it.\n\n> With the \"log --find-object\" thing, it may be that this no longer is\n> needed, but then again we haven't done anything with the other\n> Jonathan's idea to unify the --find-object thing into the --pickaxe\n> framework.\n\nI'll look into that after I finish looking at a submodule related bug.\n\n> It seems that we tend to open and then abandon new interests without\n> seeing them through completion, leaving too many loose ends untied.\n> This has to stop.\n\nI'll try to find my balance again.\n\nWhen working on too few topics at the same time, sometimes I get stalled\nwaiting for the mailing list (or internal reviewers) to respond, so I start many\ntopics, which then overwhelm me after a while once reviews catch up.\n\nIn this specific case it was not clear what the best way is, i.e. the\ninterest was\nrather broad: \"Hey I have this blob sha1, how do I get more information?\",\nwhich can be solved in many different ways, so I tried some of them.\n(Another alternative just now considered: I could have wrapped that\nscript from stackoverflow into a new command and called it a day, though that\nhas very poor benefits for the rest of ecosystem, an addition to an established\npowerful command gives more benefits IMHO)\n\nThanks,\nStefan\n"},{"id":"335203","messageId":"xmqqbmiqpdeq.fsf@gitster.mtv.corp.google.com","threadId":"47216","inReplyTo":"CAGZ79ka8PRbiHggTADNDwwO_=8rysK3sDP7tV4qerbc8C5yaSQ@mail.gmail.com","subject":"Re: [PATCHv5 7/7] builtin/describe.c: describe a blob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-22T23:10:21Z","receivedAt":"2017-12-22T23:10:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Dec 19, 2017 at 11:22 AM, Junio C Hamano wrote:\n>> I had to squash in the following to make 'pu' pass under\n>> gettext-poison build. Is this ready for 'next' otherwise?\n>\n> I saw that in pu, thanks for squashing. I should have spoken\n> up earlier confirming it.\n\nSo is this ready for 'next' if it is squashed?\n\n>> With the \"log --find-object\" thing, it may be that this no longer is\n>> needed, but then again we haven't done anything with the other\n>> Jonathan's idea to unify the --find-object thing into the --pickaxe\n>> framework.\n>\n> I'll look into that after I finish looking at a submodule related bug.\n\nThanks.\n\n>> It seems that we tend to open and then abandon new interests without\n>> seeing them through completion, leaving too many loose ends untied.\n>> This has to stop.\n>\n> I'll try to find my balance again.\n\nSorry, but this was not really about you, but was more about me. \n\nI probably should more aggressively drop a topic that stalled and\nalso merge a topic that reached the point of diminishing returns to\n'next'.\n"}]}