llvm_tools: Switch to "git -F"

Use git commit -F to get a better formatted commit
message for the created CLs.

BUG=chromium:1067029
TEST=unit tests

Change-Id: I72918274d7b565013697201ee2872b8ad33e2648
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2133227
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: George Burgess <gbiv@chromium.org>

2 files changed, 44 insertions, 43 deletions

diff --git a/llvm_tools/update_chromeos_llvm_hash.py b/llvm_tools/update_chromeos_llvm_hash.py
index ce87db94..d29b605d 100755
--- a/llvm_tools/update_chromeos_llvm_hash.py
+++ b/llvm_tools/update_chromeos_llvm_hash.py
@@ -20,6 +20,7 @@ import argparse
 import os
 import re
 import subprocess
+import tempfile
 
 from assert_not_in_chroot import VerifyOutsideChroot
 from failure_modes import FailureModes
@@ -445,7 +446,7 @@ def UploadChanges(path_to_repo_dir, branch, commit_messages):
   Args:
     path_to_repo_dir: The absolute path to the repo where changes were made.
     branch: The name of the branch to upload.
-    commit_messages: A string of commit message(s) (i.e. '-m [message]'
+    commit_messages: A string of commit message(s) (i.e. '[message]'
     of the changes made.
 
   Returns:
@@ -460,13 +461,15 @@ def UploadChanges(path_to_repo_dir, branch, commit_messages):
   if not os.path.isdir(path_to_repo_dir):
     raise ValueError('Invalid directory path provided: %s' % path_to_repo_dir)
 
-  commit_cmd = [
-      'git',
-      'commit',
-  ]
-  commit_cmd.extend(commit_messages)
+  # Create a git commit.
+  with tempfile.NamedTemporaryFile(mode='w+t') as commit_msg_file:
+    commit_msg_file.write('\n'.join(commit_messages))
+    commit_msg_file.flush()
 
-  ExecCommandAndCaptureOutput(commit_cmd, cwd=path_to_repo_dir, verbose=verbose)
+    commit_cmd = ['git', 'commit', '-F', commit_msg_file.name]
+
+    ExecCommandAndCaptureOutput(
+        commit_cmd, cwd=path_to_repo_dir, verbose=verbose)
 
   # Upload the changes for review.
   upload_change_cmd = (
@@ -582,30 +585,30 @@ def StagePackagesPatchResultsForCommit(package_info_dict, commit_messages):
     if patch_info_dict['disabled_patches'] or \
         patch_info_dict['removed_patches'] or \
         patch_info_dict['modified_metadata']:
-      cur_package_header = 'For the package %s:' % package_name
-      commit_messages.append('-m %s' % cur_package_header)
+      cur_package_header = '\nFor the package %s:' % package_name
+      commit_messages.append(cur_package_header)
 
     # Add to the commit message that the patch metadata file was modified.
     if patch_info_dict['modified_metadata']:
       patch_metadata_path = patch_info_dict['modified_metadata']
-      commit_messages.append('-m %s' % 'The patch metadata file %s was '
-                             'modified' % os.path.basename(patch_metadata_path))
+      commit_messages.append('The patch metadata file %s was modified' %
+                             os.path.basename(patch_metadata_path))
 
       StagePatchMetadataFileForCommit(patch_metadata_path)
 
     # Add each disabled patch to the commit message.
     if patch_info_dict['disabled_patches']:
-      commit_messages.append('-m %s' % 'The following patches were disabled:')
+      commit_messages.append('The following patches were disabled:')
 
       for patch_path in patch_info_dict['disabled_patches']:
-        commit_messages.append('-m %s' % os.path.basename(patch_path))
+        commit_messages.append(os.path.basename(patch_path))
 
     # Add each removed patch to the commit message.
     if patch_info_dict['removed_patches']:
-      commit_messages.append('-m %s' % 'The following patches were removed:')
+      commit_messages.append('The following patches were removed:')
 
       for patch_path in patch_info_dict['removed_patches']:
-        commit_messages.append('-m %s' % os.path.basename(patch_path))
+        commit_messages.append(os.path.basename(patch_path))
 
       RemovePatchesFromFilesDir(patch_info_dict['removed_patches'])
 
@@ -662,9 +665,10 @@ def UpdatePackages(packages, llvm_variant, git_hash, svn_version, chroot_path,
       commit_message_header += (
           ': upgrade to %s (r%d)' % (git_hash, svn_version))
 
-    commit_messages = ['-m %s' % commit_message_header]
-
-    commit_messages.append('-m The following packages have been updated:')
+    commit_messages = [
+        commit_message_header + '\n',
+        'The following packages have been updated:',
+    ]
 
     # Holds the list of packages that are updating.
     packages = []
@@ -686,10 +690,7 @@ def UpdatePackages(packages, llvm_variant, git_hash, svn_version, chroot_path,
       parent_dir_name = os.path.basename(os.path.dirname(path_to_ebuild_dir))
 
       packages.append('%s/%s' % (parent_dir_name, cur_dir_name))
-
-      new_commit_message = '%s/%s' % (parent_dir_name, cur_dir_name)
-
-      commit_messages.append('-m %s' % new_commit_message)
+      commit_messages.append('%s/%s' % (parent_dir_name, cur_dir_name))
 
     # Handle the patches for each package.
     package_info_dict = llvm_patch_management.UpdatePackagesPatchMetadataFile(
diff --git a/llvm_tools/update_chromeos_llvm_hash_unittest.py b/llvm_tools/update_chromeos_llvm_hash_unittest.py
index 84717ac7..13cdedd1 100755
--- a/llvm_tools/update_chromeos_llvm_hash_unittest.py
+++ b/llvm_tools/update_chromeos_llvm_hash_unittest.py
@@ -420,7 +420,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
   def testFailedToUploadChangesForInvalidPathDirectory(self, mock_isdir):
     path_to_repo = '/some/path/to/repo'
     branch = 'update-LLVM_NEXT_HASH-a123testhash3'
-    commit_messages = ['-m Test message']
+    commit_messages = ['Test message']
 
     # Verify exception is raised when on an invalid repo path.
     with self.assertRaises(ValueError) as err:
@@ -457,7 +457,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
 
     path_to_repo = '/some/path/to/repo'
     branch = 'update-LLVM_NEXT_HASH-a123testhash3'
-    commit_messages = ['-m Test message']
+    commit_messages = ['Test message']
 
     # Verify exception is raised when failed to upload the changes for review.
     with self.assertRaises(ValueError) as err:
@@ -469,6 +469,11 @@ class UpdateLLVMHashTest(unittest.TestCase):
     mock_isdir.assert_called_once_with(path_to_repo)
 
     mock_command_output.assert_called_once()
+    mock_command_output_args = mock_command_output.call_args_list[0][0][0]
+    expected_mock_command_output_prefix = ['git', 'commit', '-F']
+    self.assertEqual(
+        mock_command_output_args[:len(expected_mock_command_output_prefix)],
+        expected_mock_command_output_prefix)
 
     mock_repo_upload.assert_called_once()
 
@@ -501,7 +506,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
 
     path_to_repo = '/some/path/to/repo'
     branch = 'update-LLVM_NEXT_HASH-a123testhash3'
-    commit_messages = ['-m Test message']
+    commit_messages = ['Test message']
 
     change_list = update_chromeos_llvm_hash.UploadChanges(
         path_to_repo, branch, commit_messages)
@@ -689,7 +694,7 @@ class UpdateLLVMHashTest(unittest.TestCase):
         'test-packages/package2': package_2_patch_info_dict
     }
 
-    test_commit_message = ['-m %s' % 'Updated packages']
+    test_commit_message = ['Updated packages']
 
     self.assertListEqual(
         update_chromeos_llvm_hash.StagePackagesPatchResultsForCommit(
@@ -724,18 +729,15 @@ class UpdateLLVMHashTest(unittest.TestCase):
         'test-packages/package2': package_2_patch_info_dict
     }
 
-    test_commit_message = ['-m %s' % 'Updated packages']
+    test_commit_message = ['Updated packages']
 
     expected_commit_messages = [
-        '-m %s' % 'Updated packages',
-        '-m %s' % 'For the package test-packages/package1:',
-        '-m %s' % 'The patch metadata file PATCHES.json was modified',
-        '-m %s' % 'The following patches were disabled:',
-        '-m %s' % 'fixes_output.patch',
-        '-m %s' % 'For the package test-packages/package2:',
-        '-m %s' % 'The patch metadata file PATCHES.json was modified',
-        '-m %s' % 'The following patches were removed:',
-        '-m %s' % 'redirect_stdout.patch'
+        'Updated packages', '\nFor the package test-packages/package1:',
+        'The patch metadata file PATCHES.json was modified',
+        'The following patches were disabled:', 'fixes_output.patch',
+        '\nFor the package test-packages/package2:',
+        'The patch metadata file PATCHES.json was modified',
+        'The following patches were removed:', 'redirect_stdout.patch'
     ]
 
     self.assertListEqual(
@@ -972,13 +974,11 @@ class UpdateLLVMHashTest(unittest.TestCase):
     mock_uprev_ebuild.assert_called_once_with(symlink_path_to_package)
 
     expected_commit_messages = [
-        '-m %s' % 'llvm-next/tot: upgrade to a123testhash5 (r1000)',
-        '-m %s' % 'The following packages have been updated:',
-        '-m %s' % 'path/to',
-        '-m %s' % 'For the package path/to:',
-        '-m %s' % 'The patch metadata file PATCHES.json was modified',
-        '-m %s' % 'The following patches were disabled:',
-        '-m %s' % 'fix_stdout.patch'
+        'llvm-next/tot: upgrade to a123testhash5 (r1000)\n',
+        'The following packages have been updated:', 'path/to',
+        '\nFor the package path/to:',
+        'The patch metadata file PATCHES.json was modified',
+        'The following patches were disabled:', 'fix_stdout.patch'
     ]
 
     mock_update_package_metadata_file.assert_called_once()