-
-
Notifications
You must be signed in to change notification settings - Fork 235
feat(mobileapp): Update mobile app command for ipa uploads #2619
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
Changes from all commits
2ba5dff
46ebd0f
9586ada
6c078ce
b9cf3ed
0b088cb
e8cb4a3
5b8fd2e
e050988
a26b6ad
b2cef47
9daae6d
620bce8
cd04205
d681185
0eeaa67
8ff0a95
b8518fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,22 +21,28 @@ use crate::config::Config; | |
use crate::utils::args::ArgExt; | ||
use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL}; | ||
use crate::utils::fs::get_sha1_checksums; | ||
#[cfg(target_os = "macos")] | ||
use crate::utils::fs::TempDir; | ||
use crate::utils::fs::TempFile; | ||
#[cfg(target_os = "macos")] | ||
use crate::utils::mobile_app::handle_asset_catalogs; | ||
use crate::utils::mobile_app::{handle_asset_catalogs, ipa_to_xcarchive, is_ipa_file}; | ||
use crate::utils::mobile_app::{is_aab_file, is_apk_file, is_apple_app, is_zip_file}; | ||
use crate::utils::progress::ProgressBar; | ||
use crate::utils::vcs; | ||
|
||
pub fn make_command(command: Command) -> Command { | ||
#[cfg(target_os = "macos")] | ||
const HELP_TEXT: &str = "The path to the mobile app files to upload. Supported files include Apk, Aab, XCArchive, and IPA."; | ||
#[cfg(not(target_os = "macos"))] | ||
const HELP_TEXT: &str = "The path to the mobile app files to upload. Supported files include Apk, Aab, and XCArchive."; | ||
command | ||
.about("[EXPERIMENTAL] Upload mobile app files to a project.") | ||
.org_arg() | ||
.project_arg(false) | ||
.arg( | ||
Arg::new("paths") | ||
.value_name("PATH") | ||
.help("The path to the mobile app files to upload. Supported files include Apk, Aab or XCArchive.") | ||
.help(HELP_TEXT) | ||
.num_args(1..) | ||
.action(ArgAction::Append) | ||
.required(true), | ||
|
@@ -95,12 +101,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { | |
|
||
let normalized_zip = if path.is_file() { | ||
debug!("Normalizing file: {}", path.display()); | ||
normalize_file(path, &byteview).with_context(|| { | ||
format!( | ||
"Failed to generate uploadable bundle for file {}", | ||
path.display() | ||
) | ||
})? | ||
handle_file(path, &byteview)? | ||
} else if path.is_dir() { | ||
debug!("Normalizing directory: {}", path.display()); | ||
normalize_directory(path).with_context(|| { | ||
|
@@ -178,6 +179,25 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { | |
Ok(()) | ||
} | ||
|
||
fn handle_file(path: &Path, byteview: &ByteView) -> Result<TempFile> { | ||
// Handle IPA files by converting them to XCArchive | ||
#[cfg(target_os = "macos")] | ||
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)) | ||
.with_context(|| format!("Failed to process IPA file {}", path.display())); | ||
} | ||
|
||
normalize_file(path, byteview).with_context(|| { | ||
format!( | ||
"Failed to generate uploadable bundle for file {}", | ||
path.display() | ||
) | ||
}) | ||
} | ||
|
||
fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> { | ||
debug!("Validating mobile app format for: {}", path.display()); | ||
|
||
|
@@ -186,9 +206,13 @@ fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> { | |
return Ok(()); | ||
} | ||
|
||
// Check if the file is a zip file (then AAB or APK) | ||
// Check if the file is a zip file (then AAB, APK, or IPA) | ||
if is_zip_file(bytes) { | ||
#[cfg(target_os = "macos")] | ||
debug!("File is a zip, checking for AAB/APK/IPA format"); | ||
#[cfg(not(target_os = "macos"))] | ||
debug!("File is a zip, checking for AAB/APK format"); | ||
|
||
if is_aab_file(bytes)? { | ||
debug!("Detected AAB file"); | ||
return Ok(()); | ||
|
@@ -198,11 +222,22 @@ fn validate_is_mobile_app(path: &Path, bytes: &[u8]) -> Result<()> { | |
debug!("Detected APK file"); | ||
return Ok(()); | ||
} | ||
|
||
#[cfg(target_os = "macos")] | ||
if is_ipa_file(bytes)? { | ||
debug!("Detected IPA file"); | ||
return Ok(()); | ||
} | ||
} | ||
|
||
debug!("File format validation failed"); | ||
#[cfg(target_os = "macos")] | ||
let format_list = "APK, AAB, XCArchive, or IPA"; | ||
#[cfg(not(target_os = "macos"))] | ||
let format_list = "APK, AAB, or XCArchive"; | ||
Comment on lines
+234
to
+237
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. As we have to conditionally print this list in several places, it might eventually make sense to extract this into a constant. I don't think we have to do it now necessarily but could make the code more easily maintainable, especially if we add more file formats in the future |
||
|
||
Err(anyhow!( | ||
"File is not a recognized mobile app format (APK, AAB, or XCArchive): {}", | ||
"File is not a recognized mobile app format ({format_list}): {}", | ||
path.display() | ||
)) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,20 @@ pub fn is_aab_file(bytes: &[u8]) -> Result<bool> { | |
Ok(has_bundle_config && has_base_manifest) | ||
} | ||
|
||
#[cfg(target_os = "macos")] | ||
pub fn is_ipa_file(bytes: &[u8]) -> Result<bool> { | ||
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. Just wondering, why do we have this validation code in the first place? Why not just try to parse the file as all the different file types directly, then if we fail to, we assume that the file is invalid? Having the separate validation logic seems like it is leading to duplicated code paths. We already check this structure in Fixing this (especially for the other file types) is of course out of scope for this PR, but hoping you can shed some light on this architectural decision. 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. Discussed on a call, it's mostly because this was ported from our typescript code, we probably could refactor this and make it at least just file extension checking |
||
let cursor = std::io::Cursor::new(bytes); | ||
let archive = zip::ZipArchive::new(cursor)?; | ||
|
||
let is_ipa = archive.file_names().any(|name| { | ||
name.starts_with("Payload/") | ||
&& name.ends_with(".app/Info.plist") | ||
&& name.matches('/').count() == 2 | ||
}); | ||
Comment on lines
+45
to
+49
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. we could reuse the regex here, but this is also fine |
||
|
||
Ok(is_ipa) | ||
} | ||
|
||
pub fn is_xcarchive_directory<P>(path: P) -> bool | ||
where | ||
P: AsRef<Path>, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
``` | ||
$ sentry-cli mobile-app upload --help | ||
? success | ||
[EXPERIMENTAL] Upload mobile app files to a project. | ||
|
||
Usage: sentry-cli[EXE] mobile-app upload [OPTIONS] <PATH>... | ||
|
||
Arguments: | ||
<PATH>... The path to the mobile app files to upload. Supported files include Apk, Aab, | ||
XCArchive, and IPA. | ||
|
||
Options: | ||
-o, --org <ORG> | ||
The organization ID or slug. | ||
--header <KEY:VALUE> | ||
Custom headers that should be attached to all requests | ||
in key:value format. | ||
-p, --project <PROJECT> | ||
The project ID or slug. | ||
--auth-token <AUTH_TOKEN> | ||
Use the given Sentry auth token. | ||
--sha <sha> | ||
The git commit sha to use for the upload. If not provided, the current commit sha will be | ||
used. | ||
--build-configuration <build_configuration> | ||
The build configuration to use for the upload. If not provided, the current version will | ||
be used. | ||
--log-level <LOG_LEVEL> | ||
Set the log output verbosity. [possible values: trace, debug, info, warn, error] | ||
--quiet | ||
Do not print any output while preserving correct exit code. This flag is currently | ||
implemented only for selected subcommands. [aliases: silent] | ||
-h, --help | ||
Print help | ||
|
||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
``` | ||
$ sentry-cli mobile-app upload tests/integration/_fixtures/mobile_app/ipa.ipa --sha test_sha | ||
? success | ||
Successfully uploaded 1 file to Sentry | ||
- tests/integration/_fixtures/mobile_app/ipa.ipa | ||
|
||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
invalid ipa content |
Uh oh!
There was an error while loading. Please reload this page.