llvm_tools: Fix lint warnings in llvm update tools

This CL fixes some glint errors that were causing repo upload to reqiure
a no-verify flag, it also includes yapf formatting.

BUG=None
TEST=Reran unit tests in affected files

Change-Id: Ifa9827f204d2b7f1973a901722e0f5390ef850aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2912511
Commit-Queue: Ryan Beltran <ryanbeltran@chromium.org>
Tested-by: Ryan Beltran <ryanbeltran@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>

4 files changed, 87 insertions, 79 deletions

diff --git a/llvm_tools/get_llvm_hash.py b/llvm_tools/get_llvm_hash.py
index e6d36edf..5f0ef58b 100755
--- a/llvm_tools/get_llvm_hash.py
+++ b/llvm_tools/get_llvm_hash.py
@@ -9,6 +9,7 @@
 from __future__ import print_function
 
 import argparse
+import contextlib
 import functools
 import os
 import re
@@ -16,11 +17,10 @@ import shutil
 import subprocess
 import sys
 import tempfile
-from contextlib import contextmanager
 
 import git_llvm_rev
-from subprocess_helpers import CheckCommand
 from subprocess_helpers import check_output
+from subprocess_helpers import CheckCommand
 
 _LLVM_GIT_URL = ('https://chromium.googlesource.com/external/github.com/llvm'
                  '/llvm-project')
@@ -124,7 +124,7 @@ def GetLLVMMajorVersion(git_hash=None):
       CheckoutBranch(src_dir, git_llvm_rev.MAIN_BRANCH)
 
 
-@contextmanager
+@contextlib.contextmanager
 def CreateTempLLVMRepo(temp_dir):
   """Adds a LLVM worktree to 'temp_dir'.
 
@@ -138,7 +138,7 @@ def CreateTempLLVMRepo(temp_dir):
     temp_dir: An absolute path to the temporary directory to put the worktree in
     (obtained via 'tempfile.mkdtemp()').
 
-  Returns:
+  Yields:
     The absolute path to 'temp_dir'.
 
   Raises:
@@ -175,6 +175,9 @@ def GetAndUpdateLLVMProjectInLLVMTools():
   LLVM mirror. In either case, this function will return the absolute path to
   'llvm-project-copy' directory.
 
+  Returns:
+    Absolute path to 'llvm-project-copy' directory in 'llvm_tools'
+
   Raises:
     ValueError: LLVM repo (in 'llvm-project-copy' dir.) has changes or failed to
     checkout to main or failed to fetch from chromium mirror of LLVM.
@@ -212,6 +215,9 @@ def GetAndUpdateLLVMProjectInLLVMTools():
 def GetGoogle3LLVMVersion(stable):
   """Gets the latest google3 LLVM version.
 
+  Args:
+    stable: boolean, use the stable version or the unstable version
+
   Returns:
     The latest LLVM SVN version as an integer.
 
@@ -236,7 +242,7 @@ def GetGoogle3LLVMVersion(stable):
   return GetVersionFrom(GetAndUpdateLLVMProjectInLLVMTools(), git_hash.rstrip())
 
 
-def is_svn_option(svn_option):
+def IsSvnOption(svn_option):
   """Validates whether the argument (string) is a git hash option.
 
   The argument is used to find the git hash of LLVM.
@@ -244,6 +250,10 @@ def is_svn_option(svn_option):
   Args:
     svn_option: The option passed in as a command line argument.
 
+  Returns:
+    lowercase svn_option if it is a known hash source, otherwise the svn_option
+    as an int
+
   Raises:
     ValueError: Invalid svn option provided.
   """
@@ -270,7 +280,7 @@ def GetLLVMHashAndVersionFromSVNOption(svn_option):
 
   Args:
     svn_option: A valid svn option obtained from the command line.
-      Ex: 'google3', 'tot', or <svn_version> such as 365123.
+      Ex. 'google3', 'tot', or <svn_version> such as 365123.
 
   Returns:
     A tuple that is the LLVM git hash and LLVM version.
@@ -298,7 +308,7 @@ class LLVMHash(object):
   """Provides methods to retrieve a LLVM hash."""
 
   @staticmethod
-  @contextmanager
+  @contextlib.contextmanager
   def CreateTempDirectory():
     temp_dir = tempfile.mkdtemp()
 
@@ -368,7 +378,7 @@ def main():
   parser = argparse.ArgumentParser(description='Finds the LLVM hash.')
   parser.add_argument(
       '--llvm_version',
-      type=is_svn_option,
+      type=IsSvnOption,
       required=True,
       help='which git hash of LLVM to find. Either a svn revision, or one '
       'of %s' % sorted(KNOWN_HASH_SOURCES))
