Re: [RFC] notes: avoid recommitting identical trees
- From
Michael J Gruber <git@drmicha.warpmail.net>
- Date
- Aug 31, 2010, 16:15 UTC
- Message-ID
- <4C7D2AAE.9000701@drmicha.warpmail.net>
- In-Reply-To
- <20100831160145.GC11014@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 31.08.2010 18:01:
Show 11 quoted lines
> On Tue, Aug 31, 2010 at 05:16:17PM +0200, Michael J Gruber wrote: > >> Currently, "git notes" behaves like "git commit --allow-empty" when >> committing notes trees. In particular, removing nonexisting notes leads >> to empty commits "commits with no diff". >> >> Change this to avoid unnecessary notes commits. > > Is this a sufficient check in the case of notes? Is it possible that we > re-balanced the fanout of the notes tree and got a different tree sha1, > even though there is nothing interesting to commit?
Yes, but I don't think this hurts. The main thrust here is to catch the case of repeated "git notes remove". Also, we might even want to record the history when there is rebalancing since this is indeed a tree change.
Johan's (later ;) ) approach, while being more intrusive, catches this at the point of removal - if there's nothing to remove, nothing gets rewritten.
Show 6 quoted lines
>
>> + if (!parent || parse_commit(parent->item) || parse_tree(parent->item->tree) ||
>> + hashcmp(parent->item->tree->object.sha1, tree_sha1)) {
>
> I didn't check, but I can imagine you can drop the parse_tree here. We
> should know the object sha1 once the commit is parsed.parse_commit() does a lookup_tree() but I don't think that it parses the tree, i.e. I don't hink it fills in tree->object.sha1. At least it segfaulted without that ;)
Michael