feat(launchpad): Add asset catalog files to zip without adding to folder (#2667)

This improves the app upload experience by making it so assets are
written directly to the zip file rather than to the input xcarchive
directory. Previously the assets were written to the input folder and
then all the files in the folder were zipped. This meant the original
location the user passed to the CLI had extra files in it after running
the command, which is confusing.

There was also a bug that assets were not handled for ipa uploads. IPA
files are already zipped, so when we wrote assets to the user's input
directory we only did that for xcarchives (Which is a directory not a
zip file). So a side effect of not writing images to the users input
directory was also to fix this issue for IPAs, and now IPA uploads have
assets included too.

Author: noahsmartin

apple-catalog-parsing/native/swift/AssetCatalogParser/Sources/AssetCatalogParser/AssetCatalogReader.swift

@@ -7,13 +7,14 @@ import ObjcSupport
 @_cdecl("swift_inspect_asset_catalog")
 // Insepects the asset catalog and writes the results to a JSON file
 // in the xcarchive containing the asset catalog.
-public func swift_inspect_asset_catalog(_ path: UnsafePointer<CChar>) {
+public func swift_inspect_asset_catalog(_ path: UnsafePointer<CChar>, outputPath: UnsafePointer<CChar>) {
     let pathString = String(cString: path)
+    let outputPathString = String(cString: outputPath)
     if #available(macOS 13.0, *) {
         let supportedVersions = [13, 14, 15]
         let version = ProcessInfo.processInfo.operatingSystemVersion
         if supportedVersions.contains(version.majorVersion) {
-            AssetUtil.disect(file: URL(filePath: pathString))
+            AssetUtil.disect(file: URL(filePath: pathString), outputURL: URL(filePath: outputPathString))
         } else {
             print("Skipping asset catalog inspection on unsupported macOS version \(version)")
         }
@@ -48,8 +49,8 @@ typealias objectiveCMethodImp = @convention(c) (AnyObject, Selector, UnsafeRawPo
 >?
 
 enum AssetUtil {
-    private static func createResultsPath(assetPath: URL) throws -> URL {
-        var archiveURL = assetPath
+    private static func createResultsPath(assetURL: URL, outputURL: URL) throws -> URL {
+        var archiveURL = assetURL
         var tailComponents: [String] = []
         while archiveURL.pathExtension != "xcarchive" && archiveURL.pathComponents.count > 1 {
             tailComponents.insert(archiveURL.lastPathComponent, at: 0)
@@ -58,19 +59,18 @@ enum AssetUtil {
         if archiveURL.pathExtension != "xcarchive" {
             throw Error.pathError
         }
-        let parsedRoot = archiveURL.appendingPathComponent("ParsedAssets",
-                                                           isDirectory: true)
+
         let destDir = tailComponents
             .dropLast()
-            .reduce(parsedRoot) { partial, next in
+            .reduce(outputURL) { partial, next in
                 partial.appendingPathComponent(next, isDirectory: true)
             }
         try! FileManager.default.createDirectory(at: destDir,
                                                  withIntermediateDirectories: true)
         return destDir
     }
 
-    @discardableResult static func disect(file: URL) -> [AssetCatalogEntry] {
+    @discardableResult static func disect(file: URL, outputURL: URL) -> [AssetCatalogEntry] {
         var assets: [AssetCatalogEntry] = []
         var colorLength: UInt = 0
         var colorCount = 0
@@ -154,7 +154,7 @@ enum AssetUtil {
         ))
 
         let data = try! JSONEncoder().encode(assets)
-        let folder = try! createResultsPath(assetPath: file)
+        let folder = try! createResultsPath(assetURL: file, outputURL: outputURL)
         let url = folder
             .appendingPathComponent("Assets")
             .appendingPathExtension("json")

apple-catalog-parsing/native/swift/AssetCatalogParser/Tests/AssetCatalogParserTests/AssetCatalogParserTests.swift

@@ -6,7 +6,7 @@ struct AssetCatalogParserTests {
   @Test func testParseAssets() throws {
     let archivePath = try #require(Bundle.module.path(forResource: "test", ofType: "xcarchive"))
     let url = URL(filePath: "\(archivePath)/Products/Applications/DemoApp.app/Assets.car")
-    let results = AssetUtil.disect(file: url)
+    let results = AssetUtil.disect(file: url, outputURL: URL(filePath: "\(archivePath)/ParsedAssets"))
     #expect(results.count == 2)
   }
 }

apple-catalog-parsing/src/asset_catalog.rs

@@ -11,7 +11,10 @@ pub enum Error {
 }
 
 extern "C" {
-    fn swift_inspect_asset_catalog(msg: *const std::os::raw::c_char);
+    fn swift_inspect_asset_catalog(
+        catalog_path: *const std::os::raw::c_char,
+        output_path: *const std::os::raw::c_char,
+    );
 }
 
 /// This calls out to Swift code that uses Apple APIs to convert the contents
@@ -20,17 +23,19 @@ extern "C" {
 /// as duplicate image detection, xray, and image optimization insights.
 /// The path should be in an xcarchive file, results are written
 /// to a JSON file in the xcarchive’s ParsedAssets directory.
-pub fn inspect_asset_catalog<P>(path: P) -> Result<(), Error>
+pub fn inspect_asset_catalog<P>(catalog_path: P, output_path: P) -> Result<(), Error>
 where
     P: AsRef<Path>,
 {
-    let c_string = CString::new(path.as_ref().as_os_str().as_bytes())?;
-    let string_ptr = c_string.as_ptr();
+    let catalog_c_string = CString::new(catalog_path.as_ref().as_os_str().as_bytes())?;
+    let output_path_c_string = CString::new(output_path.as_ref().as_os_str().as_bytes())?;
+    let catalog_string_ptr = catalog_c_string.as_ptr();
+    let output_string_ptr = output_path_c_string.as_ptr();
     unsafe {
         // The string pointed to is immutable, in Swift we cannot change it.
         // We ensure this by using "UnsafePointer<CChar>" in Swift which is
         // immutable (as opposed to "UnsafeMutablePointer<CChar>").
-        swift_inspect_asset_catalog(string_ptr);
+        swift_inspect_asset_catalog(catalog_string_ptr, output_string_ptr);
     }
     Ok(())
 }

src/commands/mobile_app/upload.rs

@@ -1,15 +1,10 @@
 use std::borrow::Cow;
-#[cfg(not(windows))]
-use std::fs;
 use std::io::Write as _;
-#[cfg(not(windows))]
-use std::os::unix::fs::PermissionsExt as _;
 use std::path::Path;
 
 use anyhow::{anyhow, bail, Context as _, Result};
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use indicatif::ProgressStyle;
-use itertools::Itertools as _;
 use log::{debug, info, warn};
 use symbolic::common::ByteView;
 use zip::write::SimpleFileOptions;
@@ -20,14 +15,13 @@ use crate::config::Config;
 use crate::utils::args::ArgExt as _;
 use crate::utils::chunks::{upload_chunks, Chunk};
 use crate::utils::fs::get_sha1_checksums;
-#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
 use crate::utils::fs::TempDir;
 use crate::utils::fs::TempFile;
 #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
 use crate::utils::mobile_app::{
     handle_asset_catalogs, ipa_to_xcarchive, is_apple_app, is_ipa_file,
 };
-use crate::utils::mobile_app::{is_aab_file, is_apk_file, is_zip_file};
+use crate::utils::mobile_app::{is_aab_file, is_apk_file, is_zip_file, normalize_directory};
 use crate::utils::progress::ProgressBar;
 use crate::utils::vcs;
 
@@ -94,19 +88,14 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
         let byteview = ByteView::open(path)?;
         debug!("Loaded file with {} bytes", byteview.len());
 
-        #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
-        if is_apple_app(path) {
-            handle_asset_catalogs(path);
-        }
-
         validate_is_mobile_app(path, &byteview)?;
 
         let normalized_zip = if path.is_file() {
             debug!("Normalizing file: {}", path.display());
             handle_file(path, &byteview)?
         } else if path.is_dir() {
             debug!("Normalizing directory: {}", path.display());
-            normalize_directory(path).with_context(|| {
+            handle_directory(path).with_context(|| {
                 format!(
                     "Failed to generate uploadable bundle for directory {}",
                     path.display()
@@ -196,9 +185,9 @@ fn handle_file(path: &Path, byteview: &ByteView) -> Result<TempFile> {
     #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
     if is_zip_file(byteview) && is_ipa_file(byteview)? {
         debug!("Converting IPA file to XCArchive structure");
-        let temp_dir = TempDir::create()?;
-        return ipa_to_xcarchive(path, byteview, &temp_dir)
-            .and_then(|path| normalize_directory(&path))
+        let archive_temp_dir = TempDir::create()?;
+        return ipa_to_xcarchive(path, byteview, &archive_temp_dir)
+            .and_then(|path| handle_directory(&path))
             .with_context(|| format!("Failed to process IPA file {}", path.display()));
     }
 
@@ -285,65 +274,13 @@ fn normalize_file(path: &Path, bytes: &[u8]) -> Result<TempFile> {
     Ok(temp_file)
 }
 
-// For XCArchive directories, we'll zip the entire directory
-fn normalize_directory(path: &Path) -> Result<TempFile> {
-    debug!("Creating normalized zip for directory: {}", path.display());
-
-    let temp_file = TempFile::create()?;
-    let mut zip = ZipWriter::new(temp_file.open()?);
-
-    let mut file_count = 0;
-
-    // Collect and sort entries for deterministic ordering
-    // This is important to ensure stable sha1 checksums for the zip file as
-    // an optimization is used to avoid re-uploading the same chunks if they're already on the server.
-    let entries = walkdir::WalkDir::new(path)
-        .follow_links(true)
-        .into_iter()
-        .filter_map(Result::ok)
-        .filter(|entry| entry.path().is_file())
-        .map(|entry| {
-            let entry_path = entry.into_path();
-            let relative_path = entry_path
-                .strip_prefix(path.parent().ok_or_else(|| {
-                    anyhow!(
-                        "Cannot determine parent directory for path: {}",
-                        path.display()
-                    )
-                })?)?
-                .to_owned();
-            Ok((entry_path, relative_path))
-        })
-        .collect::<Result<Vec<_>>>()?
-        .into_iter()
-        .sorted_by(|(_, a), (_, b)| a.cmp(b));
-
-    // Need to set the last modified time to a fixed value to ensure consistent checksums
-    // This is important as an optimization to avoid re-uploading the same chunks if they're already on the server
-    // but the last modified time being different will cause checksums to be different.
-    let options = SimpleFileOptions::default()
-        .compression_method(zip::CompressionMethod::Stored)
-        .last_modified_time(DateTime::default());
-
-    for (entry_path, relative_path) in entries {
-        debug!("Adding file to zip: {}", relative_path.display());
-
-        #[cfg(not(windows))]
-        // On Unix, we need to preserve the file permissions.
-        let options = options.unix_permissions(fs::metadata(&entry_path)?.permissions().mode());
-
-        zip.start_file(relative_path.to_string_lossy(), options)?;
-        let file_byteview = ByteView::open(&entry_path)?;
-        zip.write_all(file_byteview.as_slice())?;
-        file_count += 1;
+fn handle_directory(path: &Path) -> Result<TempFile> {
+    let temp_dir = TempDir::create()?;
+    #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
+    if is_apple_app(path) {
+        handle_asset_catalogs(path, temp_dir.path());
     }
-
-    zip.finish()?;
-    debug!(
-        "Successfully created normalized zip for directory with {} files",
-        file_count
-    );
-    Ok(temp_file)
+    normalize_directory(path, temp_dir.path())
 }
 
 /// Returns artifact id if upload was successful.
@@ -456,12 +393,76 @@ mod tests {
         fs::create_dir_all(test_dir.join("Products"))?;
         fs::write(test_dir.join("Products").join("app.txt"), "test content")?;
 
-        let result_zip = normalize_directory(&test_dir)?;
+        let result_zip = normalize_directory(&test_dir, temp_dir.path())?;
         let zip_file = fs::File::open(result_zip.path())?;
         let mut archive = ZipArchive::new(zip_file)?;
         let file = archive.by_index(0)?;
         let file_path = file.name();
         assert_eq!(file_path, "MyApp.xcarchive/Products/app.txt");
         Ok(())
     }
+
+    #[test]
+    #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
+    fn test_xcarchive_upload_includes_parsed_assets() -> Result<()> {
+        // Test that XCArchive uploads include parsed asset catalogs
+        let xcarchive_path = Path::new("tests/integration/_fixtures/mobile_app/archive.xcarchive");
+
+        // Process the XCArchive directory
+        let result = handle_directory(xcarchive_path)?;
+
+        // Verify the resulting zip contains parsed assets
+        let zip_file = fs::File::open(result.path())?;
+        let mut archive = ZipArchive::new(zip_file)?;
+
+        let mut has_parsed_assets = false;
+        for i in 0..archive.len() {
+            let file = archive.by_index(i)?;
+            let file_name = file
+                .enclosed_name()
+                .ok_or(anyhow!("Failed to get file name"))?;
+            if file_name.to_string_lossy().contains("ParsedAssets") {
+                has_parsed_assets = true;
+                break;
+            }
+        }
+
+        assert!(
+            has_parsed_assets,
+            "XCArchive upload should include parsed asset catalogs"
+        );
+        Ok(())
+    }
+
+    #[test]
+    #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
+    fn test_ipa_upload_includes_parsed_assets() -> Result<()> {
+        // Test that IPA uploads handle missing asset catalogs gracefully
+        let ipa_path = Path::new("tests/integration/_fixtures/mobile_app/ipa_with_asset.ipa");
+        let byteview = ByteView::open(ipa_path)?;
+
+        // Process the IPA file - this should work even without asset catalogs
+        let result = handle_file(ipa_path, &byteview)?;
+
+        let zip_file = fs::File::open(result.path())?;
+        let mut archive = ZipArchive::new(zip_file)?;
+
+        let mut has_parsed_assets = false;
+        for i in 0..archive.len() {
+            let file = archive.by_index(i)?;
+            let file_name = file
+                .enclosed_name()
+                .ok_or(anyhow!("Failed to get file name"))?;
+            if file_name.to_string_lossy().contains("ParsedAssets") {
+                has_parsed_assets = true;
+                break;
+            }
+        }
+
+        assert!(
+            has_parsed_assets,
+            "XCArchive upload should include parsed asset catalogs"
+        );
+        Ok(())
+    }
 }

src/utils/mobile_app/apple.rs

@@ -12,11 +12,13 @@ use std::io::Cursor;
 use walkdir::WalkDir;
 use zip::ZipArchive;
 
-pub fn handle_asset_catalogs(path: &Path) {
+pub fn handle_asset_catalogs(archive_path: &Path, output_path: &Path) {
     // Find all asset catalogs
-    let cars = find_car_files(path);
+    let cars = find_car_files(archive_path);
     for car in &cars {
-        if let Err(e) = apple_catalog_parsing::inspect_asset_catalog(car) {
+        if let Err(e) =
+            apple_catalog_parsing::inspect_asset_catalog(car, &output_path.to_path_buf())
+        {
             eprintln!("Failed to inspect asset catalog {}: {e}", car.display());
         }
     }

src/utils/mobile_app/mod.rs

@@ -2,10 +2,12 @@
 
 #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
 mod apple;
+mod normalize;
 mod validation;
 
 #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
 pub use self::apple::{handle_asset_catalogs, ipa_to_xcarchive};
+pub use self::normalize::normalize_directory;
 pub use self::validation::{is_aab_file, is_apk_file, is_zip_file};
 #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
 pub use self::validation::{is_apple_app, is_ipa_file};