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

2018-06-03 Thread Ye Xiaolong
On 06/04, Junio C Hamano wrote:
>Ye Xiaolong  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.

Got it, I'll try to call clear_object_flags in format-patch related codepatch
only, not to touch the global reset_revision_walk.

Thanks,
Xiaolong


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

2018-06-03 Thread Junio C Hamano
Ye Xiaolong  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.


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

2018-06-03 Thread Ye Xiaolong
Hi, Junio

On 05/29, Eduardo Habkost wrote:
>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.
>

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);
}

Though I'm not sure whether it has some side effects, or whether it would impact
behavior of other reference of reset_revision_walk. If you think it's a sensible
solution, I'll submit a patch.

Thanks,
Xiaolong


>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


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

2018-05-30 Thread Ye Xiaolong
Hi, Eduardo

On 05/29, Eduardo Habkost wrote:
>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?

Thanks for reporting, it seems an unexpected behavior, I'll look into it.

Thanks,
Xiaolong

>
>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