fix

Author: adincebic

tools/dossier_codesigningtool/dossier_codesigning_reader_test.py

@@ -74,9 +74,16 @@
     'embedded_bundle_manifests': [],
 }
 
-_IPA_WORKSPACE_PATH = 'test/starlark_tests/targets_under_test/ios/app.ipa'
-_IPA_W_WATCHOS_WORKSPACE_PATH = 'test/starlark_tests/targets_under_test/watchos/app_companion.ipa'
-_COMBINED_ZIP_W_WATCHOS_WORKSPACE_PATH = 'test/starlark_tests/targets_under_test/watchos/app_companion_dossier_with_bundle.zip'
+_IPA_WORKSPACE_PATH = (
+    'test/starlark_tests/targets_under_test/ios/ipa_with_app.ipa'
+)
+_IPA_W_WATCHOS_WORKSPACE_PATH = (
+    'test/starlark_tests/targets_under_test/watchos/ipa_app_companion.ipa'
+)
+_COMBINED_ZIP_W_WATCHOS_WORKSPACE_PATH = (
+    'test/starlark_tests/targets_under_test/watchos/'
+    'ipa_app_companion_dossier_with_bundle.zip'
+)
 
 _ADDITIONAL_SIGNING_KEYCHAIN = '/tmp/Library/Keychains/ios-dev-signing.keychain'
 

tools/imported_dynamic_framework_processor/imported_dynamic_framework_processor.py

@@ -34,9 +34,11 @@
 import os
 import re
 import shutil
+import stat
 import sys
 import textwrap
 import time
+import zipfile
 from typing import List, Optional
 
 from tools.bitcode_strip import bitcode_strip
@@ -129,6 +131,47 @@ def _update_modified_timestamps(framework_temp_path: str) -> None:
     os.utime(framework_temp_path, (timestamp, timestamp))
 
 
+def _create_framework_zip(framework_temp_path: str, output_zip: str) -> None:
+  """Creates a zip archive for a processed framework bundle.
+
+  The macOS `ditto` zip writer has varied in how it records symlinked
+  directories. Write entries directly so versioned framework symlinks stay
+  symlinks across host OS versions.
+  """
+  framework_parent = os.path.dirname(framework_temp_path)
+  fixed_date_time = (2000, 1, 1, 0, 0, 0)
+
+  with zipfile.ZipFile(output_zip, "w", allowZip64=True) as out_zip:
+    for root, dirs, files in os.walk(framework_temp_path, followlinks=False):
+      dirs.sort()
+      files.sort()
+      entries = dirs + files
+      for entry in entries:
+        path = os.path.join(root, entry)
+        zip_path = os.path.relpath(path, framework_parent)
+
+        if os.path.islink(path):
+          zip_info = zipfile.ZipInfo(zip_path, fixed_date_time)
+          zip_info.create_system = 3
+          zip_info.compress_type = zipfile.ZIP_STORED
+          zip_info.external_attr = (stat.S_IFLNK | 0o755) << 16
+          out_zip.writestr(zip_info, os.readlink(path).encode("utf-8"))
+        elif os.path.isdir(path):
+          zip_info = zipfile.ZipInfo(zip_path + "/", fixed_date_time)
+          zip_info.create_system = 3
+          zip_info.compress_type = zipfile.ZIP_STORED
+          zip_info.external_attr = (stat.S_IFDIR | 0o755) << 16 | 0x10
+          out_zip.writestr(zip_info, b"")
+        elif os.path.isfile(path):
+          mode = stat.S_IMODE(os.stat(path).st_mode)
+          zip_info = zipfile.ZipInfo(zip_path, fixed_date_time)
+          zip_info.create_system = 3
+          zip_info.compress_type = zipfile.ZIP_DEFLATED
+          zip_info.external_attr = (stat.S_IFREG | mode) << 16
+          with open(path, "rb") as file:
+            out_zip.writestr(zip_info, file.read())
+
+
 def _relpath_from_framework(framework_absolute_path):
   """Returns a relative path to the root of the framework bundle."""
   framework_dir = None
@@ -381,23 +424,9 @@ def main() -> None:
     if status_code:
       return status_code
 
-  # Update modified timestamps and create archive using ditto.
+  # Update modified timestamps and create archive.
   _update_modified_timestamps(args.temp_path)
