macOS: Clean up create_macos_framework.py (#54543)

Over time, this script and others in sky/tools have accumulated a lot of additional and sometimes duplicate code.

This is a first pass cleanup of create_macos_framework.py to extract common utility code to utils.py and refactor for better readability.

The iOS analogue of this patch was #54500.

This is a reland of #54543, which failed to set the `-y` option on zip when zipping `FlutterMacOS.xcframework` but did so for the other zip steps where required. Since there is no case where we *want* to archive dereferenced symlinks, and because it's required for macOS frameworks, we now set this option for all callers in the `sky_tools.create_zip()` function to avoid future cases where someone working on this code misses one. If we ever need it, we can extract a parameter. This patch also references the correct script in its description (the original incorrectly mentioned `create_ios_framework.py`).

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

sky/tools/create_macos_framework.py

@@ -5,22 +5,13 @@
 # found in the LICENSE file.
 
 import argparse
-import platform
 import subprocess
 import shutil
 import sys
 import os
 
 from create_xcframework import create_xcframework  # pylint: disable=import-error
-
-buildroot_dir = os.path.abspath(os.path.join(os.path.realpath(__file__), '..', '..', '..', '..'))
-
-ARCH_SUBPATH = 'mac-arm64' if platform.processor() == 'arm' else 'mac-x64'
-DSYMUTIL = os.path.join(
-    os.path.dirname(__file__), '..', '..', 'buildtools', ARCH_SUBPATH, 'clang', 'bin', 'dsymutil'
-)
-
-out_dir = os.path.join(buildroot_dir, 'out')
+import sky_utils  # pylint: disable=import-error
 
 
 def main():
@@ -33,19 +24,18 @@ def main():
   parser.add_argument('--x64-out-dir', type=str, required=True)
   parser.add_argument('--strip', action='store_true', default=False)
   parser.add_argument('--dsym', action='store_true', default=False)
-  # TODO(godofredoc): Remove after recipes v2 have landed.
   parser.add_argument('--zip', action='store_true', default=False)
 
   args = parser.parse_args()
 
-  dst = (args.dst if os.path.isabs(args.dst) else os.path.join(buildroot_dir, args.dst))
+  dst = args.dst if os.path.isabs(args.dst) else sky_utils.buildroot_relative_path(args.dst)
   arm64_out_dir = (
-      args.arm64_out_dir
-      if os.path.isabs(args.arm64_out_dir) else os.path.join(buildroot_dir, args.arm64_out_dir)
+      args.arm64_out_dir if os.path.isabs(args.arm64_out_dir) else
+      sky_utils.buildroot_relative_path(args.arm64_out_dir)
   )
   x64_out_dir = (
       args.x64_out_dir
-      if os.path.isabs(args.x64_out_dir) else os.path.join(buildroot_dir, args.x64_out_dir)
+      if os.path.isabs(args.x64_out_dir) else sky_utils.buildroot_relative_path(args.x64_out_dir)
   )
 
   fat_framework_bundle = os.path.join(dst, 'FlutterMacOS.framework')
@@ -71,21 +61,15 @@ def main():
     print('Cannot find macOS x64 dylib at %s' % x64_dylib)
     return 1
 
-  if not os.path.isfile(DSYMUTIL):
-    print('Cannot find dsymutil at %s' % DSYMUTIL)
-    return 1
-
-  shutil.rmtree(fat_framework_bundle, True)
-  shutil.copytree(arm64_framework, fat_framework_bundle, symlinks=True)
+  sky_utils.copy_tree(arm64_framework, fat_framework_bundle, symlinks=True)
 
   regenerate_symlinks(fat_framework_bundle)
 
   fat_framework_binary = os.path.join(fat_framework_bundle, 'Versions', 'A', 'FlutterMacOS')
 
   # Create the arm64/x64 fat framework.
-  subprocess.check_call([
-      'lipo', arm64_dylib, x64_dylib, '-create', '-output', fat_framework_binary
-  ])
+  sky_utils.lipo([arm64_dylib, x64_dylib], fat_framework_binary)
+
   # Make the framework readable and executable: u=rwx,go=rx.
   subprocess.check_call(['chmod', '755', fat_framework_bundle])
 
@@ -107,7 +91,8 @@ def main():
   xcframeworks = [fat_framework_bundle]
   create_xcframework(location=dst, name='FlutterMacOS', frameworks=xcframeworks)
 
-  zip_framework(dst, args)
+  if args.zip:
+    zip_framework(dst)
 
   return 0
 
@@ -143,108 +128,79 @@ def regenerate_symlinks(fat_framework_bundle):
   )
 
 
