Re: [RFC] notes: avoid recommitting identical trees
- From
Johan Herland <johan@herland.net>
- Date
- Aug 31, 2010, 16:08 UTC
- Message-ID
- <201008311808.11203.johan@herland.net>
- In-Reply-To
- <bc44b3393db4018487bb956d00a12fa73f04ca9e.1283267564.git.git@drmicha.warpmail.net>
On Tuesday 31 August 2010, Michael J Gruber wrote:
Show 7 quoted lines
> 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. > > Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
I just posted a patch with the same objective, but with a different approach. Instead of parsing the previous commit and comparing tree object SHA1s, I add a few lines of notes code to let remove_note() report whether it removed a note or not (thus determining whether a commit is necessary or not).
In general, the notes_tree.dirty flag should be sufficient to determine whether a commit is needed or not (remove_note()'s unconditional setting of this flag is also fixed in my patch).
...Johan
Show 34 quoted lines
> ---
> I can't believe there's no easier way to lookup the sha1 of a tree of
> a commit but I didn't find any, and I did not want to employ the diff
> machinery for diffing the trees when their sha1 is (should be) known.
>
> builtin/notes.c | 14 +++++++++-----
> 1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index fbc347c..48da228 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -303,11 +303,15 @@ int commit_notes(struct notes_tree *t, const
> char *msg) hashclr(prev_commit);
> parent = NULL;
> }
> - if (commit_tree(buf.buf + 7, tree_sha1, parent, new_commit, NULL))
> - die("Failed to commit notes tree to database");
> -
> - /* Update notes ref with new commit */
> - update_ref(buf.buf, t->ref, new_commit, prev_commit, 0,
> DIE_ON_ERR); + if (!parent || parse_commit(parent->item) ||
> parse_tree(parent->item->tree) ||
> + hashcmp(parent->item->tree->object.sha1, tree_sha1)) {
> + /* avoid recommitting the same tree */
> + if (commit_tree(buf.buf + 7, tree_sha1, parent, new_commit, NULL))
> + die("Failed to commit notes tree to database");
> +
> + /* Update notes ref with new commit */
> + update_ref(buf.buf, t->ref, new_commit, prev_commit, 0,
> DIE_ON_ERR); + }
>
> strbuf_release(&buf);
> return 0;-- Johan Herland, <johan@herland.net> www.herland.net