-
-
Notifications
You must be signed in to change notification settings - Fork 234
feat(launchpad): Add asset catalog files to zip without adding to folder #2667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 sha1_smol::Digest; | ||
use symbolic::common::ByteView; | ||
|
@@ -21,14 +16,13 @@ use crate::config::Config; | |
use crate::utils::args::ArgExt as _; | ||
use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL}; | ||
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; | ||
|
||
|
@@ -95,19 +89,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() | ||
|
@@ -187,9 +176,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())); | ||
} | ||
|
||
|
@@ -276,65 +265,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()) | ||
} | ||
|
||
fn upload_file( | ||
|
@@ -470,12 +407,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(()) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.