-def embed_codesign_configuration(config_path, content):
-  with open(config_path, 'w') as file:
-    file.write(content)
-
-
 def process_framework(dst, args, fat_framework_bundle, fat_framework_binary):
   if args.dsym:
     dsym_out = os.path.splitext(fat_framework_bundle)[0] + '.dSYM'
-    subprocess.check_call([DSYMUTIL, '-o', dsym_out, fat_framework_binary])
+    sky_utils.extract_dsym(fat_framework_binary, dsym_out)
     if args.zip:
       dsym_dst = os.path.join(dst, 'FlutterMacOS.dSYM')
-      subprocess.check_call(['zip', '-r', '-y', 'FlutterMacOS.dSYM.zip', '.'], cwd=dsym_dst)
+      sky_utils.create_zip(dsym_dst, 'FlutterMacOS.dSYM.zip', ['.'])
       # Double zip to make it consistent with legacy artifacts.
       # TODO(fujino): remove this once https://github.com/flutter/flutter/issues/125067 is resolved
-      subprocess.check_call([
-          'zip',
-          '-y',
-          'FlutterMacOS.dSYM_.zip',
-          'FlutterMacOS.dSYM.zip',
-      ],
-                            cwd=dsym_dst)
-      # Use doubled zipped file.
+      sky_utils.create_zip(dsym_dst, 'FlutterMacOS.dSYM_.zip', ['FlutterMacOS.dSYM.zip'])
+
+      # Overwrite the FlutterMacOS.dSYM.zip with the double-zipped archive.
       dsym_final_src_path = os.path.join(dsym_dst, 'FlutterMacOS.dSYM_.zip')
       dsym_final_dst_path = os.path.join(dst, 'FlutterMacOS.dSYM.zip')
       shutil.move(dsym_final_src_path, dsym_final_dst_path)
 
   if args.strip:
-    # copy unstripped
     unstripped_out = os.path.join(dst, 'FlutterMacOS.unstripped')
-    shutil.copyfile(fat_framework_binary, unstripped_out)
+    sky_utils.strip_binary(fat_framework_binary, unstripped_out)
+
+
+def zip_framework(dst):
+  framework_dst = os.path.join(dst, 'FlutterMacOS.framework')
+  sky_utils.write_codesign_config(os.path.join(framework_dst, 'entitlements.txt'), [])
+  sky_utils.write_codesign_config(
+      os.path.join(framework_dst, 'without_entitlements.txt'),
+      [
+          # TODO(cbracken): Remove the zip file from the path when outer zip is removed.
+          'FlutterMacOS.framework.zip/Versions/A/FlutterMacOS'
+      ]
+  )
+  sky_utils.create_zip(framework_dst, 'FlutterMacOS.framework.zip', ['.'])
+
+  # Double zip to make it consistent with legacy artifacts.
+  # TODO(fujino): remove this once https://github.com/flutter/flutter/issues/125067 is resolved
+  sky_utils.create_zip(
+      framework_dst,
+      'FlutterMacOS.framework_.zip',
+      [
+          'FlutterMacOS.framework.zip',
+          # TODO(cbracken): Move these files to inner zip before removing the outer zip.
+          'entitlements.txt',
+          'without_entitlements.txt',
+      ]
+  )
 
-    subprocess.check_call(['strip', '-x', '-S', fat_framework_binary])
+  # Overwrite the FlutterMacOS.framework.zip with the double-zipped archive.
+  final_src_path = os.path.join(framework_dst, 'FlutterMacOS.framework_.zip')
+  final_dst_path = os.path.join(dst, 'FlutterMacOS.framework.zip')
+  shutil.move(final_src_path, final_dst_path)
 
