* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 4:46 ` Daniel Axtens
2020-07-06 3:41 ` [RFC PATCH 02/16] patman: Move main code out to a control module Simon Glass
` (24 subsequent siblings)
25 siblings, 1 reply; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
This handles skipped tests correctly, so use it instead of the existing
code.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/main.py | 8 ++------
tools/patman/test_util.py | 6 +++---
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 28a9a26087..03668d1bb8 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -25,6 +25,7 @@ from patman import patchstream
from patman import project
from patman import settings
from patman import terminal
+from patman import test_util
from patman import test_checkpatch
@@ -101,12 +102,7 @@ elif options.test:
suite = doctest.DocTestSuite(module)
suite.run(result)
- # TODO: Surely we can just 'print' result?
- print(result)
- for test, err in result.errors:
- print(err)
- for test, err in result.failures:
- print(err)
+ sys.exit(test_util.ReportResult('patman', None, result))
# Called from git with a patch filename as argument
# Printout a list of additional CC recipients for this patch
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index aac58fb72f..0827488f33 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult):
for test, err in result.failures:
print(err, result.failures)
if result.skipped:
- print('%d binman test%s SKIPPED:' %
- (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
+ print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
+ 's' if len(result.skipped) > 1 else ''))
for skip_info in result.skipped:
print('%s: %s' % (skip_info[0], skip_info[1]))
if result.errors or result.failures:
- print('binman tests FAILED')
+ print('%s tests FAILED' % toolname)
return 1
return 0
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-06 3:41 ` [RFC PATCH 01/16] patman: Use test_util to show test results Simon Glass
@ 2020-07-06 4:46 ` Daniel Axtens
2020-07-06 4:50 ` Daniel Axtens
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Axtens @ 2020-07-06 4:46 UTC (permalink / raw)
To: u-boot
Hi Simon,
I can't see a cover letter so apologies if I've misunderstood something
basic, but this doesn't appear to apply to the patchwork tree - I'm
guessing the patchwork relevance is with regards to the last few patches
that (AFAICT) parse the patchwork web interface for information?
I haven't fully digested the patches (and I lack a lot of context) but
is there a reason the patchwork API isn't able to meet your needs here?
(And if so, could we extend it rather than having you parse the frontend?)
Regards,
Daniel
> This handles skipped tests correctly, so use it instead of the existing
> code.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/patman/main.py | 8 ++------
> tools/patman/test_util.py | 6 +++---
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/tools/patman/main.py b/tools/patman/main.py
> index 28a9a26087..03668d1bb8 100755
> --- a/tools/patman/main.py
> +++ b/tools/patman/main.py
> @@ -25,6 +25,7 @@ from patman import patchstream
> from patman import project
> from patman import settings
> from patman import terminal
> +from patman import test_util
> from patman import test_checkpatch
>
>
> @@ -101,12 +102,7 @@ elif options.test:
> suite = doctest.DocTestSuite(module)
> suite.run(result)
>
> - # TODO: Surely we can just 'print' result?
> - print(result)
> - for test, err in result.errors:
> - print(err)
> - for test, err in result.failures:
> - print(err)
> + sys.exit(test_util.ReportResult('patman', None, result))
>
> # Called from git with a patch filename as argument
> # Printout a list of additional CC recipients for this patch
> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> index aac58fb72f..0827488f33 100644
> --- a/tools/patman/test_util.py
> +++ b/tools/patman/test_util.py
> @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult):
> for test, err in result.failures:
> print(err, result.failures)
> if result.skipped:
> - print('%d binman test%s SKIPPED:' %
> - (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
> + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
> + 's' if len(result.skipped) > 1 else ''))
> for skip_info in result.skipped:
> print('%s: %s' % (skip_info[0], skip_info[1]))
> if result.errors or result.failures:
> - print('binman tests FAILED')
> + print('%s tests FAILED' % toolname)
> return 1
> return 0
>
> --
> 2.27.0.212.ge8ba1cc988-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-06 4:46 ` Daniel Axtens
@ 2020-07-06 4:50 ` Daniel Axtens
2020-07-06 14:52 ` Simon Glass
0 siblings, 1 reply; 33+ messages in thread
From: Daniel Axtens @ 2020-07-06 4:50 UTC (permalink / raw)
To: u-boot
Daniel Axtens <dja@axtens.net> writes:
> Hi Simon,
>
> I can't see a cover letter so apologies if I've misunderstood something
> basic, but this doesn't appear to apply to the patchwork tree - I'm
> guessing the patchwork relevance is with regards to the last few patches
> that (AFAICT) parse the patchwork web interface for information?
Ah, nevermind, the cover letter got caught in moderation. I've released it.
pwclient speaks the old, less documented XML-RPC API. We have a new
REST API which is much better documented, and is explorable
(e.g. https://patchwork.ozlabs.org/api/ and
https://patchwork.readthedocs.io/en/latest/api/rest/ )
> I haven't fully digested the patches (and I lack a lot of context) but
> is there a reason the patchwork API isn't able to meet your needs here?
> (And if so, could we extend it rather than having you parse the frontend?)
So these questions still stand but for the REST API.
Regards,
Daniel
>
> Regards,
> Daniel
>
>> This handles skipped tests correctly, so use it instead of the existing
>> code.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> tools/patman/main.py | 8 ++------
>> tools/patman/test_util.py | 6 +++---
>> 2 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/patman/main.py b/tools/patman/main.py
>> index 28a9a26087..03668d1bb8 100755
>> --- a/tools/patman/main.py
>> +++ b/tools/patman/main.py
>> @@ -25,6 +25,7 @@ from patman import patchstream
>> from patman import project
>> from patman import settings
>> from patman import terminal
>> +from patman import test_util
>> from patman import test_checkpatch
>>
>>
>> @@ -101,12 +102,7 @@ elif options.test:
>> suite = doctest.DocTestSuite(module)
>> suite.run(result)
>>
>> - # TODO: Surely we can just 'print' result?
>> - print(result)
>> - for test, err in result.errors:
>> - print(err)
>> - for test, err in result.failures:
>> - print(err)
>> + sys.exit(test_util.ReportResult('patman', None, result))
>>
>> # Called from git with a patch filename as argument
>> # Printout a list of additional CC recipients for this patch
>> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
>> index aac58fb72f..0827488f33 100644
>> --- a/tools/patman/test_util.py
>> +++ b/tools/patman/test_util.py
>> @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult):
>> for test, err in result.failures:
>> print(err, result.failures)
>> if result.skipped:
>> - print('%d binman test%s SKIPPED:' %
>> - (len(result.skipped), 's' if len(result.skipped) > 1 else ''))
>> + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname,
>> + 's' if len(result.skipped) > 1 else ''))
>> for skip_info in result.skipped:
>> print('%s: %s' % (skip_info[0], skip_info[1]))
>> if result.errors or result.failures:
>> - print('binman tests FAILED')
>> + print('%s tests FAILED' % toolname)
>> return 1
>> return 0
>>
>> --
>> 2.27.0.212.ge8ba1cc988-goog
>>
>> _______________________________________________
>> Patchwork mailing list
>> Patchwork at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/patchwork
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-06 4:50 ` Daniel Axtens
@ 2020-07-06 14:52 ` Simon Glass
2020-07-07 1:09 ` Daniel Axtens
0 siblings, 1 reply; 33+ messages in thread
From: Simon Glass @ 2020-07-06 14:52 UTC (permalink / raw)
To: u-boot
Hi Daniel,
On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja@axtens.net> wrote:
>
> Daniel Axtens <dja@axtens.net> writes:
>
> > Hi Simon,
> >
> > I can't see a cover letter so apologies if I've misunderstood something
> > basic, but this doesn't appear to apply to the patchwork tree - I'm
> > guessing the patchwork relevance is with regards to the last few patches
> > that (AFAICT) parse the patchwork web interface for information?
>
> Ah, nevermind, the cover letter got caught in moderation. I've released it.
>
> pwclient speaks the old, less documented XML-RPC API. We have a new
> REST API which is much better documented, and is explorable
> (e.g. https://patchwork.ozlabs.org/api/ and
> https://patchwork.readthedocs.io/en/latest/api/rest/ )
>
> > I haven't fully digested the patches (and I lack a lot of context) but
> > is there a reason the patchwork API isn't able to meet your needs here?
> > (And if so, could we extend it rather than having you parse the frontend?)
>
> So these questions still stand but for the REST API.
So use the REST API instead of the web page? That sounds fine to me.
Is it generally enabled on patchwork servers?
What is the status of pwclient? Is it dead? Is there a replacement?
I'd love to use a Python library if one exists.
Regards,
SImon
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-06 14:52 ` Simon Glass
@ 2020-07-07 1:09 ` Daniel Axtens
2020-07-08 17:42 ` Stephen Finucane
2020-07-19 20:49 ` Simon Glass
0 siblings, 2 replies; 33+ messages in thread
From: Daniel Axtens @ 2020-07-07 1:09 UTC (permalink / raw)
To: u-boot
Simon Glass <sjg@chromium.org> writes:
> Hi Daniel,
>
> On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja@axtens.net> wrote:
>>
>> Daniel Axtens <dja@axtens.net> writes:
>>
>> > Hi Simon,
>> >
>> > I can't see a cover letter so apologies if I've misunderstood something
>> > basic, but this doesn't appear to apply to the patchwork tree - I'm
>> > guessing the patchwork relevance is with regards to the last few patches
>> > that (AFAICT) parse the patchwork web interface for information?
>>
>> Ah, nevermind, the cover letter got caught in moderation. I've released it.
>>
>> pwclient speaks the old, less documented XML-RPC API. We have a new
>> REST API which is much better documented, and is explorable
>> (e.g. https://patchwork.ozlabs.org/api/ and
>> https://patchwork.readthedocs.io/en/latest/api/rest/ )
>>
>> > I haven't fully digested the patches (and I lack a lot of context) but
>> > is there a reason the patchwork API isn't able to meet your needs here?
>> > (And if so, could we extend it rather than having you parse the frontend?)
>>
>> So these questions still stand but for the REST API.
>
> So use the REST API instead of the web page? That sounds fine to me.
> Is it generally enabled on patchwork servers?
I mean patman is your code so it's ultimately not my call :P But yes,
I'd strongly prefer you used the REST API! It is enabled on ozlabs.org
and kernel.org and has been for a while (~a couple of years).
> What is the status of pwclient? Is it dead? Is there a replacement?
> I'd love to use a Python library if one exists.
Stephen F is the expert on the client stuff, so I'm not going to make a
call on the status of pwclient. All I am confident to say is that I have
migrated to using 'git-pw' and I recommend others do so to too.
I'm not sure if a dedicated Python client library exists: the last time
I wanted to write a Python client app I found it simple enough to just
use the JSON that the API provides directly. But the place I'd start
with is git-pw.
Regards,
Daniel
> Regards,
> SImon
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-07 1:09 ` Daniel Axtens
@ 2020-07-08 17:42 ` Stephen Finucane
2020-07-19 20:49 ` Simon Glass
1 sibling, 0 replies; 33+ messages in thread
From: Stephen Finucane @ 2020-07-08 17:42 UTC (permalink / raw)
To: u-boot
On Tue, 2020-07-07 at 11:09 +1000, Daniel Axtens wrote:
> Simon Glass <sjg@chromium.org> writes:
>
> > Hi Daniel,
> >
> > On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja@axtens.net> wrote:
> > > Daniel Axtens <dja@axtens.net> writes:
> > >
> > > > Hi Simon,
> > > >
> > > > I can't see a cover letter so apologies if I've misunderstood something
> > > > basic, but this doesn't appear to apply to the patchwork tree - I'm
> > > > guessing the patchwork relevance is with regards to the last few patches
> > > > that (AFAICT) parse the patchwork web interface for information?
> > >
> > > Ah, nevermind, the cover letter got caught in moderation. I've released it.
> > >
> > > pwclient speaks the old, less documented XML-RPC API. We have a new
> > > REST API which is much better documented, and is explorable
> > > (e.g. https://patchwork.ozlabs.org/api/ and
> > > https://patchwork.readthedocs.io/en/latest/api/rest/ )
> > >
> > > > I haven't fully digested the patches (and I lack a lot of context) but
> > > > is there a reason the patchwork API isn't able to meet your needs here?
> > > > (And if so, could we extend it rather than having you parse the frontend?)
> > >
> > > So these questions still stand but for the REST API.
> >
> > So use the REST API instead of the web page? That sounds fine to me.
> > Is it generally enabled on patchwork servers?
>
> I mean patman is your code so it's ultimately not my call :P But yes,
> I'd strongly prefer you used the REST API! It is enabled on ozlabs.org
> and kernel.org and has been for a while (~a couple of years).
>
> > What is the status of pwclient? Is it dead? Is there a replacement?
> > I'd love to use a Python library if one exists.
>
> Stephen F is the expert on the client stuff, so I'm not going to make a
> call on the status of pwclient. All I am confident to say is that I have
> migrated to using 'git-pw' and I recommend others do so to too.
pwclient is alive and won't be going anywhere, but it's still using the
now-deprecated XML-RPC API. Eventually this will move to the REST API,
but I just haven't found the time to do so yet.
> I'm not sure if a dedicated Python client library exists: the last time
> I wanted to write a Python client app I found it simple enough to just
> use the JSON that the API provides directly. But the place I'd start
> with is git-pw.
There's no patchwork SDK yet and git-pw is intended for use as a tool
rather than a library (the internal API isn't stable). However, the
Patchwork API itself is very trivial so requests should get you what
you want very easily.
Stephen
> Regards,
> Daniel
>
> > Regards,
> > SImon
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 01/16] patman: Use test_util to show test results
2020-07-07 1:09 ` Daniel Axtens
2020-07-08 17:42 ` Stephen Finucane
@ 2020-07-19 20:49 ` Simon Glass
1 sibling, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
On Tue, 2020-07-07 at 11:09 +1000, Daniel Axtens wrote:
> Simon Glass <sjg@chromium.org> writes:
>
> > Hi Daniel,
> >
> > On Sun, 5 Jul 2020 at 22:50, Daniel Axtens <dja@axtens.net> wrote:
> > > Daniel Axtens <dja@axtens.net> writes:
> > >
> > > > Hi Simon,
> > > >
> > > > I can't see a cover letter so apologies if I've misunderstood something
> > > > basic, but this doesn't appear to apply to the patchwork tree - I'm
> > > > guessing the patchwork relevance is with regards to the last few patches
> > > > that (AFAICT) parse the patchwork web interface for information?
> > >
> > > Ah, nevermind, the cover letter got caught in moderation. I've released it.
> > >
> > > pwclient speaks the old, less documented XML-RPC API. We have a new
> > > REST API which is much better documented, and is explorable
> > > (e.g. https://patchwork.ozlabs.org/api/ and
> > > https://patchwork.readthedocs.io/en/latest/api/rest/ )
> > >
> > > > I haven't fully digested the patches (and I lack a lot of context) but
> > > > is there a reason the patchwork API isn't able to meet your needs here?
> > > > (And if so, could we extend it rather than having you parse the frontend?)
> > >
> > > So these questions still stand but for the REST API.
> >
> > So use the REST API instead of the web page? That sounds fine to me.
> > Is it generally enabled on patchwork servers?
>
> I mean patman is your code so it's ultimately not my call :P But yes,
> I'd strongly prefer you used the REST API! It is enabled on ozlabs.org
> and kernel.org and has been for a while (~a couple of years).
>
> > What is the status of pwclient? Is it dead? Is there a replacement?
> > I'd love to use a Python library if one exists.
>
> Stephen F is the expert on the client stuff, so I'm not going to make a
> call on the status of pwclient. All I am confident to say is that I have
> migrated to using 'git-pw' and I recommend others do so to too.
pwclient is alive and won't be going anywhere, but it's still using the
now-deprecated XML-RPC API. Eventually this will move to the REST API,
but I just haven't found the time to do so yet.
> I'm not sure if a dedicated Python client library exists: the last time
> I wanted to write a Python client app I found it simple enough to just
> use the JSON that the API provides directly. But the place I'd start
> with is git-pw.
There's no patchwork SDK yet and git-pw is intended for use as a tool
rather than a library (the internal API isn't stable). However, the
Patchwork API itself is very trivial so requests should get you what
you want very easily.
Stephen
> Regards,
> Daniel
>
> > Regards,
> > SImon
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC PATCH 02/16] patman: Move main code out to a control module
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
2020-07-06 3:41 ` [RFC PATCH 01/16] patman: Use test_util to show test results Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 03/16] patman: Add a test that uses gitpython Simon Glass
` (23 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
To make testing easier, move the code out from main into a separate
'control' module and split it into four parts: setup, preparing patches,
checking patches and emailing patches.
Add comments and fix a few code-style issues while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/checkpatch.py | 6 ++
tools/patman/control.py | 170 +++++++++++++++++++++++++++++++++++++
tools/patman/gitutil.py | 4 +-
tools/patman/main.py | 57 +------------
tools/patman/series.py | 2 +-
5 files changed, 182 insertions(+), 57 deletions(-)
create mode 100644 tools/patman/control.py
diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 07c3e2739a..263bac3fc9 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -41,6 +41,12 @@ def FindCheckPatch():
def CheckPatch(fname, verbose=False, show_types=False):
"""Run checkpatch.pl on a file.
+ Args:
+ fname: Filename to check
+ verbose: True to print out every line of the checkpatch output as it is
+ parsed
+ show_types: Tell checkpatch to show the type (number) of each message
+
Returns:
namedtuple containing:
ok: False=failure, True=ok
diff --git a/tools/patman/control.py b/tools/patman/control.py
new file mode 100644
index 0000000000..a896c924b5
--- /dev/null
+++ b/tools/patman/control.py
@@ -0,0 +1,170 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2020 Google LLC
+#
+"""Handles the main control logic of patman
+
+This module provides various functions called by the main program to implement
+the features of patman.
+"""
+
+import os
+import sys
+
+from patman import checkpatch
+from patman import gitutil
+from patman import patchstream
+from patman import terminal
+
+def setup():
+ """Do required setup before doing anything"""
+ gitutil.Setup()
+
+def prepare_patches(col, count, start, ignore_binary):
+ """Figure out what patches to generate, then generate them
+
+ The patch files are written to the current directory, e.g. 0001_xxx.patch
+ 0002_yyy.patch
+
+ Args:
+ col (terminal.Color): Colour output object
+ count (int): Number of patches to produce, or -1 to produce patches for
+ the current branch back to the upstream commit
+ start (int): Start partch to use (0=first / top of branch)
+ ignore_binary (bool): Don't generate patches for binary files
+
+ Returns:
+ Tuple:
+ Series object for this series (set of patches)
+ Filename of the cover letter as a string (None if none)
+ patch_files: List of patch filenames, each a string, e.g.
+ ['0001_xxx.patch', '0002_yyy.patch']
+ """
+ if count == -1:
+ # Work out how many patches to send if we can
+ count = (gitutil.CountCommitsToBranch() - start)
+
+ if not count:
+ sys.exit(col.Color(col.RED,
+ 'No commits found to process - please use -c flag'))
+
+ # Read the metadata from the commits
+ to_do = count
+ series = patchstream.GetMetaData(start, to_do)
+ cover_fname, patch_files = gitutil.CreatePatches(
+ start, to_do, ignore_binary, series)
+
+ # Fix up the patch files to our liking, and insert the cover letter
+ patchstream.FixPatches(series, patch_files)
+ if cover_fname and series.get('cover'):
+ patchstream.InsertCoverLetter(cover_fname, series, to_do)
+ return series, cover_fname, patch_files
+
+def check_patches(series, patch_files, run_checkpatch, verbose):
+ """Run some checks on a set of patches
+
+ This santiy-checks the patman tags like Series-version and runs the patches
+ through checkpatch
+
+ Args:
+ series (Series): Series object for this series (set of patches)
+ patch_files (list): List of patch filenames, each a string, e.g.
+ ['0001_xxx.patch', '0002_yyy.patch']
+ run_checkpatch (bool): True to run checkpatch.pl
+ verbose (bool): True to print out every line of the checkpatch output as
+ it is parsed
+
+ Returns:
+ bool: True if the patches had no errors, False if they did
+ """
+ # Do a few checks on the series
+ series.DoChecks()
+
+ # Check the patches, and run them through 'git am' just to be sure
+ if run_checkpatch:
+ ok = checkpatch.CheckPatches(verbose, patch_files)
+ else:
+ ok = True
+ return ok
+
+
+def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
+ ignore_bad_tags, add_maintainers, limit, dry_run, in_reply_to,
+ thread, smtp_server):
+ """Email patches to the recipients
+
+ This emails out the patches and cover letter using 'git send-email'. Each
+ patch is copied to recipients identified by the patch tag and output from
+ the get_maintainer.pl script. The cover letter is copied to all recipients
+ of any patch.
+
+ To make this work a CC file is created holding the recipients for each patch
+ and the cover letter. See the main program 'cc_cmd' for this logic.
+
+ Args:
+ col (terminal.Color): Colour output object
+ series (Series): Series object for this series (set of patches)
+ cover_fname (str): Filename of the cover letter as a string (None if
+ none)
+ patch_files (list): List of patch filenames, each a string, e.g.
+ ['0001_xxx.patch', '0002_yyy.patch']
+ process_tags (bool): True to process subject tags in each patch, e.g.
+ for 'dm: spi: Add SPI support' this would be 'dm' and 'spi'. The
+ tags are looked up in the configured sendemail.aliasesfile and also
+ in ~/.patman (see README)
+ its_a_go (bool): True if we are going to actually send the patches,
+ False if the patches have errors and will not be sent unless
+ @ignore_errors
+ ignore_bad_tags (bool): True to just print a warning for unknown tags,
+ False to halt with an error
+ add_maintainers (bool): Run the get_maintainer.pl script for each patch
+ limit (int): Limit on the number of people that can be cc'd on a single
+ patch or the cover letter (None if no limit)
+ dry_run (bool): Don't actually email the patches, just print out what
+ would be sent
+ in_reply_to (str): If not None we'll pass this to git as --in-reply-to.
+ Should be a message ID that this is in reply to.
+ thread (bool): True to add --thread to git send-email (make all patches
+ reply to cover-letter or first patch in series)
+ smtp_server (str): SMTP server to use to send patches (None for default)
+ """
+ cc_file = series.MakeCcFile(process_tags, cover_fname, not ignore_bad_tags,
+ add_maintainers, limit)
+
+ # Email the patches out (giving the user time to check / cancel)
+ cmd = ''
+ if its_a_go:
+ cmd = gitutil.EmailPatches(
+ series, cover_fname, patch_files, dry_run, not ignore_bad_tags,
+ cc_file, in_reply_to=in_reply_to, thread=thread,
+ smtp_server=smtp_server)
+ else:
+ print(col.Color(col.RED, "Not sending emails due to errors/warnings"))
+
+ # For a dry run, just show our actions as a sanity check
+ if dry_run:
+ series.ShowActions(patch_files, cmd, process_tags)
+ if not its_a_go:
+ print(col.Color(col.RED, "Email would not be sent"))
+
+ os.remove(cc_file)
+
+def send(options):
+ """Create, check and send patches by email
+
+ Args:
+ options (optparse.Values): Arguments to patman
+ """
+ setup()
+ col = terminal.Color()
+ series, cover_fname, patch_files = prepare_patches(
+ col, options.count, options.start, options.ignore_binary)
+ ok = check_patches(series, patch_files, options.check_patch,
+ options.verbose)
+ its_a_go = ok or options.ignore_errors
+ if its_a_go:
+ email_patches(
+ col, series, cover_fname, patch_files, options.process_tags,
+ its_a_go, options.ignore_bad_tags, options.add_maintainers,
+ options.limit, options.dry_run, options.in_reply_to, options.thread,
+ options.smtp_server)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 5189840eab..29444bf8e9 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -261,8 +261,10 @@ def CreatePatches(start, count, ignore_binary, series):
Args:
start: Commit to start from: 0=HEAD, 1=next one, etc.
count: number of commits to include
+ ignore_binary: Don't generate patches for binary files
+ series: Series object for this series (set of patches)
Return:
- Filename of cover letter
+ Filename of cover letter (None if none)
List of filenames of patch files
"""
if series.get('version'):
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 03668d1bb8..2432d31871 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -18,10 +18,9 @@ if __name__ == "__main__":
sys.path.append(os.path.join(our_path, '..'))
# Our modules
-from patman import checkpatch
from patman import command
+from patman import control
from patman import gitutil
-from patman import patchstream
from patman import project
from patman import settings
from patman import terminal
@@ -128,56 +127,4 @@ elif options.full_help:
# Process commits, produce patches files, check them, email them
else:
- gitutil.Setup()
-
- if options.count == -1:
- # Work out how many patches to send if we can
- options.count = gitutil.CountCommitsToBranch() - options.start
-
- col = terminal.Color()
- if not options.count:
- str = 'No commits found to process - please use -c flag'
- sys.exit(col.Color(col.RED, str))
-
- # Read the metadata from the commits
- if options.count:
- series = patchstream.GetMetaData(options.start, options.count)
- cover_fname, args = gitutil.CreatePatches(options.start, options.count,
- options.ignore_binary, series)
-
- # Fix up the patch files to our liking, and insert the cover letter
- patchstream.FixPatches(series, args)
- if cover_fname and series.get('cover'):
- patchstream.InsertCoverLetter(cover_fname, series, options.count)
-
- # Do a few checks on the series
- series.DoChecks()
-
- # Check the patches, and run them through 'git am' just to be sure
- if options.check_patch:
- ok = checkpatch.CheckPatches(options.verbose, args)
- else:
- ok = True
-
- cc_file = series.MakeCcFile(options.process_tags, cover_fname,
- not options.ignore_bad_tags,
- options.add_maintainers, options.limit)
-
- # Email the patches out (giving the user time to check / cancel)
- cmd = ''
- its_a_go = ok or options.ignore_errors
- if its_a_go:
- cmd = gitutil.EmailPatches(series, cover_fname, args,
- options.dry_run, not options.ignore_bad_tags, cc_file,
- in_reply_to=options.in_reply_to, thread=options.thread,
- smtp_server=options.smtp_server)
- else:
- print(col.Color(col.RED, "Not sending emails due to errors/warnings"))
-
- # For a dry run, just show our actions as a sanity check
- if options.dry_run:
- series.ShowActions(args, cmd, options.process_tags)
- if not its_a_go:
- print(col.Color(col.RED, "Email would not be sent"))
-
- os.remove(cc_file)
+ control.send(options)
diff --git a/tools/patman/series.py b/tools/patman/series.py
index b7eef37d03..9f885c8987 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -244,7 +244,7 @@ class Series(dict):
add_maintainers: Either:
True/False to call the get_maintainers to CC maintainers
List of maintainers to include (for testing)
- limit: Limit the length of the Cc list
+ limit: Limit the length of the Cc list (None if no limit)
Return:
Filename of temp file created
"""
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 03/16] patman: Add a test that uses gitpython
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
2020-07-06 3:41 ` [RFC PATCH 01/16] patman: Use test_util to show test results Simon Glass
2020-07-06 3:41 ` [RFC PATCH 02/16] patman: Move main code out to a control module Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 04/16] patman: Allow creating patches for another branch Simon Glass
` (22 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
It is convenient to use gitpython to create a real git repo for testing
patman's operation. Add a test for this. So far it just checks that patman
produces the right number of patches for a branch.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/func_test.py | 151 +++++++++++++++++++++++++++++++++++++-
tools/patman/tools.py | 4 +-
2 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index dc30078cce..211952154a 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -14,15 +14,23 @@ import unittest
from io import StringIO
+from patman import control
from patman import gitutil
from patman import patchstream
from patman import settings
+from patman import terminal
from patman import tools
+from patman.test_util import capture_sys_output
+
+try:
+ import pygit2
+ HAVE_PYGIT2= True
+except ModuleNotFoundError:
+ HAVE_PYGIT2 = False
@contextlib.contextmanager
def capture():
- import sys
oldout,olderr = sys.stdout, sys.stderr
try:
out=[StringIO(), StringIO()]
@@ -37,6 +45,8 @@ def capture():
class TestFunctional(unittest.TestCase):
def setUp(self):
self.tmpdir = tempfile.mkdtemp(prefix='patman.')
+ self.gitdir = os.path.join(self.tmpdir, 'git')
+ self.repo = None
def tearDown(self):
shutil.rmtree(self.tmpdir)
@@ -286,3 +296,142 @@ Changes in v2:
if expected:
expected = expected.splitlines()
self.assertEqual(expected, lines[start:(start+len(expected))])
+
+ def make_commit_with_file(self, subject, body, fname, text):
+ """Create a file and add it to the git repo with a new commit
+
+ Args:
+ subject (str): Subject for the commit
+ body (str): Body text of the commit
+ fname (str): Filename of file to create
+ text (str): Text to put into the file
+ """
+ path = os.path.join(self.gitdir, fname)
+ tools.WriteFile(path, text, binary=False)
+ index = self.repo.index
+ index.add(fname)
+ author = pygit2.Signature('Test user', 'test at email.com')
+ committer = author
+ tree = index.write_tree()
+ message = subject + '\n' + body
+ self.repo.create_commit('HEAD', author, committer, message, tree,
+ [self.repo.head.target])
+
+ def make_git_tree(self):
+ """Make a simple git tree suitable for testing
+
+ It has three branches:
+ 'base' has two commits: PCI, main
+ 'first' has base as upstream and two more commits: I2C, SPI
+ 'second' has base as upstream and three more: video, serial, bootm
+
+ Returns:
+ pygit2 repository
+ """
+ repo = pygit2.init_repository(self.gitdir)
+ self.repo = repo
+ new_tree = repo.TreeBuilder().write()
+
+ author = pygit2.Signature('Test user', 'test at email.com')
+ committer = author
+ commit = repo.create_commit('HEAD', author, committer,
+ 'Created master', new_tree, [])
+
+ self.make_commit_with_file('Initial commit', '''
+Add a README
+
+''', 'README', '''This is the README file
+describing this project
+in very little detail''')
+
+ self.make_commit_with_file('pci: PCI implementation', '''
+Here is a basic PCI implementation
+
+''', 'pci.c', '''This is a file
+it has some contents
+and some more things''')
+ self.make_commit_with_file('main: Main program', '''
+Hello here is the second commit.
+''', 'main.c', '''This is the main file
+there is very little here
+but we can always add more later
+if we want to
+
+Series-to: u-boot
+Series-cc: Barry Crump <bcrump@whataroa.nz>
+''')
+ base_target = repo.revparse_single('HEAD')
+ self.make_commit_with_file('i2c: I2C things', '''
+This has some stuff to do with I2C
+''', 'i2c.c', '''And this is the file contents
+with some I2C-related things in it''')
+ self.make_commit_with_file('spi: SPI fixes', '''
+SPI needs some fixes
+and here they are
+''', 'spi.c', '''Some fixes for SPI in this
+file to make SPI work
+better than before''')
+ first_target = repo.revparse_single('HEAD')
+
+ target = repo.revparse_single('HEAD~2')
+ repo.reset(target.oid, pygit2.GIT_CHECKOUT_FORCE)
+ self.make_commit_with_file('video: Some video improvements', '''
+Fix up the video so that
+it looks more purple. Purple is
+a very nice colour.
+''', 'video.c', '''More purple here
+Purple and purple
+Even more purple
+Could not be any more purple''')
+ self.make_commit_with_file('serial: Add a serial driver', '''
+Here is the serial driver
+for my chip.
+
+Cover-letter:
+Series for my board
+This series implements support
+for my glorious board.
+END
+''', 'serial.c', '''The code for the
+serial driver is here''')
+ self.make_commit_with_file('bootm: Make it boot', '''
+This makes my board boot
+with a fix to the bootm
+command
+''', 'bootm.c', '''Fix up the bootm
+command to make the code as
+complicated as possible''')
+ second_target = repo.revparse_single('HEAD')
+
+ repo.branches.local.create('first', first_target)
+ repo.config.set_multivar('branch.first.remote', '', '.')
+ repo.config.set_multivar('branch.first.merge', '', 'refs/heads/base')
+
+ repo.branches.local.create('second', second_target)
+ repo.config.set_multivar('branch.second.remote', '', '.')
+ repo.config.set_multivar('branch.second.merge', '', 'refs/heads/base')
+
+ repo.branches.local.create('base', base_target)
+ return repo
+
+ @unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2')
+ def testBranch(self):
+ """Test creating patches from a branch"""
+ repo = self.make_git_tree()
+ target = repo.lookup_reference('refs/heads/first')
+ self.repo.checkout(target, strategy=pygit2.GIT_CHECKOUT_FORCE)
+ control.setup()
+ try:
+ orig_dir = os.getcwd()
+ os.chdir(self.gitdir)
+
+ # Check that it can detect the current branch
+ self.assertEqual(2, gitutil.CountCommitsToBranch())
+ col = terminal.Color()
+ with capture_sys_output() as _:
+ _, cover_fname, patch_files = control.prepare_patches(
+ col, count=-1, start=0, ignore_binary=False)
+ self.assertIsNone(cover_fname)
+ self.assertEqual(2, len(patch_files))
+ finally:
+ os.chdir(orig_dir)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index b50370dfe8..f402b9aab8 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -270,7 +270,7 @@ def ReadFile(fname, binary=True):
#(fname, len(data), len(data)))
return data
-def WriteFile(fname, data):
+def WriteFile(fname, data, binary=True):
"""Write data into a file.
Args:
@@ -279,7 +279,7 @@ def WriteFile(fname, data):
"""
#self._out.Info("Write file '%s' size %d (%#0x)" %
#(fname, len(data), len(data)))
- with open(Filename(fname), 'wb') as fd:
+ with open(Filename(fname), binary and 'wb' or 'w') as fd:
fd.write(data)
def GetBytes(byte, size):
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 04/16] patman: Allow creating patches for another branch
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (2 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 03/16] patman: Add a test that uses gitpython Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 05/16] patman: Allow skipping patches at the end Simon Glass
` (21 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
Add a -b option to allow patches to be created from a branch other than
the current one.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/control.py | 13 ++++++++-----
tools/patman/func_test.py | 13 +++++++++++--
tools/patman/gitutil.py | 19 ++++++++++++++-----
tools/patman/main.py | 2 ++
tools/patman/patchstream.py | 8 +++++---
5 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/tools/patman/control.py b/tools/patman/control.py
index a896c924b5..b48eac41fd 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -20,7 +20,7 @@ def setup():
"""Do required setup before doing anything"""
gitutil.Setup()
-def prepare_patches(col, count, start, ignore_binary):
+def prepare_patches(col, branch, count, start, ignore_binary):
"""Figure out what patches to generate, then generate them
The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -28,6 +28,7 @@ def prepare_patches(col, count, start, ignore_binary):
Args:
col (terminal.Color): Colour output object
+ branch (str): Branch to create patches from (None = current)
count (int): Number of patches to produce, or -1 to produce patches for
the current branch back to the upstream commit
start (int): Start partch to use (0=first / top of branch)
@@ -42,7 +43,7 @@ def prepare_patches(col, count, start, ignore_binary):
"""
if count == -1:
# Work out how many patches to send if we can
- count = (gitutil.CountCommitsToBranch() - start)
+ count = (gitutil.CountCommitsToBranch(branch) - start)
if not count:
sys.exit(col.Color(col.RED,
@@ -50,9 +51,9 @@ def prepare_patches(col, count, start, ignore_binary):
# Read the metadata from the commits
to_do = count
- series = patchstream.GetMetaData(start, to_do)
+ series = patchstream.GetMetaData(branch, start, to_do)
cover_fname, patch_files = gitutil.CreatePatches(
- start, to_do, ignore_binary, series)
+ branch, start, to_do, ignore_binary, series)
# Fix up the patch files to our liking, and insert the cover letter
patchstream.FixPatches(series, patch_files)
@@ -158,9 +159,11 @@ def send(options):
setup()
col = terminal.Color()
series, cover_fname, patch_files = prepare_patches(
- col, options.count, options.start, options.ignore_binary)
+ col, options.branch, options.count, options.start,
+ options.ignore_binary)
ok = check_patches(series, patch_files, options.check_patch,
options.verbose)
+
its_a_go = ok or options.ignore_errors
if its_a_go:
email_patches(
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 211952154a..588be73ef4 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -426,12 +426,21 @@ complicated as possible''')
os.chdir(self.gitdir)
# Check that it can detect the current branch
- self.assertEqual(2, gitutil.CountCommitsToBranch())
+ self.assertEqual(2, gitutil.CountCommitsToBranch(None))
col = terminal.Color()
with capture_sys_output() as _:
_, cover_fname, patch_files = control.prepare_patches(
- col, count=-1, start=0, ignore_binary=False)
+ col, branch=None, count=-1, start=0, ignore_binary=False)
self.assertIsNone(cover_fname)
self.assertEqual(2, len(patch_files))
+
+ # Check that it can detect a different branch
+ self.assertEqual(3, gitutil.CountCommitsToBranch('second'))
+ with capture_sys_output() as _:
+ _, cover_fname, patch_files = control.prepare_patches(
+ col, branch='second', count=-1, start=0,
+ ignore_binary=False)
+ self.assertIsNotNone(cover_fname)
+ self.assertEqual(3, len(patch_files))
finally:
os.chdir(orig_dir)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 29444bf8e9..b683481a57 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -49,17 +49,24 @@ def LogCmd(commit_range, git_dir=None, oneline=False, reverse=False,
cmd.append('--')
return cmd
-def CountCommitsToBranch():
+def CountCommitsToBranch(branch):
"""Returns number of commits between HEAD and the tracking branch.
This looks back to the tracking branch and works out the number of commits
since then.
+ Args:
+ branch: Branch to count from (None for current branch)
+
Return:
Number of patches that exist on top of the branch
"""
- pipe = [LogCmd('@{upstream}..', oneline=True),
- ['wc', '-l']]
+ if branch:
+ us, msg = GetUpstream('.git', branch)
+ rev_range = '%s..%s' % (us, branch)
+ else:
+ rev_range = '@{upstream}..'
+ pipe = [LogCmd(rev_range, oneline=True), ['wc', '-l']]
stdout = command.RunPipe(pipe, capture=True, oneline=True).stdout
patch_count = int(stdout)
return patch_count
@@ -252,13 +259,14 @@ def Fetch(git_dir=None, work_tree=None):
if result.return_code != 0:
raise OSError('git fetch: %s' % result.stderr)
-def CreatePatches(start, count, ignore_binary, series):
+def CreatePatches(branch, start, count, ignore_binary, series):
"""Create a series of patches from the top of the current branch.
The patch files are written to the current directory using
git format-patch.
Args:
+ branch: Branch to create patches from (None for current branch)
start: Commit to start from: 0=HEAD, 1=next one, etc.
count: number of commits to include
ignore_binary: Don't generate patches for binary files
@@ -277,7 +285,8 @@ def CreatePatches(start, count, ignore_binary, series):
prefix = series.GetPatchPrefix()
if prefix:
cmd += ['--subject-prefix=%s' % prefix]
- cmd += ['HEAD~%d..HEAD~%d' % (start + count, start)]
+ brname = branch or 'HEAD'
+ cmd += ['%s~%d..%s~%d' % (brname, start + count, brname, start)]
stdout = command.RunList(cmd)
files = stdout.splitlines()
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 2432d31871..066754196e 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -31,6 +31,8 @@ from patman import test_checkpatch
parser = OptionParser()
parser.add_option('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
+parser.add_option('-b', '--branch', type='str',
+ help="Branch to process (by default, the current branch)")
parser.add_option('-c', '--count', dest='count', type='int',
default=-1, help='Automatically create patches from top n commits')
parser.add_option('-i', '--ignore-errors', action='store_true',
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 4fe465e9ab..2ea8ebcc3f 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -512,17 +512,19 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None,
ps.Finalize()
return series
-def GetMetaData(start, count):
+def GetMetaData(branch, start, count):
"""Reads out patch series metadata from the commits
This does a 'git log' on the relevant commits and pulls out the tags we
are interested in.
Args:
- start: Commit to start from: 0=HEAD, 1=next one, etc.
+ branch: Branch to use (None for current branch)
+ start: Commit to start from: 0=branch HEAD, 1=next one, etc.
count: Number of commits to list
"""
- return GetMetaDataForList('HEAD~%d' % start, None, count)
+ return GetMetaDataForList('%s~%d' % (branch if branch else 'HEAD', start),
+ None, count)
def GetMetaDataForTest(text):
"""Process metadata from a file containing a git log. Used for tests
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 05/16] patman: Allow skipping patches at the end
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (3 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 04/16] patman: Allow creating patches for another branch Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 06/16] patman: Convert to ArgumentParser Simon Glass
` (20 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
The -s option allows skipping patches at the top of the branch. Sometimes
there are commits at the bottom that need to be skipped. At present it is
necessary to count the number of commits and then use -c to tell patman
how many to process.
Add a -e option to easily skip a number of commits at the bottom of the
branch.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/control.py | 8 +++++---
tools/patman/func_test.py | 13 +++++++++++--
tools/patman/main.py | 2 ++
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/patman/control.py b/tools/patman/control.py
index b48eac41fd..b481ff6b27 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -20,7 +20,7 @@ def setup():
"""Do required setup before doing anything"""
gitutil.Setup()
-def prepare_patches(col, branch, count, start, ignore_binary):
+def prepare_patches(col, branch, count, start, end, ignore_binary):
"""Figure out what patches to generate, then generate them
The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -32,6 +32,8 @@ def prepare_patches(col, branch, count, start, ignore_binary):
count (int): Number of patches to produce, or -1 to produce patches for
the current branch back to the upstream commit
start (int): Start partch to use (0=first / top of branch)
+ end (int): End patch to use (0=last one in series, 1=one before that,
+ etc.)
ignore_binary (bool): Don't generate patches for binary files
Returns:
@@ -50,7 +52,7 @@ def prepare_patches(col, branch, count, start, ignore_binary):
'No commits found to process - please use -c flag'))
# Read the metadata from the commits
- to_do = count
+ to_do = count - end
series = patchstream.GetMetaData(branch, start, to_do)
cover_fname, patch_files = gitutil.CreatePatches(
branch, start, to_do, ignore_binary, series)
@@ -159,7 +161,7 @@ def send(options):
setup()
col = terminal.Color()
series, cover_fname, patch_files = prepare_patches(
- col, options.branch, options.count, options.start,
+ col, options.branch, options.count, options.start, options.end,
options.ignore_binary)
ok = check_patches(series, patch_files, options.check_patch,
options.verbose)
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 588be73ef4..810af9c604 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -430,7 +430,8 @@ complicated as possible''')
col = terminal.Color()
with capture_sys_output() as _:
_, cover_fname, patch_files = control.prepare_patches(
- col, branch=None, count=-1, start=0, ignore_binary=False)
+ col, branch=None, count=-1, start=0, end=0,
+ ignore_binary=False)
self.assertIsNone(cover_fname)
self.assertEqual(2, len(patch_files))
@@ -438,9 +439,17 @@ complicated as possible''')
self.assertEqual(3, gitutil.CountCommitsToBranch('second'))
with capture_sys_output() as _:
_, cover_fname, patch_files = control.prepare_patches(
- col, branch='second', count=-1, start=0,
+ col, branch='second', count=-1, start=0, end=0,
ignore_binary=False)
self.assertIsNotNone(cover_fname)
self.assertEqual(3, len(patch_files))
+
+ # Check that it can skip patches at the end
+ with capture_sys_output() as _:
+ _, cover_fname, patch_files = control.prepare_patches(
+ col, branch='second', count=-1, start=0, end=1,
+ ignore_binary=False)
+ self.assertIsNotNone(cover_fname)
+ self.assertEqual(2, len(patch_files))
finally:
os.chdir(orig_dir)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 066754196e..4d7a3044ea 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -35,6 +35,8 @@ parser.add_option('-b', '--branch', type='str',
help="Branch to process (by default, the current branch)")
parser.add_option('-c', '--count', dest='count', type='int',
default=-1, help='Automatically create patches from top n commits')
+parser.add_option('-e', '--end', type='int', default=0,
+ help='Commits to skip at end of patch list')
parser.add_option('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 06/16] patman: Convert to ArgumentParser
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (4 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 05/16] patman: Allow skipping patches at the end Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 07/16] patman: Allow different commands Simon Glass
` (19 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
Convert from OptionParser to ArgumentParser to match binman. With this we
can easily add sub-commands.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/control.py | 22 ++++-----
tools/patman/main.py | 97 ++++++++++++++++++++--------------------
tools/patman/settings.py | 10 +++--
3 files changed, 65 insertions(+), 64 deletions(-)
diff --git a/tools/patman/control.py b/tools/patman/control.py
index b481ff6b27..e67867b845 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -152,24 +152,24 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
os.remove(cc_file)
-def send(options):
+def send(args):
"""Create, check and send patches by email
Args:
- options (optparse.Values): Arguments to patman
+ args (argparse.Namespace): Arguments to patman
"""
setup()
col = terminal.Color()
series, cover_fname, patch_files = prepare_patches(
- col, options.branch, options.count, options.start, options.end,
- options.ignore_binary)
- ok = check_patches(series, patch_files, options.check_patch,
- options.verbose)
+ col, args.branch, args.count, args.start, args.end,
+ args.ignore_binary)
+ ok = check_patches(series, patch_files, args.check_patch,
+ args.verbose)
- its_a_go = ok or options.ignore_errors
+ its_a_go = ok or args.ignore_errors
if its_a_go:
email_patches(
- col, series, cover_fname, patch_files, options.process_tags,
- its_a_go, options.ignore_bad_tags, options.add_maintainers,
- options.limit, options.dry_run, options.in_reply_to, options.thread,
- options.smtp_server)
+ col, series, cover_fname, patch_files, args.process_tags,
+ its_a_go, args.ignore_bad_tags, args.add_maintainers,
+ args.limit, args.dry_run, args.in_reply_to, args.thread,
+ args.smtp_server)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 4d7a3044ea..34cad9a562 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -6,7 +6,7 @@
"""See README for more information"""
-from optparse import OptionParser
+from argparse import ArgumentParser
import os
import re
import sys
@@ -27,71 +27,70 @@ from patman import terminal
from patman import test_util
from patman import test_checkpatch
+epilog = '''Create patches from commits in a branch, check them and email them
+as specified by tags you place in the commits. Use -n to do a dry run first.'''
-parser = OptionParser()
-parser.add_option('-H', '--full-help', action='store_true', dest='full_help',
+parser = ArgumentParser(epilog=epilog)
+parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
-parser.add_option('-b', '--branch', type='str',
+parser.add_argument('-b', '--branch', type=str,
help="Branch to process (by default, the current branch)")
-parser.add_option('-c', '--count', dest='count', type='int',
+parser.add_argument('-c', '--count', dest='count', type=int,
default=-1, help='Automatically create patches from top n commits')
-parser.add_option('-e', '--end', type='int', default=0,
+parser.add_argument('-e', '--end', type=int, default=0,
help='Commits to skip at end of patch list')
-parser.add_option('-i', '--ignore-errors', action='store_true',
+parser.add_argument('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
-parser.add_option('-l', '--limit-cc', dest='limit', type='int',
- default=None, help='Limit the cc list to LIMIT entries [default: %default]')
-parser.add_option('-m', '--no-maintainers', action='store_false',
+parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
+ help='Limit the cc list to LIMIT entries [default: %(default)]')
+parser.add_argument('-m', '--no-maintainers', action='store_false',
dest='add_maintainers', default=True,
help="Don't cc the file maintainers automatically")
-parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
+parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help="Do a dry run (create but don't email patches)")
-parser.add_option('-p', '--project', default=project.DetectProject(),
- help="Project name; affects default option values and "
- "aliases [default: %default]")
-parser.add_option('-r', '--in-reply-to', type='string', action='store',
+parser.add_argument('-p', '--project', default=project.DetectProject(),
+ help="Project name; affects default option values and "
+ "aliases [default: %(default)]")
+parser.add_argument('-r', '--in-reply-to', type=str, action='store',
help="Message ID that this series is in reply to")
-parser.add_option('-s', '--start', dest='start', type='int',
+parser.add_argument('-s', '--start', dest='start', type=int,
default=0, help='Commit to start creating patches from (0 = HEAD)')
-parser.add_option('-t', '--ignore-bad-tags', action='store_true',
- default=False, help='Ignore bad tags / aliases')
-parser.add_option('-v', '--verbose', action='store_true', dest='verbose',
+parser.add_argument('-t', '--ignore-bad-tags', action='store_true',
+ default=False, help='Ignore bad tags / aliases')
+parser.add_argument('-v', '--verbose', action='store_true', dest='verbose',
default=False, help='Verbose output of errors and warnings')
-parser.add_option('-T', '--thread', action='store_true', dest='thread',
- default=False, help='Create patches as a single thread')
-parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store',
+parser.add_argument('-T', '--thread', action='store_true', dest='thread',
+ default=False, help='Create patches as a single thread')
+parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store',
default=None, help='Output cc list for patch file (used by git)')
-parser.add_option('--no-binary', action='store_true', dest='ignore_binary',
- default=False,
- help="Do not output contents of changes in binary files")
-parser.add_option('--no-check', action='store_false', dest='check_patch',
- default=True,
- help="Don't check for patch compliance")
-parser.add_option('--no-tags', action='store_false', dest='process_tags',
- default=True, help="Don't process subject tags as aliases")
-parser.add_option('--smtp-server', type='str',
- help="Specify the SMTP server to 'git send-email'")
-parser.add_option('--test', action='store_true', dest='test',
- default=False, help='run tests')
-
-parser.usage += """
-
-Create patches from commits in a branch, check them and email them as
-specified by tags you place in the commits. Use -n to do a dry run first."""
-
+parser.add_argument('--no-binary', action='store_true', dest='ignore_binary',
+ default=False,
+ help="Do not output contents of changes in binary files")
+parser.add_argument('--no-check', action='store_false', dest='check_patch',
+ default=True,
+ help="Don't check for patch compliance")
+parser.add_argument('--no-tags', action='store_false', dest='process_tags',
+ default=True, help="Don't process subject tags as aliases")
+parser.add_argument('--smtp-server', type=str,
+ help="Specify the SMTP server to 'git send-email'")
+parser.add_argument('--test', action='store_true', dest='test',
+ default=False, help='run tests')
+
+parser.add_argument('patchfiles', nargs='*')
# Parse options twice: first to get the project and second to handle
# defaults properly (which depends on project).
-(options, args) = parser.parse_args()
-settings.Setup(gitutil, parser, options.project, '')
-(options, args) = parser.parse_args()
+argv = sys.argv[1:]
+args = parser.parse_args(argv)
+settings.Setup(gitutil, parser, args.project, '')
+args = parser.parse_args(argv)
if __name__ != "__main__":
pass
# Run our meagre tests
-elif options.test:
+elif args.test:
import doctest
from patman import func_test
@@ -109,19 +108,19 @@ elif options.test:
# Called from git with a patch filename as argument
# Printout a list of additional CC recipients for this patch
-elif options.cc_cmd:
- fd = open(options.cc_cmd, 'r')
+elif args.cc_cmd:
+ fd = open(args.cc_cmd, 'r')
re_line = re.compile('(\S*) (.*)')
for line in fd.readlines():
match = re_line.match(line)
- if match and match.group(1) == args[0]:
+ if match and match.group(1) == args.patchfiles[0]:
for cc in match.group(2).split('\0'):
cc = cc.strip()
if cc:
print(cc)
fd.close()
-elif options.full_help:
+elif args.full_help:
pager = os.getenv('PAGER')
if not pager:
pager = 'more'
@@ -131,4 +130,4 @@ elif options.full_help:
# Process commits, produce patches files, check them, email them
else:
- control.send(options)
+ control.send(args)
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 635561ac05..732bd40106 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -233,17 +233,19 @@ def _UpdateDefaults(parser, config):
config: An instance of _ProjectConfigParser that we will query
for settings.
"""
- defaults = parser.get_default_values()
+ defaults = parser.parse_known_args()[0]
+ defaults = vars(defaults)
for name, val in config.items('settings'):
- if hasattr(defaults, name):
- default_val = getattr(defaults, name)
+ if name in defaults:
+ default_val = defaults[name]
if isinstance(default_val, bool):
val = config.getboolean('settings', name)
elif isinstance(default_val, int):
val = config.getint('settings', name)
- parser.set_default(name, val)
+ defaults[name] = val
else:
print("WARNING: Unknown setting %s" % name)
+ parser.set_defaults(**defaults)
def _ReadAliasFile(fname):
"""Read in the U-Boot git alias file if it exists.
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 07/16] patman: Allow different commands
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (5 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 06/16] patman: Convert to ArgumentParser Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 08/16] patman: Add a 'test' subcommand Simon Glass
` (18 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
At present patman only does one thing so does not have any comments. We
want to add a few more command, so create a sub-parser for the default
command ('send').
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/main.py | 77 +++++++++++++++++++++++---------------------
1 file changed, 41 insertions(+), 36 deletions(-)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 34cad9a562..fee9bc848b 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -31,60 +31,65 @@ epilog = '''Create patches from commits in a branch, check them and email them
as specified by tags you place in the commits. Use -n to do a dry run first.'''
parser = ArgumentParser(epilog=epilog)
-parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
+subparsers = parser.add_subparsers(dest='cmd')
+send = subparsers.add_parser('send')
+send.add_argument('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
-parser.add_argument('-b', '--branch', type=str,
+send.add_argument('-b', '--branch', type=str,
help="Branch to process (by default, the current branch)")
-parser.add_argument('-c', '--count', dest='count', type=int,
+send.add_argument('-c', '--count', dest='count', type=int,
default=-1, help='Automatically create patches from top n commits')
-parser.add_argument('-e', '--end', type=int, default=0,
+send.add_argument('-e', '--end', type=int, default=0,
help='Commits to skip at end of patch list')
-parser.add_argument('-i', '--ignore-errors', action='store_true',
+send.add_argument('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
-parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
- help='Limit the cc list to LIMIT entries [default: %(default)]')
-parser.add_argument('-m', '--no-maintainers', action='store_false',
+send.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
+ help='Limit the cc list to LIMIT entries [default: %(default)s]')
+send.add_argument('-m', '--no-maintainers', action='store_false',
dest='add_maintainers', default=True,
help="Don't cc the file maintainers automatically")
-parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
+send.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
default=False, help="Do a dry run (create but don't email patches)")
-parser.add_argument('-p', '--project', default=project.DetectProject(),
- help="Project name; affects default option values and "
- "aliases [default: %(default)]")
-parser.add_argument('-r', '--in-reply-to', type=str, action='store',
+send.add_argument('-p', '--project', default=project.DetectProject(),
+ help="Project name; affects default option values and "
+ "aliases [default: %(default)s]")
+send.add_argument('-r', '--in-reply-to', type=str, action='store',
help="Message ID that this series is in reply to")
-parser.add_argument('-s', '--start', dest='start', type=int,
+send.add_argument('-s', '--start', dest='start', type=int,
default=0, help='Commit to start creating patches from (0 = HEAD)')
-parser.add_argument('-t', '--ignore-bad-tags', action='store_true',
- default=False, help='Ignore bad tags / aliases')
-parser.add_argument('-v', '--verbose', action='store_true', dest='verbose',
+send.add_argument('-t', '--ignore-bad-tags', action='store_true',
+ default=False, help='Ignore bad tags / aliases')
+send.add_argument('-v', '--verbose', action='store_true', dest='verbose',
default=False, help='Verbose output of errors and warnings')
-parser.add_argument('-T', '--thread', action='store_true', dest='thread',
- default=False, help='Create patches as a single thread')
-parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store',
+send.add_argument('-T', '--thread', action='store_true', dest='thread',
+ default=False, help='Create patches as a single thread')
+send.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store',
default=None, help='Output cc list for patch file (used by git)')
-parser.add_argument('--no-binary', action='store_true', dest='ignore_binary',
- default=False,
- help="Do not output contents of changes in binary files")
-parser.add_argument('--no-check', action='store_false', dest='check_patch',
- default=True,
- help="Don't check for patch compliance")
-parser.add_argument('--no-tags', action='store_false', dest='process_tags',
- default=True, help="Don't process subject tags as aliases")
-parser.add_argument('--smtp-server', type=str,
- help="Specify the SMTP server to 'git send-email'")
-parser.add_argument('--test', action='store_true', dest='test',
- default=False, help='run tests')
-
-parser.add_argument('patchfiles', nargs='*')
+send.add_argument('--no-binary', action='store_true', dest='ignore_binary',
+ default=False,
+ help="Do not output contents of changes in binary files")
+send.add_argument('--no-check', action='store_false', dest='check_patch',
+ default=True,
+ help="Don't check for patch compliance")
+send.add_argument('--no-tags', action='store_false', dest='process_tags',
+ default=True, help="Don't process subject tags as aliases")
+send.add_argument('--smtp-server', type=str,
+ help="Specify the SMTP server to 'git send-email'")
+send.add_argument('--test', action='store_true', dest='test',
+ default=False, help='run tests')
+
+send.add_argument('patchfiles', nargs='*')
# Parse options twice: first to get the project and second to handle
# defaults properly (which depends on project).
argv = sys.argv[1:]
+if len(argv) < 1 or argv[0].startswith('-'):
+ argv = ['send'] + argv
args = parser.parse_args(argv)
-settings.Setup(gitutil, parser, args.project, '')
-args = parser.parse_args(argv)
+if hasattr(args, 'project'):
+ settings.Setup(gitutil, send, args.project, '')
+ args = parser.parse_args(argv)
if __name__ != "__main__":
pass
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 08/16] patman: Add a 'test' subcommand
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (6 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 07/16] patman: Allow different commands Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output Simon Glass
` (17 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
At present we use --test to indicate that tests should be run. It is
better to use a subcommand for list, like binman. Change it and adjust
the existing code to fit under a 'send' subcommand, the default.
Give this subcommand the same default arguments as the others.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
.azure-pipelines.yml | 2 +-
.gitlab-ci.yml | 2 +-
.travis.yml | 2 +-
test/run | 2 +-
tools/patman/main.py | 75 +++++++++++++++++++++------------------
tools/patman/test_util.py | 2 +-
6 files changed, 45 insertions(+), 40 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 9f88a539c0..45f0fabd6d 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -158,7 +158,7 @@ jobs:
./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test
./tools/buildman/buildman -t
./tools/dtoc/dtoc -t
- ./tools/patman/patman --test
+ ./tools/patman/patman test
make O=${UBOOT_TRAVIS_BUILD_DIR} testconfig
EOF
cat build.sh
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a685a7879d..c3d8ef6543 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -167,7 +167,7 @@ Run binman, buildman, dtoc, Kconfig and patman testsuites:
./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test;
./tools/buildman/buildman -t;
./tools/dtoc/dtoc -t;
- ./tools/patman/patman --test;
+ ./tools/patman/patman test;
make testconfig
Run tests for Nokia RX-51 (aka N900):
diff --git a/.travis.yml b/.travis.yml
index a042aa2c7d..4ce760d938 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -257,7 +257,7 @@ script:
export PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt";
export PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}";
./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test &&
- ./tools/patman/patman --test &&
+ ./tools/patman/patman test &&
./tools/buildman/buildman -t &&
./tools/dtoc/dtoc -t &&
make testconfig;
diff --git a/test/run b/test/run
index 27331a8e40..de87e7530b 100755
--- a/test/run
+++ b/test/run
@@ -48,7 +48,7 @@ export DTC=${DTC_DIR}/dtc
TOOLS_DIR=build-sandbox_spl/tools
run_test "binman" ./tools/binman/binman --toolpath ${TOOLS_DIR} test
-run_test "patman" ./tools/patman/patman --test
+run_test "patman" ./tools/patman/patman test
run_test "buildman" ./tools/buildman/buildman -t ${skip}
run_test "fdt" ./tools/dtoc/test_fdt -t
diff --git a/tools/patman/main.py b/tools/patman/main.py
index fee9bc848b..77f187e769 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -27,6 +27,16 @@ from patman import terminal
from patman import test_util
from patman import test_checkpatch
+def AddCommonArgs(parser):
+ parser.add_argument('-b', '--branch', type=str,
+ help="Branch to process (by default, the current branch)")
+ parser.add_argument('-c', '--count', dest='count', type=int,
+ default=-1, help='Automatically create patches from top n commits')
+ parser.add_argument('-e', '--end', type=int, default=0,
+ help='Commits to skip at end of patch list')
+ parser.add_argument('-s', '--start', dest='start', type=int,
+ default=0, help='Commit to start creating patches from (0 = HEAD)')
+
epilog = '''Create patches from commits in a branch, check them and email them
as specified by tags you place in the commits. Use -n to do a dry run first.'''
@@ -35,12 +45,6 @@ subparsers = parser.add_subparsers(dest='cmd')
send = subparsers.add_parser('send')
send.add_argument('-H', '--full-help', action='store_true', dest='full_help',
default=False, help='Display the README file')
-send.add_argument('-b', '--branch', type=str,
- help="Branch to process (by default, the current branch)")
-send.add_argument('-c', '--count', dest='count', type=int,
- default=-1, help='Automatically create patches from top n commits')
-send.add_argument('-e', '--end', type=int, default=0,
- help='Commits to skip at end of patch list')
send.add_argument('-i', '--ignore-errors', action='store_true',
dest='ignore_errors', default=False,
help='Send patches email even if patch errors are found')
@@ -56,8 +60,6 @@ send.add_argument('-p', '--project', default=project.DetectProject(),
"aliases [default: %(default)s]")
send.add_argument('-r', '--in-reply-to', type=str, action='store',
help="Message ID that this series is in reply to")
-send.add_argument('-s', '--start', dest='start', type=int,
- default=0, help='Commit to start creating patches from (0 = HEAD)')
send.add_argument('-t', '--ignore-bad-tags', action='store_true',
default=False, help='Ignore bad tags / aliases')
send.add_argument('-v', '--verbose', action='store_true', dest='verbose',
@@ -76,11 +78,13 @@ send.add_argument('--no-tags', action='store_false', dest='process_tags',
default=True, help="Don't process subject tags as aliases")
send.add_argument('--smtp-server', type=str,
help="Specify the SMTP server to 'git send-email'")
-send.add_argument('--test', action='store_true', dest='test',
- default=False, help='run tests')
+AddCommonArgs(send)
send.add_argument('patchfiles', nargs='*')
+test_parser = subparsers.add_parser('test', help='Run tests')
+AddCommonArgs(test_parser)
+
# Parse options twice: first to get the project and second to handle
# defaults properly (which depends on project).
argv = sys.argv[1:]
@@ -95,7 +99,7 @@ if __name__ != "__main__":
pass
# Run our meagre tests
-elif args.test:
+if args.cmd == 'test':
import doctest
from patman import func_test
@@ -111,28 +115,29 @@ elif args.test:
sys.exit(test_util.ReportResult('patman', None, result))
-# Called from git with a patch filename as argument
-# Printout a list of additional CC recipients for this patch
-elif args.cc_cmd:
- fd = open(args.cc_cmd, 'r')
- re_line = re.compile('(\S*) (.*)')
- for line in fd.readlines():
- match = re_line.match(line)
- if match and match.group(1) == args.patchfiles[0]:
- for cc in match.group(2).split('\0'):
- cc = cc.strip()
- if cc:
- print(cc)
- fd.close()
-
-elif args.full_help:
- pager = os.getenv('PAGER')
- if not pager:
- pager = 'more'
- fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
- 'README')
- command.Run(pager, fname)
-
# Process commits, produce patches files, check them, email them
-else:
- control.send(args)
+elif args.cmd == 'send':
+ # Called from git with a patch filename as argument
+ # Printout a list of additional CC recipients for this patch
+ if args.cc_cmd:
+ fd = open(args.cc_cmd, 'r')
+ re_line = re.compile('(\S*) (.*)')
+ for line in fd.readlines():
+ match = re_line.match(line)
+ if match and match.group(1) == args.patchfiles[0]:
+ for cc in match.group(2).split('\0'):
+ cc = cc.strip()
+ if cc:
+ print(cc)
+ fd.close()
+
+ elif args.full_help:
+ pager = os.getenv('PAGER')
+ if not pager:
+ pager = 'more'
+ fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
+ 'README')
+ command.Run(pager, fname)
+
+ else:
+ control.send(args)
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 0827488f33..a87d3cc8f3 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -47,7 +47,7 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None):
glob_list = []
glob_list += exclude_list
glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*']
- test_cmd = 'test' if 'binman' in prog else '-t'
+ test_cmd = 'test' if 'binman' in prog or 'patman' in prog else '-t'
prefix = ''
if build_dir:
prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (7 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 08/16] patman: Add a 'test' subcommand Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream Simon Glass
` (16 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
At present all text is marked bright, which makes it stand out on the
terminal. Add a way to disable that, as is done with the Color class.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/terminal.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py
index c709438bdc..60dbce3ce1 100644
--- a/tools/patman/terminal.py
+++ b/tools/patman/terminal.py
@@ -122,7 +122,7 @@ def TrimAsciiLen(text, size):
return out
-def Print(text='', newline=True, colour=None, limit_to_line=False):
+def Print(text='', newline=True, colour=None, limit_to_line=False, bright=True):
"""Handle a line of output to the terminal.
In test mode this is recorded in a list. Otherwise it is output to the
@@ -140,7 +140,7 @@ def Print(text='', newline=True, colour=None, limit_to_line=False):
else:
if colour:
col = Color()
- text = col.Color(colour, text)
+ text = col.Color(colour, text, bright=bright)
if newline:
print(text)
last_print_len = None
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (8 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 11/16] patman: Allow linking a series with patchwork Simon Glass
` (15 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
Collect response tags such as 'Reviewed-by' while parsing the stream.
This allows us to see what tags are present.
Add a new 'Fixes' tag also, since this is now quite common.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/commit.py | 14 ++++++++++++++
tools/patman/patchstream.py | 21 ++++++++++++++++-----
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/tools/patman/commit.py b/tools/patman/commit.py
index 48d0529c53..8d583c4ed3 100644
--- a/tools/patman/commit.py
+++ b/tools/patman/commit.py
@@ -2,6 +2,7 @@
# Copyright (c) 2011 The Chromium OS Authors.
#
+import collections
import re
# Separates a tag: at the beginning of the subject from the rest of it
@@ -23,6 +24,9 @@ class Commit:
notes: List of lines in the commit (not series) notes
change_id: the Change-Id: tag that was stripped from this commit
and can be used to generate the Message-Id.
+ rtags: Response tags (e.g. Reviewed-by) collected by the commit, dict:
+ key: rtag type (e.g. 'Reviewed-by')
+ value: Set of people who gave that rtag, each a name/email string
"""
def __init__(self, hash):
self.hash = hash
@@ -33,6 +37,7 @@ class Commit:
self.signoff_set = set()
self.notes = []
self.change_id = None
+ self.rtags = collections.defaultdict(set)
def AddChange(self, version, info):
"""Add a new change line to the change list for a version.
@@ -88,3 +93,12 @@ class Commit:
return False
self.signoff_set.add(signoff)
return True
+
+ def AddRtag(self, rtag_type, who):
+ """Add a response tag to a commit
+
+ Args:
+ key: rtag type (e.g. 'Reviewed-by')
+ who: Person who gave that rtag, e.g. 'Fred Bloggs <fred@bloggs.org>'
+ """
+ self.rtags[rtag_type].add(who)
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 2ea8ebcc3f..0c68c86156 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -37,7 +37,7 @@ re_change_id = re.compile('^Change-Id: *(.*)')
re_commit_tag = re.compile('^Commit-([a-z-]*): *(.*)')
# Commit tags that we want to collect and keep
-re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc): (.*)')
+re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc|Fixes): (.*)')
# The start of a new commit in the git log
re_commit = re.compile('^commit ([0-9a-f]*)$')
@@ -112,6 +112,15 @@ class PatchStream:
self.in_section = 'commit-' + name
self.skip_blank = False
+ def AddCommitRtag(self, rtag_type, who):
+ """Add a response tag to the current commit
+
+ Args:
+ key: rtag type (e.g. 'Reviewed-by')
+ who: Person who gave that rtag, e.g. 'Fred Bloggs <fred@bloggs.org>'
+ """
+ self.commit.AddRtag(rtag_type, who)
+
def CloseCommit(self):
"""Save the current commit into our commit list, and reset our state"""
if self.commit and self.is_log:
@@ -346,12 +355,14 @@ class PatchStream:
# Detect tags in the commit message
elif tag_match:
+ rtag_type, who = tag_match.groups()
+ self.AddCommitRtag(rtag_type, who)
# Remove Tested-by self, since few will take much notice
- if (tag_match.group(1) == 'Tested-by' and
- tag_match.group(2).find(os.getenv('USER') + '@') != -1):
+ if (rtag_type == 'Tested-by' and
+ who.find(os.getenv('USER') + '@') != -1):
self.warn.append("Ignoring %s" % line)
- elif tag_match.group(1) == 'Patch-cc':
- self.commit.AddCc(tag_match.group(2).split(','))
+ elif rtag_type == 'Patch-cc':
+ self.commit.AddCc(who.split(','))
else:
out = [line]
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 11/16] patman: Allow linking a series with patchwork
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (9 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:41 ` [RFC PATCH 12/16] patman: Add a -D option to enable debugging Simon Glass
` (14 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
Add a new Series-link tag to tell patman how to find the series in
patchwork.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/README | 8 ++++++++
tools/patman/func_test.py | 1 +
tools/patman/series.py | 2 +-
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/patman/README b/tools/patman/README
index 52b2cf70bd..7291e47d0c 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -187,6 +187,14 @@ Series-name: name
patman does not yet use it, but it is convenient to put the branch
name here to help you keep track of multiple upstreaming efforts.
+Series-link: url
+ Set the URL of the series in patchwork. You can set this after you send
+ out the series and look in patchwork for the resulting series. The
+ URL you want is the one for the series itself, not any particular patch.
+ E.g. http://patchwork.ozlabs.org/project/uboot/list/?series=187331
+ When this is set, patman can compare your local branch against patchwork
+ to see what new reviews your series has collected.
+
Cover-letter:
This is the patch set title
blah blah
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 810af9c604..5eb777054a 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -392,6 +392,7 @@ Series for my board
This series implements support
for my glorious board.
END
+Series-link: http://patchwork.ozlabs.org/project/uboot/list/?series=183237
''', 'serial.c', '''The code for the
serial driver is here''')
self.make_commit_with_file('bootm: Make it boot', '''
diff --git a/tools/patman/series.py b/tools/patman/series.py
index 9f885c8987..edb1141fa8 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -16,7 +16,7 @@ from patman import tools
# Series-xxx tags that we understand
valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name',
- 'cover_cc', 'process_log']
+ 'cover_cc', 'process_log', 'link']
class Series(dict):
"""Holds information about a patch series, including all tags.
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 12/16] patman: Add a -D option to enable debugging
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (10 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 11/16] patman: Allow linking a series with patchwork Simon Glass
@ 2020-07-06 3:41 ` Simon Glass
2020-07-06 3:42 ` [RFC PATCH 13/16] patchstream: Support parsing of review snippets Simon Glass
` (13 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:41 UTC (permalink / raw)
To: u-boot
Most users don't want to see traceback errors. Add an option to enable
them for debugging. Disable them by default.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/main.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 77f187e769..b96000807e 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -10,6 +10,7 @@ from argparse import ArgumentParser
import os
import re
import sys
+import traceback
import unittest
if __name__ == "__main__":
@@ -34,6 +35,8 @@ def AddCommonArgs(parser):
default=-1, help='Automatically create patches from top n commits')
parser.add_argument('-e', '--end', type=int, default=0,
help='Commits to skip at end of patch list')
+ parser.add_argument('-D', '--debug', action='store_true',
+ help='Enabling debugging (provides a full traceback on error)')
parser.add_argument('-s', '--start', dest='start', type=int,
default=0, help='Commit to start creating patches from (0 = HEAD)')
@@ -98,6 +101,9 @@ if hasattr(args, 'project'):
if __name__ != "__main__":
pass
+if not args.debug:
+ sys.tracebacklimit = 0
+
# Run our meagre tests
if args.cmd == 'test':
import doctest
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 13/16] patchstream: Support parsing of review snippets
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (11 preceding siblings ...)
2020-07-06 3:41 ` [RFC PATCH 12/16] patman: Add a -D option to enable debugging Simon Glass
@ 2020-07-06 3:42 ` Simon Glass
2020-07-06 3:42 ` [RFC PATCH 14/16] patman: Support checking for review tags in patchwork Simon Glass
` (12 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:42 UTC (permalink / raw)
To: u-boot
Add support for parsing the contents of a patchwork 'patch' web page
containing comments received from reviewers. This allows patman to show
these comments in a simple 'snippets' format.
A snippet is some quoted code plus some unquoted comments below it. Each
review is from a unique person/email and can produce multiple snippets,
one for each part of the code that attracts a comment.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/patchstream.py | 53 +++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index 0c68c86156..5e2b8e3986 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -2,10 +2,12 @@
# Copyright (c) 2011 The Chromium OS Authors.
#
+import collections
import datetime
import math
import os
import re
+import queue
import shutil
import tempfile
@@ -80,6 +82,10 @@ class PatchStream:
self.state = STATE_MSG_HEADER # What state are we in?
self.signoff = [] # Contents of signoff line
self.commit = None # Current commit
+ self.snippets = [] # List of unquoted test blocks
+ self.recent_quoted = collections.deque([], 5)
+ self.recent_unquoted = queue.Queue()
+ self.was_quoted = None
def AddToSeries(self, line, name, value):
"""Add a new Series-xxx tag.
@@ -165,6 +171,40 @@ class PatchStream:
self.commit.AddChange(self.change_version, change)
self.change_lines = []
+ def FinaliseSnippet(self):
+ """Finish off a snippet and add it to the list
+
+ This is called when we get to the end of a snippet, i.e. the we enter
+ the next block of quoted text:
+
+ This is a comment from someone.
+
+ Something else
+
+ > Now we have some code <----- end of snippet
+ > more code
+
+ Now a comment about the above code
+
+ This adds the snippet to our list
+ """
+ quoted_lines = []
+ while len(self.recent_quoted):
+ quoted_lines.append(self.recent_quoted.popleft())
+ unquoted_lines = []
+ valid = False
+ while not self.recent_unquoted.empty():
+ text = self.recent_unquoted.get()
+ unquoted_lines.append(text)
+ if text:
+ valid = True
+ if valid:
+ lines = quoted_lines + unquoted_lines
+ if lines[0].startswith('On ') and lines[0].endswith('wrote:'):
+ lines = lines[1:]
+ if lines:
+ self.snippets.append(lines)
+
def ProcessLine(self, line):
"""Process a single line of a patch file or commit log
@@ -384,6 +424,18 @@ class PatchStream:
out = [line]
self.linenum += 1
self.skip_blank = False
+
+ # If this is quoted, keep recent lines
+ if self.linenum > 1 and line:
+ if line.startswith('>'):
+ if not self.was_quoted:
+ self.FinaliseSnippet()
+ self.recent_quoted.append(line)
+ self.was_quoted = True
+ else:
+ self.recent_unquoted.put(line)
+ self.was_quoted = False
+
if self.state == STATE_DIFFS:
pass
@@ -407,6 +459,7 @@ class PatchStream:
def Finalize(self):
"""Close out processing of this patch stream"""
+ self.FinaliseSnippet()
self.FinalizeChange()
self.CloseCommit()
if self.lines_after_test:
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 14/16] patman: Support checking for review tags in patchwork
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (12 preceding siblings ...)
2020-07-06 3:42 ` [RFC PATCH 13/16] patchstream: Support parsing of review snippets Simon Glass
@ 2020-07-06 3:42 ` Simon Glass
2020-07-06 3:42 ` [RFC PATCH 15/16] patman: Support updating a branch with review tags Simon Glass
` (11 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:42 UTC (permalink / raw)
To: u-boot
Before sending out a new version of a series for review, it is important
to add any review tags (e.g. Reviewed-by, Acked-by) collected by
patchwork. Otherwise people waste time reviewing the same patch
repeatedly, become frustrated and stop reviewing your patches.
To help with this, add a new 'status' subcommand that checks patchwork
for review tags, showing those which are not present in the local branch.
This allows users to see what new review tags have been received and then
add them.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/README | 34 ++++
tools/patman/control.py | 31 ++++
tools/patman/main.py | 17 ++
tools/patman/status.py | 334 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 416 insertions(+)
create mode 100644 tools/patman/status.py
diff --git a/tools/patman/README b/tools/patman/README
index 7291e47d0c..a85974740f 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -11,6 +11,8 @@ This tool is a Python script which:
- Runs the patches through checkpatch.pl and its own checks
- Optionally emails them out to selected people
+It also shows review tags from Patchwork so you can update your local patches.
+
It is intended to automate patch creation and make it a less
error-prone process. It is useful for U-Boot and Linux work so far,
since it uses the checkpatch.pl script.
@@ -345,6 +347,38 @@ These people will get the cover letter even if they are not on the To/Cc
list for any of the patches.
+Patchwork Integration
+=====================
+
+Patman has a very basic integration with Patchwork. If you point patman to
+your series on patchwork it can show you what new reviews have appears since
+you sent your series.
+
+To set this up, add a Series-link tag to one of the commits in your series
+(see above).
+
+Then you can type
+
+ patman status
+
+and patman will show you each patch and what review tags have been collected,
+for example:
+
+...
+ 21 x86: mtrr: Update the command to use the new mtrr
+ Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
+ + Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
+ 22 x86: mtrr: Restructure so command execution is in
+ Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
+ + Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
+...
+
+This shows that patch 21 and 22 were sent out with one review but have since
+attracted another review each. If the series needs changes, you can update
+these commits with the new review tag before sending the next version of the
+series.
+
+
Example Work Flow
=================
diff --git a/tools/patman/control.py b/tools/patman/control.py
index e67867b845..3bb3c236e4 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -173,3 +173,34 @@ def send(args):
its_a_go, args.ignore_bad_tags, args.add_maintainers,
args.limit, args.dry_run, args.in_reply_to, args.thread,
args.smtp_server)
+
+def patchwork_status(branch, count, start, end):
+ """Check the status of patches in patchwork
+
+ This finds the series in patchwork using the Series-link tag, checks for new
+ comments / review tags and displays them
+
+ Args:
+ branch (str): Branch to create patches from (None = current)
+ count (int): Number of patches to produce, or -1 to produce patches for
+ the current branch back to the upstream commit
+ start (int): Start partch to use (0=first / top of branch)
+ end (int): End patch to use (0=last one in series, 1=one before that,
+ etc.)
+
+ Raises:
+ ValueError: if the branch has no Series-link value
+ """
+ if count == -1:
+ # Work out how many patches to send if we can
+ count = (gitutil.CountCommitsToBranch(branch) - start)
+
+ series = patchstream.GetMetaData(branch, start, count - end)
+ link = series.get('link')
+ if not link:
+ raise ValueError("Branch has no Series-link value")
+
+ # Import this here to avoid failing on other commands if the dependencies
+ # are not present
+ from patman import status
+ status.check_patchwork_status(series, link, branch)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index b96000807e..227eb3b228 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -88,6 +88,10 @@ send.add_argument('patchfiles', nargs='*')
test_parser = subparsers.add_parser('test', help='Run tests')
AddCommonArgs(test_parser)
+status = subparsers.add_parser('status',
+ help='Check status of patches in patchwork')
+AddCommonArgs(status)
+
# Parse options twice: first to get the project and second to handle
# defaults properly (which depends on project).
argv = sys.argv[1:]
@@ -147,3 +151,16 @@ elif args.cmd == 'send':
else:
control.send(args)
+
+# Check status of patches in patchwork
+elif args.cmd == 'status':
+ ret_code = 0
+ try:
+ control.patchwork_status(args.branch, args.count, args.start, args.end)
+ except Exception as e:
+ print('patman: %s: %s' % (type(e).__name__, e))
+ if args.debug:
+ print()
+ traceback.print_exc()
+ ret_code = 1
+ sys.exit(ret_code)
diff --git a/tools/patman/status.py b/tools/patman/status.py
new file mode 100644
index 0000000000..c2e98a5d11
--- /dev/null
+++ b/tools/patman/status.py
@@ -0,0 +1,334 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2020 Google LLC
+#
+# This talks to the patchwork service to figure out what patches have been
+# reviewed and commented on.
+
+from bs4 import BeautifulSoup
+import collections
+import concurrent.futures
+import io
+import pygit2
+from itertools import repeat
+import re
+import requests
+import time
+from urllib.parse import urljoin
+
+from patman import commit
+from patman import gitutil
+from patman import patchstream
+from patman import terminal
+from patman.terminal import Color
+
+# Patches which are part of a multi-patch series are shown with a prefix like
+# [prefix, version, sequence], for example '[RFC, v2, 3/5]'. All but the last
+# part is optional. This decodes the string into groups. For single patches
+# the [] part is not present:
+# Groups: (ignore, ignore, ignore, prefix, version, sequence, subject)
+re_patch = re.compile('(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$')
+
+# This decodes the sequence string into a patch number and patch count
+re_seq = re.compile('(\d+)/(\d+)')
+
+def to_int(vals):
+ """Convert a lsit of strings into integers, using 0 if not an integer
+
+ Args:
+ vals: List of strings
+
+ Returns:
+ List of integers, one for each input string
+ """
+ out = [int(val) if val.isdigit() else 0 for val in vals]
+ return out
+
+
+class Patch(dict):
+ """Models a patch in patchwork
+
+ This class records information obtained from patchwork
+
+ Some of this information comes from the 'Patch' column:
+
+ [RFC,v2,1/3] dm: Driver and uclass changes for tiny-dm
+
+ This shows the prefix, version, seq, count and subject.
+
+ The other properties come from other columns in the display.
+
+ Properties:
+ seq: Sequence number within series (1=first) parsed from sequence string
+ count: Number of patches in series, parsed from sequence string
+ prefix: Prefix string or None
+ subject: Patch subject with [..] part removed (same as commit subject)
+ series: Series name
+ acked: Number of Acked-by rtags
+ fixes: Number of Fixes rtags
+ reviewed: Number of Reviewed-by rtags
+ tested: Number of Tested-by rtags
+ success: Success value
+ warning: Warning value
+ fail: Failure value
+ date: Date as a string
+ Submitter: Submitter name
+ Delegate: Deleted ID
+ state: Current state (e.g. 'New')
+ url: URL pointing to the patch on patchwork
+ """
+ def __init__(self):
+ self.seq = None
+ self.count = None
+ self.prefix = None
+ self.subject = None
+ self.series = None
+ self.acked = None
+ self.fixes = None
+ self.reviewed = None
+ self.tested = None
+ self.success = None
+ self.warning = None
+ self.fail = None
+ self.date = None
+ self.submitter = None
+ self.delegate = None
+ self.state = None
+ self.url = None
+
+ # These make us more like a dictionary
+ def __setattr__(self, name, value):
+ self[name] = value
+
+ def __getattr__(self, name):
+ return self[name]
+
+ def Add(self, name, value):
+ """Add information obtained from the web page
+
+ This sets the object properties based on @name.
+
+ Args:
+ name: Column name in lower-case, e.g. 'Series' or 'State'
+ value: Text obtained for this patch from that column, e.g. 'New'
+ """
+ if name in self:
+ self[name] = value
+ elif name == 'patch':
+ mat = re_patch.search(value.strip())
+ if not mat:
+ raise ValueError("Cannot parse subject '%s'" % value)
+ self.prefix = mat.group(1)
+ self.prefix, self.version, seq_info, self.subject = mat.groups()[3:]
+ mat_seq = re_seq.match(seq_info)
+ if not mat_seq:
+ raise ValueError("Cannot parse sequence spec '%s'" % seq_info)
+ self.seq = int(mat_seq.group(1))
+ self.count = int(mat_seq.group(2))
+ elif name == 'a/f/r/t':
+ self.acked, self.fixes, self.reviewed, self.tested = to_int(
+ value.split())
+ elif name == 's/w/f':
+ self.success, self.warning, self.fail = to_int(value)
+ else:
+ print('Unknown field: %s\n' % name)
+
+ def SetUrl(self, url):
+ """Sets the URL for a patch
+
+ Args:
+ url: URL to set
+ """
+ self.url = url
+
+
+def FindResponses(url):
+ """Collect the response tags (rtags) to a patch found in patchwork
+
+ Reads the URL and checks each comment for an rtags (e.g. Reviewed-by).
+ This does not look at the original patch (which have have rtags picked
+ up from previous rounds of reviews). It only looks at comments which add
+ new rtags.
+
+ Args:
+ url: URL of the patch
+
+ Returns:
+ Dict of rtags:
+ key: rtag type, e.g. 'Acked-by'
+ value: Set of people providing that rtag. Each member of the set
+ is a name/email, e.g. 'Bin Meng <bmeng.cn@gmail.com>'
+ """
+ web_data = requests.get(url)
+ if web_data.status_code != 200:
+ raise ValueError("Could not read URL '%s'" % url)
+ soup = BeautifulSoup(web_data.text, 'html.parser')
+
+ # The first comment is the patch itself, so skip that
+ comments = soup.findAll(class_='comment')[1:]
+ rtags = collections.defaultdict(set)
+ reviews = []
+
+ if comments:
+ for comment in comments:
+ ps = patchstream.PatchStream(None)
+ ps.commit = commit.Commit(None)
+ meta = comment.find(class_='meta')
+ content = comment.find(class_='content')
+ infd = io.StringIO(content.get_text())
+ outfd = io.StringIO()
+ ps.ProcessStream(infd, outfd)
+ for response, people in ps.commit.rtags.items():
+ rtags[response].update(people)
+ return rtags
+
+def CollectPatches(series, url):
+ """Collect patch information about a series from patchwork
+
+ Reads the web page and collects information provided by patchwork about
+ the status of each patch.
+
+ Args:
+ series: Series object corresponding to the local branch containing the
+ series
+ url: URL pointing to patchwork's view of this series, possibly an
+ earlier versions of it
+
+ Returns:
+ List of patches sorted by sequence number, each a Patch object
+ """
+ response = requests.get(url)
+ if response.status_code != 200:
+ raise ValueError("Could not read URL '%s'" % url)
+ soup = BeautifulSoup(response.text, 'html.parser')
+
+ # Find the main table
+ tables = soup.findAll('table')
+ if len(tables) != 1:
+ raise ValueError('Failed to parse page (expected only one table)')
+ tab = tables[0]
+
+ # Get all the rows, which are patches
+ rows = tab.tbody.findAll('tr')
+ count = len(rows)
+ num_commits = len(series.commits)
+ if count != num_commits:
+ print('Warning: Web page reports %d patches, series has %d' %
+ (count, num_commits))
+
+ # Get the column headers, which we use to figure out which column is which
+ hdrs = tab.thead.findAll('th')
+ cols = []
+ for hdr in hdrs:
+ cols.append(hdr.get_text().strip())
+ patches = []
+
+ # Work through each row (patch) one at a time, collecting the information
+ for row in rows:
+ patch = Patch()
+ for seq, col in enumerate(row.findAll('td')):
+ name = cols[seq].lower()
+ patch.Add(name, col.get_text())
+ if name == 'patch':
+ patch.SetUrl(urljoin(url, col.a['href']))
+ if patch.count != count:
+ raise ValueError("Patch %d '%s' has count of %d, expected %d" %
+ (patch.seq, patch.subject, patch.count, num_commits))
+ patches.append(patch)
+
+ # Sort patches by patch number
+ patches = sorted(patches, key=lambda x: x.seq)
+ return patches
+
+def FindNewResponses(new_rtag_list, seq, commit, patch):
+ """Find new rtags collected by patchwork that we don't know about
+
+ This is designed to be run in parallel, once for each commit/patch
+
+ Args:
+ new_rtag_list: New rtags are written to new_rtag_list[seq]
+ list, each a dict:
+ key: Response tag (e.g. 'Reviewed-by')
+ value: Set of people who gave that response, each a name/email
+ string
+ seq: Position in new_rtag_list to update
+ commit: Commit object for this commit
+ patch: Corresponding Patch object for this patch
+ """
+ acked = commit.rtags['Acked-by']
+ reviewed = commit.rtags['Reviewed-by']
+ tested = commit.rtags['Tested-by']
+ fixes = commit.rtags['Fixes']
+
+ # Figure out the number of new rtags by subtracting what we have in
+ # the commit from what patchwork's summary reports.
+ extra = 0
+ extra += patch.acked - len(acked)
+ extra += patch.fixes - len(fixes)
+ extra += patch.reviewed - len(reviewed)
+ extra += patch.tested - len(tested)
+
+ # If patchwork agrees with the commit, we don't need to read the web
+ # page, so save some time. If not, we need to add the new ones to the commit
+ new_rtags = collections.defaultdict(set)
+ if extra:
+ pw_rtags = FindResponses(patch.url)
+ base_rtags = commit.rtags
+ new_count = 0
+ for tag, people in pw_rtags.items():
+ for who in people:
+ is_new = (tag not in base_rtags or
+ who not in base_rtags[tag])
+ if is_new:
+ new_rtags[tag].add(who)
+ new_count += 1
+ new_rtag_list[seq] = new_rtags
+
+def ShowResponses(rtags, indent, is_new):
+ """Show rtags collected
+
+ Args:
+ rtags: dict:
+ key: Response tag (e.g. 'Reviewed-by')
+ value: Set of people who gave that response, each a name/email string
+ indent: Indentation string to write before each line
+ is_new: True if this output should be highlighted
+ """
+ col = terminal.Color()
+ count = 0
+ for tag, people in rtags.items():
+ for who in people:
+ terminal.Print(indent + '%s %s: ' % ('+' if is_new else ' ', tag),
+ newline=False, colour=col.GREEN, bright=is_new)
+ terminal.Print(who, colour=col.WHITE, bright=is_new)
+ count += 1
+ return count
+
+def check_patchwork_status(series, url, branch):
+ patches = CollectPatches(series, url)
+ col = terminal.Color()
+ count = len(patches)
+ new_rtag_list = [None] * count
+
+ with concurrent.futures.ThreadPoolExecutor(max_workers=16) as executor:
+ futures = executor.map(
+ FindNewResponses, repeat(new_rtag_list), range(count),
+ series.commits, patches)
+ for fresponse in futures:
+ if fresponse:
+ raise fresponse.exception()
+
+ num_to_add = 0
+ for seq, patch in enumerate(patches):
+ terminal.Print('%3d %s' % (patch.seq, patch.subject[:50]),
+ colour=col.BLUE)
+ commit = series.commits[seq]
+ base_rtags = commit.rtags
+ new_rtags = new_rtag_list[seq]
+
+ indent = ' ' * 2
+ ShowResponses(base_rtags, indent, False)
+ num_to_add += ShowResponses(new_rtags, indent, True)
+
+ print("%d new response%s available in patchwork" %
+ (num_to_add, 's' if num_to_add != 1 else ''))
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 15/16] patman: Support updating a branch with review tags
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (13 preceding siblings ...)
2020-07-06 3:42 ` [RFC PATCH 14/16] patman: Support checking for review tags in patchwork Simon Glass
@ 2020-07-06 3:42 ` Simon Glass
2020-07-06 3:42 ` [RFC PATCH 16/16] patman: Support listing comments from patchwork Simon Glass
` (10 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:42 UTC (permalink / raw)
To: u-boot
It is tedious to add review tags into the local branch and errors can
sometimes be made. Add an option to create a new branch with the review
tags obtained from patchwork.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/README | 17 +++++++++++--
tools/patman/control.py | 9 ++++---
tools/patman/main.py | 7 +++++-
tools/patman/status.py | 54 ++++++++++++++++++++++++++++++++++++++---
4 files changed, 78 insertions(+), 9 deletions(-)
diff --git a/tools/patman/README b/tools/patman/README
index a85974740f..595a0d616b 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -11,11 +11,13 @@ This tool is a Python script which:
- Runs the patches through checkpatch.pl and its own checks
- Optionally emails them out to selected people
-It also shows review tags from Patchwork so you can update your local patches.
+It also has some Patchwork features:
+- shows review tags from Patchwork so you can update your local patches
+- pulls these down into a new branch on request
It is intended to automate patch creation and make it a less
error-prone process. It is useful for U-Boot and Linux work so far,
-since it uses the checkpatch.pl script.
+since they use the checkpatch.pl script.
It is configured almost entirely by tags it finds in your commits.
This means that you can work on a number of different branches at
@@ -378,6 +380,17 @@ attracted another review each. If the series needs changes, you can update
these commits with the new review tag before sending the next version of the
series.
+To automatically pull into these tags into a new branch, use the -d option:
+
+ patman status -d mtrr4
+
+This will create a new 'mtrr4' branch which is the same as your current branch
+but has the new review tags in it. You can check that this worked with:
+
+ patman -b mtrr4 status
+
+which should show that there are no new responses compared to this new branch.
+
Example Work Flow
=================
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 3bb3c236e4..b6ba0a56c0 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -174,11 +174,11 @@ def send(args):
args.limit, args.dry_run, args.in_reply_to, args.thread,
args.smtp_server)
-def patchwork_status(branch, count, start, end):
+def patchwork_status(branch, count, start, end, dest_branch, force):
"""Check the status of patches in patchwork
This finds the series in patchwork using the Series-link tag, checks for new
- comments / review tags and displays them
+ review tags, displays then and creates a new branch with the review tags.
Args:
branch (str): Branch to create patches from (None = current)
@@ -187,6 +187,9 @@ def patchwork_status(branch, count, start, end):
start (int): Start partch to use (0=first / top of branch)
end (int): End patch to use (0=last one in series, 1=one before that,
etc.)
+ dest_branch (str): Name of new branch to create with the updated tags
+ (None to not create a branch)
+ force (bool): With dest_branch, force overwriting an existing branch
Raises:
ValueError: if the branch has no Series-link value
@@ -203,4 +206,4 @@ def patchwork_status(branch, count, start, end):
# Import this here to avoid failing on other commands if the dependencies
# are not present
from patman import status
- status.check_patchwork_status(series, link, branch)
+ status.check_patchwork_status(series, link, branch, dest_branch, force)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 227eb3b228..c8fc035c10 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -90,6 +90,10 @@ AddCommonArgs(test_parser)
status = subparsers.add_parser('status',
help='Check status of patches in patchwork')
+status.add_argument('-d', '--dest-branch', type=str,
+ help='Name of branch to create with collected responses')
+status.add_argument('-f', '--force', action='store_true',
+ help='Force overwriting an existing branch')
AddCommonArgs(status)
# Parse options twice: first to get the project and second to handle
@@ -156,7 +160,8 @@ elif args.cmd == 'send':
elif args.cmd == 'status':
ret_code = 0
try:
- control.patchwork_status(args.branch, args.count, args.start, args.end)
+ control.patchwork_status(args.branch, args.count, args.start, args.end,
+ args.dest_branch, args.force)
except Exception as e:
print('patman: %s: %s' % (type(e).__name__, e))
if args.debug:
diff --git a/tools/patman/status.py b/tools/patman/status.py
index c2e98a5d11..71124d6e5e 100644
--- a/tools/patman/status.py
+++ b/tools/patman/status.py
@@ -304,7 +304,48 @@ def ShowResponses(rtags, indent, is_new):
count += 1
return count
-def check_patchwork_status(series, url, branch):
+def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite):
+ if branch == dest_branch:
+ raise ValueError(
+ 'Destination branch must not be the same as the original branch')
+ repo = pygit2.Repository('.')
+ count = len(series.commits)
+ old_br = repo.branches[branch]
+ new_br = repo.branches.get(dest_branch)
+ if new_br:
+ if not overwrite:
+ raise ValueError("Branch '%s' already exists (-f to overwrite)" %
+ dest_branch)
+ new_br.delete()
+ target = repo.revparse_single('%s~%d' % (branch, count))
+ repo.branches.local.create(dest_branch, target)
+
+ num_added = 0
+ for seq in range(count):
+ basket = repo.branches.get(dest_branch)
+ cherry = repo.revparse_single('%s~%d' % (branch, count - seq - 1))
+
+ base = repo.merge_base(cherry.oid, basket.target)
+ base_tree = cherry.parents[0].tree
+
+ index = repo.merge_trees(base_tree, basket, cherry)
+ tree_id = index.write_tree(repo)
+
+ author = cherry.author
+ committer = cherry.committer
+ lines = []
+ for tag, people in new_rtag_list[seq].items():
+ for who in people:
+ lines.append('%s: %s' % (tag, who))
+ num_added += 1
+ message = cherry.message + '\n'.join(lines)
+
+ basket = repo.create_commit(
+ basket.name, cherry.author, cherry.committer, message, tree_id,
+ [basket.target])
+ return num_added
+
+def check_patchwork_status(series, url, branch, dest_branch, force):
patches = CollectPatches(series, url)
col = terminal.Color()
count = len(patches)
@@ -330,5 +371,12 @@ def check_patchwork_status(series, url, branch):
ShowResponses(base_rtags, indent, False)
num_to_add += ShowResponses(new_rtags, indent, True)
- print("%d new response%s available in patchwork" %
- (num_to_add, 's' if num_to_add != 1 else ''))
+ print("%d new response%s available in patchwork%s" %
+ (num_to_add, 's' if num_to_add != 1 else '',
+ '' if dest_branch else ' (use -d to write them to a new branch)'))
+
+ if dest_branch:
+ num_added = CreateBranch(series, new_rtag_list, branch,
+ dest_branch, force)
+ print("%d response%s added from patchwork into new branch '%s'" %
+ (num_added, 's' if num_added != 1 else '', dest_branch))
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 16/16] patman: Support listing comments from patchwork
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (14 preceding siblings ...)
2020-07-06 3:42 ` [RFC PATCH 15/16] patman: Support updating a branch with review tags Simon Glass
@ 2020-07-06 3:42 ` Simon Glass
2020-07-15 9:10 ` [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Michal Simek
` (9 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-06 3:42 UTC (permalink / raw)
To: u-boot
While reviewing feedback it is helpful to see the review comments on the
command line to check that each has been addressed. Add an option to
support that.
Update the workflow documentation to describe the new features.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/README | 36 +++++-
tools/patman/control.py | 11 +-
tools/patman/main.py | 5 +-
tools/patman/status.py | 278 +++++++++++++++++++++++++++-------------
4 files changed, 227 insertions(+), 103 deletions(-)
diff --git a/tools/patman/README b/tools/patman/README
index 595a0d616b..0ce299d2c0 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -14,6 +14,7 @@ This tool is a Python script which:
It also has some Patchwork features:
- shows review tags from Patchwork so you can update your local patches
- pulls these down into a new branch on request
+- lists comments received on a series
It is intended to automate patch creation and make it a less
error-prone process. It is useful for U-Boot and Linux work so far,
@@ -391,6 +392,8 @@ but has the new review tags in it. You can check that this worked with:
which should show that there are no new responses compared to this new branch.
+There is also a -C option to list the comments received for each patch.
+
Example Work Flow
=================
@@ -475,17 +478,33 @@ people on the list don't see your secret info.
Of course patches often attract comments and you need to make some updates.
Let's say one person sent comments and you get an Acked-by: on one patch.
Also, the patch on the list that you were waiting for has been merged,
-so you can drop your wip commit. So you resync with upstream:
+so you can drop your wip commit.
+
+Take a look on patchwork and find out the URL of the series. This will be
+something like http://patchwork.ozlabs.org/project/uboot/list/?series=187331
+Add this to a tag in your top commit:
+
+ Series-link: http://patchwork.ozlabs.org/project/uboot/list/?series=187331
+
+You can use then patman to collect the Acked-by tag to the correct commit,
+creating a new 'version 2' branch for us-cmd:
+
+ patman status -d us-cmd2
+ git checkout us-cmd2
+
+You can look at the comments in Patchwork or with:
+
+ patman status -C
+
+Then you can resync with upstream:
git fetch origin (or whatever upstream is called)
git rebase origin/master
-and use git rebase -i to edit the commits, dropping the wip one. You add
-the ack tag to one commit:
-
- Acked-by: Heiko Schocher <hs@denx.de>
+and use git rebase -i to edit the commits, dropping the wip one.
-update the Series-cc: in the top commit:
+Then update the Series-cc: in the top commit to add the person who reviewed
+the v1 series:
Series-cc: bfin, marex, Heiko Schocher <hs@denx.de>
@@ -524,7 +543,9 @@ so to send them:
and it will create and send the version 2 series.
-General points:
+
+General points
+==============
1. When you change back to the us-cmd branch days or weeks later all your
information is still there, safely stored in the commits. You don't need
@@ -606,3 +627,4 @@ a bad thing.
Simon Glass <sjg@chromium.org>
v1, v2, 19-Oct-11
revised v3 24-Nov-11
+revised v4 Independence Day 2020, with Patchwork integration
diff --git a/tools/patman/control.py b/tools/patman/control.py
index b6ba0a56c0..b7d23b58c1 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -174,11 +174,13 @@ def send(args):
args.limit, args.dry_run, args.in_reply_to, args.thread,
args.smtp_server)
-def patchwork_status(branch, count, start, end, dest_branch, force):
+def patchwork_status(branch, count, start, end, dest_branch, force,
+ show_comments):
"""Check the status of patches in patchwork
This finds the series in patchwork using the Series-link tag, checks for new
- review tags, displays then and creates a new branch with the review tags.
+ comments and review tags, displays then and creates a new branch with the
+ review tags.
Args:
branch (str): Branch to create patches from (None = current)
@@ -190,6 +192,8 @@ def patchwork_status(branch, count, start, end, dest_branch, force):
dest_branch (str): Name of new branch to create with the updated tags
(None to not create a branch)
force (bool): With dest_branch, force overwriting an existing branch
+ show_comments (bool): True to display snippets from the comments
+ provided by reviewers
Raises:
ValueError: if the branch has no Series-link value
@@ -206,4 +210,5 @@ def patchwork_status(branch, count, start, end, dest_branch, force):
# Import this here to avoid failing on other commands if the dependencies
# are not present
from patman import status
- status.check_patchwork_status(series, link, branch, dest_branch, force)
+ status.check_patchwork_status(series, link, branch, dest_branch, force,
+show_comments)
diff --git a/tools/patman/main.py b/tools/patman/main.py
index c8fc035c10..6e4e23763d 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -90,6 +90,8 @@ AddCommonArgs(test_parser)
status = subparsers.add_parser('status',
help='Check status of patches in patchwork')
+status.add_argument('-C', '--show-comments', action='store_true',
+ help='Show comments from each patch')
status.add_argument('-d', '--dest-branch', type=str,
help='Name of branch to create with collected responses')
status.add_argument('-f', '--force', action='store_true',
@@ -161,7 +163,8 @@ elif args.cmd == 'status':
ret_code = 0
try:
control.patchwork_status(args.branch, args.count, args.start, args.end,
- args.dest_branch, args.force)
+ args.dest_branch, args.force,
+ args.show_comments)
except Exception as e:
print('patman: %s: %s' % (type(e).__name__, e))
if args.debug:
diff --git a/tools/patman/status.py b/tools/patman/status.py
index 71124d6e5e..9e3cb97e72 100644
--- a/tools/patman/status.py
+++ b/tools/patman/status.py
@@ -2,44 +2,45 @@
#
# Copyright 2020 Google LLC
#
-# This talks to the patchwork service to figure out what patches have been
-# reviewed and commented on.
-from bs4 import BeautifulSoup
+"""Talks to the patchwork service to figure out what patches have been
+reviewed and commented on. Provides a way to display review tags and comments.
+Allows creation of a new branch based on the old but with the review tags
+collected from patchwork.
+"""
+
import collections
import concurrent.futures
import io
-import pygit2
from itertools import repeat
import re
-import requests
-import time
from urllib.parse import urljoin
+import pygit2
+import requests
+from bs4 import BeautifulSoup
from patman import commit
-from patman import gitutil
from patman import patchstream
from patman import terminal
-from patman.terminal import Color
# Patches which are part of a multi-patch series are shown with a prefix like
# [prefix, version, sequence], for example '[RFC, v2, 3/5]'. All but the last
# part is optional. This decodes the string into groups. For single patches
# the [] part is not present:
# Groups: (ignore, ignore, ignore, prefix, version, sequence, subject)
-re_patch = re.compile('(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$')
+RE_PATCH = re.compile(r'(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$')
# This decodes the sequence string into a patch number and patch count
-re_seq = re.compile('(\d+)/(\d+)')
+RE_SEQ = re.compile(r'(\d+)/(\d+)')
def to_int(vals):
- """Convert a lsit of strings into integers, using 0 if not an integer
+ """Convert a list of strings into integers, using 0 if not an integer
Args:
- vals: List of strings
+ vals (list): List of strings
Returns:
- List of integers, one for each input string
+ list: List of integers, one for each input string
"""
out = [int(val) if val.isdigit() else 0 for val in vals]
return out
@@ -59,28 +60,33 @@ class Patch(dict):
The other properties come from other columns in the display.
Properties:
- seq: Sequence number within series (1=first) parsed from sequence string
- count: Number of patches in series, parsed from sequence string
- prefix: Prefix string or None
- subject: Patch subject with [..] part removed (same as commit subject)
- series: Series name
- acked: Number of Acked-by rtags
- fixes: Number of Fixes rtags
- reviewed: Number of Reviewed-by rtags
- tested: Number of Tested-by rtags
- success: Success value
- warning: Warning value
- fail: Failure value
- date: Date as a string
- Submitter: Submitter name
- Delegate: Deleted ID
- state: Current state (e.g. 'New')
- url: URL pointing to the patch on patchwork
+ seq (int): Sequence number within series (1=first) parsed from sequence
+ string
+ count (int): Number of patches in series, parsed from sequence string
+ prefix (str): Prefix string or None (e.g. 'RFC')
+ version (str): Version string or None (e.g. 'v2')
+ subject (str): Patch subject with [..] part removed (same as commit
+ subject)
+ series (str): Series name
+ acked (int): Number of Acked-by rtags, or None if not known
+ fixes (int): Number of Fixes rtags, or None if not known
+ reviewed (int): Number of Reviewed-by rtags, or None if not known
+ tested (int): Number of Tested-by rtags, or None if not known
+ success (int): Success value, or None if not known
+ warning (int): Warning value, or None if not known
+ fail (int): Failure value, or None if not known
+ date (str): Date as a string
+ Submitter (str): Submitter name
+ Delegate (str): Delegate ID
+ state (str): Current state (e.g. 'New')
+ url (str): URL pointing to the patch on patchwork
"""
def __init__(self):
+ super().__init__()
self.seq = None
self.count = None
self.prefix = None
+ self.version = None
self.subject = None
self.series = None
self.acked = None
@@ -103,24 +109,28 @@ class Patch(dict):
def __getattr__(self, name):
return self[name]
- def Add(self, name, value):
+ def add(self, name, value):
"""Add information obtained from the web page
This sets the object properties based on @name.
Args:
- name: Column name in lower-case, e.g. 'Series' or 'State'
- value: Text obtained for this patch from that column, e.g. 'New'
+ name (str): Column name in lower-case, e.g. 'Series' or 'State'
+ value (str): Text obtained for this patch from that column,
+ e.g. 'New'
+
+ Raises:
+ ValueError: if the name cannot be parsed or is not recognised
"""
if name in self:
self[name] = value
elif name == 'patch':
- mat = re_patch.search(value.strip())
+ mat = RE_PATCH.search(value.strip())
if not mat:
raise ValueError("Cannot parse subject '%s'" % value)
self.prefix = mat.group(1)
self.prefix, self.version, seq_info, self.subject = mat.groups()[3:]
- mat_seq = re_seq.match(seq_info)
+ mat_seq = RE_SEQ.match(seq_info)
if not mat_seq:
raise ValueError("Cannot parse sequence spec '%s'" % seq_info)
self.seq = int(mat_seq.group(1))
@@ -133,16 +143,35 @@ class Patch(dict):
else:
print('Unknown field: %s\n' % name)
- def SetUrl(self, url):
+ def set_url(self, url):
"""Sets the URL for a patch
Args:
- url: URL to set
+ url (str): URL to set
"""
self.url = url
-def FindResponses(url):
+class Review:
+ """Represents a single review email collected in Patchwork
+
+ Patches can attract multiple reviews. Each consists of an author/date and
+ a variable number of 'snippets', which are groups of quoted and unquoted
+ text.
+ """
+ def __init__(self, meta, snippets):
+ """Create new Review object
+
+ Args:
+ meta (str): Text containing review author and date
+ snippets (list): List of snippets in th review, each a list of text
+ lines
+ """
+ self.meta = ' : '.join([line for line in meta.splitlines() if line])
+ self.snippets = snippets
+
+
+def find_responses(url):
"""Collect the response tags (rtags) to a patch found in patchwork
Reads the URL and checks each comment for an rtags (e.g. Reviewed-by).
@@ -151,13 +180,18 @@ def FindResponses(url):
new rtags.
Args:
- url: URL of the patch
+ url (str): URL of the patch
Returns:
- Dict of rtags:
- key: rtag type, e.g. 'Acked-by'
- value: Set of people providing that rtag. Each member of the set
- is a name/email, e.g. 'Bin Meng <bmeng.cn@gmail.com>'
+ Tuple:
+ Dict of rtags:
+ key: rtag type, e.g. 'Acked-by'
+ value: Set of people providing that rtag. Each member of the set
+ is a name/email, e.g. 'Bin Meng <bmeng.cn@gmail.com>'
+ List of Review objects for the patch
+
+ Raises:
+ ValueError: if the URL could not be read
"""
web_data = requests.get(url)
if web_data.status_code != 200:
@@ -171,31 +205,37 @@ def FindResponses(url):
if comments:
for comment in comments:
- ps = patchstream.PatchStream(None)
- ps.commit = commit.Commit(None)
+ pstrm = patchstream.PatchStream(None)
+ pstrm.commit = commit.Commit(None)
meta = comment.find(class_='meta')
content = comment.find(class_='content')
infd = io.StringIO(content.get_text())
outfd = io.StringIO()
- ps.ProcessStream(infd, outfd)
- for response, people in ps.commit.rtags.items():
+ pstrm.ProcessStream(infd, outfd)
+ for response, people in pstrm.commit.rtags.items():
rtags[response].update(people)
- return rtags
+ if pstrm.snippets:
+ reviews.append(Review(meta.get_text(), pstrm.snippets))
+ return rtags, reviews
-def CollectPatches(series, url):
+def collect_patches(series, url):
"""Collect patch information about a series from patchwork
Reads the web page and collects information provided by patchwork about
the status of each patch.
Args:
- series: Series object corresponding to the local branch containing the
- series
- url: URL pointing to patchwork's view of this series, possibly an
+ series (Series): Series object corresponding to the local branch
+ containing the series
+ url (str): URL pointing to patchwork's view of this series, possibly an
earlier versions of it
Returns:
- List of patches sorted by sequence number, each a Patch object
+ list: List of patches sorted by sequence number, each a Patch object
+
+ Raises:
+ ValueError: if the URL could not be read or the web page does not follow
+ the expected structure
"""
response = requests.get(url)
if response.status_code != 200:
@@ -228,11 +268,12 @@ def CollectPatches(series, url):
patch = Patch()
for seq, col in enumerate(row.findAll('td')):
name = cols[seq].lower()
- patch.Add(name, col.get_text())
+ patch.add(name, col.get_text())
if name == 'patch':
- patch.SetUrl(urljoin(url, col.a['href']))
+ patch.set_url(urljoin(url, col.a['href']))
if patch.count != count:
- raise ValueError("Patch %d '%s' has count of %d, expected %d" %
+ raise ValueError(
+ "Patch %d '%s' has count of %d, expected %d" %
(patch.seq, patch.subject, patch.count, num_commits))
patches.append(patch)
@@ -240,25 +281,31 @@ def CollectPatches(series, url):
patches = sorted(patches, key=lambda x: x.seq)
return patches
-def FindNewResponses(new_rtag_list, seq, commit, patch):
+def find_new_responses(new_rtag_list, review_list, seq, cmt, patch,
+ force_load):
"""Find new rtags collected by patchwork that we don't know about
This is designed to be run in parallel, once for each commit/patch
Args:
- new_rtag_list: New rtags are written to new_rtag_list[seq]
+ new_rtag_list (list): New rtags are written to new_rtag_list[seq]
list, each a dict:
key: Response tag (e.g. 'Reviewed-by')
value: Set of people who gave that response, each a name/email
string
- seq: Position in new_rtag_list to update
- commit: Commit object for this commit
- patch: Corresponding Patch object for this patch
+ review_list (list): New reviews are written to review_list[seq]
+ list, each a
+ List of reviews for the patch, each a Review
+ seq (int): Position in new_rtag_list to update
+ cmt (Commit): Commit object for this commit
+ patch (Patch): Corresponding Patch object for this patch
+ force_load (bool): True to always load the patch from patchwork, False
+ to only load it if patchwork has additional response tags
"""
- acked = commit.rtags['Acked-by']
- reviewed = commit.rtags['Reviewed-by']
- tested = commit.rtags['Tested-by']
- fixes = commit.rtags['Fixes']
+ acked = cmt.rtags['Acked-by']
+ reviewed = cmt.rtags['Reviewed-by']
+ tested = cmt.rtags['Tested-by']
+ fixes = cmt.rtags['Fixes']
# Figure out the number of new rtags by subtracting what we have in
# the commit from what patchwork's summary reports.
@@ -271,9 +318,10 @@ def FindNewResponses(new_rtag_list, seq, commit, patch):
# If patchwork agrees with the commit, we don't need to read the web
# page, so save some time. If not, we need to add the new ones to the commit
new_rtags = collections.defaultdict(set)
- if extra:
- pw_rtags = FindResponses(patch.url)
- base_rtags = commit.rtags
+ reviews = None
+ if extra or force_load:
+ pw_rtags, reviews = find_responses(patch.url)
+ base_rtags = cmt.rtags
new_count = 0
for tag, people in pw_rtags.items():
for who in people:
@@ -283,16 +331,20 @@ def FindNewResponses(new_rtag_list, seq, commit, patch):
new_rtags[tag].add(who)
new_count += 1
new_rtag_list[seq] = new_rtags
+ review_list[seq] = reviews
-def ShowResponses(rtags, indent, is_new):
+def show_responses(rtags, indent, is_new):
"""Show rtags collected
Args:
- rtags: dict:
+ rtags (dict): review tags to show
key: Response tag (e.g. 'Reviewed-by')
value: Set of people who gave that response, each a name/email string
- indent: Indentation string to write before each line
- is_new: True if this output should be highlighted
+ indent (str): Indentation string to write before each line
+ is_new (bool): True if this output should be highlighted
+
+ Returns:
+ int: Number of review tags displayed
"""
col = terminal.Color()
count = 0
@@ -304,13 +356,32 @@ def ShowResponses(rtags, indent, is_new):
count += 1
return count
-def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite):
+def create_branch(series, new_rtag_list, branch, dest_branch, overwrite):
+ """Create a new branch with review tags added
+
+ Args:
+ series (Series): Series object for the existing branch
+ new_rtag_list (list): List of review tags to add, one for each commit,
+ each a dict:
+ key: Response tag (e.g. 'Reviewed-by')
+ value: Set of people who gave that response, each a name/email
+ string
+ branch (str): Existing branch to update
+ dest_branch (str): Name of new branch to create
+ overwrite (bool): True to force overwriting dest_branch if it exists
+
+ Returns:
+ int: Total number of review tags added across all commits
+
+ Raises:
+ ValueError: if the destination branch name is the same as the original
+ branch, or it already exists and @overwrite is False
+ """
if branch == dest_branch:
raise ValueError(
'Destination branch must not be the same as the original branch')
repo = pygit2.Repository('.')
count = len(series.commits)
- old_br = repo.branches[branch]
new_br = repo.branches.get(dest_branch)
if new_br:
if not overwrite:
@@ -322,17 +393,15 @@ def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite):
num_added = 0
for seq in range(count):
- basket = repo.branches.get(dest_branch)
+ parent = repo.branches.get(dest_branch)
cherry = repo.revparse_single('%s~%d' % (branch, count - seq - 1))
- base = repo.merge_base(cherry.oid, basket.target)
+ repo.merge_base(cherry.oid, parent.target)
base_tree = cherry.parents[0].tree
- index = repo.merge_trees(base_tree, basket, cherry)
+ index = repo.merge_trees(base_tree, parent, cherry)
tree_id = index.write_tree(repo)
- author = cherry.author
- committer = cherry.committer
lines = []
for tag, people in new_rtag_list[seq].items():
for who in people:
@@ -340,21 +409,37 @@ def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite):
num_added += 1
message = cherry.message + '\n'.join(lines)
- basket = repo.create_commit(
- basket.name, cherry.author, cherry.committer, message, tree_id,
- [basket.target])
+ repo.create_commit(
+ parent.name, cherry.author, cherry.committer, message, tree_id,
+ [parent.target])
return num_added
-def check_patchwork_status(series, url, branch, dest_branch, force):
- patches = CollectPatches(series, url)
+def check_patchwork_status(series, url, branch, dest_branch, force,
+ show_comments):
+ """Check the status of a series on Patchwork
+
+ This finds review tags and comments for a series in Patchwork, displaying
+ them and optionally creating a new branch like the old but with the new
+ review tags.
+
+ Args:
+ series (Series): Series object for the existing branch
+ url (str): URL pointing to the patch on patchwork
+ branch (str): Existing branch to update
+ dest_branch (str): Name of new branch to create
+ force (bool): True to force overwriting dest_branch if it exists
+ show_comments (bool): True to show the comments on each patch
+ """
+ patches = collect_patches(series, url)
col = terminal.Color()
count = len(patches)
new_rtag_list = [None] * count
+ review_list = [None] * count
with concurrent.futures.ThreadPoolExecutor(max_workers=16) as executor:
futures = executor.map(
- FindNewResponses, repeat(new_rtag_list), range(count),
- series.commits, patches)
+ find_new_responses, repeat(new_rtag_list), repeat(review_list),
+ range(count), series.commits, patches, repeat(show_comments))
for fresponse in futures:
if fresponse:
raise fresponse.exception()
@@ -363,20 +448,29 @@ def check_patchwork_status(series, url, branch, dest_branch, force):
for seq, patch in enumerate(patches):
terminal.Print('%3d %s' % (patch.seq, patch.subject[:50]),
colour=col.BLUE)
- commit = series.commits[seq]
- base_rtags = commit.rtags
+ cmt = series.commits[seq]
+ base_rtags = cmt.rtags
new_rtags = new_rtag_list[seq]
indent = ' ' * 2
- ShowResponses(base_rtags, indent, False)
- num_to_add += ShowResponses(new_rtags, indent, True)
+ show_responses(base_rtags, indent, False)
+ num_to_add += show_responses(new_rtags, indent, True)
+ if show_comments:
+ for review in review_list[seq]:
+ terminal.Print('Review: %s' % review.meta, colour=col.RED)
+ for snippet in review.snippets:
+ for line in snippet:
+ quoted = line.startswith('>')
+ terminal.Print(' %s' % line,
+ colour=col.MAGENTA if quoted else None)
+ print()
print("%d new response%s available in patchwork%s" %
(num_to_add, 's' if num_to_add != 1 else '',
'' if dest_branch else ' (use -d to write them to a new branch)'))
if dest_branch:
- num_added = CreateBranch(series, new_rtag_list, branch,
- dest_branch, force)
+ num_added = create_branch(series, new_rtag_list, branch,
+ dest_branch, force)
print("%d response%s added from patchwork into new branch '%s'" %
(num_added, 's' if num_added != 1 else '', dest_branch))
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (15 preceding siblings ...)
2020-07-06 3:42 ` [RFC PATCH 16/16] patman: Support listing comments from patchwork Simon Glass
@ 2020-07-15 9:10 ` Michal Simek
2020-07-19 20:49 ` [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream Simon Glass
` (8 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Michal Simek @ 2020-07-15 9:10 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 06. 07. 20 5:41, Simon Glass wrote:
> Patman is a useful tool for creating, checking sending out patches. It
> automates the creation of patches, simplifies checking them and handles
> cover letters and change logs.
>
> However once patches are sent and reviewers add Reviewed-by tags, etc.,
> these must be manually added into the commits using git before the next
> version of the series is sent. Also, the only way to look at patches is
> in the web interface.
>
> It would be nice if patman could talk to Patchwork and collect the review
> tags itself. Even better if it could show the review comments in a
> command-line view patch-by-patch to speed up addressing comments.
>
> This series adds these features to patman, talking directly to the web
> URLs. While pwclient seems like it could handle some of this, or at least
> provide a library to patman, the documentation[1] doesn't really explain
> what the commands do and it doesn't seem to work with the current
> Patchwork (e.g. pwclient git-am requires a patch number but Patchwork
> seems to use an ID now).
>
> With these changes, which currently lack tests, it is possible to type
> 'patman status' and see a list of new review tags. It is also possible
> to create a new branch with those tags and list out the review comments
> as small snippets on the command line.
>
> I have been using these features for a short while and they seem useful,
> so herewith a series.
>
> This series depends on u-boot-dm/next so you might find it easier to get
> it from u-boot-dm/patman-working[2]
>
> [1] https://patchwork.readthedocs.io/projects/pwclient/en/latest/usage/
> [2] https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/patman-working
>
>
> Simon Glass (16):
> patman: Use test_util to show test results
> patman: Move main code out to a control module
> patman: Add a test that uses gitpython
> patman: Allow creating patches for another branch
> patman: Allow skipping patches at the end
> patman: Convert to ArgumentParser
> patman: Allow different commands
> patman: Add a 'test' subcommand
> patman: Allow disabling 'bright' mode with Print output
> patman: Support collecting response tags in Patchstream
> patman: Allow linking a series with patchwork
> patman: Add a -D option to enable debugging
> patchstream: Support parsing of review snippets
> patman: Support checking for review tags in patchwork
> patman: Support updating a branch with review tags
> patman: Support listing comments from patchwork
>
> .azure-pipelines.yml | 2 +-
> .gitlab-ci.yml | 2 +-
> .travis.yml | 2 +-
> test/run | 2 +-
> tools/patman/README | 93 ++++++-
> tools/patman/checkpatch.py | 6 +
> tools/patman/commit.py | 14 ++
> tools/patman/control.py | 214 ++++++++++++++++
> tools/patman/func_test.py | 170 ++++++++++++-
> tools/patman/gitutil.py | 23 +-
> tools/patman/main.py | 219 ++++++++---------
> tools/patman/patchstream.py | 82 ++++++-
> tools/patman/series.py | 4 +-
> tools/patman/settings.py | 10 +-
> tools/patman/status.py | 476 ++++++++++++++++++++++++++++++++++++
> tools/patman/terminal.py | 4 +-
> tools/patman/test_util.py | 8 +-
> tools/patman/tools.py | 4 +-
> 18 files changed, 1178 insertions(+), 157 deletions(-)
> create mode 100644 tools/patman/control.py
> create mode 100644 tools/patman/status.py
>
I have tried your series because I use patman for quite a long time.
I have done these steps:
...
git pull https://gitlab.denx.de/u-boot/custodians/u-boot-dm.git
patman-working
git-cherry-pick <sha1> (my sifive v2 series)
-- Add Series-link based on patchwork which points to v2
HEAD commit looks like this:
commit b99568c40f4a2017a5dccbf96c4e5bb5ab0c7ea3 (HEAD -> simon)
Author: Michal Simek <michal.simek@xilinx.com>
AuthorDate: Thu Jul 9 10:58:47 2020 +0200
Commit: Michal Simek <michal.simek@xilinx.com>
CommitDate: Wed Jul 15 10:05:51 2020 +0200
serial: Fix SIFIVE debug serial dependency
The commit 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for
debug consoles") has added incorrect dependency for SIFIVE debug
uart which
should depend on SIFIVE driver instead of PL01x.
Fixes: 4cc24aeaf420 ("serial: Add missing Kconfig dependencies for
debug consoles")
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Series-to: uboot
Series-version: 2
Series-changes: 2
- Add fixes tag - asked by Simon
Series-link:
https://patchwork.ozlabs.org/project/uboot/list/?series=188865
And the running status on last one patch (because your series has also
other patches.
[u-boot](simon)$ rm -f 0* && /mnt/disk/u-boot/tools/patman/patman status
-c 1 -D
patman: ValueError: Cannot parse sequence spec 'v2'
Traceback (most recent call last):
File "/mnt/disk/u-boot/tools/patman/patman", line 167, in <module>
args.show_comments)
File
"/home/monstr/data/disk/u-boot/tools/patman/../patman/control.py", line
214, in patchwork_status
show_comments)
File "/home/monstr/data/disk/u-boot/tools/patman/../patman/status.py",
line 433, in check_patchwork_status
patches = collect_patches(series, url)
File "/home/monstr/data/disk/u-boot/tools/patman/../patman/status.py",
line 271, in collect_patches
patch.add(name, col.get_text())
File "/home/monstr/data/disk/u-boot/tools/patman/../patman/status.py",
line 135, in add
raise ValueError("Cannot parse sequence spec '%s'" % seq_info)
ValueError: Cannot parse sequence spec 'v2'
What do you think can be wrong? I expect your regex strings.
Anyway here are my comments.
1) -e options is not recorded in readme
2) this is the first time I read about patman test and I see that README
should be also updated and remove at least --test which you switched to
subcommand test
Would be also good to see what that command is really doing. It means
having verbose mode or so would be helpful.
Anyway based on strace I see that it touches patman/test/ folder that's
why it indicates that it is just patman internal tests.
If yes - this patchwork feature should be also added there.
3) You are collecting responses as Fixes, Tested-by, etc.
I am missing there also Reported-by:
4)
Last but not least is that Series-link. It seems to me that it is quite
time consuming to search for it and this would require more work to do.
When patman send message to mailing list it also generate Message-id. I
think you could be able to append/amend it when message is sent.
Then you should use this message-id to get status back based on it.
It could be enough for communication with patchwork and collect tags and
create another series.
Another thing what I see is that after sending you have all message ID
and you can use the same style as Linux is using for setting up Link: to
lore.kernel.org/r/
With the same style you can add Link:
https://patchwork.ozlabs.org/project/uboot/patch/<Message-id>
when you send patches.
Then you can use this Link which will be recorded in every patch to show
status/feedback/collect tags, etc.
Thanks,
Michal
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (16 preceding siblings ...)
2020-07-15 9:10 ` [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Michal Simek
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output Simon Glass
` (7 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
Collect response tags such as 'Reviewed-by' while parsing the stream.
This allows us to see what tags are present.
Add a new 'Fixes' tag also, since this is now quite common.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/commit.py | 14 ++++++++++++++
tools/patman/patchstream.py | 21 ++++++++++++++++-----
2 files changed, 30 insertions(+), 5 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (17 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 08/16] patman: Add a 'test' subcommand Simon Glass
` (6 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
At present all text is marked bright, which makes it stand out on the
terminal. Add a way to disable that, as is done with the Color class.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/terminal.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 08/16] patman: Add a 'test' subcommand
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (18 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 07/16] patman: Allow different commands Simon Glass
` (5 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
At present we use --test to indicate that tests should be run. It is
better to use a subcommand for list, like binman. Change it and adjust
the existing code to fit under a 'send' subcommand, the default.
Give this subcommand the same default arguments as the others.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
.azure-pipelines.yml | 2 +-
.gitlab-ci.yml | 2 +-
.travis.yml | 2 +-
test/run | 2 +-
tools/patman/main.py | 75 +++++++++++++++++++++------------------
tools/patman/test_util.py | 2 +-
6 files changed, 45 insertions(+), 40 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 07/16] patman: Allow different commands
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (19 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 08/16] patman: Add a 'test' subcommand Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 06/16] patman: Convert to ArgumentParser Simon Glass
` (4 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
At present patman only does one thing so does not have any comments. We
want to add a few more command, so create a sub-parser for the default
command ('send').
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/main.py | 77 +++++++++++++++++++++++---------------------
1 file changed, 41 insertions(+), 36 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 06/16] patman: Convert to ArgumentParser
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (20 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 07/16] patman: Allow different commands Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 05/16] patman: Allow skipping patches at the end Simon Glass
` (3 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
Convert from OptionParser to ArgumentParser to match binman. With this we
can easily add sub-commands.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/control.py | 22 ++++-----
tools/patman/main.py | 97 ++++++++++++++++++++--------------------
tools/patman/settings.py | 10 +++--
3 files changed, 65 insertions(+), 64 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 05/16] patman: Allow skipping patches at the end
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (21 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 06/16] patman: Convert to ArgumentParser Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 04/16] patman: Allow creating patches for another branch Simon Glass
` (2 subsequent siblings)
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
The -s option allows skipping patches at the top of the branch. Sometimes
there are commits at the bottom that need to be skipped. At present it is
necessary to count the number of commits and then use -c to tell patman
how many to process.
Add a -e option to easily skip a number of commits at the bottom of the
branch.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/control.py | 8 +++++---
tools/patman/func_test.py | 13 +++++++++++--
tools/patman/main.py | 2 ++
3 files changed, 18 insertions(+), 5 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 04/16] patman: Allow creating patches for another branch
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (22 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 05/16] patman: Allow skipping patches at the end Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 03/16] patman: Add a test that uses gitpython Simon Glass
2020-07-19 20:49 ` [RFC PATCH 02/16] patman: Move main code out to a control module Simon Glass
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
Add a -b option to allow patches to be created from a branch other than
the current one.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/control.py | 13 ++++++++-----
tools/patman/func_test.py | 13 +++++++++++--
tools/patman/gitutil.py | 19 ++++++++++++++-----
tools/patman/main.py | 2 ++
tools/patman/patchstream.py | 8 +++++---
5 files changed, 40 insertions(+), 15 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 03/16] patman: Add a test that uses gitpython
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (23 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 04/16] patman: Allow creating patches for another branch Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
2020-07-19 20:49 ` [RFC PATCH 02/16] patman: Move main code out to a control module Simon Glass
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
It is convenient to use gitpython to create a real git repo for testing
patman's operation. Add a test for this. So far it just checks that patman
produces the right number of patches for a branch.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/func_test.py | 151 +++++++++++++++++++++++++++++++++++++-
tools/patman/tools.py | 4 +-
2 files changed, 152 insertions(+), 3 deletions(-)
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread* [RFC PATCH 02/16] patman: Move main code out to a control module
2020-07-06 3:41 [RFC PATCH 00/16] RFC: patman: Collect review tags and comments from Patchwork Simon Glass
` (24 preceding siblings ...)
2020-07-19 20:49 ` [RFC PATCH 03/16] patman: Add a test that uses gitpython Simon Glass
@ 2020-07-19 20:49 ` Simon Glass
25 siblings, 0 replies; 33+ messages in thread
From: Simon Glass @ 2020-07-19 20:49 UTC (permalink / raw)
To: u-boot
To make testing easier, move the code out from main into a separate
'control' module and split it into four parts: setup, preparing patches,
checking patches and emailing patches.
Add comments and fix a few code-style issues while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/patman/checkpatch.py | 6 ++
tools/patman/control.py | 170 +++++++++++++++++++++++++++++++++++++
tools/patman/gitutil.py | 4 +-
tools/patman/main.py | 57 +------------
tools/patman/series.py | 2 +-
5 files changed, 182 insertions(+), 57 deletions(-)
create mode 100644 tools/patman/control.py
Applied to u-boot-dm
^ permalink raw reply [flat|nested] 33+ messages in thread