[Asterisk-code-review] build: Improve handling of CHANGES and UPGRADE.txt for releases. (...repotools[master])

Joshua Colp asteriskteam at digium.com
Thu Apr 18 05:06:59 CDT 2019


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/repotools/+/10941 )

Change subject: build: Improve handling of CHANGES and UPGRADE.txt for releases.
......................................................................

build: Improve handling of CHANGES and UPGRADE.txt for releases.

The release script now handles the merging of the CHANGES and
UPGRADE.txt files for us! When a release is being done, the script will
go through the staging changes in the Asterisk working directory
(<asterisk-home>/doc/<file>-staging) and add each change to the
corresponding file. A separate script (process-staging-changes) has also
been added that can be used when creating a new version from master. It
takes 3 arguments: -l/--local-root, which is optional,
-s/--start-version, and -e/--end-version. You can use -h for more
information on each option. Another script has been added that should be
run after process-staging-changes. This script is called
commit-staging-changes. It will go through all unstaged changes, add
them, and then commit them to the local repository. Then it will push
these changes to the remote. These scripts should be used for master or
if we need to do things manually (for some reason).

This means that there's a new way to document our major changes. All
changes for CHANGES will go into doc/CHANGES-staging and all changes for
UPGRADE.txt will go into doc/UPGRADE-staging. Each of these files should
have a meaningful name related to what the change is. For example, if
you made a change to something in pjsip, your file might be called
"res_pjsip_relative_title", where "relative_title" will be something a
little more descriptive than that. Inside of each file, you will have a
subject (1 or more) and corresponding headers (as of right now, the only
header is 'Master-Only'), with the description of the change following
that. You can have multiple subject lines in one file. For example, it
may look something like this:

   Subject: res_pjsip
   Subject: Core

   This is a detailed description of what I changed.

   You can have new lines in between as well, spacing is handled by the
   release script!

The header lines (Subject:) are case sensative. One thing to note is
that master is different. Since changes that are master-only will only
go into the master branch, we add another special tag (Master-Only) that
will go underneath the "Subject:" header. Note that it doesn't have to
go under the "Subject:" headers, but it makes it easier to read. There
should be no space between these.

   Subject: res_ari
   Master-Only: true

   This is a master-only change! You can put "True" or "true".

Note that the "Master-Only" header will only ever be true. There should
not be any scenarios where there will be a "Master-Only" header with the
value false.

For master (and possibly other releases), it will be necessary to do
some of this manually before running the mkrelease.py script. This is
because when cutting a major release (like from 16 to 17), the mainline
branch will never be master, so master must have these changes made
manually. The easiest way to do this is to make the changes before
cutting the new branch, so that the work does not need to be done twice.
Here's an example (run from the repotools dir):

   ./process-staging-changes -s 16 -e 17
   (check the working dir to make sure it looks correct)
   ./commit-staging-changes -v 17

Note that both of these scripts take an optional parameter (-l /
--local-root). This works the exact same way as in the mkrelease.py
script. If you are not working from the default directory (/tmp), you
MUST specify the path with this option.

Fore more information, check out the wiki page:
https://wiki.asterisk.org/wiki/display/AST/CHANGES+and+UPGRADE.txt

First step for ASTERISK_28111. The directories will need to be added,
and all changes will need to be refactored into these directories. Also,
the UPGRADE.txt file will no longer branch off into different files for
each release of a new major version, so these should be merged together
as well to reduce clutter.