-
-  # Previous implementation of creating the processed framework archive
-  # using shutil/zip already stripped the extended attributes of the bundle.
-  execute.execute_and_filter_output(
-      cmd_args=[
-          "/usr/bin/ditto",
-          "-c",
-          "-k",  # use PKZip format for bundletool compatibility.
-          "--keepParent",  # preserves the .framework directory.
-          "--norsrc",  # strip resource forks and HFS metadata.
-          "--noextattr",  # strip extended attributes.
-          args.temp_path,
-          args.output_zip
-      ],
-      raise_on_failure=True)
+  _create_framework_zip(args.temp_path, args.output_zip)
 
 
 if __name__ == "__main__":

tools/imported_dynamic_framework_processor/imported_dynamic_framework_processor_test.py

@@ -15,8 +15,10 @@
 
 import os
 import shutil
+import stat
 import tempfile
 import unittest
+import zipfile
 from unittest import mock
 
 from tools.imported_dynamic_framework_processor import imported_dynamic_framework_processor
@@ -41,6 +43,12 @@ def _scratch_file(self, path, content=""):
       fp.write(content)
     return full_path
 
+  def _scratch_symlink(self, path, target):
+    full_path = os.path.join(self._scratch_dir, path)
+    os.makedirs(os.path.dirname(full_path), exist_ok=True)
+    os.symlink(target, full_path)
+    return full_path
+
   @mock.patch.object(execute, "execute_and_filter_output")
   def test_get_install_path_for_binary(self, mock_execute):
     with self.assertRaisesRegex(
@@ -194,7 +202,7 @@ def test_strip_or_copy_binary_skips_lipo_with_matching_archs_bin(
       imported_dynamic_framework_processor.codesigningtool,
       "find_identity_and_sign_bundle_paths")
   @mock.patch.object(
-      imported_dynamic_framework_processor.execute, "execute_and_filter_output")
+      imported_dynamic_framework_processor, "_create_framework_zip")
   @mock.patch.object(
       imported_dynamic_framework_processor, "_update_modified_timestamps")
   @mock.patch.object(
@@ -206,7 +214,7 @@ def test_main_reconstructs_versioned_framework_and_signs_current_version(
       mock_parse_args,
       mock_try_get_framework_version_from_structure,
       mock_update_modified_timestamps,
-      mock_execute,
+      mock_create_framework_zip,
       mock_sign):
     framework_binary = self._scratch_file(
         "Foo.framework/Versions/A/Foo", "framework-binary")
@@ -270,7 +278,38 @@ def test_main_reconstructs_versioned_framework_and_signs_current_version(
     )
     mock_sign.assert_called_once()
     mock_update_modified_timestamps.assert_called_once_with(temp_path)
-    mock_execute.assert_called_once()
+    mock_create_framework_zip.assert_called_once_with(temp_path, output_zip)
+
+  def test_create_framework_zip_preserves_versioned_framework_symlinks(self):
+    temp_path = os.path.join(self._scratch_dir, "out", "Foo.framework")
+    output_zip = os.path.join(self._scratch_dir, "Foo.zip")
+    self._scratch_file("out/Foo.framework/Versions/A/Foo", "binary")
+    self._scratch_file(
+        "out/Foo.framework/Versions/A/Resources/Info.plist", "plist")
+    self._scratch_symlink("out/Foo.framework/Versions/Current", "A")
+    self._scratch_symlink(
+        "out/Foo.framework/Foo", "Versions/Current/Foo")
+    self._scratch_symlink(
+        "out/Foo.framework/Resources", "Versions/Current/Resources")
+
+    imported_dynamic_framework_processor._create_framework_zip(
+        temp_path, output_zip)
+
+    with zipfile.ZipFile(output_zip) as zip_file:
+      self.assertNotIn("Foo.framework/Resources/", zip_file.namelist())
+      self.assertEqual(
+          b"Versions/Current/Resources",
+          zip_file.read("Foo.framework/Resources"),
+      )
+      self.assertTrue(stat.S_ISLNK(
+          zip_file.getinfo("Foo.framework/Resources").external_attr >> 16))
+      self.assertEqual(
+          b"A",
+          zip_file.read("Foo.framework/Versions/Current"),
+      )
+      self.assertTrue(stat.S_ISLNK(
+          zip_file.getinfo(
+              "Foo.framework/Versions/Current").external_attr >> 16))
 
 if __name__ == "__main__":
   unittest.main()