NAK: [PATCH kteam-tools] maint-modify-patch: add support for mbox files

Kamal Mostafa kamal at canonical.com
Fri Jul 21 17:33:25 UTC 2017


On Fri, Jul 21, 2017 at 01:00:23PM -0300, Marcelo Henrique Cerri wrote:
> Add the --mbox option to modify mbox files instead of patches.

Sadly, this patch will break on commits with log lines that contain the
text "From " at the start of a paragraph, since that's also the mbox
message delimiter.  That results in all the text up to that being
silently omitted from the resulting modified patch.

A sample of one such "From blah blah" commit:

    (xenial) d282f41 power: supply: bq24190_charger: Don't read fault register
    From the data sheet: "When a fault occurs,   ...

Further experiments with Marcelo indicate that 'git mailsplit' is the tool
which can handle this task.  Revised patch to come in future.

 -Kamal


> 
> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>
> ---
>  maintscripts/maint-modify-patch | 267 +++++++++++++++++++++++-----------------
>  1 file changed, 153 insertions(+), 114 deletions(-)
> 
> diff --git a/maintscripts/maint-modify-patch b/maintscripts/maint-modify-patch
> index 99673ec0ff52..787a1f9ce12d 100755
> --- a/maintscripts/maint-modify-patch
> +++ b/maintscripts/maint-modify-patch
> @@ -8,6 +8,8 @@ from tempfile                           import NamedTemporaryFile
>  from ktl.utils                          import stdo, error
>  from ktl.std_app                        import StdApp
>  from re                                 import compile
> +from mailbox                            import mbox
> +from io                                 import StringIO
>  
>  # CmdlineError
>  #
> @@ -92,6 +94,9 @@ class Cmdline:
>          stdo("                         A list of git commit ids that will make up 'back-port' lines        \n")
>          stdo("                         in the signed-off-by block of the commit.                           \n")
>          stdo("                                                                                             \n")
> +        stdo("        --mbox                                                                               \n")
> +        stdo("                         Treat the imput files as mbox files.                                \n")
> +        stdo("                                                                                             \n")
>          stdo("    Examples:                                                                                \n")
>          stdo("        %s --list-aliases                                                                    \n" % self.cfg['app_name'])
>          stdo('        %s --bug=772543 --sob=bjf --ack="smb,ogasawara" *.patch                              \n' % self.cfg['app_name'])
> @@ -108,7 +113,9 @@ class Cmdline:
>          result = True
>          try:
>              optsShort = 'va:b:c:s:d:'
> -            optsLong  = ['help', 'verbose', 'config=', 'debug=', 'ack=', 'bugid=', 'cve=', 'sob=', 'list-aliases', 'cp=', 'bp=', 'dry-run']
> +            optsLong  = ['help', 'verbose', 'config=', 'debug=', 'ack=',
> +                    'bugid=', 'cve=', 'sob=', 'list-aliases', 'cp=', 'bp=',
> +                    'dry-run', 'mbox']
>              opts, args = getopt(argv[1:], optsShort, optsLong)
>  
>              for opt, val in opts:
> @@ -164,6 +171,9 @@ class Cmdline:
>                  elif opt in ('--dry-run'):                    # dry-run
>                      self.cfg['dry-run'] = True
>  
> +                elif opt in ('--mbox'):                       # mbox
> +                    self.cfg['mbox'] = True
> +
>              if result: # No errors yet
>                  if len(args) > 0:
>                      self.cfg['modify-operation'] = True
> @@ -207,6 +217,7 @@ class ModifyPatch(StdApp):
>          self.defaults['list-aliases']     = False
>          self.defaults['modify-operation'] = False
>          self.defaults['dry-run']          = False
> +        self.defaults['mbox']             = False
>          self.defaults['buglink_base_url'] = "http://bugs.launchpad.net/bugs/"
>  
>          self.cp_rc = compile('\(cherry picked from commit ([0-9a-zA-Z]+)\)')
> @@ -272,7 +283,10 @@ class ModifyPatch(StdApp):
>              self.initialize()
>  
>              for patch_file in self.cfg['patch-files']:
> -                self.modify(patch_file)
> +                if self.cfg['mbox']:
> +                    self.modify_mbox(patch_file)
> +                else:
> +                    self.modify_patch(patch_file)
>  
>          # Handle the user presses <ctrl-C>.
>          #
> @@ -286,13 +300,44 @@ class ModifyPatch(StdApp):
>  
>          return
>  
> -    # modify
> +    # modify_mbox
> +    #
> +    def modify_mbox(self, mbox_file):
> +        """
> +        Open a mbox file and write it's modified contents out to a temp file.
> +        The temp file will be renamed to the original file as the last step.
> +        """
> +        src = mbox(mbox_file, create=False)
> +        dst_file = mbox_file + '.tmp'
> +        dst = mbox(dst_file)
> +        for msg in src:
> +            src_msg = StringIO(msg.as_string().decode('utf-8', 'ignore'))
> +            dst_msg = StringIO()
> +            self.modify(src_msg, dst_msg)
> +            dst_msg.seek(0)
> +            dst.add(dst_msg)
> +
> +        if not self.cfg['dry-run']:
> +            rename(dst_file, mbox_file)
> +        return
> +
> +    # modify_patch
>      #
> -    def modify(self, patch):
> +    def modify_patch(self, patch):
>          """
>          Open a file and write it's modified contents out to a temp file.
>          The temp file will be renamed to the original file as the last step.
>          """
> +        with open(patch, 'r') as src:
> +            with NamedTemporaryFile(dir='./', delete=False) as dst:
> +                self.modify(src, dst)
> +        temp = dst.name
> +        if not self.cfg['dry-run']:
> +            rename(temp, patch)
> +
> +    # modify
> +    #
> +    def modify(self, src, dst):
>          existing_cves     = []
>          existing_buglinks = []
>          existing_acks     = []
> @@ -310,119 +355,113 @@ class ModifyPatch(StdApp):
>          sob_insertion_point               = False
>          just_copy                         = False
>  
> -        with open(patch, 'r') as src:
> -            with NamedTemporaryFile(dir='./', delete=False) as dst:
> -
> -                for line in src:
> -
> -                    if just_copy:
> -                        dst.write(line)
> -                        continue
> -
> -                    if looking_4_sob_insertion_point:
> -                        if line.strip() == '---':
> -                            sob_insertion_point = True
> -                            looking_4_sob_insertion_point = False
> -                        else:
> -                            if 'Acked-by:' in line:
> -                                user_id = line.replace('Acked-by:', '').strip()
> -                                if user_id not in existing_acks:
> -                                    existing_acks.append(user_id)
> -                            elif 'Signed-off-by:' in line:
> -                                user_id = line.replace('Signed-off-by:', '').strip()
> -                                if user_id not in existing_sobs:
> -                                    existing_sobs.append(user_id)
> -                            elif 'cherry picked' in line:
> -                                m = self.cp_rc.search(line)
> -                                if m is not None:
> -                                    existing_cps.append(m.group(1))
> -                                pass
> -                            elif 'backported from' in line:
> -                                m = self.bp_rc.search(line)
> -                                if m is not None:
> -                                    existing_bps.append(m.group(1))
> -                                pass
> -
> -                    if sob_insertion_point:
> -                        dst.write(self.sob_block(existing_acks, existing_sobs, existing_cps, existing_bps))
> -                        sob_insertion_point = False
> -                        just_copy = True
> -
> -                    # After the first blank line after the subject line is where we
> -                    # want to insert out CVE lines if we need to insert any.
> +        for line in src:
> +
> +            if just_copy:
> +                dst.write(line)
> +                continue
> +
> +            if looking_4_sob_insertion_point:
> +                if line.strip() == '---':
> +                    sob_insertion_point = True
> +                    looking_4_sob_insertion_point = False
> +                else:
> +                    if 'Acked-by:' in line:
> +                        user_id = line.replace('Acked-by:', '').strip()
> +                        if user_id not in existing_acks:
> +                            existing_acks.append(user_id)
> +                    elif 'Signed-off-by:' in line:
> +                        user_id = line.replace('Signed-off-by:', '').strip()
> +                        if user_id not in existing_sobs:
> +                            existing_sobs.append(user_id)
> +                    elif 'cherry picked' in line:
> +                        m = self.cp_rc.search(line)
> +                        if m is not None:
> +                            existing_cps.append(m.group(1))
> +                        pass
> +                    elif 'backported from' in line:
> +                        m = self.bp_rc.search(line)
> +                        if m is not None:
> +                            existing_bps.append(m.group(1))
> +                        pass
> +
> +            if sob_insertion_point:
> +                dst.write(self.sob_block(existing_acks, existing_sobs, existing_cps, existing_bps))
> +                sob_insertion_point = False
> +                just_copy = True
> +
> +            # After the first blank line after the subject line is where we
> +            # want to insert out CVE lines if we need to insert any.
> +            #
> +            if cve_insertion_point:
> +                # Skip past any existing CVE lines and don't duplicate any
> +                # existing CVE lines
> +                #
> +                if 'CVE' in line:
> +                    cve_id = line.strip().replace('CVE-', '')
> +                    existing_cves.append(cve_id)
> +                else:
> +                    # Add the CVE id here.
>                      #
> -                    if cve_insertion_point:
> -                        # Skip past any existing CVE lines and don't duplicate any
> -                        # existing CVE lines
> -                        #
> -                        if 'CVE' in line:
> -                            cve_id = line.strip().replace('CVE-', '')
> -                            existing_cves.append(cve_id)
> -                        else:
> -                            # Add the CVE id here.
> -                            #
> -                            if 'cveid' in self.cfg and len(self.cfg['cveid']) > 0:
> -                                if self.cfg['cveid'] not in existing_cves:
> -                                    dst.write('CVE-%s\n' % self.cfg['cveid'])
> -                                    dst.write('\n') # One blank line after the CVE line
> -
> -                            cve_insertion_point = False # Done!
> -                            looking_4_buglink_insertion_point = True
> -
> -                            # We don't know at this point, if we are going to insert a BugLink
> -                            # so we can't write out the current line of text.
> -
> -                    # After the first blank line after the CVE lines is where the BugLinks are
> -                    # to be inserted.
> +                    if 'cveid' in self.cfg and len(self.cfg['cveid']) > 0:
> +                        if self.cfg['cveid'] not in existing_cves:
> +                            dst.write('CVE-%s\n' % self.cfg['cveid'])
> +                            dst.write('\n') # One blank line after the CVE line
> +
> +                    cve_insertion_point = False # Done!
> +                    looking_4_buglink_insertion_point = True
> +
> +                    # We don't know at this point, if we are going to insert a BugLink
> +                    # so we can't write out the current line of text.
> +
> +            # After the first blank line after the CVE lines is where the BugLinks are
> +            # to be inserted.
> +            #
> +            if looking_4_buglink_insertion_point:
> +                if line.strip() != '':
> +                    looking_4_buglink_insertion_point = False
> +                    buglink_insertion_point = True
> +
> +            if buglink_insertion_point:
> +                # Skip past any existing BugLink lines and build a list of existing
> +                # buglinks so we don't duplicate any.
> +                #
> +                if line.startswith('BugLink:'):
> +                    bug_id = line.rstrip().split('/')[-1]
> +                    existing_buglinks.append(bug_id)
> +                else:
> +                    # Add the buglinks.
>                      #
> -                    if looking_4_buglink_insertion_point:
> -                        if line.strip() != '':
> -                            looking_4_buglink_insertion_point = False
> -                            buglink_insertion_point = True
> -
> -                    if buglink_insertion_point:
> -                        # Skip past any existing BugLink lines and build a list of existing
> -                        # buglinks so we don't duplicate any.
> -                        #
> -                        if line.startswith('BugLink:'):
> -                            bug_id = line.rstrip().split('/')[-1]
> -                            existing_buglinks.append(bug_id)
> -                        else:
> -                            # Add the buglinks.
> -                            #
> -                            if 'bugids' in self.cfg and len(self.cfg['bugids']) > 0:
> -                                for bug_id in self.cfg['bugids']:
> -                                    if bug_id not in existing_buglinks:
> -                                        dst.write('BugLink: %s%s\n' % (self.cfg['buglink_base_url'], bug_id))
> -                                        dst.write('\n') # One blank line after the BugLink line
> -                            buglink_insertion_point = False
> -                            looking_4_sob_insertion_point = True
> -
> -                            # We've inserted any BugLinks that we intend to, so we should now
> -                            # print out the line of text we've been holding. We do that by falling
> -                            # through to the end of the loop.
> -
> -                    # Once we've found the subject line, we look for the first blank
> -                    # line after it.
> -                    #
> -                    if subject_line:
> -                        if line.strip() == '':
> -                            cve_insertion_point = True
> -                            subject_line = False
> -
> -                    # All the modifications that we make, are made after the subject
> -                    # line, so that's the first things we look for.
> -                    #
> -                    if looking_4_subject_line:
> -                        if 'Subject:' in line:
> -                            subject_line = True
> -                            looking_4_subject_line = False
> +                    if 'bugids' in self.cfg and len(self.cfg['bugids']) > 0:
> +                        for bug_id in self.cfg['bugids']:
> +                            if bug_id not in existing_buglinks:
> +                                dst.write('BugLink: %s%s\n' % (self.cfg['buglink_base_url'], bug_id))
> +                                dst.write('\n') # One blank line after the BugLink line
> +                    buglink_insertion_point = False
> +                    looking_4_sob_insertion_point = True
> +
> +                    # We've inserted any BugLinks that we intend to, so we should now
> +                    # print out the line of text we've been holding. We do that by falling
> +                    # through to the end of the loop.
> +
> +            # Once we've found the subject line, we look for the first blank
> +            # line after it.
> +            #
> +            if subject_line:
> +                if line.strip() == '':
> +                    cve_insertion_point = True
> +                    subject_line = False
> +
> +            # All the modifications that we make, are made after the subject
> +            # line, so that's the first things we look for.
> +            #
> +            if looking_4_subject_line:
> +                if 'Subject:' in line:
> +                    subject_line = True
> +                    looking_4_subject_line = False
> +
> +            dst.write(line) # Print out the current line of text.
>  
> -                    dst.write(line) # Print out the current line of text.
> -
> -                temp = dst.name
> -        if not self.cfg['dry-run']:
> -            rename(temp, patch)
>          return
>  
>  if __name__ == '__main__':
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team




More information about the kernel-team mailing list