Change-Id: I6dc084afedaeecaf36aaec66c3cf6a5a8ed4ef3c
---
A commit-staging-changes
M digium_git.py
M mkrelease.py
A process-staging-changes
A staging_changes.py
5 files changed, 429 insertions(+), 6 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/commit-staging-changes b/commit-staging-changes
new file mode 100755
index 0000000..fc4c1f7
--- /dev/null
+++ b/commit-staging-changes
@@ -0,0 +1,39 @@
+#!/usr/bin/env python
+
+import os
+import sys
+import logging
+from argparse import ArgumentParser
+from digium_git import get_repo,DigiumGitRepo
+
+LOGGER = logging.getLogger(__name__)
+
+if __name__ == '__main__':
+
+    parser = ArgumentParser()
+    parser.add_argument("-l", "--local-root", dest="local_root",
+                        help="The local root to work from", default="/tmp")
+    parser.add_argument("-v", "--version", dest="version",
+                        help="The version to be released", required=True)
+
+    (ns, args) = parser.parse_known_args()
+
+    logging.basicConfig(level=logging.DEBUG, format="%(module)s:%(lineno)d - %(message)s")
+
+    # Default to asterisk and gerrit URL
+    repo = get_repo(local_root=ns.local_root)
+
+    LOGGER.debug("Adding and committing all staging changes")
+
+    commit_msg = "Update CHANGES and UPGRADE.txt for {0}".format(ns.version)
+    repo.add_and_commit_all_unstaged(commit_msg)
+
+    prompt = "About to push changes to remote\n\tContinue [yes/no]?"
+    cont = raw_input(prompt)
+    if 'yes' in cont.lower() or 'y' in cont.lower():
+        repo.push_changes()
+    else:
+        print "Not pushing changes to remote (HEAD needs to be reset)"
+        sys.exit(1)
+
+    print "Done! Changes pushed to remote"
diff --git a/digium_git.py b/digium_git.py
index 0512642..d15699d 100644
--- a/digium_git.py
+++ b/digium_git.py
@@ -532,6 +532,23 @@
         self.repo.index.add(files)
         self.repo.index.commit(commit_msg)
 
+    def add_and_commit_all_unstaged(self, commit_msg):
+        """Add and commit all unstaged changes
+
+        Keyword Arguments:
+        commit_msg - Out commit message for the changes
+        """
+
+        LOGGER.debug("Adding and commit all unstaged changes to branch {0}"
+                     .format(self.current_branch))
+
+        files = self.repo.index.diff(None)
+        for f in files:
+            self.repo.git.add(f.a_path)
+        self.repo.index.commit(commit_msg)
+        if self.current_branch not in self.updated_branches:
+            self.updated_branches.append(self.current_branch)
+
     def push_changes(self):
         """Push any changes (branches, tags, etc...) upstream"""
 
diff --git a/mkrelease.py b/mkrelease.py
index 10e1a7e..81d4423 100755
--- a/mkrelease.py
+++ b/mkrelease.py
@@ -23,6 +23,7 @@
 from release_summary import ReleaseSummary, ReleaseSummaryOptions
 from alembic_creator import create_db_script
 from testsuite import update_testsuite
+from staging_changes import StagingChangesExtractor
 
 LOGGER = logging.getLogger(__name__)
 
@@ -123,6 +124,8 @@
     global branch
     global mainline
 
