[v3,1/2] parsemail.py: Avoid skipping patches when parsing

Avoids some email patch notifications to be wrongly
 skipped when the pull URL includes the branch.

An example of an email patch notification that would
be skipped can be found in [1].

[1] http://patchwork.openembedded.org/patch/96385/

Signed-off-by: Jose Lamego <jose.a.lamego@linux.intel.com>
---
 patchwork/bin/parsemail.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Jose,

On Thu, Sep 24, 2015 at 12:09:57PM -0500, Jose Lamego wrote:
> Avoids some email patch notifications to be wrongly
>  skipped when the pull URL includes the branch.
> 
> An example of an email patch notification that would
> be skipped can be found in [1].
> 
> [1] http://patchwork.openembedded.org/patch/96385/
> 
> Signed-off-by: Jose Lamego <jose.a.lamego@linux.intel.com>

I'm not quite sure why this pull request was skipped, but the commit
message doesn't seem to be the reason and [^\n]+$ matches everything
until the end of the line, which includes the branch name.

I doubled checked that:
  - Your test still passes with this commit reverted,
  - patchwork/tests/mail/0003-git-pull-request-with-diff.mbox and
    GitPullWithDiffTest test a pull URL with a branch name already,
  - The current version of find_pull_request() work on the example
    above. Here is the interactive session:

	>>> content = '''
	... The following changes since commit 968973d55d4b33e1a929ed4cdf9387fcaba2d93f:
	... 
	...   qt4: unconditionally disable gstreamer 0.10 support in qt webkit (2015-05-30 22:25:12 +0100)
	... 
	... are available in the git repository at:
	... 
	...   git://git.openembedded.org/openembedded-core-contrib rbt/PU
	...   http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/PU
	... 
	... Robert Yang (6):
	...   liberror-perl: 0.17023 -> 0.17024
	...   python-mako: 0.9.1 -> 1.0.1
	...   python-nose: 1.2.1 -> 1.3.6
	...   debianutils: 4.5 -> 4.5.1
	...   ethtool: 3.16 -> 4.0
	...   glib-2.0: 2.44.0 -> 2.44.1
	... '''
	>>> from patchwork.bin.parsemail import find_pull_request
	>>> find_pull_request(content)
	'git://git.openembedded.org/openembedded-core-contrib rbt/PU'

So I don't think this commit fixes anything. There must be another
explanation?

HTH,

Hi Damien,

Glad you verified this, let me share my findings: I checked the original
patch [2] that included the following changes but that I wrongly cut
after finding that they collide with testGitPullWithDiff test in upstream:

     def find_pull_request(content):
        git_re = re.compile('^The following changes since commit.*' +
    - '^are available in the git repository at:\n'
    - '^\s*([\S]+://[^\n]+)$',
    + '^are available in the git repository at.*:'
    + '^\s*([\S]+://[^\n]+)',
         re.DOTALL | re.MULTILINE)
        match = git_re.search(content)
	if match:

this patch is currently implemented both in our Open Embedded Patchwork
[3] and in a staging instance that I use for tests. Both these instances
are running Patchwork 8904a7dcaf959da8db4a9a5d92b91a61eed05201

I just verified that the staging instance do skip this same email if I
revert parsemail.py to:
	'^are available in the git repository at:\n'

I previously had verified that some messages were skipped when
parsemail.py contained:
	'^\s*([\S]+://[^\n]+)$'
but unfortunately I couldn't find an example message right now.

I'm assuming that this behavior is caused by something (maybe
parser.py?) that is already fixed/updated in patchwork's upstream, but
not in our version, then the patch is still needed for us.

so, as you already pointed out, this patch can be superseded as it is
actually not needed when using latest patchwork version.

This also adds to the list of why we need to upgrade our patchwork.

Sorry for the long message, but I figured that this would be of interest
for you.

Regards
Jose


[2] https://patchwork.ozlabs.org/patch/510090/
[3] http://patchwork.openembedded.org/



On 26/10/15 06:32, Damien Lespiau wrote:
> Hi Jose,
> 
> On Thu, Sep 24, 2015 at 12:09:57PM -0500, Jose Lamego wrote:
>> Avoids some email patch notifications to be wrongly skipped when
>> the pull URL includes the branch.
>> 
>> An example of an email patch notification that would be skipped can
>> be found in [1].
>> 
>> [1] http://patchwork.openembedded.org/patch/96385/
>> 
>> Signed-off-by: Jose Lamego <jose.a.lamego@linux.intel.com>
> 
> I'm not quite sure why this pull request was skipped, but the commit 
> message doesn't seem to be the reason and [^\n]+$ matches everything 
> until the end of the line, which includes the branch name.
> 
> I doubled checked that: - Your test still passes with this commit
> reverted, - patchwork/tests/mail/0003-git-pull-request-with-diff.mbox
> and GitPullWithDiffTest test a pull URL with a branch name already, -
> The current version of find_pull_request() work on the example above.
> Here is the interactive session:
> 
> >>> content = ''' ... The following changes since commit
> 968973d55d4b33e1a929ed4cdf9387fcaba2d93f: ... ...   qt4:
> unconditionally disable gstreamer 0.10 support in qt webkit
> (2015-05-30 22:25:12 +0100) ... ... are available in the git
> repository at: ... ...
> git://git.openembedded.org/openembedded-core-contrib rbt/PU ...
> http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/PU
>
> 
...
> ... Robert Yang (6): ...   liberror-perl: 0.17023 -> 0.17024 ...
> python-mako: 0.9.1 -> 1.0.1 ...   python-nose: 1.2.1 -> 1.3.6 ...
> debianutils: 4.5 -> 4.5.1 ...   ethtool: 3.16 -> 4.0 ...   glib-2.0:
> 2.44.0 -> 2.44.1 ... ''' >>> from patchwork.bin.parsemail import
> find_pull_request >>> find_pull_request(content) 
> 'git://git.openembedded.org/openembedded-core-contrib rbt/PU'
> 
> So I don't think this commit fixes anything. There must be another 
> explanation?
> 
> HTH,
>