diff --git a/llvm_tools/update_chromeos_llvm_hash.py b/llvm_tools/update_chromeos_llvm_hash.py
index a5227cc9..8d8030b9 100755
--- a/llvm_tools/update_chromeos_llvm_hash.py
+++ b/llvm_tools/update_chromeos_llvm_hash.py
@@ -4,32 +4,30 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Updates the LLVM hash and uprevs the build of the specified
-
-packages.
+"""Updates the LLVM hash and uprevs the build of the specified packages.
 
 For each package, a temporary repo is created and the changes are uploaded
 for review.
 """
 
 from __future__ import print_function
-from datetime import datetime
-from enum import Enum
 
 import argparse
+import datetime
+import enum
 import os
 import re
 import subprocess
 
-from failure_modes import FailureModes
 import chroot
+import failure_modes
 import get_llvm_hash
 import git
 import llvm_patch_management
 
 
 # Specify which LLVM hash to update
-class LLVMVariant(Enum):
+class LLVMVariant(enum.Enum):
   """Represent the LLVM hash in an ebuild file to update."""
 
   current = 'LLVM_HASH'
@@ -71,8 +69,8 @@ def GetCommandLineArgs():
       default=['sys-devel/llvm'],
       required=False,
       nargs='+',
-      help='the ebuilds to update their hash for llvm-next ' \
-          '(default: %(default)s)')
+      help='the ebuilds to update their hash for llvm-next '
+      '(default: %(default)s)')
 
   # Add argument for whether to display command contents to `stdout`.
   parser.add_argument(
@@ -91,7 +89,7 @@ def GetCommandLineArgs():
   # Add argument for the LLVM version to use.
   parser.add_argument(
       '--llvm_version',
-      type=get_llvm_hash.is_svn_option,
+      type=get_llvm_hash.IsSvnOption,
       required=True,
       help='which git hash to use. Either a svn revision, or one '
       'of %s' % sorted(get_llvm_hash.KNOWN_HASH_SOURCES))
@@ -99,12 +97,15 @@ def GetCommandLineArgs():
   # Add argument for the mode of the patch management when handling patches.
   parser.add_argument(
       '--failure_mode',
-      default=FailureModes.FAIL.value,
-      choices=[FailureModes.FAIL.value, FailureModes.CONTINUE.value,
-               FailureModes.DISABLE_PATCHES.value,
-               FailureModes.REMOVE_PATCHES.value],
-      help='the mode of the patch manager when handling failed patches ' \
-          '(default: %(default)s)')
+      default=failure_modes.FailureModes.FAIL.value,
+      choices=[
+          failure_modes.FailureModes.FAIL.value,
+          failure_modes.FailureModes.CONTINUE.value,
+          failure_modes.FailureModes.DISABLE_PATCHES.value,
+          failure_modes.FailureModes.REMOVE_PATCHES.value
+      ],
+      help='the mode of the patch manager when handling failed patches '
+      '(default: %(default)s)')
 
   # Add argument for the patch metadata file.
   parser.add_argument(
@@ -216,6 +217,9 @@ def ReplaceLLVMHash(ebuild_lines, llvm_variant, git_hash, svn_version):
     llvm_variant: The LLVM hash to update.
     git_hash: The new git hash.
     svn_version: The SVN-style revision number of git_hash.
+
+  Yields:
+    lines of the modified ebuild file
   """
   is_updated = False
   llvm_regex = re.compile('^' + re.escape(llvm_variant.value) +
@@ -294,8 +298,8 @@ def UprevEbuildToVersion(symlink, svn_version, git_hash):
     llvm_major_version = get_llvm_hash.GetLLVMMajorVersion(git_hash)
     new_ebuild, is_changed = re.subn(
         r'(\d+)\.(\d+)_pre([0-9]+)_p([0-9]+)',
-        '%s.\\2_pre%s_p%s' %
-        (llvm_major_version, svn_version, datetime.today().strftime('%Y%m%d')),
+        '%s.\\2_pre%s_p%s' % (llvm_major_version, svn_version,
+                              datetime.datetime.today().strftime('%Y%m%d')),
         ebuild,
         count=1)
   # any other package
@@ -395,15 +399,18 @@ def StagePackagesPatchResultsForCommit(package_info_dict, commit_messages):
     package (key).
     commit_messages: The commit message that has the updated ebuilds and
     upreving information.
+
+  Returns:
+    commit_messages with new additions
   """
 
   # For each package, check if any patches for that package have
   # changed, if so, add which patches have changed to the commit
   # message.
   for package_name, patch_info_dict in package_info_dict.items():
-    if patch_info_dict['disabled_patches'] or \
-        patch_info_dict['removed_patches'] or \
-        patch_info_dict['modified_metadata']:
+    if (patch_info_dict['disabled_patches'] or
+        patch_info_dict['removed_patches'] or
+        patch_info_dict['modified_metadata']):
       cur_package_header = '\nFor the package %s:' % package_name
       commit_messages.append(cur_package_header)
 
@@ -434,8 +441,8 @@ def StagePackagesPatchResultsForCommit(package_info_dict, commit_messages):
   return commit_messages
 
 
-def UpdatePackages(packages, llvm_variant, git_hash, svn_version,
-                   chroot_path, patch_metadata_file, mode, git_hash_source,
+def UpdatePackages(packages, llvm_variant, git_hash, svn_version, chroot_path,
+                   patch_metadata_file, mode, git_hash_source,
                    extra_commit_msg):
   """Updates an LLVM hash and uprevs the ebuild of the packages.
 
@@ -452,9 +459,9 @@ def UpdatePackages(packages, llvm_variant, git_hash, svn_version,
     the patches and its metadata.
     mode: The mode of the patch manager when handling an applicable patch
     that failed to apply.
-      Ex: 'FailureModes.FAIL'
+      Ex. 'FailureModes.FAIL'
     git_hash_source: The source of which git hash to use based off of.
-      Ex: 'google3', 'tot', or <version> such as 365123
+      Ex. 'google3', 'tot', or <version> such as 365123
     extra_commit_msg: extra test to append to the commit message.
 
   Returns:
@@ -589,7 +596,7 @@ def main():
       svn_version,
       args_output.chroot_path,
       args_output.patch_metadata_file,
-      FailureModes(args_output.failure_mode),
+      failure_modes.FailureModes(args_output.failure_mode),
       git_hash_source,
       extra_commit_msg=None)
 
diff --git a/llvm_tools/update_chromeos_llvm_hash_unittest.py b/llvm_tools/update_chromeos_llvm_hash_unittest.py
index 2e928be9..744a1aae 100755
--- a/llvm_tools/update_chromeos_llvm_hash_unittest.py
+++ b/llvm_tools/update_chromeos_llvm_hash_unittest.py
@@ -8,8 +8,8 @@
 
 from __future__ import print_function
 
-from collections import namedtuple
-from datetime import datetime
+import collections
+import datetime
 import os
 import re
 import subprocess
@@ -72,8 +72,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
                                                        llvm_variant, git_hash,
                                                        svn_version)
 
-        self.assertEqual(
-            str(err.exception), ('Failed to update %s.', 'LLVM_HASH'))
+      self.assertEqual(str(err.exception), 'Failed to update LLVM_HASH')
 
       llvm_variant = update_chromeos_llvm_hash.LLVMVariant.next
 
@@ -101,8 +100,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
                                                        llvm_variant, git_hash,
                                                        svn_version)
 
-        self.assertEqual(
-            str(err.exception), ('Failed to update %s.', 'LLVM_NEXT_HASH'))
+      self.assertEqual(str(err.exception), 'Failed to update LLVM_NEXT_HASH')
 
     self.assertEqual(mock_isfile.call_count, 2)
 
@@ -280,7 +278,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
 
     # Verify commands
     symlink_dir = os.path.dirname(symlink)
-    timestamp = datetime.today().strftime('%Y%m%d')
+    timestamp = datetime.datetime.today().strftime('%Y%m%d')
     new_ebuild = '/abs/path/to/llvm/llvm-1234.0_pre1000_p%s.ebuild' % timestamp
     new_symlink = new_ebuild[:-len('.ebuild')] + '-r1.ebuild'
 
@@ -322,7 +320,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
 
     # Verify commands
     symlink_dir = os.path.dirname(symlink)
-    new_ebuild, _is_changed = re.subn(
+    new_ebuild, _ = re.subn(
         r'pre([0-9]+)', 'pre%s' % svn_version, ebuild, count=1)
     new_symlink = new_ebuild[:-len('.ebuild')] + '-r1.ebuild'
 
@@ -370,7 +368,8 @@ class UpdateLLVMHashTest(unittest.TestCase):
 
     # Test function to simulate 'ConvertChrootPathsToAbsolutePaths' when a
     # symlink does not start with the prefix '/mnt/host/source'.
-    def BadPrefixChrootPath(_chroot_path, _chroot_file_paths):
+    def BadPrefixChrootPath(*args):
+      assert len(args) == 2
       raise ValueError('Invalid prefix for the chroot path: '
                        '%s' % package_chroot_path)
 
@@ -478,8 +477,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
 
   @mock.patch.object(os.path, 'isfile', return_value=True)
   @mock.patch.object(subprocess, 'check_output', return_value=None)
-  def testSuccessfullyStagedPatchMetadataFileForCommit(self, mock_run_cmd,
-                                                       _mock_isfile):
+  def testSuccessfullyStagedPatchMetadataFileForCommit(self, mock_run_cmd, _):
 
     patch_metadata_path = '/abs/path/to/filesdir/PATCHES.json'
 
@@ -583,19 +581,17 @@ class UpdateLLVMHashTest(unittest.TestCase):
       mock_uprev_symlink, mock_update_llvm_next, mock_create_repo,
       mock_create_path_dict, mock_llvm_major_version):
 
-    abs_path_to_package = '/some/path/to/chroot/src/path/to/package.ebuild'
-
-    symlink_path_to_package = \
-        '/some/path/to/chroot/src/path/to/package-r1.ebuild'
-
     path_to_package_dir = '/some/path/to/chroot/src/path/to'
+    abs_path_to_package = os.path.join(path_to_package_dir, 'package.ebuild')
+    symlink_path_to_package = os.path.join(path_to_package_dir,
+                                           'package-r1.ebuild')
 
     mock_llvm_major_version.return_value = '1234'
+
     # Test function to simulate 'CreateBranch' when successfully created the
     # branch on a valid repo path.
-    def SuccessfullyCreateBranchForChanges(_repo_path, branch):
+    def SuccessfullyCreateBranchForChanges(_, branch):
       self.assertEqual(branch, 'update-LLVM_NEXT_HASH-a123testhash4')
-      return
 
     # Test function to simulate 'UpdateEbuildLLVMHash' when successfully
     # updated the ebuild's 'LLVM_NEXT_HASH'.
@@ -603,23 +599,23 @@ class UpdateLLVMHashTest(unittest.TestCase):
       self.assertEqual(ebuild_path, abs_path_to_package)
       self.assertEqual(git_hash, 'a123testhash4')
       self.assertEqual(svn_version, 1000)
-      return
 
     # Test function to simulate 'UprevEbuildSymlink' when the symlink to the
     # ebuild does not have a revision number.
-    def FailedToUprevEbuildSymlink(_symlink_path):
+    def FailedToUprevEbuildSymlink(_):
       # Raises a 'ValueError' exception because the symlink did not have have a
       # revision number.
       raise ValueError('Failed to uprev the ebuild.')
 
     # Test function to fail on 'UploadChanges' if the function gets called
     # when an exception is raised.
-    def ShouldNotExecuteUploadChanges(_repo_path, _git_hash, _commit_messages):
+    def ShouldNotExecuteUploadChanges(*args):
       # Test function should not be called (i.e. execution should resume in the
       # 'finally' block) because 'UprevEbuildSymlink' raised an
       # exception.
-      assert False, 'Failed to go to "finally" block ' \
-          'after the exception was raised.'
+      assert len(args) == 3
+      assert False, ('Failed to go to "finally" block '
+                     'after the exception was raised.')
 
     test_package_path_dict = {symlink_path_to_package: abs_path_to_package}
 
@@ -649,10 +645,12 @@ class UpdateLLVMHashTest(unittest.TestCase):
     # Verify exception is raised when an exception is thrown within
     # the 'try' block by UprevEbuildSymlink function.
     with self.assertRaises(ValueError) as err:
-      update_chromeos_llvm_hash.UpdatePackages(
-          packages_to_update, llvm_variant, git_hash, svn_version,
-          chroot_path, patch_metadata_file, failure_modes.FailureModes.FAIL,
-          git_hash_source, extra_commit_msg)
+      update_chromeos_llvm_hash.UpdatePackages(packages_to_update, llvm_variant,
+                                               git_hash, svn_version,
+                                               chroot_path, patch_metadata_file,
+                                               failure_modes.FailureModes.FAIL,
+                                               git_hash_source,
+                                               extra_commit_msg)
 
     self.assertEqual(str(err.exception), 'Failed to uprev the ebuild.')
 
@@ -690,18 +688,15 @@ class UpdateLLVMHashTest(unittest.TestCase):
                                       mock_create_repo, mock_create_path_dict,
                                       mock_llvm_version, mock_mask_contains):
 
-    abs_path_to_package = '/some/path/to/chroot/src/path/to/package.ebuild'
-
-    symlink_path_to_package = \
-        '/some/path/to/chroot/src/path/to/package-r1.ebuild'
-
     path_to_package_dir = '/some/path/to/chroot/src/path/to'
+    abs_path_to_package = os.path.join(path_to_package_dir, 'package.ebuild')
+    symlink_path_to_package = os.path.join(path_to_package_dir,
+                                           'package-r1.ebuild')
 
     # Test function to simulate 'CreateBranch' when successfully created the
     # branch for the changes to be made to the ebuild files.
-    def SuccessfullyCreateBranchForChanges(_repo_path, branch):
+    def SuccessfullyCreateBranchForChanges(_, branch):
       self.assertEqual(branch, 'update-LLVM_NEXT_HASH-a123testhash5')
-      return
 
     # Test function to simulate 'UploadChanges' after a successfull update of
     # 'LLVM_NEXT_HASH" of the ebuild file.
@@ -710,7 +705,6 @@ class UpdateLLVMHashTest(unittest.TestCase):
                        '/some/path/to/chroot/src/path/to/package.ebuild')
       self.assertEqual(git_hash, 'a123testhash5')
       self.assertEqual(svn_version, 1000)
-      return
 
     # Test function to simulate 'UprevEbuildSymlink' when successfully
     # incremented the revision number by 1.
@@ -718,8 +712,6 @@ class UpdateLLVMHashTest(unittest.TestCase):
       self.assertEqual(symlink_path,
                        '/some/path/to/chroot/src/path/to/package-r1.ebuild')
 
-      return
-
     # Test function to simulate 'UpdatePackagesPatchMetadataFile()' when the
     # patch results contains a disabled patch in 'disable_patches' mode.
     def RetrievedPatchResults(chroot_path, svn_version, patch_metadata_file,
@@ -731,7 +723,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
       self.assertListEqual(packages, ['path/to'])
       self.assertEqual(mode, failure_modes.FailureModes.DISABLE_PATCHES)
 
-      PatchInfo = namedtuple('PatchInfo', [
+      PatchInfo = collections.namedtuple('PatchInfo', [
           'applied_patches', 'failed_patches', 'non_applicable_patches',
           'disabled_patches', 'removed_patches', 'modified_metadata'
       ])
@@ -754,9 +746,9 @@ class UpdateLLVMHashTest(unittest.TestCase):
     # Test function to simulate 'UploadChanges()' when successfully created a
     # commit for the changes made to the packages and their patches and
     # retrieved the change list of the commit.
-    def SuccessfullyUploadedChanges(_repo_path, _branch, _commit_messages):
+    def SuccessfullyUploadedChanges(*args):
+      assert len(args) == 3
       commit_url = 'https://some_name/path/to/commit/+/12345'
-
       return git.CommitContents(url=commit_url, cl_number=12345)
 
     test_package_path_dict = {symlink_path_to_package: abs_path_to_package}
@@ -787,10 +779,9 @@ class UpdateLLVMHashTest(unittest.TestCase):
     extra_commit_msg = '\ncommit-message-end'
 
     change_list = update_chromeos_llvm_hash.UpdatePackages(
-        packages_to_update, llvm_variant, git_hash, svn_version,
-        chroot_path, patch_metadata_file,
-        failure_modes.FailureModes.DISABLE_PATCHES, git_hash_source,
-        extra_commit_msg)
+        packages_to_update, llvm_variant, git_hash, svn_version, chroot_path,
+        patch_metadata_file, failure_modes.FailureModes.DISABLE_PATCHES,
+        git_hash_source, extra_commit_msg)
 
     self.assertEqual(change_list.url,
                      'https://some_name/path/to/commit/+/12345')
diff --git a/llvm_tools/update_packages_and_run_tests.py b/llvm_tools/update_packages_and_run_tests.py
index 66819ebc..dd01253e 100755
--- a/llvm_tools/update_packages_and_run_tests.py
+++ b/llvm_tools/update_packages_and_run_tests.py
@@ -73,7 +73,7 @@ def GetCommandLineArgs():
   # Add argument for the LLVM version to use.
   parser.add_argument(
       '--llvm_version',
-      type=get_llvm_hash.is_svn_option,
+      type=get_llvm_hash.IsSvnOption,
       required=True,
       help='which git hash of LLVM to find '
       '{google3, ToT, <svn_version>} '