patman: Move main code out to a control module

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>

5 files changed, 182 insertions, 57 deletions

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