format-patch: no 'prerequisite-patch-id' info when specifying commit range

> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
> `--cover-letter` or using `Z..C` instead of `-3 C` to specify the
> range), the base tree information block is shown at the end of the
> first message the command outputs (either the first patch, or the
> cover letter), like this:
> 
> ------------
> base-commit: P
> prerequisite-patch-id: X
> prerequisite-patch-id: Y
> prerequisite-patch-id: Z
> ------------

>Hi,
>
>I'm trying to use git-format-patch --base to generate the list of
>prerequisite patches for a series, but the behavior of git
>doesn't seem to match the documentation:
>
>When using a commit count (e.g.: "-2"), git-format-patch generates the
>prerequisite-patch-id lines as expected.  But when using a commit range like
>"Z..C", the prerequisite-patch-id lines are missing.
>
>Is this intentional, or it is a bug?

>
>Example using git.git commits:
>
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 -2 b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  prerequisite-patch-id: 080ac2faf21a6a7f9b23cb68286866d026a92930
>  prerequisite-patch-id: e3ee77500c9aa70248e7ee814662d01f79d0dcdb
>  prerequisite-patch-id: 6d831e23e33075681e6b74553151a32b73092013
>  (ehabkost@localhost:~/rh/proj/git (ok) 1j)
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 b7b1fca17~2..b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  $ git --version
>  git version 2.17.1
>  $ git log --graph --pretty=oneline -6 b7b1fca17
>  * b7b1fca175f1ed7933f361028c631b9ac86d868d fsck: complain when .gitmodules is a symlink
>  * 73c3f0f704a91b6792e0199a3f3ab6e3a1971675 index-pack: check .gitmodules files with --strict
>  * 6e328d6caef218db320978e3e251009135d87d0e unpack-objects: call fsck_finish() after fscking objects
>  * 1995b5e03e1cc97116be58cdc0502d4a23547856 fsck: call fsck_finish() after fscking objects
>  * ed8b10f631c9a71df3351d46187bf7f3fa4f9b7e fsck: check .gitmodules content
>  * 2738744426c161a98c2ec494d41241a4c5eef9ef fsck: handle promisor objects in .gitmodules check
>  $ 
>
>If I understand the documentation correctly, both "-3 C" or "Z..C" were
>supposed to be equivalent:
>
>> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
>> `--cover-letter` or using `Z..C` instead of `-3 C` to specify the
>> range), the base tree information block is shown at the end of the
>> first message the command outputs (either the first patch, or the
>> cover letter), like this:
>> 
>> ------------
>> base-commit: P
>> prerequisite-patch-id: X
>> prerequisite-patch-id: Y
>> prerequisite-patch-id: Z
>> ------------
>
>-- 
>Eduardo

>Hi,
>
>I'm trying to use git-format-patch --base to generate the list of
>prerequisite patches for a series, but the behavior of git
>doesn't seem to match the documentation:
>
>When using a commit count (e.g.: "-2"), git-format-patch generates the
>prerequisite-patch-id lines as expected.  But when using a commit range like
>"Z..C", the prerequisite-patch-id lines are missing.
>

>Is this intentional, or it is a bug?
>
>Example using git.git commits:
>
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 -2 b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  prerequisite-patch-id: 080ac2faf21a6a7f9b23cb68286866d026a92930
>  prerequisite-patch-id: e3ee77500c9aa70248e7ee814662d01f79d0dcdb
>  prerequisite-patch-id: 6d831e23e33075681e6b74553151a32b73092013
>  (ehabkost@localhost:~/rh/proj/git (ok) 1j)
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 b7b1fca17~2..b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  $ git --version
>  git version 2.17.1
>  $ git log --graph --pretty=oneline -6 b7b1fca17
>  * b7b1fca175f1ed7933f361028c631b9ac86d868d fsck: complain when .gitmodules is a symlink
>  * 73c3f0f704a91b6792e0199a3f3ab6e3a1971675 index-pack: check .gitmodules files with --strict
>  * 6e328d6caef218db320978e3e251009135d87d0e unpack-objects: call fsck_finish() after fscking objects
>  * 1995b5e03e1cc97116be58cdc0502d4a23547856 fsck: call fsck_finish() after fscking objects
>  * ed8b10f631c9a71df3351d46187bf7f3fa4f9b7e fsck: check .gitmodules content
>  * 2738744426c161a98c2ec494d41241a4c5eef9ef fsck: handle promisor objects in .gitmodules check
>  $ 
>
>If I understand the documentation correctly, both "-3 C" or "Z..C" were
>supposed to be equivalent:
>
>> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
>> `--cover-letter` or using `Z..C` instead of `-3 C` to specify the
>> range), the base tree information block is shown at the end of the
>> first message the command outputs (either the first patch, or the
>> cover letter), like this:
>> 
>> ------------
>> base-commit: P
>> prerequisite-patch-id: X
>> prerequisite-patch-id: Y
>> prerequisite-patch-id: Z
>> ------------
>
>-- 
>Eduardo

