Speedup of calc_release_version.py (#1383)

* Cleaner argument handling and type-checking in calc_release_version
* Speedup of calc_release_version by minimizing subprocesses

Instead of continually asking Git whether a particular commit hash is
pointed-to by a given tag, ask Git upfront for a mapping between all
commit hashes and the associated tags. Then, look for the commit in
the mapping as a single step.

Author: vector-of-bool

build/calc_release_version.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python
+# -*- coding: utf-8 -*-
 
 #
 # Copyright 2018-present MongoDB, Inc.
@@ -20,6 +21,9 @@
 A script that calculates the release version number (based on the current Git
 branch and/or recent tags in history) to assign to a tarball generated from the
 current Git commit.
+
+This script needs to remain compatible with its target platforms, which currently
+includes RHEL 6, which uses Python 2.6!
 """
 
 # XXX NOTE XXX NOTE XXX NOTE XXX
@@ -34,9 +38,13 @@
 # of each command is desired, then add the -x option to the bash invocation.
 # XXX NOTE XXX NOTE XXX NOTE XXX
 
+# pyright: reportTypeCommentUsage=false
+
 import datetime
+import errno
 import re
 import subprocess
+import optparse  # No 'argparse' on Python 2.6
 import sys
 
 try:
@@ -46,21 +54,39 @@
 except ImportError:
     # Fallback to deprecated pkg_resources.
     try:
-        from pkg_resources.extern.packaging.version import Version
+        from pkg_resources.extern.packaging.version import Version  # type: ignore
         from pkg_resources import parse_version
     except ImportError:
         # Fallback to deprecated distutils.
         from distutils.version import LooseVersion as Version
         from distutils.version import LooseVersion as parse_version
 
-DEBUG = len(sys.argv) > 1 and '-d' in sys.argv
-if DEBUG:
-    print('Debugging output enabled.')
+parser = optparse.OptionParser(description=__doc__)
+parser.add_option("--debug", "-d", action="store_true", help="Enable debug output")
+parser.add_option("--previous", "-p", action="store_true", help="Calculate the previous version instead of the current")
+parser.add_option("--next-minor", action="store_true", help="Calculate the next minor version instead of the current")
+args, pos = parser.parse_args()
+assert not pos, "No positional arguments are expected"
+
+
+_DEBUG = args.debug  # type: bool
+
+
+def debug(msg):  # type: (str) -> None
+    if _DEBUG:
+        sys.stderr.write(msg)
+        sys.stderr.write("\n")
+        sys.stderr.flush()
+
+
+debug("Debugging output enabled.")
 
 # This option indicates we are to determine the previous release version
-PREVIOUS = len(sys.argv) > 1 and '-p' in sys.argv
+PREVIOUS = args.previous  # type: bool
 # This options indicates to output the next minor release version
-NEXT_MINOR = len(sys.argv) > 1 and '--next-minor' in sys.argv
+NEXT_MINOR = args.next_minor  # type: bool
+
+# fmt: off
 
 PREVIOUS_TAG_RE = re.compile('(?P<ver>(?P<vermaj>[0-9]+)\\.(?P<vermin>[0-9]+)'
                              '\\.(?P<verpatch>[0-9]+)(?:-(?P<verpre>.*))?)')
@@ -69,29 +95,33 @@
 RELEASE_BRANCH_RE = re.compile('(?:(?:refs/remotes/)?origin/)?(?P<brname>r'
                                '(?P<vermaj>[0-9]+)\\.(?P<vermin>[0-9]+))')
 
-def check_output(args):
+
+def check_output(args):  # type: (list[str]) -> str
     """
     Delegates to subprocess.check_output() if it is available, otherwise
     provides a reasonable facsimile.
     """
-    if 'check_output' in dir(subprocess):
-        out = subprocess.check_output(args)
-    else:
+    debug('Run command: {0}'.format(args))
+    try:
         proc = subprocess.Popen(args, stdout=subprocess.PIPE)
-        out, err = proc.communicate()
-        ret = proc.poll()
-        if ret:
-            raise subprocess.CalledProcessError(ret, args[0], output=out)
-
-    if type(out) is bytes:
-        # git isn't guaranteed to always return UTF-8, but for our purposes
-        # this should be fine as tags and hashes should be ASCII only.
-        out = out.decode('utf-8')
+    except OSError as e:
+        suppl = ''
+        if e.errno == errno.ENOENT:
+            suppl = 'Does the executable “{0}” not exist?'.format(args[0])
+        raise RuntimeError("Failed to execute subprocess {0}: {1} [{2}]".format(args, e, suppl))
+    out = proc.communicate()[0]
+    ret = proc.poll()
+    if ret:
+        raise subprocess.CalledProcessError(ret, args[0])
+
+    # git isn't guaranteed to always return UTF-8, but for our purposes
+    # this should be fine as tags and hashes should be ASCII only.
+    out = out.decode('utf-8')
 
     return out
 
 
-def check_head_tag():
+def check_head_tag():  # type: () -> str | None
     """
     Checks the current HEAD to see if it has been tagged with a tag that matches
     the pattern for a release tag.  Returns release version calculated from the
@@ -115,21 +145,19 @@ def check_head_tag():
     if release_tag_match:
         new_version_str = release_tag_match.group('ver')
         new_version_parsed = parse_version(new_version_str)
-        if new_version_parsed > version_parsed:
-            if DEBUG:
-                print('HEAD release tag: ' + new_version_str)
+        if new_version_parsed > version_parsed: # type: ignore
+            debug('HEAD release tag: ' + new_version_str)
             version_str = new_version_str
             version_parsed = new_version_parsed
             found_tag = True
 
     if found_tag:
-        if DEBUG:
-            print('Calculated version: ' + version_str)
+        debug('Calculated version: ' + version_str)
         return version_str
 
     return None
 
-def get_next_minor(prerelease_marker):
+def get_next_minor(prerelease_marker):  # type: (str) -> str
     """
     get_next_minor does the following:
         - Inspect the branches that fit the convention for a release branch.
@@ -157,49 +185,70 @@ def get_next_minor(prerelease_marker):
                               str(version_new['patch']) + '-' + \
                               version_new['prerelease']
             new_version_parsed = parse_version(new_version_str)
-            if new_version_parsed > version_parsed:
+            if new_version_parsed > version_parsed: # type: ignore
                 version_str = new_version_str
                 version_parsed = new_version_parsed
-                if DEBUG:
-                    print('Found new best version "' + version_str \
+                debug('Found new best version "' + version_str \
                             + '" based on branch "' \
                             + release_branch_match.group('brname') + '"')
     return version_str
 
-def get_branch_tags(active_branch_name):
+def get_branch_tags(active_branch_name):  # type: (str) -> list[str]
     """
-    Returns the tag or tags (as a single string with newlines between tags)
-    corresponding to the current branch, which must not be master.  If the
-    specified branch is a release branch then return all tags based on the
-    major/minor X.Y release version.  If the specified branch is neither master
-    nor a release branch, then walk backwards in history until the first tag
-    matching the glob '1.*' and return that tag.
+    Returns a list of tags corresponding to the current branch, which must not
+    be master.  If the specified branch is a release branch then return all tags
+    based on the major/minor X.Y release version.  If the specified branch is
+    neither master nor a release branch, then walk backwards in history until
+    the first tag matching the glob '1.*' and return that tag.
     """
 
     if active_branch_name == 'master':
         raise Exception('this method is not meant to be called while on "master"')
-    tags = ''
+
     release_branch_match = RELEASE_BRANCH_RE.match(active_branch_name)
     if release_branch_match:
         # This is a release branch, so look for tags only on this branch
         tag_glob = release_branch_match.group('vermaj') + '.' \
                 + release_branch_match.group('vermin') + '.*'
-        tags = check_output(['git', 'tag', '--list', tag_glob])
-    else:
-        # Not a release branch, so look for the most recent tag in history
-        commits = check_output(['git', 'log', '--pretty=format:%H',
-                                '--no-merges'])
-        if len(commits) > 0:
-            for commit in commits.splitlines():
-                tags = check_output(['git', 'tag', '--points-at',
-                                     commit, '--list', '1.*'])
-                if len(tags) > 0:
-                    # found a tag, we should be done
-                    break
-
-    return tags
-
-def process_and_sort_tags(tags):
+        return check_output(['git', 'tag', '--list', tag_glob]).splitlines()
+
+    # Not a release branch, so look for the most recent tag in history
+    commits = check_output(['git', 'log', '--pretty=format:%H', '--no-merges'])
+    tags_by_obj = get_object_tags()
+    for commit in commits.splitlines():
+        got = tags_by_obj.get(commit)
+        if got:
+            return got
+    # No tags
+    return []
+
+
+def iter_tag_lines():
+    """
+    Generate a list of pairs of strings, where the first is a commit hash, and
+    the second is a tag that is associated with that commit. Duplicate commits
+    are possible.
+    """
+    output = check_output(['git', 'tag', '--list', '1.*', '--format=%(*objectname)|%(tag)'])
+    lines = output.splitlines()
+    for l in lines:
+        obj, tag = l.split('|', 1)
+        if tag:
+            yield obj, tag
+
+
+def get_object_tags():  # type: () -> dict[str, list[str]]
+    """
+    Obtain a mapping between commit hashes and a list of tags that point to
+    that commit. Untagged commits will not be included in the resulting map.
+    """
+    ret = {}  # type: dict[str, list[str]]
+    for obj, tag in iter_tag_lines():
+        ret.setdefault(obj, []).append(tag)
+    return ret
+
+
+def process_and_sort_tags(tags):  # type: (list[str]) -> list[str]
     """
     Given a string (as returned from get_branch_tags), return a sorted list of
     zero or more tags (sorted based on the Version comparison) which meet
@@ -209,23 +258,22 @@ def process_and_sort_tags(tags):
           1.x.y-preX iff 1.x.y does not already exist)
     """
 
-    processed_and_sorted_tags = []
+    processed_and_sorted_tags = []  # type: list[str]
     if not tags or len(tags) == 0:
         return processed_and_sorted_tags
 
-    raw_tags = tags.splitlines()
     # find all the final release tags
-    for tag in raw_tags:
+    for tag in tags:
         release_tag_match = RELEASE_TAG_RE.match(tag)
         if release_tag_match and not release_tag_match.group('verpre'):
             processed_and_sorted_tags.append(tag)
     # collect together final release tags and pre-release tags for
     # versions that have not yet had a final release
-    for tag in raw_tags:
+    for tag in tags:
         tag_parts = tag.split('-')
         if len(tag_parts) >= 2 and tag_parts[0] not in processed_and_sorted_tags:
             processed_and_sorted_tags.append(tag)
-    processed_and_sorted_tags.sort(key=Version)
+    processed_and_sorted_tags.sort(key=Version)  # type: ignore
 
     return processed_and_sorted_tags
 
@@ -254,8 +302,7 @@ def main():
             + '+git' + head_commit_short
 
     if NEXT_MINOR:
-        if DEBUG:
-            print('Calculating next minor release')
+        debug('Calculating next minor release')
         return get_next_minor(prerelease_marker)
 
     head_tag_ver = check_head_tag()
@@ -264,8 +311,7 @@ def main():
 
     active_branch_name = check_output(['git', 'rev-parse',
                                        '--abbrev-ref', 'HEAD']).strip()
-    if DEBUG:
-        print('Calculating release version for branch: ' + active_branch_name)
+    debug('Calculating release version for branch: ' + active_branch_name)
     if active_branch_name == 'master':
         return get_next_minor(prerelease_marker)
 
@@ -287,27 +333,25 @@ def main():
                           str(version_new['patch']) + '-' + \
                           version_new['prerelease']
         new_version_parsed = parse_version(new_version_str)
-        if new_version_parsed > version_parsed:
+        if new_version_parsed > version_parsed: # type: ignore
             version_str = new_version_str
             version_parsed = new_version_parsed
-            if DEBUG:
-                print('Found new best version "' + version_str \
+            debug('Found new best version "' + version_str \
                         + '" from tag "' + release_tag_match.group('ver') + '"')
 
     return version_str
 
-def previous(rel_ver):
+def previous(rel_ver):  # type: (str) -> str
     """
     Given a release version, find the previous version based on the latest Git
     tag that is strictly a lower version than the given release version.
     """
-    if DEBUG:
-        print('Calculating previous release version (option -p was specified).')
+    debug('Calculating previous release version (option -p was specified).')
     version_str = '0.0.0'
     version_parsed = parse_version(version_str)
     rel_ver_str = rel_ver
     rel_ver_parsed = parse_version(rel_ver_str)
-    tags = check_output(['git', 'tag', '--list', '1.*'])
+    tags = check_output(['git', 'tag', '--list', '1.*']).splitlines()
     processed_and_sorted_tags = process_and_sort_tags(tags)
     for tag in processed_and_sorted_tags:
         previous_tag_match = PREVIOUS_TAG_RE.match(tag)
@@ -323,17 +367,15 @@ def previous(rel_ver):
             if version_new['prerelease'] is not None:
                 new_version_str += '-' + version_new['prerelease']
             new_version_parsed = parse_version(new_version_str)
-            if new_version_parsed < rel_ver_parsed and new_version_parsed > version_parsed:
+            if new_version_parsed < rel_ver_parsed and new_version_parsed > version_parsed: # type: ignore
                 version_str = new_version_str
                 version_parsed = new_version_parsed
-                if DEBUG:
-                    print('Found new best version "' + version_str \
+                debug('Found new best version "' + version_str \
                             + '" from tag "' + tag + '"')
 
     return version_str
 
 RELEASE_VER = previous(main()) if PREVIOUS else main()
 
-if DEBUG:
-    print('Final calculated release version:')
+debug('Final calculated release version:')
 print(RELEASE_VER)

build/calc_release_version_selftest.sh

@@ -40,9 +40,9 @@ cp calc_release_version.py calc_release_version_test.py
 echo "Test a tagged commit ... begin"
 {
     git checkout 1.23.4 --quiet
-    got=$("${PYTHON_INTERP}" calc_release_version_test.py)
+    got=$("${PYTHON_INTERP}" calc_release_version_test.py --debug)
     assert_eq "$got" "1.23.4"
-    got=$("${PYTHON_INTERP}" calc_release_version_test.py -p)
+    got=$("${PYTHON_INTERP}" calc_release_version_test.py --debug -p)
     assert_eq "$got" "1.23.3"
     git checkout - --quiet
 }
@@ -53,9 +53,9 @@ echo "Test an untagged commit ... begin"
 {
     # 42a818429d6d586a6abf22367ac6fea1e9ce3f2c is commit before 1.23.4
     git checkout 42a818429d6d586a6abf22367ac6fea1e9ce3f2c --quiet
-    got=$("${PYTHON_INTERP}" calc_release_version_test.py)
+    got=$("${PYTHON_INTERP}" calc_release_version_test.py --debug)
     assert_eq "$got" "1.23.4-$DATE+git42a818429d"
-    got=$("${PYTHON_INTERP}" calc_release_version_test.py -p)
+    got=$("${PYTHON_INTERP}" calc_release_version_test.py --debug -p)
     assert_eq "$got" "1.23.4"
     git checkout - --quiet
 }
@@ -64,14 +64,14 @@ echo "Test an untagged commit ... end"
 echo "Test next minor version ... begin"
 {
     CURRENT_SHORTREF=$(git rev-parse --revs-only --short=10 HEAD)
-    got=$("${PYTHON_INTERP}" calc_release_version_test.py --next-minor)
+    got=$("${PYTHON_INTERP}" calc_release_version_test.py --debug --next-minor)
     # XXX NOTE XXX NOTE XXX
     # If you find yourself looking at this line because the assertion below
     # failed, then it is probably because a new major/minor release was made.
     # Update the expected output to represent the correct next version.
     # XXX NOTE XXX NOTE XXX
     assert_eq "$got" "1.25.0-$DATE+git$CURRENT_SHORTREF"
-    got=$("${PYTHON_INTERP}" calc_release_version_test.py --next-minor -p)
+    got=$("${PYTHON_INTERP}" calc_release_version_test.py --debug --next-minor -p)
     # XXX NOTE XXX NOTE XXX
     # If you find yourself looking at this line because the assertion below
     # failed, then it is probably because a new major/minor release was made.