Re: [RFC] notes: avoid recommitting identical trees

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <git@xxxxxxxxxxxxxxxxxxxx>

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

> ---
> 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@xxxxxxxxxxx>
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]