+    project = options.project.lower()
+
     if version_object.prefix:
         mainline = '{0}/{1}.{2}'.format(version_object.prefix,
                                         version_object.major,
@@ -133,22 +136,42 @@
         branch = '{0}.{1}'.format(version_object.major,
                                   version_object.minor)
 
-    if options.project.lower() != 'asterisk':
+    if project != 'asterisk':
         # Assume non asterisk project release branches are off of master
         mainline = 'master'
 
     if repo.branch_exists(branch):
         LOGGER.debug("Local branch {0} exists already".format(branch))
+        prompt_to_continue("Local branch {0} may not have updated CHANGES and"
+                           " UPGRADE.txt files".format(branch))
     else:
         LOGGER.debug("Local branch {0} does not exist".format(branch))
-        prompt_to_continue()
-        # Switch to the mainline branch first that way the new release
-        # branch is created off of that
-        repo.checkout(mainline)
+
+    # We checkout mainline here anyways since we need to update CHANGES
+    # and UPGRADE.txt. If the branch does not exist, the new release will
+    # need to be created off of that anyways
+    repo.checkout(mainline)
+
+    # Only update CHANGES and UPGRADE.txt if this is a full release (not beta,
+    # rc, etc.) for the Asterisk project
+    if len(version_object.modifiers) == 0 and project == 'asterisk':
+        s_version = version_object.get_previous_version()
+        start = "{0}.{1}.{2}".format(s_version.major, s_version.minor,
+                                     s_version.patch)
+        end = "{0}.{1}.{2}".format(version_object.major, version_object.minor,
+                                   version_object.patch)
+
+        sce = StagingChangesExtractor(options.local_root, start, end)
+        ret = sce.run()
+        if ret is not 0:
+            LOGGER.error("Failed to prepare staging changes")
+            sys.exit(1)
+
+        repo.add_and_commit_all_unstaged("Update CHANGES and UPGRADE.txt for {0}"
+                                         .format(version))
 
     repo.checkout(branch)
 
-
 def extract_tags(options, repo):
     """Extract the tags from the version to be created and the prepared repo
 
diff --git a/process-staging-changes b/process-staging-changes
new file mode 100755
index 0000000..4efe98b
--- /dev/null
+++ b/process-staging-changes
@@ -0,0 +1,32 @@
+#!/usr/bin/env python
+
+from argparse import ArgumentParser
+from staging_changes import StagingChangesExtractor
+
+if __name__ == '__main__':
+
+    parser = ArgumentParser()
+    parser.add_argument("-l", "--local-root", dest="local_root",
+                        help="The local root to work from", default="/tmp")
+    parser.add_argument("-s", "--start-version", dest="start_version",
+                        help="The version to start history at", required=True)
+    parser.add_argument("-e", "--end-version", dest="end_version",
+                        help="The version to release", required=True)
+
+    (ns, args) = parser.parse_known_args()
+
+    sce = StagingChangesExtractor(ast_path=ns.local_root, start_version=ns.start_version, end_version=ns.end_version)
+    ret = sce.run()
+
+    # Only add trailing slash for printing message; it's not needed otherwise
+    if not ns.local_root.endswith("/"):
+        ns.local_root += "/"
+
+    if ret is 0:
+        used_l_opt = ""
+        if ns.local_root != "/tmp/":
+            used_l_opt = "-l {0} ".format(ns.local_root)
+        print("Done! Check {0}asterisk and make sure everything looks right. "
+              "Then run 'commit-staging-changes {1}-v {2}'.".format(ns.local_root, used_l_opt, ns.end_version))
+    else:
+        print("Error! Check the logs and remember to clean up {0}asterisk".format(ns.local_root))
diff --git a/staging_changes.py b/staging_changes.py
new file mode 100644
index 0000000..8eceb48
--- /dev/null
+++ b/staging_changes.py
@@ -0,0 +1,312 @@
+#!/usr/bin/env python
+'''
+Used to extract information from <asterisk_home>/doc/<file>-staging/
+and add that information to <asterisk_home>/<file>
+
+Ben Ford <bford at digium.com>
+'''
+
+import logging
+import os
+import fileinput
+from collections import defaultdict
+from enum import Enum
+
+LOGGER = logging.getLogger(__name__)
+
+MODES = ["CHANGES", "UPGRADE"]
+FUNCTIONALITY_SEPARATOR_DASH_COUNT = 78
+CATEGORY_SEPARATOR_DASH_COUNT = 18
+
+class HeaderFlags(Enum):
+    '''
+    Header flags that are set based on what headers are present within each
+    subject in the change files
+
+    Values should be 1, 2, 4, 8, and so on
+    '''
+    MASTER_ONLY = 1
+
+class StagingChangesExtractor:
+    '''
+    Holds all the information needed to update the target files
+    (CHANGES / UPGRADE.txt)
+    '''
+
+    class Container:
+        '''
+        Holds the commit subject and summary for CHANGES/UPGRADE.txt as well as
+        the timestamp of the commit
+        '''
+
+        def __init__(self, subject, message, timestamp):
+            '''
+            Initializer
+
+            subject     The commit subject
+            message     The commit summary
+            timestamp   The commit time (epoch)
+            '''
+            self.subject = subject
+            self.message = message
+            self.timestamp = timestamp
+
+        def by_timestamp(self):
+            '''
+            Returns the container timestamp for sorting
+
+            returns     timestamp (epoch)
+            '''
+            return self.timestamp
+
+    def __init__(self, ast_path, start_version, end_version):
+        '''
+        Initializer
+
+        ast_path        The location of the Asterisk directory we are working with
+        start_version   The version we are starting from
+        end_version     The version we are ending on
+        '''
+        if ast_path is None:
+            raise Exception("ast_path cannot be NULL")
+        if start_version is None:
+            raise Exception("start_version cannot be NULL")
+        if end_version is None:
+            raise Exception("end_version cannot be NULL")
+
+        self.data = defaultdict(list)
+        self.master_data = defaultdict(list)
+        self.ast_path = os.path.join(ast_path, "asterisk")
+        self.start_version = start_version
+        self.end_version = end_version
+        # Bits set based on what headers are present for each subject
+        self.header_flags = 0
+
+    def gen_separator(self, count):
+        '''
+        Generate a number of dashes to be used as a separator
+
+        count       The number of dashes to generate
+
+        returns     A string with <count> dash(es)
+        '''
+        ret = ""
+        for num in range(count):
+            ret += "-"
+        return ret
+
+    def gen_banner(self, master=False):
+        '''
+        Generate the banner that will precede data in the target file
+
+        master      True if this is a master-only change
+
+        returns     The formatted banner to use above the changes
+        '''
+        banner = ""
+        if master is True:
+            banner = "--- New functionality introduced in Asterisk {0} ".format(self.end_version)
+        else:
+            banner = "--- Functionality changes from Asterisk {0} to Asterisk {1} ".format(self.start_version, self.end_version)
+        remaining = FUNCTIONALITY_SEPARATOR_DASH_COUNT - len(banner)
+        banner += self.gen_separator(remaining)
+        separator = self.gen_separator(FUNCTIONALITY_SEPARATOR_DASH_COUNT)
+        banner = separator + "\n" + banner + "\n" + separator + "\n\n"
+
+        return banner
+
+    def set_header_flag(self, line):
+        '''
+        Set a header flag based on the line provided
+
+        line        The header to parse
+
+        returns     0 on success
+        returns     -1 on failure
+        '''
+        data = line.split(":")
+        header = data[0].strip()
+        value = data[1].strip()
+        if header == "Master-Only":
+            if value == "True" or value == "true":
+                self.header_flags |= HeaderFlags.MASTER_ONLY.value
+            else:
+                LOGGER.error("Master-Only can only have a value of true!")
+                return -1
+        return 0
+
+    def add_data(self, subject, message, timestamp):
+        '''
+        Creates a container based on the parameters and adds the contents to
+        the correct dictionary, based on what flags were set during parsing
+
+        subject     The subject of the change
+        message     The change message
+        timestamp   The file timestamp for sorting
+        '''
+        container = self.Container(subject, message, timestamp)
+        if self.header_flags & HeaderFlags.MASTER_ONLY.value:
+            self.master_data[subject].append(container)
+        else:
+            self.data[subject].append(container)
+        # We should be done with header flags after adding an entry; reset
+        self.header_flags = 0
+
+    def parse_file(self, staging_path, filename):
+        '''
+        Given a path and filename, parse the contents and store the results
+
+        staging_path    The directory the file is in
+        filename        The name of the file to parse
+
+        returns         0 on success
+        returns         -1 on failure
+        '''
+        # Get the file timestamp (epoch)
+        timestamp = os.popen("(cd {0} && git log -1 --format=%ct {1})".format(staging_path, filename)).read().rstrip()
+        with open(os.path.join(staging_path, filename), 'r') as f:
+            LOGGER.debug("Parsing file '{0}'".format(filename))
+            subjects = []
+            message = ""
+            line = f.readline()
+            while line != "\n":
+                # Get the subjects as they appear
+                if line.startswith("Subject:"):
+                    subject = line.split("Subject:")[1].strip()
+                    subjects.append(subject)
+                # If it's not a subject, it's a header, so set the appropriate flag
+                else:
+                    ret = self.set_header_flag(line)
+                    if ret != 0:
+                        LOGGER.error("Failed to set header for line '{0}'".format(line.rstrip()))
+                        return -1
+                line = f.readline()
+            line = f.readline()
+            message = " * " + line
+            line = f.readline()
+            while line != "":
+                if line == "\n":
+                    # This is a special case because if there is an empty line separating some
+                    # text, we don't want to add 3 spaces and a new line.
+                    message += "\n"
+                else:
+                    message += "   " + line
+                line = f.readline()
+            for sub in subjects:
+                self.add_data(sub, message, timestamp)
+        # Once we are done with the file, remove it from the staging directory.
+        # We don't need it anymore.
+        LOGGER.debug("Removing file {0}".format(filename))
+        os.remove(os.path.join(staging_path, filename))
+        return 0
+
+    def get_staging_changes(self, mode):
+        '''
+        Retrieve changes from <self.ast_path>/doc/<mode>-staging/
+
+        returns     0 on success
+        returns     -1 on failure
+        '''
+        if mode is not "CHANGES" and mode is not "UPGRADE":
+            return -1
+
+        staging_path = "{0}/doc/{1}-staging".format(self.ast_path, mode)
+        try:
+            for filename in os.listdir(staging_path):
+                # Only process text files
+                if not os.path.isfile(staging_path + "/" + filename) or not filename.endswith(".txt"):
+                    continue
+                ret = self.parse_file(staging_path, filename)
+                if ret != 0:
+                    LOGGER.error("Failed during parsing of file '{0}'".format(filename))
+                    return -1
+        except:
+            LOGGER.debug("Could not extract data from {0} (does it exist?)".format(staging_path))
+            return -1
+
+        return 0
+
+    def add_staging_changes(self, mode):
+        '''
+        Add the changes retrieved to <self.ast_path/<mode>(.txt)
+
+        returns     0 on success
+        returns     -1 on failure
+        '''
+        if mode is not "CHANGES" and mode is not "UPGRADE":
+            return -1
+
+        if bool(self.data) is False and bool(self.master_data) is False:
+            # There was no data to retrieve during get_staging_changes
+            return 0
+
+        # If this is the master branch, we put the new stuff first
+        text_to_insert = ""
+        if bool(self.master_data) is True:
+            text_to_insert += self.gen_banner(master=True)
+            for key in sorted(self.master_data):
+                text_to_insert += key + "\n" + self.gen_separator(CATEGORY_SEPARATOR_DASH_COUNT) + "\n"
+                for obj in sorted(self.master_data[key], key=self.Container.by_timestamp):
+                    text_to_insert += obj.message.rstrip() + "\n\n"
+
+        # Insert the changes for the new version
+        if bool(self.data) is True:
+            text_to_insert += self.gen_banner()
+            for key in sorted(self.data):
+                text_to_insert += key + "\n" + self.gen_separator(CATEGORY_SEPARATOR_DASH_COUNT) + "\n"
+                for obj in sorted(self.data[key], key=self.Container.by_timestamp):
+                    text_to_insert += obj.message.rstrip() + "\n\n"
+
+        file_path = "{0}/{1}".format(self.ast_path, mode)
+        if mode is "UPGRADE":
+            file_path += ".txt"
+        try:
+            # insert will be set to "1" when we reach the first new line in file
+            insert = 0
+            # This is solely used to insert in place
+            for line in fileinput.FileInput(file_path, inplace = 1):
+                if insert is 0 and line in ["\n", "\r\n"]:
+                    # First new line, this is where we actually want to insert data
+                    insert = 1
+                    continue
+                if insert is 1:
+                    # We insert here, and then just print every line back to the file
+                    line = line.replace(line, "\n" + text_to_insert + line)
+                    insert = 2
+                print line,
+        except:
+            LOGGER.debug("Could not add data to {0}".format(file_path))
+            return -1
+
+        return 0
+
+    def run(self):
+        '''
+        Put everything together and run it
+
+        returns     0 on success
+        returns     -1 on failure
+        '''
+
+        logging.basicConfig(level=logging.DEBUG, format="%(module)s:%(lineno)d - %(message)s")
+
+        for mode in MODES:
+            LOGGER.debug("Extracting changes for {0}".format(mode))
+
+            # Clear any data we fetched previously before we do anything
+            self.data.clear()
+            self.master_data.clear()
+
+            ret = self.get_staging_changes(mode)
+            if ret is not 0:
+                LOGGER.debug("Failed during {0} get_staging_changes step - aborting "
+                             "(working directory probably needs to be reset!!!)".format(mode))
+                return -1
+
+            ret = self.add_staging_changes(mode)
+            if ret is not 0:
+                LOGGER.debug("Failed during {0} add_staging_changes step - aborting "
+                             "(working directory probably needs to be reset!!!)".format(mode))
+                return -1
+
+        return 0

-- 
To view, visit https://gerrit.asterisk.org/c/repotools/+/10941
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-Change-Id: I6dc084afedaeecaf36aaec66c3cf6a5a8ed4ef3c
Gerrit-Change-Number: 10941
Gerrit-PatchSet: 9
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190418/9339e09e/attachment-0001.html>


More information about the asterisk-code-review mailing list