From 9ef6028da0c73ff0186ef107870f73d95370da9d Mon Sep 17 00:00:00 2001
From: Jaeden Amero <jaeden.amero@arm.com>
Date: Fri, 2 Nov 2018 16:22:37 +0000
Subject: [PATCH 01/24] abi_check: Allow checking current checkout

Without a "--detach" option, git worktree will refuse to checkout a branch
that's already checked out. This makes the abi_check.py script not very
useful for checking the currently checked out branch, as git will error
that the branch is already checked out. Add the "--detach" option to check
out the new temporary worktree in detached head mode. This is acceptable
because we aren't planning on working on the branch and just want a
checkout to do ABI checking from.
---
 scripts/abi_check.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 2a90b68f7..d458d56e3 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -76,7 +76,7 @@ class AbiChecker(object):
         )
         git_worktree_path = tempfile.mkdtemp()
         worktree_process = subprocess.Popen(
-            [self.git_command, "worktree", "add", git_worktree_path, git_rev],
+            [self.git_command, "worktree", "add", "--detach", git_worktree_path, git_rev],
             cwd=self.repo_path,
             stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT

From 4cd4b4bf835bab6145b6865ab1c54ff21ac027dd Mon Sep 17 00:00:00 2001
From: Jaeden Amero <jaeden.amero@arm.com>
Date: Fri, 2 Nov 2018 16:35:09 +0000
Subject: [PATCH 02/24] abi_check: Update submodules

When grabbing a fresh copy of a branch, it's required to also fetch the
submodule. Add fetching the submodule to abi_check.py.
---
 scripts/abi_check.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index d458d56e3..7926bc8cd 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -87,6 +87,18 @@ class AbiChecker(object):
             raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
 
+    def update_git_submodules(self, git_worktree_path):
+        process = subprocess.Popen(
+            [self.git_command, "submodule", "update", "--init", '--recursive'],
+            cwd=git_worktree_path,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT
+        )
+        output, _ = process.communicate()
+        self.log.info(output.decode("utf-8"))
+        if process.returncode != 0:
+            raise Exception("git submodule update failed, aborting")
+
     def build_shared_libraries(self, git_worktree_path):
         """Build the shared libraries in the specified worktree."""
         my_environment = os.environ.copy()
@@ -149,6 +161,7 @@ class AbiChecker(object):
     def get_abi_dump_for_ref(self, git_rev):
         """Generate the ABI dumps for the specified git revision."""
         git_worktree_path = self.get_clean_worktree_for_git_revision(git_rev)
+        self.update_git_submodules(git_worktree_path)
         self.build_shared_libraries(git_worktree_path)
         abi_dumps = self.get_abi_dumps_from_shared_libraries(
             git_rev, git_worktree_path

From 5a301f08689c6a6fc1eb46ac8a1e7f5119a9cb0c Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Tue, 19 Feb 2019 16:59:33 +0000
Subject: [PATCH 03/24] Extend abi-checking to different repos

---
 scripts/abi_check.py | 79 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 16 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 7926bc8cd..d14236ead 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -28,11 +28,14 @@ import tempfile
 class AbiChecker(object):
     """API and ABI checker."""
 
-    def __init__(self, report_dir, old_rev, new_rev, keep_all_reports):
+    def __init__(self, report_dir, old_repo, old_rev, new_repo, new_rev,
+                 keep_all_reports):
         """Instantiate the API/ABI checker.
 
         report_dir: directory for output files
+        old_repo: repository for git revision to compare against
         old_rev: reference git revision to compare against
+        new_repo: repository for git revision to check
         new_rev: git revision to check
         keep_all_reports: if false, delete old reports
         """
@@ -42,7 +45,9 @@ class AbiChecker(object):
         self.report_dir = os.path.abspath(report_dir)
         self.keep_all_reports = keep_all_reports
         self.should_keep_report_dir = os.path.isdir(self.report_dir)
+        self.old_repo = old_repo
         self.old_rev = old_rev
+        self.new_repo = new_repo
         self.new_rev = new_rev
         self.mbedtls_modules = ["libmbedcrypto", "libmbedtls", "libmbedx509"]
         self.old_dumps = {}
@@ -68,15 +73,35 @@ class AbiChecker(object):
             if not shutil.which(command):
                 raise Exception("{} not installed, aborting".format(command))
 
-    def get_clean_worktree_for_git_revision(self, git_rev):
+    def get_clean_worktree_for_git_revision(self, remote_repo, git_rev):
         """Make a separate worktree with git_rev checked out.
         Do not modify the current worktree."""
-        self.log.info(
-            "Checking out git worktree for revision {}".format(git_rev)
-        )
         git_worktree_path = tempfile.mkdtemp()
+        if remote_repo:
+            self.log.info(
+                "Checking out git worktree for revision {} from {}".format(
+                    git_rev, remote_repo
+                )
+            )
+            fetch_process = subprocess.Popen(
+                [self.git_command, "fetch", remote_repo, git_rev],
+                cwd=self.repo_path,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.STDOUT
+            )
+            fetch_output, _ = fetch_process.communicate()
+            self.log.info(fetch_output.decode("utf-8"))
+            if fetch_process.returncode != 0:
+                raise Exception("Fetching revision failed, aborting")
+            worktree_rev = "FETCH_HEAD"
+        else:
+            self.log.info(
+                "Checking out git worktree for revision {}".format(git_rev)
+            )
+            worktree_rev = git_rev
         worktree_process = subprocess.Popen(
-            [self.git_command, "worktree", "add", "--detach", git_worktree_path, git_rev],
+            [self.git_command, "worktree", "add", "--detach",
+             git_worktree_path, worktree_rev],
             cwd=self.repo_path,
             stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT
@@ -158,9 +183,11 @@ class AbiChecker(object):
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
-    def get_abi_dump_for_ref(self, git_rev):
+    def get_abi_dump_for_ref(self, remote_repo, git_rev):
         """Generate the ABI dumps for the specified git revision."""
-        git_worktree_path = self.get_clean_worktree_for_git_revision(git_rev)
+        git_worktree_path = self.get_clean_worktree_for_git_revision(
+            remote_repo, git_rev
+        )
         self.update_git_submodules(git_worktree_path)
         self.build_shared_libraries(git_worktree_path)
         abi_dumps = self.get_abi_dumps_from_shared_libraries(
@@ -226,8 +253,8 @@ class AbiChecker(object):
         between self.old_rev and self.new_rev."""
         self.check_repo_path()
         self.check_abi_tools_are_installed()
-        self.old_dumps = self.get_abi_dump_for_ref(self.old_rev)
-        self.new_dumps = self.get_abi_dump_for_ref(self.new_rev)
+        self.old_dumps = self.get_abi_dump_for_ref(self.old_repo, self.old_rev)
+        self.new_dumps = self.get_abi_dump_for_ref(self.new_repo, self.new_rev)
         return self.get_abi_compatibility_report()
 
 
@@ -254,17 +281,37 @@ def run_main():
             help="keep all reports, even if there are no compatibility issues",
         )
         parser.add_argument(
-            "-o", "--old-rev", type=str, help="revision for old version",
-            required=True
+            "-o", "--old-rev", type=str,
+            help=("revision for old version."
+                  "Can include repository before revision"),
+            required=True, nargs="+"
         )
         parser.add_argument(
-            "-n", "--new-rev", type=str, help="revision for new version",
-            required=True
+            "-n", "--new-rev", type=str,
+            help=("revision for new version"
+                  "Can include repository before revision"),
+            required=True, nargs="+"
         )
         abi_args = parser.parse_args()
+        if len(abi_args.old_rev) == 1:
+            old_repo = None
+            old_rev = abi_args.old_rev[0]
+        elif len(abi_args.old_rev) == 2:
+            old_repo = abi_args.old_rev[0]
+            old_rev = abi_args.old_rev[1]
+        else:
+            raise Exception("Too many arguments passed for old version")
+        if len(abi_args.new_rev) == 1:
+            new_repo = None
+            new_rev = abi_args.new_rev[0]
+        elif len(abi_args.new_rev) == 2:
+            new_repo = abi_args.new_rev[0]
+            new_rev = abi_args.new_rev[1]
+        else:
+            raise Exception("Too many arguments passed for new version")
         abi_check = AbiChecker(
-            abi_args.report_dir, abi_args.old_rev,
-            abi_args.new_rev, abi_args.keep_all_reports
+            abi_args.report_dir, old_repo, old_rev,
+            new_repo, new_rev, abi_args.keep_all_reports
         )
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)

From 668063bca2884e5e2f010eec65fc8294e7e1cd32 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Wed, 20 Feb 2019 15:01:56 +0000
Subject: [PATCH 04/24] Add option to skip identifiers in ABI checks

By default abi-compliance-checker will check the entire ABI/API.
There are internal identifiers that we do not promise compatibility
for, so we want the ability to skip them when checking the ABI/API.
---
 scripts/abi_check.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index d14236ead..559501dee 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -29,7 +29,7 @@ class AbiChecker(object):
     """API and ABI checker."""
 
     def __init__(self, report_dir, old_repo, old_rev, new_repo, new_rev,
-                 keep_all_reports):
+                 keep_all_reports, skip_file=None):
         """Instantiate the API/ABI checker.
 
         report_dir: directory for output files
@@ -38,6 +38,7 @@ class AbiChecker(object):
         new_repo: repository for git revision to check
         new_rev: git revision to check
         keep_all_reports: if false, delete old reports
+        skip_file: path to file containing symbols and types to skip
         """
         self.repo_path = "."
         self.log = None
@@ -49,6 +50,7 @@ class AbiChecker(object):
         self.old_rev = old_rev
         self.new_repo = new_repo
         self.new_rev = new_rev
+        self.skip_file = skip_file
         self.mbedtls_modules = ["libmbedcrypto", "libmbedtls", "libmbedx509"]
         self.old_dumps = {}
         self.new_dumps = {}
@@ -216,6 +218,9 @@ class AbiChecker(object):
                 "-strict",
                 "-report-path", output_path
             ]
+            if self.skip_file:
+                abi_compliance_command += ["-skip-symbols", self.skip_file,
+                                           "-skip-types", self.skip_file]
             abi_compliance_process = subprocess.Popen(
                 abi_compliance_command,
                 stdout=subprocess.PIPE,
@@ -292,6 +297,10 @@ def run_main():
                   "Can include repository before revision"),
             required=True, nargs="+"
         )
+        parser.add_argument(
+            "-s", "--skip-file", type=str,
+            help="path to file containing symbols and types to skip"
+        )
         abi_args = parser.parse_args()
         if len(abi_args.old_rev) == 1:
             old_repo = None
@@ -311,7 +320,8 @@ def run_main():
             raise Exception("Too many arguments passed for new version")
         abi_check = AbiChecker(
             abi_args.report_dir, old_repo, old_rev,
-            new_repo, new_rev, abi_args.keep_all_reports
+            new_repo, new_rev, abi_args.keep_all_reports,
+            abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)

From 0da4578faecb73efb7c028cd827afdfedac997c7 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Thu, 21 Feb 2019 13:09:26 +0000
Subject: [PATCH 05/24] Add option for a brief report of problems only

---
 scripts/abi_check.py | 71 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 54 insertions(+), 17 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 559501dee..afc2afe1c 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -9,10 +9,10 @@ Purpose
 This script is a small wrapper around the abi-compliance-checker and
 abi-dumper tools, applying them to compare the ABI and API of the library
 files from two different Git revisions within an Mbed TLS repository.
-The results of the comparison are formatted as HTML and stored at
-a configurable location. Returns 0 on success, 1 on ABI/API non-compliance,
-and 2 if there is an error while running the script.
-Note: must be run from Mbed TLS root.
+The results of the comparison are either formatted as HTML and stored at
+a configurable location, or a brief list of problems found is output.
+Returns 0 on success, 1 on ABI/API non-compliance, and 2 if there is an error
+while running the script. Note: must be run from Mbed TLS root.
 """
 
 import os
@@ -24,12 +24,14 @@ import argparse
 import logging
 import tempfile
 
+import xml.etree.ElementTree as ET
+
 
 class AbiChecker(object):
     """API and ABI checker."""
 
     def __init__(self, report_dir, old_repo, old_rev, new_repo, new_rev,
-                 keep_all_reports, skip_file=None):
+                 keep_all_reports, brief, skip_file=None):
         """Instantiate the API/ABI checker.
 
         report_dir: directory for output files
@@ -38,6 +40,7 @@ class AbiChecker(object):
         new_repo: repository for git revision to check
         new_rev: git revision to check
         keep_all_reports: if false, delete old reports
+        brief: if true, output shorter report to stdout
         skip_file: path to file containing symbols and types to skip
         """
         self.repo_path = "."
@@ -51,6 +54,7 @@ class AbiChecker(object):
         self.new_repo = new_repo
         self.new_rev = new_rev
         self.skip_file = skip_file
+        self.brief = brief
         self.mbedtls_modules = ["libmbedcrypto", "libmbedtls", "libmbedx509"]
         self.old_dumps = {}
         self.new_dumps = {}
@@ -198,6 +202,24 @@ class AbiChecker(object):
         self.cleanup_worktree(git_worktree_path)
         return abi_dumps
 
+    def remove_children_with_tag(self, parent, tag):
+        children = parent.getchildren()
+        for child in children:
+            if child.tag == tag:
+                parent.remove(child)
+            else:
+                self.remove_children_with_tag(child, tag)
+
+    def remove_extra_detail_from_report(self, report_root):
+        for tag in ['test_info', 'test_results', 'problem_summary',
+                'added_symbols', 'removed_symbols', 'affected']:
+            self.remove_children_with_tag(report_root, tag)
+
+        for report in report_root:
+            for problems in report.getchildren()[:]:
+                if not problems.getchildren():
+                    report.remove(problems)
+
     def get_abi_compatibility_report(self):
         """Generate a report of the differences between the reference ABI
         and the new ABI. ABI dumps from self.old_rev and self.new_rev must
@@ -216,31 +238,41 @@ class AbiChecker(object):
                 "-old", self.old_dumps[mbed_module],
                 "-new", self.new_dumps[mbed_module],
                 "-strict",
-                "-report-path", output_path
+                "-report-path", output_path,
             ]
             if self.skip_file:
                 abi_compliance_command += ["-skip-symbols", self.skip_file,
                                            "-skip-types", self.skip_file]
+            if self.brief:
+                abi_compliance_command += ["-report-format", "xml",
+                                           "-stdout"]
             abi_compliance_process = subprocess.Popen(
                 abi_compliance_command,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
             )
             abi_compliance_output, _ = abi_compliance_process.communicate()
-            self.log.info(abi_compliance_output.decode("utf-8"))
             if abi_compliance_process.returncode == 0:
                 compatibility_report += (
                     "No compatibility issues for {}\n".format(mbed_module)
                 )
-                if not self.keep_all_reports:
+                if not (self.keep_all_reports or self.brief):
                     os.remove(output_path)
             elif abi_compliance_process.returncode == 1:
-                compliance_return_code = 1
-                self.should_keep_report_dir = True
-                compatibility_report += (
-                    "Compatibility issues found for {}, "
-                    "for details see {}\n".format(mbed_module, output_path)
-                )
+                if self.brief:
+                    self.log.info(
+                        "Compatibility issues found for {}".format(mbed_module)
+                    )
+                    report_root = ET.fromstring(abi_compliance_output.decode("utf-8"))
+                    self.remove_extra_detail_from_report(report_root)
+                    self.log.info(ET.tostring(report_root).decode("utf-8"))
+                else:
+                    compliance_return_code = 1
+                    self.can_remove_report_dir = False
+                    compatibility_report += (
+                        "Compatibility issues found for {}, "
+                        "for details see {}\n".format(mbed_module, output_path)
+                    )
             else:
                 raise Exception(
                     "abi-compliance-checker failed with a return code of {},"
@@ -271,8 +303,9 @@ def run_main():
                 abi-compliance-checker and abi-dumper tools, applying them
                 to compare the ABI and API of the library files from two
                 different Git revisions within an Mbed TLS repository.
-                The results of the comparison are formatted as HTML and stored
-                at a configurable location. Returns 0 on success, 1 on ABI/API
+                The results of the comparison are either formatted as HTML and
+                stored at a configurable location, or a brief list of problems
+                found is output. Returns 0 on success, 1 on ABI/API
                 non-compliance, and 2 if there is an error while running the
                 script. Note: must be run from Mbed TLS root."""
             )
@@ -301,6 +334,10 @@ def run_main():
             "-s", "--skip-file", type=str,
             help="path to file containing symbols and types to skip"
         )
+        parser.add_argument(
+            "-b", "--brief", action="store_true",
+            help="output only the list of issues to stdout, instead of a full report",
+        )
         abi_args = parser.parse_args()
         if len(abi_args.old_rev) == 1:
             old_repo = None
@@ -321,7 +358,7 @@ def run_main():
         abi_check = AbiChecker(
             abi_args.report_dir, old_repo, old_rev,
             new_repo, new_rev, abi_args.keep_all_reports,
-            abi_args.skip_file
+            abi_args.brief, abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)

From 131e24b1b56818907595575922ab1f1c4ab92463 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Mon, 25 Feb 2019 17:01:55 +0000
Subject: [PATCH 06/24] Simplify logic for checking if report folder can be
 removed

---
 scripts/abi_check.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index afc2afe1c..30f38a9bb 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -48,7 +48,8 @@ class AbiChecker(object):
         self.setup_logger()
         self.report_dir = os.path.abspath(report_dir)
         self.keep_all_reports = keep_all_reports
-        self.should_keep_report_dir = os.path.isdir(self.report_dir)
+        self.can_remove_report_dir = not (os.path.isdir(self.report_dir) or
+                                          keep_all_reports)
         self.old_repo = old_repo
         self.old_rev = old_rev
         self.new_repo = new_repo
@@ -280,7 +281,7 @@ class AbiChecker(object):
                 )
             os.remove(self.old_dumps[mbed_module])
             os.remove(self.new_dumps[mbed_module])
