Re: [PATCH 2/3 v2] add new Perl API: Git::Repo, Git::Commit, Git::Tag, and Git::RepoRoot

From: Lea Wiemann <lewiemann@gmail.com>
Date: Aug 19, 2008, 15:06 UTC
Message-ID: <48AAE172.3000307@gmail.com>
In-Reply-To: <7v8wutkcx4.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:

> Lea Wiemann <lewiemann@gmail.com> writes:
>> +The author, committer and message methods return Unicoded strings,
> 
> Unicoded?

Fixed. :)

Show 5 quoted lines

>> +# Auxiliary method to load (and parse) the commit object from the
>> +# repository if it hasn't already been loaded. [...]
>> +sub _load {
>
> Aside from seeming repetitive and quite similar to Git::Tag::_load(),

I didn't see a clean way to extract common code since there are differences in how some headers are parsed (e.g. 'parents' is an array).

> I have to wonder how parent rewriting and grafts come into the picture.
> Git::Repo::get_object is just a cat-file which means you are ignoring any
> grafts.  As a design decision it is fine, but it needs to be documented.

I'm not sure if I understand grafts correctly, but would something like the following piece of text in the documentation for Git::Commit->parents cover it?

"Note that this method does not currently respect grafts. Hence, if you are operating on a repository that contains grafts (e.g. a shallow clone), the commit objects returned by the 'parents' method may not exist in the repository; your program will die if you call an attribute accessor method (such as 'author' or 'message') of any parent of an object with grafted parents. Thus, if you want your program to work with shallow clones, you will have to check the .git/info/grafts file yourself before accessing any parents.

You must not rely on this behavior. In the future, the 'parents' method may return the grafted parents instead of the parents recorded in the commit object."

> Also if you run "per path history" using "rev-list $head -- $path" to
> collect commits that touch the named $path, you might want to get
> rewritten parents for each commit and use it in the presentation,

You're referring to gitweb here, right? (The perl API doesn't use rev-list.) I don't think gitweb is currently tested with grafted repositories at all, but in general it should respect rewritten parents, since the parse_commit function uses rev-list without the 'raw' format, and thus it should get the rewritten parents.

-- Lea