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

Re: [PATCH] diff-tree.c: load notes machinery when required

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 21, 2020, 00:13 UTC
Message-ID
<20200421001315.GA32669@syl.local>
In-Reply-To
<20200421000619.GB2879931@coredump.intra.peff.net>
On Mon, Apr 20, 2020 at 08:06:19PM -0400, Jeff King wrote:
Show 10 quoted lines
> On Mon, Apr 20, 2020 at 06:05:31PM -0600, Taylor Blau wrote:
>
> > Here is an updated version of the patch that should be ready for
> > queuing.
> >
> > -- 8< --
> >
> > Subject: [PATCH] tempfile.c: introduce 'create_tempfile_mode'
>
> Er, right patch?
Ugh, how embarrassing. See below:
-- >8 --
Subject: [PATCH] diff-tree.c: load notes machinery when required

Since its introduction in 7249e91 (revision.c: support --notes command-line option, 2011-03-29), combining '--notes' with any option that causes us to format notes (e.g., '--pretty', '--format="%N"', etc) results in a failed assertion at runtime.

  $ git rev-list HEAD | git diff-tree --stdin --pretty=medium --notes
  commit 8f3d9f354286745c751374f5f1fcafee6b3f3136
  git: notes.c:1308: format_display_notes: Assertion `display_notes_trees' failed.
  Aborted

This failure is due to diff-tree not calling 'load_display_notes' to initialize the notes machinery.

Ordinarily, this failure isn't triggered, because it requires passing both '--notes' and another of the above mentioned options. In the case of '--pretty', for example, we set 'opt->verbose_header', causing 'show_log()' to eventually call 'format_display_notes()', which expects a non-NULL 'display_note_trees'.

Without initializing the notes machinery, 'display_note_trees' remains NULL, and thus triggers an assertion failure.

Fix this by initializing the notes machinery after parsing our options, and harden this behavior against regression with a test in t4013. (Note that the added ref in this test requires updating two unrelated tests which use 'log --all', and thus need to learn about the new refs).

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/diff-tree.c                          |  9 +++++++++
 t/t4013-diff-various.sh                      | 12 ++++++++++++
 t/t4013/diff.diff-tree_--format=%N_note      |  6 ++++++
 t/t4013/diff.diff-tree_--pretty_--notes_note | 12 ++++++++++++
 t/t4013/diff.diff-tree_--pretty_note         |  9 +++++++++
 t/t4013/diff.log_--decorate=full_--all       | 15 +++++++++++++++
 t/t4013/diff.log_--decorate_--all            | 15 +++++++++++++++
 7 files changed, 78 insertions(+)
 create mode 100644 t/t4013/diff.diff-tree_--format=%N_note
 create mode 100644 t/t4013/diff.diff-tree_--pretty_--notes_note
 create mode 100644 t/t4013/diff.diff-tree_--pretty_note
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index cb9ea79367..802363d0a2 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -109,6 +109,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	struct object *tree1, *tree2;
 	static struct rev_info *opt = &log_tree_opt;
 	struct setup_revision_opt s_r_opt;
+	struct userformat_want w;
 	int read_stdin = 0;

 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -127,6 +128,14 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	precompose_argv(argc, argv);
 	argc = setup_revisions(argc, argv, opt, &s_r_opt);

+	memset(&w, 0, sizeof(w));
+	userformat_find_requirements(NULL, &w);
+
+	if (!opt->show_notes_given && w.notes)
+		opt->show_notes = 1;
+	if (opt->show_notes)
+		load_display_notes(&opt->notes_opt);
+
 	while (--argc > 0) {
 		const char *arg = *++argv;

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index dde3f11fec..3f60f7d96c 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -95,6 +95,15 @@ test_expect_success setup '
 	git commit -m "update mode" &&
 	git checkout -f master &&

+	GIT_AUTHOR_DATE="2006-06-26 00:06:00 +0000" &&
+	GIT_COMMITTER_DATE="2006-06-26 00:06:00 +0000" &&
+	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
+	git checkout -b note initial &&
+	git update-index --chmod=+x file2 &&
+	git commit -m "update mode (file2)" &&
+	git notes add -m "note" &&
+	git checkout -f master &&
+
 	# Same merge as master, but with parents reversed. Hide it in a
 	# pseudo-ref to avoid impacting tests with --all.
 	commit=$(echo reverse |
@@ -398,6 +407,9 @@ diff --no-index --raw --no-abbrev dir2 dir

 diff-tree --pretty --root --stat --compact-summary initial
 diff-tree --pretty -R --root --stat --compact-summary initial
+diff-tree --pretty note
+diff-tree --pretty --notes note
+diff-tree --format=%N note
 diff-tree --stat --compact-summary initial mode
 diff-tree -R --stat --compact-summary initial mode
 EOF
diff --git a/t/t4013/diff.diff-tree_--format=%N_note b/t/t4013/diff.diff-tree_--format=%N_note
new file mode 100644
index 0000000000..93042ed539
--- /dev/null
+++ b/t/t4013/diff.diff-tree_--format=%N_note
@@ -0,0 +1,6 @@
+$ git diff-tree --format=%N note
+note
+
+
+:100644 100755 01e79c32a8c99c557f0757da7cb6d65b3414466d 01e79c32a8c99c557f0757da7cb6d65b3414466d M	file2
+$
diff --git a/t/t4013/diff.diff-tree_--pretty_--notes_note b/t/t4013/diff.diff-tree_--pretty_--notes_note
new file mode 100644
index 0000000000..4d0bde601c
--- /dev/null
+++ b/t/t4013/diff.diff-tree_--pretty_--notes_note
@@ -0,0 +1,12 @@
+$ git diff-tree --pretty --notes note
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
+:100644 100755 01e79c32a8c99c557f0757da7cb6d65b3414466d 01e79c32a8c99c557f0757da7cb6d65b3414466d M	file2
+$
diff --git a/t/t4013/diff.diff-tree_--pretty_note b/t/t4013/diff.diff-tree_--pretty_note
new file mode 100644
index 0000000000..1fa5967083
--- /dev/null
+++ b/t/t4013/diff.diff-tree_--pretty_note
@@ -0,0 +1,9 @@
+$ git diff-tree --pretty note
+commit a6f364368ca320bc5a92e18912e16fa6b3dff598
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+:100644 100755 01e79c32a8c99c557f0757da7cb6d65b3414466d 01e79c32a8c99c557f0757da7cb6d65b3414466d M	file2
+$
diff --git a/t/t4013/diff.log_--decorate=full_--all b/t/t4013/diff.log_--decorate=full_--all
index 2afe91f116..3f9b872ece 100644
--- a/t/t4013/diff.log_--decorate=full_--all
+++ b/t/t4013/diff.log_--decorate=full_--all
@@ -5,12 +5,27 @@ Date:   Mon Jun 26 00:06:00 2006 +0000

     update mode

+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (refs/heads/note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
 commit cd4e72fd96faed3f0ba949dc42967430374e2290 (refs/heads/rearrange)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:06:00 2006 +0000

     Rearranged lines in dir/sub

+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
 commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> refs/heads/master)
 Merge: 9a6d494 c7a2ab9
 Author: A U Thor <author@example.com>
diff --git a/t/t4013/diff.log_--decorate_--all b/t/t4013/diff.log_--decorate_--all
index d0f308ab2b..f5e20e1e14 100644
--- a/t/t4013/diff.log_--decorate_--all
+++ b/t/t4013/diff.log_--decorate_--all
@@ -5,12 +5,27 @@ Date:   Mon Jun 26 00:06:00 2006 +0000

     update mode

+commit a6f364368ca320bc5a92e18912e16fa6b3dff598 (note)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    update mode (file2)
+
+Notes:
+    note
+
 commit cd4e72fd96faed3f0ba949dc42967430374e2290 (rearrange)
 Author: A U Thor <author@example.com>
 Date:   Mon Jun 26 00:06:00 2006 +0000

     Rearranged lines in dir/sub

+commit cbacedd14cb8b89255a2c02b59e77a2e9a8021a0 (refs/notes/commits)
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:06:00 2006 +0000
+
+    Notes added by 'git notes add'
+
 commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (HEAD -> master)
 Merge: 9a6d494 c7a2ab9
 Author: A U Thor <author@example.com>
--
2.26.2.108.g048abe1751
Previous: Jeff KingNext: Jeff King
Message 7 of 8 in “diff-tree.c: load notes machinery when required”
  1. diff-tree.c: load notes machinery when requiredTaylor Blau, Apr 16, 2020
  2. Junio C HamanoApr 16, 2020
  3. Jeff KingApr 16, 2020
  4. Taylor BlauApr 21, 2020
  5. Taylor BlauApr 21, 2020
  6. Jeff KingApr 21, 2020
  7. Taylor BlauApr 21, 2020
  8. Jeff KingApr 21, 2020

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.