-        if not self.should_keep_report_dir and not self.keep_all_reports:
+        if self.can_remove_report_dir:
             os.rmdir(self.report_dir)
         self.log.info(compatibility_report)
         return compliance_return_code

From ae5d66c61209fc1a258617446845861da037e4c0 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Mon, 25 Feb 2019 11:35:05 +0000
Subject: [PATCH 07/24] Extend functionality to allow setting crypto submodule
 version

As going forward we will have Crypto in a submodule, we will need to
be able to check ABI compatibility between versions using different
submodule versions. For TLS versions that support the submodule, we
will always build using the submodule.

If the Crypto submodule is used, libmbedcrypto.so is not in the main
library folder, but in crypto/library instead. Given this, the script
searches for *.so files and notes their path, in order to create the
dumps correctly.
---
 scripts/abi_check.py | 63 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 30f38a9bb..6475a7e64 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -23,6 +23,7 @@ import subprocess
 import argparse
 import logging
 import tempfile
+import fnmatch
 
 import xml.etree.ElementTree as ET
 
@@ -30,15 +31,18 @@ import xml.etree.ElementTree as ET
 class AbiChecker(object):
     """API and ABI checker."""
 
-    def __init__(self, report_dir, old_repo, old_rev, new_repo, new_rev,
-                 keep_all_reports, brief, skip_file=None):
+    def __init__(self, report_dir, old_repo, old_rev, old_crypto_rev,
+                 new_repo, new_rev, new_crypto_rev, keep_all_reports, brief,
+                 skip_file=None):
         """Instantiate the API/ABI checker.
 
         report_dir: directory for output files
         old_repo: repository for git revision to compare against
         old_rev: reference git revision to compare against
+        old_crypto_rev: reference git revision for old crypto submodule
         new_repo: repository for git revision to check
         new_rev: git revision to check
+        new_crypto_rev: reference git revision for new crypto submodule
         keep_all_reports: if false, delete old reports
         brief: if true, output shorter report to stdout
         skip_file: path to file containing symbols and types to skip
@@ -52,11 +56,13 @@ class AbiChecker(object):
                                           keep_all_reports)
         self.old_repo = old_repo
         self.old_rev = old_rev
+        self.old_crypto_rev = old_crypto_rev
         self.new_repo = new_repo
         self.new_rev = new_rev
+        self.new_crypto_rev = new_crypto_rev
         self.skip_file = skip_file
         self.brief = brief
-        self.mbedtls_modules = ["libmbedcrypto", "libmbedtls", "libmbedx509"]
+        self.mbedtls_modules = {}
         self.old_dumps = {}
         self.new_dumps = {}
         self.git_command = "git"
@@ -119,7 +125,7 @@ class AbiChecker(object):
             raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
 
-    def update_git_submodules(self, git_worktree_path):
+    def update_git_submodules(self, git_worktree_path, crypto_rev):
         process = subprocess.Popen(
             [self.git_command, "submodule", "update", "--init", '--recursive'],
             cwd=git_worktree_path,
@@ -130,12 +136,25 @@ class AbiChecker(object):
         self.log.info(output.decode("utf-8"))
         if process.returncode != 0:
             raise Exception("git submodule update failed, aborting")
+        if (os.path.exists(os.path.join(git_worktree_path, "crypto"))
+                and crypto_rev):
+            checkout_process = subprocess.Popen(
+                [self.git_command, "checkout", crypto_rev],
+                cwd=os.path.join(git_worktree_path, "crypto"),
+                stdout=subprocess.PIPE,
+                stderr=subprocess.STDOUT
+            )
+            checkout_output, _ = checkout_process.communicate()
+            self.log.info(checkout_output.decode("utf-8"))
+            if checkout_process.returncode != 0:
+                raise Exception("git checkout failed, aborting")
 
     def build_shared_libraries(self, git_worktree_path):
         """Build the shared libraries in the specified worktree."""
         my_environment = os.environ.copy()
         my_environment["CFLAGS"] = "-g -Og"
         my_environment["SHARED"] = "1"
+        my_environment["USE_CRYPTO_SUBMODULE"] = "1"
         make_process = subprocess.Popen(
             self.make_command,
             env=my_environment,
@@ -145,6 +164,11 @@ class AbiChecker(object):
         )
         make_output, _ = make_process.communicate()
         self.log.info(make_output.decode("utf-8"))
+        for root, dirs, files in os.walk(git_worktree_path):
+            for file in fnmatch.filter(files, "*.so"):
+                self.mbedtls_modules[os.path.splitext(file)[0]] = os.path.join(
+                    root, file
+                )
         if make_process.returncode != 0:
             raise Exception("make failed, aborting")
 
@@ -153,14 +177,13 @@ class AbiChecker(object):
         It must be checked out in git_worktree_path and the shared libraries
         must have been built."""
         abi_dumps = {}
-        for mbed_module in self.mbedtls_modules:
+        for mbed_module, module_path in self.mbedtls_modules.items():
             output_path = os.path.join(
                 self.report_dir, "{}-{}.dump".format(mbed_module, git_ref)
             )
             abi_dump_command = [
                 "abi-dumper",
-                os.path.join(
-                    git_worktree_path, "library", mbed_module + ".so"),
+                module_path,
                 "-o", output_path,
                 "-lver", git_ref
             ]
