On Tue, Oct 31, 2017 at 9:11 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Stefan Beller <sbel...@google.com> 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!
