git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Junio C HamanoNext: Johan Herland
Message 13 of 16 in “Q. about usage of notes”
  1. Stefan NaeweAug 31, 2010
  2. Johan HerlandAug 31, 2010
  3. Stefan NaeweAug 31, 2010
  4. Stefan NaeweAug 31, 2010
  5. [RFC] notes: avoid recommitting identical treesMichael J Gruber, Aug 31, 2010
  6. Jeff KingAug 31, 2010
  7. Michael J GruberAug 31, 2010
  8. Junio C HamanoAug 31, 2010
  9. Michael J GruberAug 31, 2010
  10. Jeff KingAug 31, 2010
  11. Michael J GruberAug 31, 2010
  12. Junio C HamanoAug 31, 2010
  13. Johan HerlandAug 31, 2010
  14. Johan HerlandAug 31, 2010
  15. 1/2 notes: Don't create (empty) commit when removing non-existing notesJohan Herland, Aug 31, 2010
  16. 2/2 notes.h: Extend remove_note to return the SHA1 of the removed note, if anyJohan Herland, Aug 31, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.