@@ -190,12 +213,12 @@ class AbiChecker(object):
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
-    def get_abi_dump_for_ref(self, remote_repo, git_rev):
+    def get_abi_dump_for_ref(self, remote_repo, git_rev, crypto_rev):
         """Generate the ABI dumps for the specified git revision."""
         git_worktree_path = self.get_clean_worktree_for_git_revision(
             remote_repo, git_rev
         )
-        self.update_git_submodules(git_worktree_path)
+        self.update_git_submodules(git_worktree_path, crypto_rev)
         self.build_shared_libraries(git_worktree_path)
         abi_dumps = self.get_abi_dumps_from_shared_libraries(
             git_rev, git_worktree_path
@@ -227,7 +250,7 @@ class AbiChecker(object):
         be available."""
         compatibility_report = ""
         compliance_return_code = 0
-        for mbed_module in self.mbedtls_modules:
+        for mbed_module, module_path in self.mbedtls_modules.items():
             output_path = os.path.join(
                 self.report_dir, "{}-{}-{}.html".format(
                     mbed_module, self.old_rev, self.new_rev
@@ -291,8 +314,10 @@ class AbiChecker(object):
         between self.old_rev and self.new_rev."""
         self.check_repo_path()
         self.check_abi_tools_are_installed()
-        self.old_dumps = self.get_abi_dump_for_ref(self.old_repo, self.old_rev)
-        self.new_dumps = self.get_abi_dump_for_ref(self.new_repo, self.new_rev)
+        self.old_dumps = self.get_abi_dump_for_ref(self.old_repo, self.old_rev,
+                                                   self.old_crypto_rev)
+        self.new_dumps = self.get_abi_dump_for_ref(self.new_repo, self.new_rev,
+                                                   self.new_crypto_rev)
         return self.get_abi_compatibility_report()
 
 
@@ -325,12 +350,20 @@ def run_main():
                   "Can include repository before revision"),
             required=True, nargs="+"
         )
+        parser.add_argument(
+            "-oc", "--old-crypto-rev", type=str,
+            help="revision for old crypto version",
+        )
         parser.add_argument(
             "-n", "--new-rev", type=str,
             help=("revision for new version"
                   "Can include repository before revision"),
             required=True, nargs="+"
         )