-
-def zip_framework(dst, args):
-  # Zip FlutterMacOS.framework.
-  if args.zip:
-    filepath_with_entitlements = ''
-
-    framework_dst = os.path.join(dst, 'FlutterMacOS.framework')
-    # TODO(xilaizhang): Remove the zip file from the path when outer zip is removed.
-    filepath_without_entitlements = 'FlutterMacOS.framework.zip/Versions/A/FlutterMacOS'
-
-    embed_codesign_configuration(
-        os.path.join(framework_dst, 'entitlements.txt'), filepath_with_entitlements
-    )
-
-    embed_codesign_configuration(
-        os.path.join(framework_dst, 'without_entitlements.txt'), filepath_without_entitlements
-    )
-    subprocess.check_call([
-        'zip',
-        '-r',
-        '-y',
-        'FlutterMacOS.framework.zip',
-        '.',
-    ],
-                          cwd=framework_dst)
-    # Double zip to make it consistent with legacy artifacts.
-    # TODO(fujino): remove this once https://github.com/flutter/flutter/issues/125067 is resolved
-    subprocess.check_call(
-        [
-            'zip',
-            '-y',
-            'FlutterMacOS.framework_.zip',
-            'FlutterMacOS.framework.zip',
-            # TODO(xilaizhang): Move these files to inner zip before removing the outer zip.
-            'entitlements.txt',
-            'without_entitlements.txt',
-        ],
-        cwd=framework_dst
-    )
-    # Use doubled zipped file.
-    final_src_path = os.path.join(framework_dst, 'FlutterMacOS.framework_.zip')
-    final_dst_path = os.path.join(dst, 'FlutterMacOS.framework.zip')
-    shutil.move(final_src_path, final_dst_path)
-
-    zip_xcframework_archive(dst)
+  zip_xcframework_archive(dst)
 
 
 def zip_xcframework_archive(dst):
-  filepath_with_entitlements = ''
-  filepath_without_entitlements = (
-      'FlutterMacOS.xcframework/macos-arm64_x86_64/'
-      'FlutterMacOS.framework/Versions/A/FlutterMacOS'
-  )
-  embed_codesign_configuration(os.path.join(dst, 'entitlements.txt'), filepath_with_entitlements)
+  sky_utils.write_codesign_config(os.path.join(dst, 'entitlements.txt'), [])
 
-  embed_codesign_configuration(
-      os.path.join(dst, 'without_entitlements.txt'), filepath_without_entitlements
+  sky_utils.write_codesign_config(
+      os.path.join(dst, 'without_entitlements.txt'), [
+          'FlutterMacOS.xcframework/macos-arm64_x86_64/'
+          'FlutterMacOS.framework/Versions/A/FlutterMacOS'
+      ]
   )
 
-  subprocess.check_call([
-      'zip',
-      '-r',
-      '-y',
+  sky_utils.create_zip(
+      dst,
       'framework.zip',
-      'FlutterMacOS.xcframework',
-      'entitlements.txt',
-      'without_entitlements.txt',
-  ],
-                        cwd=dst)
+      [
+          'FlutterMacOS.xcframework',
+          'entitlements.txt',
+          'without_entitlements.txt',
+      ],
+  )
 
 
 if __name__ == '__main__':

sky/tools/sky_utils.py

@@ -37,17 +37,21 @@ def copy_binary(source_path, destination_path):
   shutil.copy2(source_path, destination_path)
 
 
-def copy_tree(source_path, destination_path):
-  """Performs a recursive copy of a directory.
-  If the destination path is present, it is deleted first."""
+def copy_tree(source_path, destination_path, symlinks=False):
+  """Performs a recursive copy of a directory. If the destination path is
+  present, it is deleted first."""
   assert_directory(source_path, 'directory to copy')
   shutil.rmtree(destination_path, True)
-  shutil.copytree(source_path, destination_path)
+  shutil.copytree(source_path, destination_path, symlinks=symlinks)
 
 
 def create_zip(cwd, zip_filename, paths):
-  """Creates a zip archive in cwd, containing a set of cwd-relative files."""
-  subprocess.check_call(['zip', '-r', zip_filename] + paths, cwd=cwd)
+  """Creates a zip archive in cwd, containing a set of cwd-relative files.
+
+  In order to preserve the correct internal structure of macOS frameworks,
+  symlinks are preserved.
+  """
+  subprocess.check_call(['zip', '-r', '-y', zip_filename] + paths, cwd=cwd)
 
 
 def _dsymutil_path():
@@ -86,4 +90,5 @@ def strip_binary(binary_path, unstripped_copy_path):
 def write_codesign_config(output_path, paths):
   """Writes an Apple codesign configuration file containing the specified paths."""
   with open(output_path, mode='w', encoding='utf-8') as file:
-    file.write('\n'.join(paths) + '\n')
+    if paths:
+      file.write('\n'.join(paths) + '\n')