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

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> 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>
> ---
> This is the remainder of the fix that I started earlier today, with some
> additional suggestions from Peff incorporated.

Yeah, with the auto-detection of userformat requirement, the fix
looks good to me, too.

Will queue.  Thanks.

>
>  builtin/diff-tree.c                          |  9 +++++++++
>  t/t4013-diff-various.sh                      | 11 +++++++++++
>  t/t4013/diff.diff-tree_--format=%N_note      |  6 ++++++
>  t/t4013/diff.diff-tree_--pretty_--notes_note | 12 ++++++++++++
>  t/t4013/diff.log_--decorate=full_--all       | 15 +++++++++++++++
>  t/t4013/diff.log_--decorate_--all            | 15 +++++++++++++++
>  6 files changed, 68 insertions(+)
>  create mode 100644 t/t4013/diff.diff-tree_--format=%N_note
>  create mode 100644 t/t4013/diff.diff-tree_--pretty_--notes_note
>
> diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
> index cb9ea79367..11551a20cc 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 && (!opt->pretty_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..4263b95ca6 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,8 @@ 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 --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.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.0.121.gefe3874640.dirty