+        parser.add_argument(
+            "-nc", "--new-crypto-rev", type=str,
+            help="revision for new crypto version",
+        )
         parser.add_argument(
             "-s", "--skip-file", type=str,
             help="path to file containing symbols and types to skip"
@@ -357,9 +390,9 @@ def run_main():
         else:
             raise Exception("Too many arguments passed for new version")
         abi_check = AbiChecker(
-            abi_args.report_dir, old_repo, old_rev,
-            new_repo, new_rev, abi_args.keep_all_reports,
-            abi_args.brief, abi_args.skip_file
+            abi_args.report_dir, old_repo, old_rev, abi_args.old_crypto_rev,
+            new_repo, new_rev, abi_args.new_crypto_rev,
+            abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)

From de118091f21722933466c38ffe894d8346778963 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Wed, 27 Feb 2019 16:53:40 +0000
Subject: [PATCH 08/24] Add handling for cases when not all .so files are
 present

We may wish to compare ABI/API between Mbed TLS and Mbed Crypto,
which will cause issues as not all .so files are shared. Only
compare .so files which both libraries have.
---
 scripts/abi_check.py | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 6475a7e64..3f697fd47 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -62,7 +62,7 @@ class AbiChecker(object):
         self.new_crypto_rev = new_crypto_rev
         self.skip_file = skip_file
         self.brief = brief
-        self.mbedtls_modules = {}
+        self.mbedtls_modules = {"old": {}, "new": {}}
         self.old_dumps = {}
         self.new_dumps = {}
         self.git_command = "git"
@@ -149,7 +149,7 @@ class AbiChecker(object):
             if checkout_process.returncode != 0:
                 raise Exception("git checkout failed, aborting")
 
-    def build_shared_libraries(self, git_worktree_path):
+    def build_shared_libraries(self, git_worktree_path, version):
         """Build the shared libraries in the specified worktree."""
         my_environment = os.environ.copy()
         my_environment["CFLAGS"] = "-g -Og"
@@ -166,20 +166,23 @@ class AbiChecker(object):
         self.log.info(make_output.decode("utf-8"))
         for root, dirs, files in os.walk(git_worktree_path):
             for file in fnmatch.filter(files, "*.so"):
-                self.mbedtls_modules[os.path.splitext(file)[0]] = os.path.join(
-                    root, file
+                self.mbedtls_modules[version][os.path.splitext(file)[0]] = (
+                    os.path.join(root, file)
                 )
         if make_process.returncode != 0:
             raise Exception("make failed, aborting")
 
-    def get_abi_dumps_from_shared_libraries(self, git_ref, git_worktree_path):
+    def get_abi_dumps_from_shared_libraries(self, git_ref, git_worktree_path,
+                                            version):
         """Generate the ABI dumps for the specified git revision.
         It must be checked out in git_worktree_path and the shared libraries
         must have been built."""
         abi_dumps = {}
-        for mbed_module, module_path in self.mbedtls_modules.items():
+        for mbed_module, module_path in self.mbedtls_modules[version].items():
             output_path = os.path.join(
-                self.report_dir, "{}-{}.dump".format(mbed_module, git_ref)
+                self.report_dir, version, "{}-{}.dump".format(
+                    mbed_module, git_ref
+                )
             )
             abi_dump_command = [
                 "abi-dumper",
@@ -213,15 +216,15 @@ class AbiChecker(object):
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
-    def get_abi_dump_for_ref(self, remote_repo, git_rev, crypto_rev):
+    def get_abi_dump_for_ref(self, remote_repo, git_rev, crypto_rev, version):
         """Generate the ABI dumps for the specified git revision."""
         git_worktree_path = self.get_clean_worktree_for_git_revision(
             remote_repo, git_rev
         )
         self.update_git_submodules(git_worktree_path, crypto_rev)
-        self.build_shared_libraries(git_worktree_path)
+        self.build_shared_libraries(git_worktree_path, version)
         abi_dumps = self.get_abi_dumps_from_shared_libraries(
-            git_rev, git_worktree_path
+            git_rev, git_worktree_path, version
         )
         self.cleanup_worktree(git_worktree_path)
         return abi_dumps
@@ -250,7 +253,9 @@ class AbiChecker(object):
         be available."""
         compatibility_report = ""
         compliance_return_code = 0
-        for mbed_module, module_path in self.mbedtls_modules.items():
+        shared_modules = list(set(self.mbedtls_modules["old"].keys()) &
+                              set(self.mbedtls_modules["new"].keys()))
+        for mbed_module in shared_modules:
             output_path = os.path.join(
                 self.report_dir, "{}-{}-{}.html".format(
                     mbed_module, self.old_rev, self.new_rev
@@ -315,9 +320,9 @@ class AbiChecker(object):
         self.check_repo_path()
         self.check_abi_tools_are_installed()
         self.old_dumps = self.get_abi_dump_for_ref(self.old_repo, self.old_rev,
-                                                   self.old_crypto_rev)
+                                                   self.old_crypto_rev, "old")
         self.new_dumps = self.get_abi_dump_for_ref(self.new_repo, self.new_rev,
-                                                   self.new_crypto_rev)
+                                                   self.new_crypto_rev, "new")
         return self.get_abi_compatibility_report()
 
 

From 879f2509dcb7169e6a5da89553b9fc97699372f8 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Wed, 27 Feb 2019 17:33:31 +0000
Subject: [PATCH 09/24] Add ability to compare submodules from different
 repositories

As before with wanting to compare revisions across different
repositories, the ability to select the crypto submodule from a
different repository is useful.
---
 scripts/abi_check.py | 86 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 65 insertions(+), 21 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 3f697fd47..30baadcdb 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -32,17 +32,19 @@ class AbiChecker(object):
     """API and ABI checker."""
 
     def __init__(self, report_dir, old_repo, old_rev, old_crypto_rev,
-                 new_repo, new_rev, new_crypto_rev, keep_all_reports, brief,
-                 skip_file=None):
+                 old_crypto_repo, new_repo, new_rev, new_crypto_rev,
+                 new_crypto_repo, keep_all_reports, brief, skip_file=None):
         """Instantiate the API/ABI checker.
 
         report_dir: directory for output files
         old_repo: repository for git revision to compare against
         old_rev: reference git revision to compare against
         old_crypto_rev: reference git revision for old crypto submodule
+        old_crypto_repo: repository for git revision for old crypto submodule
         new_repo: repository for git revision to check
         new_rev: git revision to check
         new_crypto_rev: reference git revision for new crypto submodule
+        new_crypto_repo: repository for git revision for new crypto submodule
         keep_all_reports: if false, delete old reports
         brief: if true, output shorter report to stdout
         skip_file: path to file containing symbols and types to skip
@@ -57,9 +59,11 @@ class AbiChecker(object):
         self.old_repo = old_repo
         self.old_rev = old_rev
         self.old_crypto_rev = old_crypto_rev
+        self.old_crypto_repo = old_crypto_repo
         self.new_repo = new_repo
         self.new_rev = new_rev
         self.new_crypto_rev = new_crypto_rev
+        self.new_crypto_repo = new_crypto_repo
         self.skip_file = skip_file
         self.brief = brief
         self.mbedtls_modules = {"old": {}, "new": {}}
@@ -125,7 +129,8 @@ class AbiChecker(object):
             raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
 
-    def update_git_submodules(self, git_worktree_path, crypto_rev):
+    def update_git_submodules(self, git_worktree_path, crypto_repo,
+                              crypto_rev):
         process = subprocess.Popen(
             [self.git_command, "submodule", "update", "--init", '--recursive'],
             cwd=git_worktree_path,
@@ -138,16 +143,30 @@ class AbiChecker(object):
             raise Exception("git submodule update failed, aborting")
         if (os.path.exists(os.path.join(git_worktree_path, "crypto"))
                 and crypto_rev):
-            checkout_process = subprocess.Popen(
-                [self.git_command, "checkout", crypto_rev],
-                cwd=os.path.join(git_worktree_path, "crypto"),
-                stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT
-            )
-            checkout_output, _ = checkout_process.communicate()
-            self.log.info(checkout_output.decode("utf-8"))
-            if checkout_process.returncode != 0:
-                raise Exception("git checkout failed, aborting")
+            if crypto_repo:
+                shutil.rmtree(os.path.join(git_worktree_path, "crypto"))
+                clone_process = subprocess.Popen(
+                    [self.git_command, "clone", crypto_repo,
+                     "--branch", crypto_rev, "crypto"],
+                    cwd=git_worktree_path,
+                    stdout=subprocess.PIPE,
+                    stderr=subprocess.STDOUT
+                )
+                clone_output, _ = clone_process.communicate()
+                self.log.info(clone_output.decode("utf-8"))
+                if clone_process.returncode != 0:
+                    raise Exception("git clone failed, aborting")
+            else:
+                checkout_process = subprocess.Popen(
+                    [self.git_command, "checkout", crypto_rev],
+                    cwd=os.path.join(git_worktree_path, "crypto"),
+                    stdout=subprocess.PIPE,
+                    stderr=subprocess.STDOUT
+                )
+                checkout_output, _ = checkout_process.communicate()
+                self.log.info(checkout_output.decode("utf-8"))
+                if checkout_process.returncode != 0:
+                    raise Exception("git checkout failed, aborting")
 
     def build_shared_libraries(self, git_worktree_path, version):
         """Build the shared libraries in the specified worktree."""
@@ -216,12 +235,13 @@ class AbiChecker(object):
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
-    def get_abi_dump_for_ref(self, remote_repo, git_rev, crypto_rev, version):
+    def get_abi_dump_for_ref(self, remote_repo, git_rev, crypto_repo,
+                             crypto_rev, version):
         """Generate the ABI dumps for the specified git revision."""
         git_worktree_path = self.get_clean_worktree_for_git_revision(
             remote_repo, git_rev
         )
-        self.update_git_submodules(git_worktree_path, crypto_rev)
+        self.update_git_submodules(git_worktree_path, crypto_repo, crypto_rev)
         self.build_shared_libraries(git_worktree_path, version)
         abi_dumps = self.get_abi_dumps_from_shared_libraries(
             git_rev, git_worktree_path, version
@@ -320,8 +340,10 @@ class AbiChecker(object):
         self.check_repo_path()
         self.check_abi_tools_are_installed()
         self.old_dumps = self.get_abi_dump_for_ref(self.old_repo, self.old_rev,
+                                                   self.old_crypto_repo,
                                                    self.old_crypto_rev, "old")
         self.new_dumps = self.get_abi_dump_for_ref(self.new_repo, self.new_rev,
+                                                   self.new_crypto_repo,
                                                    self.new_crypto_rev, "new")
         return self.get_abi_compatibility_report()
 
@@ -356,8 +378,9 @@ def run_main():
             required=True, nargs="+"
         )
         parser.add_argument(
-            "-oc", "--old-crypto-rev", type=str,
-            help="revision for old crypto version",
+            "-oc", "--old-crypto-rev", type=str, nargs="+",
+            help=("revision for old crypto version."
+                  "Can include repository before revision"),
         )
         parser.add_argument(
             "-n", "--new-rev", type=str,
@@ -366,8 +389,9 @@ def run_main():
             required=True, nargs="+"
         )
         parser.add_argument(
-            "-nc", "--new-crypto-rev", type=str,
-            help="revision for new crypto version",
+            "-nc", "--new-crypto-rev", type=str, nargs="+",
+            help=("revision for new crypto version"
+                  "Can include repository before revision"),
         )
         parser.add_argument(
             "-s", "--skip-file", type=str,
@@ -394,9 +418,29 @@ def run_main():
             new_rev = abi_args.new_rev[1]
         else:
             raise Exception("Too many arguments passed for new version")
+        old_crypto_repo = None
+        old_crypto_rev = None
+        if abi_args.old_crypto_rev:
+            if len(abi_args.old_crypto_rev) == 1:
+                old_crypto_rev = abi_args.old_crypto_rev[0]
+            elif len(abi_args.old_crypto_rev) == 2:
+                old_crypto_repo = abi_args.old_crypto_rev[0]
+                old_crypto_rev = abi_args.old_crypto_rev[1]
+            else:
+                raise Exception("Too many arguments passed for old crypto version")
+        new_crypto_repo = None
+        new_crypto_rev = None
+        if abi_args.new_crypto_rev:
+            if len(abi_args.new_crypto_rev) == 1:
+                new_crypto_rev = abi_args.new_crypto_rev[0]
+            elif len(abi_args.new_crypto_rev) == 2:
+                new_crypto_repo = abi_args.new_crypto_rev[0]
+                new_crypto_rev = abi_args.new_crypto_rev[1]
+            else:
+                raise Exception("Too many arguments passed for new crypto version")
         abi_check = AbiChecker(
-            abi_args.report_dir, old_repo, old_rev, abi_args.old_crypto_rev,
-            new_repo, new_rev, abi_args.new_crypto_rev,
+            abi_args.report_dir, old_repo, old_rev, old_crypto_rev,
+            old_crypto_repo, new_repo, new_rev, new_crypto_rev, new_crypto_repo,
             abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()

From c8e6ad4ace4fab318f1c2bd98fe220265c73f9ed Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Thu, 28 Feb 2019 11:52:39 +0000
Subject: [PATCH 10/24] Only build the library

We only need the .so files, so only build the library
---
 scripts/abi_check.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 30baadcdb..6c8be0d68 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -175,7 +175,7 @@ class AbiChecker(object):
         my_environment["SHARED"] = "1"
         my_environment["USE_CRYPTO_SUBMODULE"] = "1"
         make_process = subprocess.Popen(
-            self.make_command,
+            [self.make_command, "lib"],
             env=my_environment,
             cwd=git_worktree_path,
             stdout=subprocess.PIPE,

From 06c51d047032a6c695a1eb25fce2d7c89266b83a Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 1 Mar 2019 09:54:44 +0000
Subject: [PATCH 11/24] Use optional arguments for setting repositories

---
 scripts/abi_check.py | 80 +++++++++++++++-----------------------------
 1 file changed, 27 insertions(+), 53 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 6c8be0d68..12ee44ed2 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -372,26 +372,34 @@ def run_main():
             help="keep all reports, even if there are no compatibility issues",
         )
         parser.add_argument(
-            "-o", "--old-rev", type=str,
-            help=("revision for old version."
-                  "Can include repository before revision"),
-            required=True, nargs="+"
+            "-o", "--old-rev", type=str, help="revision for old version.",
+            required=True,
         )
         parser.add_argument(
-            "-oc", "--old-crypto-rev", type=str, nargs="+",
-            help=("revision for old crypto version."
-                  "Can include repository before revision"),
+            "-or", "--old-repo", type=str, help="repository for old version."
         )
         parser.add_argument(
-            "-n", "--new-rev", type=str,
-            help=("revision for new version"
-                  "Can include repository before revision"),
-            required=True, nargs="+"
+            "-oc", "--old-crypto-rev", type=str,
+            help="revision for old crypto submodule."
         )
         parser.add_argument(
-            "-nc", "--new-crypto-rev", type=str, nargs="+",
-            help=("revision for new crypto version"
-                  "Can include repository before revision"),
+            "-ocr", "--old-crypto-repo", type=str,
+            help="repository for old crypto submodule."
+        )
+        parser.add_argument(
+            "-n", "--new-rev", type=str, help="revision for new version",
+            required=True,
+        )
+        parser.add_argument(
+            "-nr", "--new-repo", type=str, help="repository for new version."
+        )
+        parser.add_argument(
+            "-nc", "--new-crypto-rev", type=str,
+            help="revision for new crypto version"
+        )
+        parser.add_argument(
+            "-ncr", "--new-crypto-repo", type=str,
+            help="repository for new crypto submodule."
         )
         parser.add_argument(
             "-s", "--skip-file", type=str,
@@ -402,46 +410,12 @@ def run_main():
             help="output only the list of issues to stdout, instead of a full report",
         )
         abi_args = parser.parse_args()
-        if len(abi_args.old_rev) == 1:
-            old_repo = None
-            old_rev = abi_args.old_rev[0]
-        elif len(abi_args.old_rev) == 2:
-            old_repo = abi_args.old_rev[0]
-            old_rev = abi_args.old_rev[1]
-        else:
-            raise Exception("Too many arguments passed for old version")
-        if len(abi_args.new_rev) == 1:
-            new_repo = None
-            new_rev = abi_args.new_rev[0]
-        elif len(abi_args.new_rev) == 2:
-            new_repo = abi_args.new_rev[0]
-            new_rev = abi_args.new_rev[1]
-        else:
-            raise Exception("Too many arguments passed for new version")
-        old_crypto_repo = None
-        old_crypto_rev = None
-        if abi_args.old_crypto_rev:
-            if len(abi_args.old_crypto_rev) == 1:
-                old_crypto_rev = abi_args.old_crypto_rev[0]
-            elif len(abi_args.old_crypto_rev) == 2:
-                old_crypto_repo = abi_args.old_crypto_rev[0]
-                old_crypto_rev = abi_args.old_crypto_rev[1]
-            else:
-                raise Exception("Too many arguments passed for old crypto version")
-        new_crypto_repo = None
-        new_crypto_rev = None
-        if abi_args.new_crypto_rev:
-            if len(abi_args.new_crypto_rev) == 1:
-                new_crypto_rev = abi_args.new_crypto_rev[0]
-            elif len(abi_args.new_crypto_rev) == 2:
-                new_crypto_repo = abi_args.new_crypto_rev[0]
-                new_crypto_rev = abi_args.new_crypto_rev[1]
-            else:
-                raise Exception("Too many arguments passed for new crypto version")
         abi_check = AbiChecker(
-            abi_args.report_dir, old_repo, old_rev, old_crypto_rev,
-            old_crypto_repo, new_repo, new_rev, new_crypto_rev, new_crypto_repo,
-            abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file
+            abi_args.report_dir, abi_args.old_repo, abi_args.old_rev,
+            abi_args.old_crypto_rev, abi_args.old_crypto_repo,
+            abi_args.new_repo, abi_args.new_rev, abi_args.new_crypto_rev,
+            abi_args.new_crypto_repo, abi_args.keep_all_reports,
+            abi_args.brief, abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)

From 7c0e0522760fd7e40c848993ea88f8a346c01d6e Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Tue, 5 Mar 2019 15:21:32 +0000
Subject: [PATCH 12/24] Improve documentation

---
 scripts/abi_check.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 12ee44ed2..7e6d7fcfd 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -10,7 +10,7 @@ This script is a small wrapper around the abi-compliance-checker and
 abi-dumper tools, applying them to compare the ABI and API of the library
 files from two different Git revisions within an Mbed TLS repository.
 The results of the comparison are either formatted as HTML and stored at
-a configurable location, or a brief list of problems found is output.
+a configurable location, or are given as a brief list of problems.
 Returns 0 on success, 1 on ABI/API non-compliance, and 2 if there is an error
 while running the script. Note: must be run from Mbed TLS root.
 """
@@ -357,10 +357,10 @@ def run_main():
                 to compare the ABI and API of the library files from two
                 different Git revisions within an Mbed TLS repository.
                 The results of the comparison are either formatted as HTML and
-                stored at a configurable location, or a brief list of problems
-                found is output. Returns 0 on success, 1 on ABI/API
-                non-compliance, and 2 if there is an error while running the
-                script. Note: must be run from Mbed TLS root."""
+                stored at a configurable location, or are given as a brief list
+                of problems. Returns 0 on success, 1 on ABI/API non-compliance,
+                and 2 if there is an error while running the script.
+                Note: must be run from Mbed TLS root."""
             )
         )
         parser.add_argument(

From 0478a321622207650dac700fa8b4df85391c0a92 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Tue, 5 Mar 2019 15:23:25 +0000
Subject: [PATCH 13/24] Reduce indentation levels

---
 scripts/abi_check.py | 52 +++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 7e6d7fcfd..bab2fcdac 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -141,32 +141,34 @@ class AbiChecker(object):
         self.log.info(output.decode("utf-8"))
         if process.returncode != 0:
             raise Exception("git submodule update failed, aborting")
-        if (os.path.exists(os.path.join(git_worktree_path, "crypto"))
+        if not (os.path.exists(os.path.join(git_worktree_path, "crypto"))
                 and crypto_rev):
-            if crypto_repo:
-                shutil.rmtree(os.path.join(git_worktree_path, "crypto"))
-                clone_process = subprocess.Popen(
-                    [self.git_command, "clone", crypto_repo,
-                     "--branch", crypto_rev, "crypto"],
-                    cwd=git_worktree_path,
-                    stdout=subprocess.PIPE,
-                    stderr=subprocess.STDOUT
-                )
-                clone_output, _ = clone_process.communicate()
-                self.log.info(clone_output.decode("utf-8"))
-                if clone_process.returncode != 0:
-                    raise Exception("git clone failed, aborting")
-            else:
-                checkout_process = subprocess.Popen(
-                    [self.git_command, "checkout", crypto_rev],
-                    cwd=os.path.join(git_worktree_path, "crypto"),
-                    stdout=subprocess.PIPE,
-                    stderr=subprocess.STDOUT
-                )
-                checkout_output, _ = checkout_process.communicate()
-                self.log.info(checkout_output.decode("utf-8"))
-                if checkout_process.returncode != 0:
-                    raise Exception("git checkout failed, aborting")
+            return
+
+        if crypto_repo:
+            shutil.rmtree(os.path.join(git_worktree_path, "crypto"))
+            clone_process = subprocess.Popen(
+                [self.git_command, "clone", crypto_repo,
+                 "--branch", crypto_rev, "crypto"],
+                cwd=git_worktree_path,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.STDOUT
+            )
+            clone_output, _ = clone_process.communicate()
+            self.log.info(clone_output.decode("utf-8"))
+            if clone_process.returncode != 0:
+                raise Exception("git clone failed, aborting")
+        else:
+            checkout_process = subprocess.Popen(
+                [self.git_command, "checkout", crypto_rev],
+                cwd=os.path.join(git_worktree_path, "crypto"),
+                stdout=subprocess.PIPE,
+                stderr=subprocess.STDOUT
+            )
+            checkout_output, _ = checkout_process.communicate()
+            self.log.info(checkout_output.decode("utf-8"))
+            if checkout_process.returncode != 0:
+                raise Exception("git checkout failed, aborting")
 
     def build_shared_libraries(self, git_worktree_path, version):
         """Build the shared libraries in the specified worktree."""

From 7381bea6be0c257282f731d2b2081be81974f9d4 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Tue, 5 Mar 2019 16:25:38 +0000
Subject: [PATCH 14/24] Add RepoVersion class to make handling of many
 arguments easier

There are a number of arguments being passed around, nearly all of
which are duplicated between the old and new versions. Moving these
into a separate class should hopefully make it simpler to follow
what is being done.
---
 scripts/abi_check.py | 145 +++++++++++++++++++++----------------------
 1 file changed, 70 insertions(+), 75 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index bab2fcdac..cc8667899 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -28,23 +28,37 @@ import fnmatch
 import xml.etree.ElementTree as ET
 
 
+class RepoVersion(object):
+
+    def __init__(self, version, repository, revision,
+                 crypto_repository, crypto_revision):
+        """Class containing details for a particular revision.
+
+        version: either 'old' or 'new'
+        repository: repository for git revision
+        revision: git revision for comparison
+        crypto_repository: repository for git revision of crypto submodule
+        crypto_revision: git revision of crypto submodule
+        """
+        self.version = version
+        self.repository = repository
+        self.revision = revision
+        self.crypto_repository = crypto_repository
+        self.crypto_revision = crypto_revision
+        self.abi_dumps = {}
+        self.modules = {}
+
+
 class AbiChecker(object):
     """API and ABI checker."""
 
-    def __init__(self, report_dir, old_repo, old_rev, old_crypto_rev,
-                 old_crypto_repo, new_repo, new_rev, new_crypto_rev,
-                 new_crypto_repo, keep_all_reports, brief, skip_file=None):
+    def __init__(self, old_version, new_version, report_dir,
+                 keep_all_reports, brief, skip_file=None):
         """Instantiate the API/ABI checker.
 
+        old_version: RepoVersion containing details to compare against
+        new_version: RepoVersion containing details to check
         report_dir: directory for output files
-        old_repo: repository for git revision to compare against
-        old_rev: reference git revision to compare against
-        old_crypto_rev: reference git revision for old crypto submodule
-        old_crypto_repo: repository for git revision for old crypto submodule
-        new_repo: repository for git revision to check
-        new_rev: git revision to check
-        new_crypto_rev: reference git revision for new crypto submodule
-        new_crypto_repo: repository for git revision for new crypto submodule
         keep_all_reports: if false, delete old reports
         brief: if true, output shorter report to stdout
         skip_file: path to file containing symbols and types to skip
@@ -56,19 +70,10 @@ class AbiChecker(object):
         self.keep_all_reports = keep_all_reports
         self.can_remove_report_dir = not (os.path.isdir(self.report_dir) or
                                           keep_all_reports)
-        self.old_repo = old_repo
-        self.old_rev = old_rev
-        self.old_crypto_rev = old_crypto_rev
-        self.old_crypto_repo = old_crypto_repo
-        self.new_repo = new_repo
-        self.new_rev = new_rev
-        self.new_crypto_rev = new_crypto_rev
-        self.new_crypto_repo = new_crypto_repo
+        self.old_version = old_version
+        self.new_version = new_version
         self.skip_file = skip_file
         self.brief = brief
-        self.mbedtls_modules = {"old": {}, "new": {}}
-        self.old_dumps = {}
-        self.new_dumps = {}
         self.git_command = "git"
         self.make_command = "make"
 
@@ -90,18 +95,19 @@ class AbiChecker(object):
             if not shutil.which(command):
                 raise Exception("{} not installed, aborting".format(command))
 
-    def get_clean_worktree_for_git_revision(self, remote_repo, git_rev):
-        """Make a separate worktree with git_rev checked out.
+    def get_clean_worktree_for_git_revision(self, version):
+        """Make a separate worktree with version.revision checked out.
         Do not modify the current worktree."""
         git_worktree_path = tempfile.mkdtemp()
-        if remote_repo:
+        if version.repository:
             self.log.info(
                 "Checking out git worktree for revision {} from {}".format(
-                    git_rev, remote_repo
+                    version.revision, version.repository
                 )
             )
             fetch_process = subprocess.Popen(
-                [self.git_command, "fetch", remote_repo, git_rev],
+                [self.git_command, "fetch",
+                 version.repository, version.revision],
                 cwd=self.repo_path,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
@@ -112,10 +118,10 @@ class AbiChecker(object):
                 raise Exception("Fetching revision failed, aborting")
             worktree_rev = "FETCH_HEAD"
         else:
-            self.log.info(
-                "Checking out git worktree for revision {}".format(git_rev)
-            )
-            worktree_rev = git_rev
+            self.log.info("Checking out git worktree for revision {}".format(
+                version.revision
+            ))
+            worktree_rev = version.revision
         worktree_process = subprocess.Popen(
             [self.git_command, "worktree", "add", "--detach",
              git_worktree_path, worktree_rev],
@@ -129,8 +135,7 @@ class AbiChecker(object):
             raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
 
-    def update_git_submodules(self, git_worktree_path, crypto_repo,
-                              crypto_rev):
+    def update_git_submodules(self, git_worktree_path, version):
         process = subprocess.Popen(
             [self.git_command, "submodule", "update", "--init", '--recursive'],
             cwd=git_worktree_path,
@@ -142,14 +147,14 @@ class AbiChecker(object):
         if process.returncode != 0:
             raise Exception("git submodule update failed, aborting")
         if not (os.path.exists(os.path.join(git_worktree_path, "crypto"))
-                and crypto_rev):
+                and version.crypto_revision):
             return
 
-        if crypto_repo:
+        if version.crypto_repository:
             shutil.rmtree(os.path.join(git_worktree_path, "crypto"))
             clone_process = subprocess.Popen(
-                [self.git_command, "clone", crypto_repo,
-                 "--branch", crypto_rev, "crypto"],
+                [self.git_command, "clone", version.crypto_repository,
+                 "--branch", version.crypto_revision, "crypto"],
                 cwd=git_worktree_path,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
@@ -160,7 +165,7 @@ class AbiChecker(object):
                 raise Exception("git clone failed, aborting")
         else:
             checkout_process = subprocess.Popen(
-                [self.git_command, "checkout", crypto_rev],
+                [self.git_command, "checkout", version.crypto_revision],
                 cwd=os.path.join(git_worktree_path, "crypto"),
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
@@ -187,29 +192,28 @@ class AbiChecker(object):
         self.log.info(make_output.decode("utf-8"))
         for root, dirs, files in os.walk(git_worktree_path):
             for file in fnmatch.filter(files, "*.so"):
-                self.mbedtls_modules[version][os.path.splitext(file)[0]] = (
+                version.modules[os.path.splitext(file)[0]] = (
                     os.path.join(root, file)
                 )
         if make_process.returncode != 0:
             raise Exception("make failed, aborting")
 
-    def get_abi_dumps_from_shared_libraries(self, git_ref, git_worktree_path,
+    def get_abi_dumps_from_shared_libraries(self, git_worktree_path,
                                             version):
         """Generate the ABI dumps for the specified git revision.
         It must be checked out in git_worktree_path and the shared libraries
         must have been built."""
-        abi_dumps = {}
-        for mbed_module, module_path in self.mbedtls_modules[version].items():
+        for mbed_module, module_path in version.modules.items():
             output_path = os.path.join(
-                self.report_dir, version, "{}-{}.dump".format(
-                    mbed_module, git_ref
+                self.report_dir, version.version, "{}-{}.dump".format(
+                    mbed_module, version.revision
                 )
             )
             abi_dump_command = [
                 "abi-dumper",
                 module_path,
                 "-o", output_path,
-                "-lver", git_ref
+                "-lver", version.revision
             ]
             abi_dump_process = subprocess.Popen(
                 abi_dump_command,
@@ -220,8 +224,7 @@ class AbiChecker(object):
             self.log.info(abi_dump_output.decode("utf-8"))
             if abi_dump_process.returncode != 0:
                 raise Exception("abi-dumper failed, aborting")
-            abi_dumps[mbed_module] = output_path
-        return abi_dumps
+            version.abi_dumps[mbed_module] = output_path
 
     def cleanup_worktree(self, git_worktree_path):
         """Remove the specified git worktree."""
@@ -237,19 +240,13 @@ class AbiChecker(object):
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
-    def get_abi_dump_for_ref(self, remote_repo, git_rev, crypto_repo,
-                             crypto_rev, version):
+    def get_abi_dump_for_ref(self, version):
         """Generate the ABI dumps for the specified git revision."""
-        git_worktree_path = self.get_clean_worktree_for_git_revision(
-            remote_repo, git_rev
-        )
-        self.update_git_submodules(git_worktree_path, crypto_repo, crypto_rev)
+        git_worktree_path = self.get_clean_worktree_for_git_revision(version)
+        self.update_git_submodules(git_worktree_path, version)
         self.build_shared_libraries(git_worktree_path, version)
-        abi_dumps = self.get_abi_dumps_from_shared_libraries(
-            git_rev, git_worktree_path, version
-        )
+        self.get_abi_dumps_from_shared_libraries(git_worktree_path, version)
         self.cleanup_worktree(git_worktree_path)
-        return abi_dumps
 
     def remove_children_with_tag(self, parent, tag):
         children = parent.getchildren()
@@ -275,19 +272,20 @@ class AbiChecker(object):
         be available."""
         compatibility_report = ""
         compliance_return_code = 0
-        shared_modules = list(set(self.mbedtls_modules["old"].keys()) &
-                              set(self.mbedtls_modules["new"].keys()))
+        shared_modules = list(set(self.old_version.modules.keys()) &
+                              set(self.new_version.modules.keys()))
         for mbed_module in shared_modules:
             output_path = os.path.join(
                 self.report_dir, "{}-{}-{}.html".format(
-                    mbed_module, self.old_rev, self.new_rev
+                    mbed_module, self.old_version.revision,
+                    self.new_version.revision
                 )
             )
             abi_compliance_command = [
                 "abi-compliance-checker",
                 "-l", mbed_module,
-                "-old", self.old_dumps[mbed_module],
-                "-new", self.new_dumps[mbed_module],
+                "-old", self.old_version.abi_dumps[mbed_module],
+                "-new", self.new_version.abi_dumps[mbed_module],
                 "-strict",
                 "-report-path", output_path,
             ]
@@ -329,8 +327,8 @@ class AbiChecker(object):
                     "abi-compliance-checker failed with a return code of {},"
                     " aborting".format(abi_compliance_process.returncode)
                 )
-            os.remove(self.old_dumps[mbed_module])
-            os.remove(self.new_dumps[mbed_module])
+            os.remove(self.old_version.abi_dumps[mbed_module])
+            os.remove(self.new_version.abi_dumps[mbed_module])
         if self.can_remove_report_dir:
             os.rmdir(self.report_dir)
         self.log.info(compatibility_report)
@@ -341,12 +339,8 @@ class AbiChecker(object):
         between self.old_rev and self.new_rev."""
         self.check_repo_path()
         self.check_abi_tools_are_installed()
-        self.old_dumps = self.get_abi_dump_for_ref(self.old_repo, self.old_rev,
-                                                   self.old_crypto_repo,
-                                                   self.old_crypto_rev, "old")
-        self.new_dumps = self.get_abi_dump_for_ref(self.new_repo, self.new_rev,
-                                                   self.new_crypto_repo,
-                                                   self.new_crypto_rev, "new")
+        self.get_abi_dump_for_ref(self.old_version)
+        self.get_abi_dump_for_ref(self.new_version)
         return self.get_abi_compatibility_report()
 
 
@@ -412,12 +406,13 @@ def run_main():
             help="output only the list of issues to stdout, instead of a full report",
         )
         abi_args = parser.parse_args()
+        old_version = RepoVersion("old", abi_args.old_repo, abi_args.old_rev,
+                 abi_args.old_crypto_repo, abi_args.old_crypto_rev)
+        new_version = RepoVersion("new", abi_args.new_repo, abi_args.new_rev,
+                 abi_args.new_crypto_repo, abi_args.new_crypto_rev)
         abi_check = AbiChecker(
-            abi_args.report_dir, abi_args.old_repo, abi_args.old_rev,
-            abi_args.old_crypto_rev, abi_args.old_crypto_repo,
-            abi_args.new_repo, abi_args.new_rev, abi_args.new_crypto_rev,
-            abi_args.new_crypto_repo, abi_args.keep_all_reports,
-            abi_args.brief, abi_args.skip_file
+            old_version, new_version, abi_args.report_dir,
+            abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)

From 88bfbc2deb78847679b1385a1eecb14ce2f00d01 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Tue, 5 Mar 2019 16:30:39 +0000
Subject: [PATCH 15/24] Prefix internal functions with underscore

---
 scripts/abi_check.py | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index cc8667899..193169154 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -65,7 +65,7 @@ class AbiChecker(object):
         """
         self.repo_path = "."
         self.log = None
-        self.setup_logger()
+        self._setup_logger()
         self.report_dir = os.path.abspath(report_dir)
         self.keep_all_reports = keep_all_reports
         self.can_remove_report_dir = not (os.path.isdir(self.report_dir) or
@@ -84,7 +84,7 @@ class AbiChecker(object):
         if current_dir != root_dir:
             raise Exception("Must be run from Mbed TLS root")
 
-    def setup_logger(self):
+    def _setup_logger(self):
         self.log = logging.getLogger()
         self.log.setLevel(logging.INFO)
         self.log.addHandler(logging.StreamHandler())
@@ -95,7 +95,7 @@ class AbiChecker(object):
             if not shutil.which(command):
                 raise Exception("{} not installed, aborting".format(command))
 
-    def get_clean_worktree_for_git_revision(self, version):
+    def _get_clean_worktree_for_git_revision(self, version):
         """Make a separate worktree with version.revision checked out.
         Do not modify the current worktree."""
         git_worktree_path = tempfile.mkdtemp()
@@ -135,7 +135,7 @@ class AbiChecker(object):
             raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
 
-    def update_git_submodules(self, git_worktree_path, version):
+    def _update_git_submodules(self, git_worktree_path, version):
         process = subprocess.Popen(
             [self.git_command, "submodule", "update", "--init", '--recursive'],
             cwd=git_worktree_path,
@@ -175,7 +175,7 @@ class AbiChecker(object):
             if checkout_process.returncode != 0:
                 raise Exception("git checkout failed, aborting")
 
-    def build_shared_libraries(self, git_worktree_path, version):
+    def _build_shared_libraries(self, git_worktree_path, version):
         """Build the shared libraries in the specified worktree."""
         my_environment = os.environ.copy()
         my_environment["CFLAGS"] = "-g -Og"
@@ -198,8 +198,8 @@ class AbiChecker(object):
         if make_process.returncode != 0:
             raise Exception("make failed, aborting")
 
-    def get_abi_dumps_from_shared_libraries(self, git_worktree_path,
-                                            version):
+    def _get_abi_dumps_from_shared_libraries(self, git_worktree_path,
+                                             version):
         """Generate the ABI dumps for the specified git revision.
         It must be checked out in git_worktree_path and the shared libraries
         must have been built."""
@@ -226,7 +226,7 @@ class AbiChecker(object):
                 raise Exception("abi-dumper failed, aborting")
             version.abi_dumps[mbed_module] = output_path
 
-    def cleanup_worktree(self, git_worktree_path):
+    def _cleanup_worktree(self, git_worktree_path):
         """Remove the specified git worktree."""
         shutil.rmtree(git_worktree_path)
         worktree_process = subprocess.Popen(
@@ -240,26 +240,26 @@ class AbiChecker(object):
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
-    def get_abi_dump_for_ref(self, version):
+    def _get_abi_dump_for_ref(self, version):
         """Generate the ABI dumps for the specified git revision."""
-        git_worktree_path = self.get_clean_worktree_for_git_revision(version)
-        self.update_git_submodules(git_worktree_path, version)
-        self.build_shared_libraries(git_worktree_path, version)
-        self.get_abi_dumps_from_shared_libraries(git_worktree_path, version)
-        self.cleanup_worktree(git_worktree_path)
+        git_worktree_path = self._get_clean_worktree_for_git_revision(version)
+        self._update_git_submodules(git_worktree_path, version)
+        self._build_shared_libraries(git_worktree_path, version)
+        self._get_abi_dumps_from_shared_libraries(git_worktree_path, version)
+        self._cleanup_worktree(git_worktree_path)
 
-    def remove_children_with_tag(self, parent, tag):
+    def _remove_children_with_tag(self, parent, tag):
         children = parent.getchildren()
         for child in children:
             if child.tag == tag:
                 parent.remove(child)
             else:
-                self.remove_children_with_tag(child, tag)
+                self._remove_children_with_tag(child, tag)
 
-    def remove_extra_detail_from_report(self, report_root):
+    def _remove_extra_detail_from_report(self, report_root):
         for tag in ['test_info', 'test_results', 'problem_summary',
                 'added_symbols', 'removed_symbols', 'affected']:
-            self.remove_children_with_tag(report_root, tag)
+            self._remove_children_with_tag(report_root, tag)
 
         for report in report_root:
             for problems in report.getchildren()[:]:
@@ -313,7 +313,7 @@ class AbiChecker(object):
                         "Compatibility issues found for {}".format(mbed_module)
                     )
                     report_root = ET.fromstring(abi_compliance_output.decode("utf-8"))
-                    self.remove_extra_detail_from_report(report_root)
+                    self._remove_extra_detail_from_report(report_root)
                     self.log.info(ET.tostring(report_root).decode("utf-8"))
                 else:
                     compliance_return_code = 1
@@ -339,8 +339,8 @@ class AbiChecker(object):
         between self.old_rev and self.new_rev."""
         self.check_repo_path()
         self.check_abi_tools_are_installed()
-        self.get_abi_dump_for_ref(self.old_version)
-        self.get_abi_dump_for_ref(self.new_version)
+        self._get_abi_dump_for_ref(self.old_version)
+        self._get_abi_dump_for_ref(self.new_version)
         return self.get_abi_compatibility_report()
 
 

From 3f742987b86a7c9146c5980ac916a851763995fc Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 8 Mar 2019 11:12:19 +0000
Subject: [PATCH 16/24] Fetch the remote crypto branch, rather than cloning it

---
 scripts/abi_check.py | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 193169154..d23b038ea 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -151,29 +151,31 @@ class AbiChecker(object):
             return
 
         if version.crypto_repository:
-            shutil.rmtree(os.path.join(git_worktree_path, "crypto"))
-            clone_process = subprocess.Popen(
-                [self.git_command, "clone", version.crypto_repository,
-                 "--branch", version.crypto_revision, "crypto"],
-                cwd=git_worktree_path,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT
-            )
-            clone_output, _ = clone_process.communicate()
-            self.log.info(clone_output.decode("utf-8"))
-            if clone_process.returncode != 0:
-                raise Exception("git clone failed, aborting")
-        else:
-            checkout_process = subprocess.Popen(
-                [self.git_command, "checkout", version.crypto_revision],
+            fetch_process = subprocess.Popen(
+                [self.git_command, "fetch", version.crypto_repository,
+                 version.crypto_revision],
                 cwd=os.path.join(git_worktree_path, "crypto"),
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
             )
-            checkout_output, _ = checkout_process.communicate()
-            self.log.info(checkout_output.decode("utf-8"))
-            if checkout_process.returncode != 0:
-                raise Exception("git checkout failed, aborting")
+            fetch_output, _ = fetch_process.communicate()
+            self.log.info(fetch_output.decode("utf-8"))
+            if fetch_process.returncode != 0:
+                raise Exception("git fetch failed, aborting")
+            crypto_rev = "FETCH_HEAD"
+        else:
+            crypto_rev = version.crypto_revision
+
+        checkout_process = subprocess.Popen(
+            [self.git_command, "checkout", crypto_rev],
+            cwd=os.path.join(git_worktree_path, "crypto"),
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT
+        )
+        checkout_output, _ = checkout_process.communicate()
+        self.log.info(checkout_output.decode("utf-8"))
+        if checkout_process.returncode != 0:
+            raise Exception("git checkout failed, aborting")
 
     def _build_shared_libraries(self, git_worktree_path, version):
         """Build the shared libraries in the specified worktree."""

From 66025385443b93cb09e5058658db5b7632c57c13 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 8 Mar 2019 11:30:04 +0000
Subject: [PATCH 17/24] Add verbose switch to silence all output except the
 final report

---
 scripts/abi_check.py | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index d23b038ea..a25c85fc1 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -52,7 +52,7 @@ class RepoVersion(object):
 class AbiChecker(object):
     """API and ABI checker."""
 
-    def __init__(self, old_version, new_version, report_dir,
+    def __init__(self, verbose, old_version, new_version, report_dir,
                  keep_all_reports, brief, skip_file=None):
         """Instantiate the API/ABI checker.
 
@@ -65,6 +65,7 @@ class AbiChecker(object):
         """
         self.repo_path = "."
         self.log = None
+        self.verbose = verbose
         self._setup_logger()
         self.report_dir = os.path.abspath(report_dir)
         self.keep_all_reports = keep_all_reports
@@ -86,7 +87,10 @@ class AbiChecker(object):
 
     def _setup_logger(self):
         self.log = logging.getLogger()
-        self.log.setLevel(logging.INFO)
+        if self.verbose:
+            self.log.setLevel(logging.DEBUG)
+        else:
+            self.log.setLevel(logging.INFO)
         self.log.addHandler(logging.StreamHandler())
 
     @staticmethod
@@ -100,7 +104,7 @@ class AbiChecker(object):
         Do not modify the current worktree."""
         git_worktree_path = tempfile.mkdtemp()
         if version.repository:
-            self.log.info(
+            self.log.debug(
                 "Checking out git worktree for revision {} from {}".format(
                     version.revision, version.repository
                 )
@@ -113,12 +117,12 @@ class AbiChecker(object):
                 stderr=subprocess.STDOUT
             )
             fetch_output, _ = fetch_process.communicate()
-            self.log.info(fetch_output.decode("utf-8"))
+            self.log.debug(fetch_output.decode("utf-8"))
             if fetch_process.returncode != 0:
                 raise Exception("Fetching revision failed, aborting")
             worktree_rev = "FETCH_HEAD"
         else:
-            self.log.info("Checking out git worktree for revision {}".format(
+            self.log.debug("Checking out git worktree for revision {}".format(
                 version.revision
             ))
             worktree_rev = version.revision
@@ -130,7 +134,7 @@ class AbiChecker(object):
             stderr=subprocess.STDOUT
         )
         worktree_output, _ = worktree_process.communicate()
-        self.log.info(worktree_output.decode("utf-8"))
+        self.log.debug(worktree_output.decode("utf-8"))
         if worktree_process.returncode != 0:
             raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
@@ -143,7 +147,7 @@ class AbiChecker(object):
             stderr=subprocess.STDOUT
         )
         output, _ = process.communicate()
-        self.log.info(output.decode("utf-8"))
+        self.log.debug(output.decode("utf-8"))
         if process.returncode != 0:
             raise Exception("git submodule update failed, aborting")
         if not (os.path.exists(os.path.join(git_worktree_path, "crypto"))
@@ -159,7 +163,7 @@ class AbiChecker(object):
                 stderr=subprocess.STDOUT
             )
             fetch_output, _ = fetch_process.communicate()
-            self.log.info(fetch_output.decode("utf-8"))
+            self.log.debug(fetch_output.decode("utf-8"))
             if fetch_process.returncode != 0:
                 raise Exception("git fetch failed, aborting")
             crypto_rev = "FETCH_HEAD"
@@ -173,7 +177,7 @@ class AbiChecker(object):
             stderr=subprocess.STDOUT
         )
         checkout_output, _ = checkout_process.communicate()
-        self.log.info(checkout_output.decode("utf-8"))
+        self.log.debug(checkout_output.decode("utf-8"))
         if checkout_process.returncode != 0:
             raise Exception("git checkout failed, aborting")
 
@@ -191,7 +195,7 @@ class AbiChecker(object):
             stderr=subprocess.STDOUT
         )
         make_output, _ = make_process.communicate()
-        self.log.info(make_output.decode("utf-8"))
+        self.log.debug(make_output.decode("utf-8"))
         for root, dirs, files in os.walk(git_worktree_path):
             for file in fnmatch.filter(files, "*.so"):
                 version.modules[os.path.splitext(file)[0]] = (
@@ -223,7 +227,7 @@ class AbiChecker(object):
                 stderr=subprocess.STDOUT
             )
             abi_dump_output, _ = abi_dump_process.communicate()
-            self.log.info(abi_dump_output.decode("utf-8"))
+            self.log.debug(abi_dump_output.decode("utf-8"))
             if abi_dump_process.returncode != 0:
                 raise Exception("abi-dumper failed, aborting")
             version.abi_dumps[mbed_module] = output_path
@@ -238,7 +242,7 @@ class AbiChecker(object):
             stderr=subprocess.STDOUT
         )
         worktree_output, _ = worktree_process.communicate()
-        self.log.info(worktree_output.decode("utf-8"))
+        self.log.debug(worktree_output.decode("utf-8"))
         if worktree_process.returncode != 0:
             raise Exception("Worktree cleanup failed, aborting")
 
@@ -361,6 +365,10 @@ def run_main():
                 Note: must be run from Mbed TLS root."""
             )
         )
+        parser.add_argument(
+            "-v", "--verbose", action="store_true",
+            help="set verbosity level",
+        )
         parser.add_argument(
             "-r", "--report-dir", type=str, default="reports",
             help="directory where reports are stored, default is reports",
@@ -413,7 +421,7 @@ def run_main():
         new_version = RepoVersion("new", abi_args.new_repo, abi_args.new_rev,
                  abi_args.new_crypto_repo, abi_args.new_crypto_rev)
         abi_check = AbiChecker(
-            old_version, new_version, abi_args.report_dir,
+            abi_args.verbose, old_version, new_version, abi_args.report_dir,
             abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file
         )
         return_code = abi_check.check_for_abi_changes()

From 5783847c639be6c7aaf684b631a78a041adadc83 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Thu, 4 Apr 2019 14:39:33 +0100
Subject: [PATCH 18/24] Don't put abi dumps in subfolders

---
 scripts/abi_check.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index a25c85fc1..a5ef7e648 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -211,8 +211,8 @@ class AbiChecker(object):
         must have been built."""
         for mbed_module, module_path in version.modules.items():
             output_path = os.path.join(
-                self.report_dir, version.version, "{}-{}.dump".format(
-                    mbed_module, version.revision
+                self.report_dir, "{}-{}-{}.dump".format(
+                    mbed_module, version.revision, version.version
                 )
             )
             abi_dump_command = [

From 26dff8e68d4029441d7857bf67dbcc0bd390ca65 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 5 Apr 2019 17:06:17 +0100
Subject: [PATCH 19/24] Fix pylint issues

---
 scripts/abi_check.py | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index a5ef7e648..9f99309af 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -140,6 +140,9 @@ class AbiChecker(object):
         return git_worktree_path
 
     def _update_git_submodules(self, git_worktree_path, version):
+        """If the crypto submodule is present, initialize it.
+        if version.crypto_revision exists, update it to that revision,
+        otherwise update it to the default revision"""
         process = subprocess.Popen(
             [self.git_command, "submodule", "update", "--init", '--recursive'],
             cwd=git_worktree_path,
@@ -196,7 +199,7 @@ class AbiChecker(object):
         )
         make_output, _ = make_process.communicate()
         self.log.debug(make_output.decode("utf-8"))
-        for root, dirs, files in os.walk(git_worktree_path):
+        for root, dirs, files in os.walk(git_worktree_path): # pylint: disable=unused-variable
             for file in fnmatch.filter(files, "*.so"):
                 version.modules[os.path.splitext(file)[0]] = (
                     os.path.join(root, file)
@@ -204,11 +207,10 @@ class AbiChecker(object):
         if make_process.returncode != 0:
             raise Exception("make failed, aborting")
 
-    def _get_abi_dumps_from_shared_libraries(self, git_worktree_path,
-                                             version):
+    def _get_abi_dumps_from_shared_libraries(self, version):
         """Generate the ABI dumps for the specified git revision.
-        It must be checked out in git_worktree_path and the shared libraries
-        must have been built."""
+        The shared libraries must have been built and the module paths
+        present in version.modules."""
         for mbed_module, module_path in version.modules.items():
             output_path = os.path.join(
                 self.report_dir, "{}-{}-{}.dump".format(
@@ -251,7 +253,7 @@ class AbiChecker(object):
         git_worktree_path = self._get_clean_worktree_for_git_revision(version)
         self._update_git_submodules(git_worktree_path, version)
         self._build_shared_libraries(git_worktree_path, version)
-        self._get_abi_dumps_from_shared_libraries(git_worktree_path, version)
+        self._get_abi_dumps_from_shared_libraries(version)
         self._cleanup_worktree(git_worktree_path)
 
     def _remove_children_with_tag(self, parent, tag):
@@ -264,7 +266,7 @@ class AbiChecker(object):
 
     def _remove_extra_detail_from_report(self, report_root):
         for tag in ['test_info', 'test_results', 'problem_summary',
-                'added_symbols', 'removed_symbols', 'affected']:
+                    'added_symbols', 'removed_symbols', 'affected']:
             self._remove_children_with_tag(report_root, tag)
 
         for report in report_root:
@@ -274,8 +276,8 @@ class AbiChecker(object):
 
     def get_abi_compatibility_report(self):
         """Generate a report of the differences between the reference ABI
-        and the new ABI. ABI dumps from self.old_rev and self.new_rev must
-        be available."""
+        and the new ABI. ABI dumps from self.old_version and self.new_version
+        must be available."""
         compatibility_report = ""
         compliance_return_code = 0
         shared_modules = list(set(self.old_version.modules.keys()) &
@@ -416,10 +418,14 @@ def run_main():
             help="output only the list of issues to stdout, instead of a full report",
         )
         abi_args = parser.parse_args()
-        old_version = RepoVersion("old", abi_args.old_repo, abi_args.old_rev,
-                 abi_args.old_crypto_repo, abi_args.old_crypto_rev)
-        new_version = RepoVersion("new", abi_args.new_repo, abi_args.new_rev,
-                 abi_args.new_crypto_repo, abi_args.new_crypto_rev)
+        old_version = RepoVersion(
+            "old", abi_args.old_repo, abi_args.old_rev,
+            abi_args.old_crypto_repo, abi_args.old_crypto_rev
+        )
+        new_version = RepoVersion(
+            "new", abi_args.new_repo, abi_args.new_rev,
+            abi_args.new_crypto_repo, abi_args.new_crypto_rev
+        )
         abi_check = AbiChecker(
             abi_args.verbose, old_version, new_version, abi_args.report_dir,
             abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file

From 30dc6d5bb9364dec520763fe6528bb13a5eb3066 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Tue, 9 Apr 2019 09:14:17 +0100
Subject: [PATCH 20/24] Use namespaces instead of full classes

---
 scripts/abi_check.py | 69 ++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 38 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 9f99309af..5a92a7d40 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -24,36 +24,15 @@ import argparse
 import logging
 import tempfile
 import fnmatch
+from types import SimpleNamespace
 
 import xml.etree.ElementTree as ET
 
 
-class RepoVersion(object):
-
-    def __init__(self, version, repository, revision,
-                 crypto_repository, crypto_revision):
-        """Class containing details for a particular revision.
-
-        version: either 'old' or 'new'
-        repository: repository for git revision
-        revision: git revision for comparison
-        crypto_repository: repository for git revision of crypto submodule
-        crypto_revision: git revision of crypto submodule
-        """
-        self.version = version
-        self.repository = repository
-        self.revision = revision
-        self.crypto_repository = crypto_repository
-        self.crypto_revision = crypto_revision
-        self.abi_dumps = {}
-        self.modules = {}
-
-
 class AbiChecker(object):
     """API and ABI checker."""
 
-    def __init__(self, verbose, old_version, new_version, report_dir,
-                 keep_all_reports, brief, skip_file=None):
+    def __init__(self, old_version, new_version, configuration):
         """Instantiate the API/ABI checker.
 
         old_version: RepoVersion containing details to compare against
@@ -65,16 +44,16 @@ class AbiChecker(object):
         """
         self.repo_path = "."
         self.log = None
-        self.verbose = verbose
+        self.verbose = configuration.verbose
         self._setup_logger()
-        self.report_dir = os.path.abspath(report_dir)
-        self.keep_all_reports = keep_all_reports
+        self.report_dir = os.path.abspath(configuration.report_dir)
+        self.keep_all_reports = configuration.keep_all_reports
         self.can_remove_report_dir = not (os.path.isdir(self.report_dir) or
-                                          keep_all_reports)
+                                          self.keep_all_reports)
         self.old_version = old_version
         self.new_version = new_version
-        self.skip_file = skip_file
-        self.brief = brief
+        self.skip_file = configuration.skip_file
+        self.brief = configuration.brief
         self.git_command = "git"
         self.make_command = "make"
 
@@ -418,18 +397,32 @@ def run_main():
             help="output only the list of issues to stdout, instead of a full report",
         )
         abi_args = parser.parse_args()
-        old_version = RepoVersion(
-            "old", abi_args.old_repo, abi_args.old_rev,
-            abi_args.old_crypto_repo, abi_args.old_crypto_rev
+        old_version = SimpleNamespace(
+            version="old",
+            repository=abi_args.old_repo,
+            revision=abi_args.old_rev,
+            crypto_repository=abi_args.old_crypto_repo,
+            crypto_revision=abi_args.old_crypto_rev,
+            abi_dumps={},
+            modules={}
         )
-        new_version = RepoVersion(
-            "new", abi_args.new_repo, abi_args.new_rev,
-            abi_args.new_crypto_repo, abi_args.new_crypto_rev
+        new_version = SimpleNamespace(
+            version="new",
+            repository=abi_args.new_repo,
+            revision=abi_args.new_rev,
+            crypto_repository=abi_args.new_crypto_repo,
+            crypto_revision=abi_args.new_crypto_rev,
+            abi_dumps={},
+            modules={}
         )
-        abi_check = AbiChecker(
-            abi_args.verbose, old_version, new_version, abi_args.report_dir,
-            abi_args.keep_all_reports, abi_args.brief, abi_args.skip_file
+        configuration = SimpleNamespace(
+            verbose=abi_args.verbose,
+            report_dir=abi_args.report_dir,
+            keep_all_reports=abi_args.keep_all_reports,
+            brief=abi_args.brief,
+            skip_file=abi_args.skip_file
         )
+        abi_check = AbiChecker(old_version, new_version, configuration)
         return_code = abi_check.check_for_abi_changes()
         sys.exit(return_code)
     except Exception: # pylint: disable=broad-except

From 6b3cbfa370c4025af3b349e843e4d47db8f78477 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Thu, 11 Apr 2019 15:50:41 +0100
Subject: [PATCH 21/24] Check that the report directory is a directory

---
 scripts/abi_check.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 5a92a7d40..e6253e99b 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -48,7 +48,7 @@ class AbiChecker(object):
         self._setup_logger()
         self.report_dir = os.path.abspath(configuration.report_dir)
         self.keep_all_reports = configuration.keep_all_reports
-        self.can_remove_report_dir = not (os.path.isdir(self.report_dir) or
+        self.can_remove_report_dir = not (os.path.exists(self.report_dir) or
                                           self.keep_all_reports)
         self.old_version = old_version
         self.new_version = new_version
@@ -397,6 +397,9 @@ def run_main():
             help="output only the list of issues to stdout, instead of a full report",
         )
         abi_args = parser.parse_args()
+        if os.path.isfile(abi_args.report_dir):
+            print("Error: {} is not a directory".format(abi_args.report_dir))
+            parser.exit()
         old_version = SimpleNamespace(
             version="old",
             repository=abi_args.old_repo,

From 3c9e7d23b1f62785566872276110ace98931715f Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 12 Apr 2019 15:17:02 +0100
Subject: [PATCH 22/24] Correct documentation

---
 scripts/abi_check.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index e6253e99b..0bbb9f872 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -37,10 +37,10 @@ class AbiChecker(object):
 
         old_version: RepoVersion containing details to compare against
         new_version: RepoVersion containing details to check
-        report_dir: directory for output files
-        keep_all_reports: if false, delete old reports
-        brief: if true, output shorter report to stdout
-        skip_file: path to file containing symbols and types to skip
+        configuration.report_dir: directory for output files
+        configuration.keep_all_reports: if false, delete old reports
+        configuration.brief: if true, output shorter report to stdout
+        configuration.skip_file: path to file containing symbols and types to skip
         """
         self.repo_path = "."
         self.log = None

From 03c5e856e87508891e9afa5b165fff31428b40e7 Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 12 Apr 2019 15:18:02 +0100
Subject: [PATCH 23/24] Start unused variable with underscore

---
 scripts/abi_check.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index 0bbb9f872..af293cd2a 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -178,7 +178,7 @@ class AbiChecker(object):
         )
         make_output, _ = make_process.communicate()
         self.log.debug(make_output.decode("utf-8"))
-        for root, dirs, files in os.walk(git_worktree_path): # pylint: disable=unused-variable
+        for root, _dirs, files in os.walk(git_worktree_path):
             for file in fnmatch.filter(files, "*.so"):
                 version.modules[os.path.splitext(file)[0]] = (
                     os.path.join(root, file)

From a0415bc779905987607a0cf3908fa7971d07dc4d Mon Sep 17 00:00:00 2001
From: Darryl Green <darryl.green@arm.com>
Date: Fri, 12 Apr 2019 16:24:25 +0100
Subject: [PATCH 24/24] Use check_output instead of Popen

---
 scripts/abi_check.py | 101 ++++++++++++++-----------------------------
 1 file changed, 33 insertions(+), 68 deletions(-)

diff --git a/scripts/abi_check.py b/scripts/abi_check.py
index af293cd2a..f837f7a79 100755
--- a/scripts/abi_check.py
+++ b/scripts/abi_check.py
@@ -88,80 +88,60 @@ class AbiChecker(object):
                     version.revision, version.repository
                 )
             )
-            fetch_process = subprocess.Popen(
+            fetch_output = subprocess.check_output(
                 [self.git_command, "fetch",
                  version.repository, version.revision],
                 cwd=self.repo_path,
-                stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
             )
-            fetch_output, _ = fetch_process.communicate()
             self.log.debug(fetch_output.decode("utf-8"))
-            if fetch_process.returncode != 0:
-                raise Exception("Fetching revision failed, aborting")
             worktree_rev = "FETCH_HEAD"
         else:
             self.log.debug("Checking out git worktree for revision {}".format(
                 version.revision
             ))
             worktree_rev = version.revision
-        worktree_process = subprocess.Popen(
+        worktree_output = subprocess.check_output(
             [self.git_command, "worktree", "add", "--detach",
              git_worktree_path, worktree_rev],
             cwd=self.repo_path,
-            stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT
         )
-        worktree_output, _ = worktree_process.communicate()
         self.log.debug(worktree_output.decode("utf-8"))
-        if worktree_process.returncode != 0:
-            raise Exception("Checking out worktree failed, aborting")
         return git_worktree_path
 
     def _update_git_submodules(self, git_worktree_path, version):
         """If the crypto submodule is present, initialize it.
         if version.crypto_revision exists, update it to that revision,
         otherwise update it to the default revision"""
-        process = subprocess.Popen(
+        update_output = subprocess.check_output(
             [self.git_command, "submodule", "update", "--init", '--recursive'],
             cwd=git_worktree_path,
-            stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT
         )
-        output, _ = process.communicate()
-        self.log.debug(output.decode("utf-8"))
-        if process.returncode != 0:
-            raise Exception("git submodule update failed, aborting")
+        self.log.debug(update_output.decode("utf-8"))
         if not (os.path.exists(os.path.join(git_worktree_path, "crypto"))
                 and version.crypto_revision):
             return
 
         if version.crypto_repository:
-            fetch_process = subprocess.Popen(
+            fetch_output = subprocess.check_output(
                 [self.git_command, "fetch", version.crypto_repository,
                  version.crypto_revision],
                 cwd=os.path.join(git_worktree_path, "crypto"),
-                stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
             )
-            fetch_output, _ = fetch_process.communicate()
             self.log.debug(fetch_output.decode("utf-8"))
-            if fetch_process.returncode != 0:
-                raise Exception("git fetch failed, aborting")
             crypto_rev = "FETCH_HEAD"
         else:
             crypto_rev = version.crypto_revision
 
-        checkout_process = subprocess.Popen(
+        checkout_output = subprocess.check_output(
             [self.git_command, "checkout", crypto_rev],
             cwd=os.path.join(git_worktree_path, "crypto"),
-            stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT
         )
-        checkout_output, _ = checkout_process.communicate()
         self.log.debug(checkout_output.decode("utf-8"))
-        if checkout_process.returncode != 0:
-            raise Exception("git checkout failed, aborting")
 
     def _build_shared_libraries(self, git_worktree_path, version):
         """Build the shared libraries in the specified worktree."""
@@ -169,22 +149,18 @@ class AbiChecker(object):
         my_environment["CFLAGS"] = "-g -Og"
         my_environment["SHARED"] = "1"
         my_environment["USE_CRYPTO_SUBMODULE"] = "1"
-        make_process = subprocess.Popen(
+        make_output = subprocess.check_output(
             [self.make_command, "lib"],
             env=my_environment,
             cwd=git_worktree_path,
-            stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT
         )
-        make_output, _ = make_process.communicate()
         self.log.debug(make_output.decode("utf-8"))
         for root, _dirs, files in os.walk(git_worktree_path):
             for file in fnmatch.filter(files, "*.so"):
                 version.modules[os.path.splitext(file)[0]] = (
                     os.path.join(root, file)
                 )
-        if make_process.returncode != 0:
-            raise Exception("make failed, aborting")
 
     def _get_abi_dumps_from_shared_libraries(self, version):
         """Generate the ABI dumps for the specified git revision.
@@ -202,30 +178,22 @@ class AbiChecker(object):
                 "-o", output_path,
                 "-lver", version.revision
             ]
-            abi_dump_process = subprocess.Popen(
+            abi_dump_output = subprocess.check_output(
                 abi_dump_command,
-                stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
             )
-            abi_dump_output, _ = abi_dump_process.communicate()
             self.log.debug(abi_dump_output.decode("utf-8"))
-            if abi_dump_process.returncode != 0:
-                raise Exception("abi-dumper failed, aborting")
             version.abi_dumps[mbed_module] = output_path
 
     def _cleanup_worktree(self, git_worktree_path):
         """Remove the specified git worktree."""
         shutil.rmtree(git_worktree_path)
-        worktree_process = subprocess.Popen(
+        worktree_output = subprocess.check_output(
             [self.git_command, "worktree", "prune"],
             cwd=self.repo_path,
-            stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT
         )
-        worktree_output, _ = worktree_process.communicate()
         self.log.debug(worktree_output.decode("utf-8"))
-        if worktree_process.returncode != 0:
-            raise Exception("Worktree cleanup failed, aborting")
 
     def _get_abi_dump_for_ref(self, version):
         """Generate the ABI dumps for the specified git revision."""
@@ -282,38 +250,35 @@ class AbiChecker(object):
             if self.brief:
                 abi_compliance_command += ["-report-format", "xml",
                                            "-stdout"]
-            abi_compliance_process = subprocess.Popen(
-                abi_compliance_command,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT
-            )
-            abi_compliance_output, _ = abi_compliance_process.communicate()
-            if abi_compliance_process.returncode == 0:
+            try:
+                subprocess.check_output(
+                    abi_compliance_command,
+                    stderr=subprocess.STDOUT
+                )
+            except subprocess.CalledProcessError as err:
+                if err.returncode == 1:
+                    compliance_return_code = 1
+                    if self.brief:
+                        self.log.info(
+                            "Compatibility issues found for {}".format(mbed_module)
+                        )
+                        report_root = ET.fromstring(err.output.decode("utf-8"))
+                        self._remove_extra_detail_from_report(report_root)
+                        self.log.info(ET.tostring(report_root).decode("utf-8"))
+                    else:
+                        self.can_remove_report_dir = False
+                        compatibility_report += (
+                            "Compatibility issues found for {}, "
+                            "for details see {}\n".format(mbed_module, output_path)
+                        )
+                else:
+                    raise err
+            else:
                 compatibility_report += (
                     "No compatibility issues for {}\n".format(mbed_module)
                 )
                 if not (self.keep_all_reports or self.brief):
                     os.remove(output_path)
-            elif abi_compliance_process.returncode == 1:
-                if self.brief:
-                    self.log.info(
-                        "Compatibility issues found for {}".format(mbed_module)
-                    )
-                    report_root = ET.fromstring(abi_compliance_output.decode("utf-8"))
-                    self._remove_extra_detail_from_report(report_root)
-                    self.log.info(ET.tostring(report_root).decode("utf-8"))
-                else:
-                    compliance_return_code = 1
-                    self.can_remove_report_dir = False
-                    compatibility_report += (
-                        "Compatibility issues found for {}, "
-                        "for details see {}\n".format(mbed_module, output_path)
-                    )
-            else:
-                raise Exception(
-                    "abi-compliance-checker failed with a return code of {},"
-                    " aborting".format(abi_compliance_process.returncode)
-                )
             os.remove(self.old_version.abi_dumps[mbed_module])
             os.remove(self.new_version.abi_dumps[mbed_module])
         if self.can_remove_report_dir: