Re: [RFC] notes: avoid recommitting identical trees
From: Johan Herland <hidden>
Date: 2016-06-15 22:49:25
On Tuesday 31 August 2010, 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. Signed-off-by: Michael J Gruber <redacted>
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
quoted hunk ↗ jump to hunk
--- 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, constchar *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, [off-list ref] www.herland.net