On Tue, Oct 31, 2017 at 9:11 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> +If the given object refers to a blob, it will be described >> +as `<commit-ish>:<path>`, such that the blob can be found >> +at `<path>` in the `<commit-ish>`. Note, that the commit is likely > > Does the code describe a9dbc3f12c as v2.15.0:GIT-VERSION-GEN, or > would it always be <commit>:<path>? > >> +not the commit that introduced the blob, but the one that was found >> +first; to find the commit that introduced the blob, you need to find > > Because we do not want to descend into the same tree object we saw > earlier, this "we show the name we happened to find first without > attempting to refine it for a better name" is a rather fundamental > limitation of this approach. Hopefully we can later improve it with > more thought, but for now it is better than nothing (and much better > than "git log --raw | grep"). ok. > >> +the commit that last touched the path, e.g. >> +`git log <commit-description> -- <path>`. >> +As blobs do not point at the commits they are contained in, >> +describing blobs is slow as we have to walk the whole graph. > > Is it true that we walk the "whole graph", or do we stop immediately > we find a path to the blob? The former makes it sound like > completely useless. Unfortunately we walk the whole graph, as I have not figured out how to stop the walking from the callback in 'traverse_commit_list'. I assume I have to modify the struct rev_info that we operate on, clearing any pending commits? > >> -#define SEEN (1u << 0) > > Interesting. Now we include revision.h this becomes redundant. > Correct. In a way this small part is a revert of 8713ab3079 (Improve git-describe performance by reducing revision listing., 2007-01-13) >> + argv_array_pushl(&args, "internal: The first arg is not parsed", >> + "--all", "--reflog", /* as many starting points as possible */ > > Interesting. > > Do we also search in the reflog in the normal "describe" operation? > If not, perhaps we should? I wonder what's the performance > implications would be. "normal" git describe doesn't need to walk the whole graph as we only walk down from the given commit-ish until a land mark is found. For --contains, this might be an interesting though, as there we also have to "walk backwards without pointers to follow". > > That tangent aside, as "describe blob" is most likely a "what > reaches and is holding onto this blob?" query, finding something > that can only be reached from a reflog entry would make it more > helpful than without the option. Yeah that is my reasoning as well. > >> + /* NEEDSWORK: --all is incompatible with worktrees for now: */ > > What's that last colon about? will replace with a dot, it ought to hint at the line that follows, the --single-worktree flag. > >> + "--single-worktree", >> + "--objects", >> + "--in-commit-order", >> + NULL); >> + >> + init_revisions(&revs, NULL); >> + if (setup_revisions(args.argc, args.argv, &revs, NULL) > 1) >> + BUG("setup_revisions could not handle all args?"); >> + >> + if (prepare_revision_walk(&revs)) >> + die("revision walk setup failed"); >> + >> + traverse_commit_list(&revs, process_commit, process_object, &pcd); >> + reset_revision_walk(); >> +} >> + > > OK. > >> diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh >> index 1c0e8659d9..3be01316e8 100755 >> --- a/t/t6120-describe.sh >> +++ b/t/t6120-describe.sh >> @@ -310,6 +310,21 @@ test_expect_success 'describe ignoring a borken submodule' ' >> grep broken out >> ' >> >> +test_expect_success 'describe a blob at a tag' ' >> + echo "make it a unique blob" >file && >> + git add file && git commit -m "content in file" && >> + git tag -a -m "latest annotated tag" unique-file && >> + git describe HEAD:file >actual && >> + echo "unique-file:file" >expect && >> + test_cmp expect actual >> +' >> + >> +test_expect_success 'describe a surviving blob' ' >> + git commit --allow-empty -m "empty commit" && >> + git describe HEAD:file >actual && >> + grep unique-file-1-g actual >> +' >> + > > I am not sure what "surviving" means in this context. Maybe "unchanged", "still kept around" ? > The word > hinted that the test would be finding a blob that only appears in a > commit that only appears as a reflog entry, but that wasn't the > case, which was a bit disappointing. oh!