rev-list/tree committer/author information.

quoted

quoted

quoted

quoted

"LT" == Linus Torvalds [off-list ref] writes:

LT> Anyway, everything I've read so far makes sense, and it
LT> might make sense to continue git development using just
LT> git-pb. The only thing I personally think sucks is the
LT> author/committer matching of git-rev-list/tree, since it
LT> would seem like somebody might well like to match on an
LT> arbitrary part of a commit, and special-casing
LT> author/committer seems somewhat broken.

Well, that author/committer thing is not in git-pb yet, if I am
not mistaken [*1].

The only reason why I did it that way was because the strategy
taken by "struct object" derivatives seemed to pick up bare
absolute minimum to support actual callers that have immediate
need for information stored in structural fields, as opposed to
designing for helping yet to be written callers by adding fields
to hold information of "having this might also help somebody in
the future" type.  And the author and committer names are in the
structured fields while signed-off-by and others are not.  Also
when author / committer name strings are intern'ed like the way
I did, the memory consumption even for a long sequence of
commits are kept reasonably low.  However,...

LT> I personally suspect that both git-rev-list and git-rev-tree
LT> should have an alternate output format that could be more
LT> easily grepped by subsequent commands. For example, right
LT> now git-rev-list just outputs a list of commit ID's, and it
LT> might make sense to have a flag to just append the commit
LT> message to the output, and zero-terminate it (and if the
LT> commit message has a NUL byte in it, just truncate it at
LT> that point).

I think what you said here makes more sense [*2*].  The above
implies to keep the unpacked raw data as a whole to be
accessible to the callers for at least commit objects and if we
go that route I think it would make more sense to do that
uniformly for everything (probably except for pure "blob"
objects for size concerns but we might as well do them while we
are at it).  On the other hand, the current lifetime rules being
what it is, that strategy may introduce memory consumption
problems when working on a huge project.

[Footnote]

*1* Petr has been applying quite good judgements. I would have
polluted git-jc with that patch already if I were still running
it.  So far, I have been generally happy with his acceptance
criteria for external patches.  Anything he places on hold or
just outright returns to me, I later find rooms for improvements
myself, and the later rounds that eventually get accepted always
turn out to be far cleaner, thanks to his comments.

*2* At least in principle.  I am not quite sure what the output
should look like for rev-tree.

Dear diary, on Mon, May 16, 2005 at 11:33:11PM CEST, I got a letter
where Junio C Hamano [off-list ref] told me that...

quoted

quoted

quoted

quoted

quoted

"LT" == Linus Torvalds [off-list ref] writes:

LT> Anyway, everything I've read so far makes sense, and it
LT> might make sense to continue git development using just
LT> git-pb. The only thing I personally think sucks is the
LT> author/committer matching of git-rev-list/tree, since it
LT> would seem like somebody might well like to match on an
LT> arbitrary part of a commit, and special-casing
LT> author/committer seems somewhat broken.

Well, that author/committer thing is not in git-pb yet, if I am
not mistaken [*1].

The only reason why I did it that way was because the strategy
taken by "struct object" derivatives seemed to pick up bare
absolute minimum to support actual callers that have immediate
need for information stored in structural fields, as opposed to
designing for helping yet to be written callers by adding fields
to hold information of "having this might also help somebody in
the future" type.  And the author and committer names are in the
structured fields while signed-off-by and others are not.  Also
when author / committer name strings are intern'ed like the way
I did, the memory consumption even for a long sequence of
commits are kept reasonably low.  However,...

I like Linus' suggestion. At the very least, what about making the
matching generic for the header? Something like --match-header or
whatever.

*1* Petr has been applying quite good judgements. I would have
polluted git-jc with that patch already if I were still running
it.  So far, I have been generally happy with his acceptance
criteria for external patches.  Anything he places on hold or
just outright returns to me, I later find rooms for improvements
myself, and the later rounds that eventually get accepted always
turn out to be far cleaner, thanks to his comments.

Thanks for the praise :-), but I'm actually quite unhappy with putting
patches "on hold" and it's not intentional. It's just that I don't feel
right about the patch enough to apply it immediately, and I either don't
have time to voice my concerns if they are non-trivial, or I just want
some time to think about it.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

On Mon, 16 May 2005, Junio C Hamano wrote:

The only reason why I did it that way was because the strategy
taken by "struct object" derivatives seemed to pick up bare
absolute minimum to support actual callers that have immediate
need for information stored in structural fields, as opposed to
designing for helping yet to be written callers by adding fields
to hold information of "having this might also help somebody in
the future" type.  And the author and committer names are in the
structured fields while signed-off-by and others are not.  Also
when author / committer name strings are intern'ed like the way
I did, the memory consumption even for a long sequence of
commits are kept reasonably low.  However,...

I think if we had a good reason to make author and committer structured,
there must be some reason we'll want them parsed out eventually, although
we may not have reached the point at which it makes sense to do it.

The above implies to keep the unpacked raw data as a whole to be
accessible to the callers for at least commit objects and if we
go that route I think it would make more sense to do that
uniformly for everything (probably except for pure "blob"
objects for size concerns but we might as well do them while we
are at it).  On the other hand, the current lifetime rules being
what it is, that strategy may introduce memory consumption
problems when working on a huge project.

Perhaps have a struct object field for the unpacked data, have it filled
by a unpack_object(struct object *) function, have the parse functions use
it if it's filled, and have a function to free it (and NULL the field).

So, if you want the actual contents, you'd call
unpack_object(&commit->obj) before parse_commit(commit), and
close_object(&commit_obj) when you're done, and the parse wouldn't
duplicate the work. The lifetime rules only really care that the return
value from lookup_*() stays the same for the duration of the program, not
that the fields in it don't get cleared (aside from the flags and the
hash).

This could be useful even for blobs; I'm working on a program to do
diff/merge on commits and everything under them (taking advantage of all
available information if the fast path leads to a conflict); it would be
nice not to have to go through the filesystem with the blob data.

	-Daniel
*This .sig left intentionally blank*