> I narrowed down the problem to revision walk, if users specify the commit range
> via "Z..C" pattern, the first prepare_revision_walk function called in
> cmd_format_patch would mark all parents (ancestors) of Z to be uninteresting,
> thus the next revision walk in prepare_bases wouldn't be able to reach
> prerequisite patches, one quick solution I can think of is to clear
> UNINTERESTING flag in reset_revision_walk, like below:
>
> void reset_revision_walk(void)
> {
> 	clear_object_flags(SEEN | ADDED | SHOWN| UNINTERESTING);
> }

>Ye Xiaolong <xiaolong.ye@intel.com> writes:
>
>> I narrowed down the problem to revision walk, if users specify the commit range
>> via "Z..C" pattern, the first prepare_revision_walk function called in
>> cmd_format_patch would mark all parents (ancestors) of Z to be uninteresting,
>> thus the next revision walk in prepare_bases wouldn't be able to reach
>> prerequisite patches, one quick solution I can think of is to clear
>> UNINTERESTING flag in reset_revision_walk, like below:
>>
>> void reset_revision_walk(void)
>> {
>> 	clear_object_flags(SEEN | ADDED | SHOWN| UNINTERESTING);
>> }
>
>When you are done with objects that are UNINTERESTING in your
>application (i.e. only when "format-patch" is told to compute list
>of prereq patches by doing an extra revision walk), your application
>can call clear_object_flags() on the flags you are done with, I
>would think.
>
>But the current callers of reset_revision_walk() do not expect any
>flags other than the ones that are used to keep track of the
>traversal state, so it is likely you will break them if you suddenly
>started to clear flags randomly.

> When users specify the commit range with 'Z..C' pattern for format-patch, all
> the parents of Z (including Z) would be marked as UNINTERESTING which would
> prevent revision walk in prepare_bases from getting the prerequisite commits,
> thus `git format-patch --base <base_commit_sha> Z..C` won't be able to generate
> the list of prerequisite patch ids. Clear UNINTERESTING flag with
> clear_object_flags solves this issue.
> 
> Reported-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

>When users specify the commit range with 'Z..C' pattern for format-patch, all
>the parents of Z (including Z) would be marked as UNINTERESTING which would
>prevent revision walk in prepare_bases from getting the prerequisite commits,
>thus `git format-patch --base <base_commit_sha> Z..C` won't be able to generate
>the list of prerequisite patch ids. Clear UNINTERESTING flag with
>clear_object_flags solves this issue.
>
>Reported-by: Eduardo Habkost <ehabkost@redhat.com>
>Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>---
> builtin/log.c           | 1 +
> t/t4014-format-patch.sh | 6 ++++--
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/builtin/log.c b/builtin/log.c
>index 4686f68594..01993de6fe 100644
>--- a/builtin/log.c
>+++ b/builtin/log.c
>@@ -1746,6 +1746,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> 	if (base_commit || base_auto) {
> 		struct commit *base = get_base_commit(base_commit, list, nr);
> 		reset_revision_walk();
>+		clear_object_flags(UNINTERESTING);
> 		prepare_bases(&bases, base, list, nr);
> 	}
> 
>diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>index 028d5507a6..53880da7bb 100755
>--- a/t/t4014-format-patch.sh
>+++ b/t/t4014-format-patch.sh
>@@ -1554,13 +1554,15 @@ test_expect_success 'format-patch -o overrides format.outputDirectory' '
> 
> test_expect_success 'format-patch --base' '
> 	git checkout side &&
>-	git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual &&
>+	git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual1 &&
>+	git format-patch --stdout --base=HEAD~3 HEAD~.. | tail -n 7 >actual2 &&
> 	echo >expected &&
> 	echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
> 	echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --stable | awk "{print \$1}")" >>expected &&
> 	echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --stable | awk "{print \$1}")" >>expected &&
> 	signature >> expected &&
>-	test_cmp expected actual
>+	test_cmp expected actual1 &&
>+	test_cmp expected actual2
> '
> 
> test_expect_success 'format-patch --base errors out when base commit is in revision list' '
>-- 
>2.16.GIT
>

>
> When users specify the commit range with 'Z..C' pattern for format-patch, all
> the parents of Z (including Z) would be marked as UNINTERESTING which would
> prevent revision walk in prepare_bases from getting the prerequisite commits,
> thus `git format-patch --base <base_commit_sha> Z..C` won't be able to generate
> the list of prerequisite patch ids. Clear UNINTERESTING flag with
> clear_object_flags solves this issue.