Re: [PATCH 1/2] revision: keep track of the end-user input from the command line

[Junio C Hamano <gitster@pobox.com>]

Sverre Rabbelier <srabbelier@gmail.com> writes:

>> Prepare a separate array to keep track of what syntactic element was used
>> to cause each object to appear in the pending array from the command line,
>> and populate it as setup_revisions() parses the command line.
>
> Thank you! I was really dreading looking into this myself, so I'm very
> glad that you could find the time to look into it yourself.

I debated long and hard if I should instead fatten object array entry and
shove this information there without adding a new structure, which would
have resulted in something very similar to what you had, so you should
take some credit for the code, and also credit for a large part of the
motivation (the second paragraph in the log is entirely your use case).

We might end up unifying this command line information array and the
pending object array after reviewing what other future callers would want
from this new information, but at least by doing it this way I can rest
assured that no existing code that is unaware of the mechanism would get
any unintended side effects in the earlier rounds.

>> @@ -835,6 +853,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
>>                struct object *o = parse_object(sha1);
>>                if (o) {
>>                        o->flags |= cb->all_flags;
>> +                       /* ??? CMDLINEFLAGS ??? */
>>                        add_pending_object(cb->all_revs, o, "");
>>                }
>>                else if (!cb->warned_bad_reflog) {
>
> What is happening here?

We could have add_rev_cmdline() call there if we really wanted to, but I
decided not to do so for two reasons:

(1) with "rev-list -g HEAD", the user is not explicitly mentioning all
    objects in the reflog of the HEAD---it might still make sense to mark
    HEAD as explicitly mentioned as positive, but the code to do so will
    not sit here anyway; and more importantly,

(2) I was appalled by how broken the design and the implementation of
    walking the reflog was (it shoves _all_ the objects in the entire
    history recorded in the reflog to the pending queue before letting the
    caller do an iota of work). It is against the general design of Git,
    and the design of the revision walk machinery in particular that tries
    very hard to be incremental. It goes against a good software design
    taste.

For the latter reason, I think in the longer term we should correct the
implementation to walk the reflog to keep an iterator (a structure that
holds an open file descriptor to a reflog file, and perhaps a little bit
of buffer) in struct rev_info, and teach get_revisions() to lazily read
from there, reading the file backward. Once that happens, this callback
from for_each_reflog_ent() will go away, so I didn't bother.