diff --git a/README.md b/README.md index ae02a61..4b5b1dd 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ The upload flow itself. ProGuard, Rust, ELF, dSYM, PDB, sourcemap, and Unity IL2 #### Android NDK — `--type elf` -Takes one or more paths: AGP's pre-built `native-debug-symbols.zip`, and/or directories, which are walked recursively for libraries (e.g. `build/intermediates/merged_native_libs/`) and read in place — nothing is re-zipped. Directory symlinks are not descended; an unreadable entry fails the run (exit 11) and a directory with no libraries exits 10. Every library found is uploaded as its own symbol, keyed by its GNU build-id — `--uuid` (the SDK's `BUILD_UUID`) is still required but only correlates logs. Built-in entry names are `.so`, `.so.dbg` (`ndk.debugSymbolLevel = 'FULL'`) and `.so.sym` (`'SYMBOL_TABLE'`, function names only — no `file:line`). A library without a build-id is warned about and skipped; two entries with the same build-id upload once (across all the given paths), preferring the one with DWARF. The server dedups on the build-id, so switching a library from `SYMBOL_TABLE` to `FULL` needs `--force` to replace the stored symbols. +Takes one or more paths: AGP's pre-built `native-debug-symbols.zip`, and/or directories, which are walked recursively for libraries (e.g. `build/intermediates/merged_native_libs/`) and read in place — nothing is re-zipped. Directory symlinks are not descended; an unreadable entry fails the run (exit 11) and a directory with no libraries exits 10. Every library found is uploaded as its own symbol, keyed by its GNU build-id — `--uuid` (the SDK's `BUILD_UUID`) is still required but only correlates logs. Built-in entry names are `.so`, `.so.dbg` (`ndk.debugSymbolLevel = 'FULL'`) and `.so.sym` (`'SYMBOL_TABLE'`, function names only — no `file:line`). A library without a build-id is warned about and skipped; two entries with the same build-id upload once (across all the given paths), preferring the one with DWARF. The server dedups on the build-id, and each library declares whether it carries debug info (`dwarf`) or only a symbol table (`symtab`), read from the file itself rather than its name. Switching a library from `SYMBOL_TABLE` to `FULL` therefore replaces the stored symbols on its own, transferring the bytes once (the run logs `upgraded SYMBOL_TABLE -> FULL`); re-running with the same file, or uploading a symbol table when the server holds debug info, transfers nothing. `--force` still means "always replace" and re-sends every library. A server that predates this behaviour skips the `FULL` upload as already present; the run then says so and `--force` is the only way to replace it. #### Unity IL2CPP — `--type il2cpp-linemap` diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index 068ca0c..510be28 100644 --- a/src/cli/debug_files.rs +++ b/src/cli/debug_files.rs @@ -102,11 +102,14 @@ pub enum DebugFilesCommand { /// `--type sourcemaps` — sets `overwrite` on the metadata POST so /// format-scoped peers are replaced. Proguard ignores this flag today. /// - /// For `--type elf` this is how richer symbols replace poorer ones: an - /// AGP `SYMBOL_TABLE` upload (`.so.sym`, function names only) and a later - /// `FULL` one (`.so.dbg`, with line info) of the same library share its - /// GNU build-id, so without `--force` the `FULL` upload is skipped as - /// already present. + /// For `--type elf` it is rarely needed: each library declares whether it + /// carries debug info or only a symbol table, and the server replaces a + /// stored symbol table with a later `FULL` one (`.so.dbg`, with line info) + /// of the same GNU build-id on its own — transferring it once, and nothing + /// when the server already holds the same or a richer copy (it never + /// downgrades). `--force` means "always replace", re-sending every + /// library, so use it only to replace a symbol a server that predates + /// that behaviour is still holding. #[arg(long)] force: bool, @@ -587,6 +590,8 @@ async fn run_proguard_upload( format: Some("mapping"), uuids: None, overwrite: None, + format_variant: None, + replace_if_richer: None, }; let client = client.as_ref().expect("client constructed when !dry_run"); let outcome = presigned::upload( @@ -821,6 +826,8 @@ async fn run_pdb_upload( format: Some("pdb"), uuids: None, overwrite: if force { Some(true) } else { None }, + format_variant: None, + replace_if_richer: None, }; let outcome = presigned::upload( client.as_ref().expect("client built when not dry-run"), @@ -1019,6 +1026,8 @@ async fn run_rust_elf_upload( format: Some("elf"), uuids: None, overwrite: if force { Some(true) } else { None }, + format_variant: None, + replace_if_richer: None, }; let outcome = presigned::upload( client.as_ref().expect("client built when not dry-run"), @@ -1182,6 +1191,7 @@ async fn run_elf_upload( // Skipped libraries that carry full debug info (`.so` / `.so.dbg`). The // server dedups on build-id alone, so one of these may have been skipped in // favour of an earlier `SYMBOL_TABLE` (`.so.sym`) upload of the same library. + let mut upgraded = 0u32; let mut full_already_existed = 0u32; let mut skipped_no_build_id = 0u32; @@ -1282,27 +1292,38 @@ async fn run_elf_upload( let client = client.as_ref().expect("client constructed when !dry_run"); // Run each `.so`'s pack → register → dedup-or-PUT pipeline concurrently. - let outcomes: Vec<(bool, anyhow::Result)> = - futures_util::stream::iter(uploadable.into_iter().enumerate()) - .map(|(i, lib)| { - let client = client.clone(); - let work = work_dir.path().to_path_buf(); - async move { - let symbol_table_only = lib.name.ends_with(".so.sym"); - let outcome = upload_one_so(&client, ctx, &lib, i, &work).await; - (symbol_table_only, outcome) - } - }) - .buffer_unordered(ELF_UPLOAD_CONCURRENCY) - .collect() - .await; + let outcomes: Vec<( + Option<&'static str>, + anyhow::Result, + )> = futures_util::stream::iter(uploadable.into_iter().enumerate()) + .map(|(i, lib)| { + let client = client.clone(); + let work = work_dir.path().to_path_buf(); + async move { + let variant = lib.format_variant(); + let detail = upload_one_so(&client, ctx, &lib, i, &work).await; + (variant, detail) + } + }) + .buffer_unordered(ELF_UPLOAD_CONCURRENCY) + .collect() + .await; - for (symbol_table_only, outcome) in outcomes { - match outcome? { - presigned::Outcome::Uploaded => uploaded += 1, + for (variant, detail) in outcomes { + let detail = detail?; + match detail.outcome { + presigned::Outcome::Uploaded => { + uploaded += 1; + if detail.upgraded { + upgraded += 1; + } + } presigned::Outcome::AlreadyExists => { already_existed += 1; - if !symbol_table_only { + // A full-debug-info library skipped WITHOUT the server saying what + // it holds: a server that predates `replace_if_richer` (a current + // one replaces a poorer copy, or reports the richer one it keeps). + if variant == Some("dwarf") && detail.stored.is_none() { full_already_existed += 1; } } @@ -1315,20 +1336,22 @@ async fn run_elf_upload( } else { tracing::info!( uploaded, + upgraded, already_existed, skipped_no_build_id, "native upload complete" ); - // Never overwrite automatically: that would re-upload every unchanged - // library on every build. The CLI cannot see what the server holds, so - // say how to replace it when that is what the caller wants. + // A current server replaces a poorer stored copy on its own, so this + // only fires against one that predates that: it skipped a library with + // debug info without saying what it holds, and `--force` is then the + // only way to replace it (it re-sends every library, so it is not the default). if full_already_existed > 0 && !force { tracing::info!( libraries = full_already_existed, "full-debug-info libraries were skipped as already on the server; if \ they were previously uploaded with ndk.debugSymbolLevel = \ - 'SYMBOL_TABLE' (.so.sym, no line info), re-run with --force to \ - replace them" + 'SYMBOL_TABLE' (.so.sym, no line info), this server cannot upgrade \ + them on its own — re-run with --force to replace them" ); } } @@ -1345,7 +1368,7 @@ async fn upload_one_so( lib: &elf::ElfLib, index: usize, work_dir: &Path, -) -> anyhow::Result { +) -> anyhow::Result { let build_id = lib .build_id .as_deref() @@ -1375,8 +1398,19 @@ async fn upload_one_so( format: Some("elf"), uuids: None, overwrite: if ctx.force { Some(true) } else { None }, + // Declare how rich this file is and let the server decide whether it + // beats what it holds: a `symtab` -> `dwarf` upgrade transfers once, an + // unchanged or poorer library transfers nothing. `--force` (overwrite) + // still means "always replace". + format_variant: lib.format_variant(), + // Sent for EVERY library, not only `dwarf` ones. The server never lets a + // `symtab` replace anything (a stored copy with no variant is replaceable only + // by `dwarf`), so this cannot downgrade. It also opts a plain Android app out + // of the legacy find-or-update, which has no duplicate check: without it an + // unchanged symbol-table library would re-transfer on every build. + replace_if_richer: Some(true), }; - let outcome = presigned::upload( + let detail = presigned::upload_detailed( client, RetryPolicy::default(), ctx.endpoint, @@ -1385,15 +1419,33 @@ async fn upload_one_so( &zip_path, ) .await?; - match outcome { + match detail.outcome { + presigned::Outcome::Uploaded if detail.upgraded => { + tracing::info!( + lib = %lib.name, + build_id, + "uploaded (upgraded SYMBOL_TABLE -> FULL)" + ) + } presigned::Outcome::Uploaded => { tracing::info!(lib = %lib.name, build_id, "uploaded") } + // The server said what it keeps: it is the same as, or richer than, this file. + presigned::Outcome::AlreadyExists if detail.stored.is_some() => { + tracing::info!( + lib = %lib.name, + build_id, + stored = detail.stored.map(presigned::Variant::as_str), + "already on server at the same or better level, skipped" + ) + } + // It did not say (a server that predates this, or a copy with no variant), so + // all that is known is that the build-id is held. presigned::Outcome::AlreadyExists => { tracing::info!(lib = %lib.name, build_id, "already on server, skipped") } } - Ok(outcome) + Ok(detail) } /// Pack and upload a Unity IL2CPP LineNumberMappings bundle. @@ -1513,6 +1565,8 @@ async fn run_il2cpp_linemap_upload( format: Some("il2cpp-linemap"), uuids: Some(&uuids), overwrite: if force { Some(true) } else { None }, + format_variant: None, + replace_if_richer: None, }; let outcome = presigned::upload( client.as_ref().expect("client built when not dry-run"), @@ -1631,6 +1685,8 @@ async fn upload_one_sourcemap( format: Some("sourcemap"), uuids: None, overwrite: if ctx.force { Some(true) } else { None }, + format_variant: None, + replace_if_richer: None, }; let client = client.expect("client constructed when !dry_run"); let outcome = presigned::upload( @@ -2108,6 +2164,8 @@ pub(crate) async fn run_dsym_upload( format: Some("dsym"), uuids: Some(&slice_uuids), overwrite: if force { Some(true) } else { None }, + format_variant: None, + replace_if_richer: None, }; if dry_run { @@ -2132,7 +2190,7 @@ pub(crate) async fn run_dsym_upload( ) .await? { - presigned::Registration::AlreadyExists => { + presigned::Registration::AlreadyExists { .. } => { already_existed += 1; tracing::info!( bundle = bundle_name, @@ -2140,7 +2198,7 @@ pub(crate) async fn run_dsym_upload( ); continue; } - presigned::Registration::Proceed { presigned_url } => presigned_url, + presigned::Registration::Proceed { presigned_url, .. } => presigned_url, }; // The server wants it — only NOW pack the (possibly large) bundle. diff --git a/src/symbols/elf.rs b/src/symbols/elf.rs index dc20bfa..06ab364 100644 --- a/src/symbols/elf.rs +++ b/src/symbols/elf.rs @@ -74,6 +74,19 @@ pub struct ElfLib { richness: (bool, bool, u64), } +impl ElfLib { + /// What this library can symbolicate, as the server's `format_variant`: + /// `"dwarf"` (debug info), `"symtab"` (symbol table only), or `None` for a + /// library carrying neither — it has no richness to declare. + pub fn format_variant(&self) -> Option<&'static str> { + match self.richness { + (true, _, _) => Some("dwarf"), + (false, true, _) => Some("symtab"), + _ => None, + } + } +} + /// Extract every ELF `.so` from `archive_path` into `out_dir`, reading each /// library's GNU build-id. An entry counts as a library when its name ends in /// a built-in suffix (see [`is_native_lib_entry`]) or one of `extra`. The wire `code_id` is produced by the same @@ -782,6 +795,47 @@ mod tests { } } + #[test] + fn format_variant_reports_what_a_library_can_symbolicate() { + // Debug info wins whatever else the file carries; a symbol table alone is + // `symtab`; a library with neither has no richness to declare. + assert_eq!( + lib("a.so", None, (true, true, 1)).format_variant(), + Some("dwarf") + ); + assert_eq!( + lib("a.so", None, (true, false, 1)).format_variant(), + Some("dwarf") + ); + assert_eq!( + lib("a.so", None, (false, true, 1)).format_variant(), + Some("symtab") + ); + assert_eq!(lib("a.so", None, (false, false, 1)).format_variant(), None); + } + + /// The real fixtures, end to end through the identity reader: the unstripped + /// library is `dwarf`, the same library with its debug sections removed is + /// `symtab`, and both carry ONE build-id — which is exactly why the server needs + /// the variant to tell them apart. + #[test] + fn real_fixtures_classify_as_dwarf_and_symtab_with_one_build_id() { + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/elf"); + let scanned = scan_dir(&dir, &ExtraSuffixes::default()).unwrap(); + let variant_of = |name: &str| { + scanned + .iter() + .find(|l| l.name == name) + .unwrap_or_else(|| panic!("{name} not scanned")) + }; + let full = variant_of("libsymbol1.so"); + let table = variant_of("libsymbol1.symtab.so"); + assert_eq!(full.format_variant(), Some("dwarf")); + assert_eq!(table.format_variant(), Some("symtab")); + assert_eq!(full.build_id, table.build_id); + assert!(full.build_id.is_some()); + } + fn names(libs: &[ElfLib]) -> Vec<&str> { libs.iter().map(|l| l.name.as_str()).collect() } diff --git a/src/upload/presigned.rs b/src/upload/presigned.rs index 99711fb..1b62d89 100644 --- a/src/upload/presigned.rs +++ b/src/upload/presigned.rs @@ -71,6 +71,57 @@ pub struct Metadata<'a> { /// has these symbols (maps to the server's `overwrite`). #[serde(skip_serializing_if = "Option::is_none")] pub overwrite: Option, + + /// Native ELF only: what this file can symbolicate — `"symtab"` (symbol + /// table, function names) or `"dwarf"` (debug info, file:line). The server + /// stores it so a later upload of the same build-id can be compared with it. + /// Absent for a library that carries neither. + #[serde(skip_serializing_if = "Option::is_none")] + pub format_variant: Option<&'a str>, + + /// Native ELF only: replace the stored copy when this file is richer + /// (`symtab` -> `dwarf`), never otherwise. Unlike `overwrite` it does not + /// re-transfer a library the server already holds at the same or better level. + #[serde(skip_serializing_if = "Option::is_none")] + pub replace_if_richer: Option, +} + +/// What a stored native symbol can symbolicate (the server's `format_variant`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Variant { + /// Symbol table only (AGP `SYMBOL_TABLE`): function names, no file:line. + Symtab, + /// Full debug info. + Dwarf, +} + +impl Variant { + pub fn as_str(self) -> &'static str { + match self { + Variant::Symtab => "symtab", + Variant::Dwarf => "dwarf", + } + } + + fn parse(s: &str) -> Option { + match s { + "symtab" => Some(Variant::Symtab), + "dwarf" => Some(Variant::Dwarf), + _ => None, + } + } +} + +/// An upload attempt with the detail [`Outcome`] leaves out. Only the native ELF +/// flow reads it; every other flow uses [`upload`] and the plain `Outcome`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct UploadDetail { + pub outcome: Outcome, + /// Uploaded AND replaced a poorer stored copy (`replace_if_richer`). + pub upgraded: bool, + /// On `AlreadyExists`: what the server holds, when it says. `None` means it + /// did not say — an older server, or a stored copy that predates the field. + pub stored: Option, } /// Outcome of a successful upload attempt. @@ -89,6 +140,9 @@ struct MetadataResponse { code: Option, #[serde(default)] endpoint: Option, + /// Set when the upload replaces a poorer stored copy (`replace_if_richer`). + #[serde(default)] + upgraded: Option, #[serde(default)] error: Option, } @@ -103,6 +157,15 @@ struct ErrorPayload { /// non-integer here must not fail the parse of an otherwise readable error. #[serde(default)] code: Option, + /// On a duplicate, the server's `{format_variant}` for the stored copy. + #[serde(default)] + params: Option, +} + +#[derive(Debug, Deserialize)] +struct ErrorParams { + #[serde(default)] + format_variant: Option, } /// Whether the metadata response says the server already has this symbol. @@ -129,9 +192,14 @@ fn is_already_exists(parsed: &MetadataResponse) -> bool { #[derive(Debug)] pub enum Registration { /// Server already has these symbols (`DuplicateSymbolsFoundError`, 16004). - AlreadyExists, - /// Proceed: PUT the payload to this presigned URL. - Proceed { presigned_url: String }, + /// `stored` is what it holds, when it says. + AlreadyExists { stored: Option }, + /// Proceed: PUT the payload to this presigned URL. `upgraded` is set when the + /// upload replaces a poorer stored copy. + Proceed { + presigned_url: String, + upgraded: bool, + }, } /// Stage 1: POST the metadata and interpret the response. Lets the caller dedup @@ -191,7 +259,13 @@ pub async fn register( if is_already_exists(&parsed) { tracing::debug!("server reports SymbolAlreadyExists ({CODE_ALREADY_EXISTS})"); - return Ok(Registration::AlreadyExists); + let stored = parsed + .error + .as_ref() + .and_then(|e| e.params.as_ref()) + .and_then(|p| p.format_variant.as_deref()) + .and_then(Variant::parse); + return Ok(Registration::AlreadyExists { stored }); } let presigned = match parsed.endpoint.as_deref() { @@ -217,6 +291,7 @@ pub async fn register( Ok(Registration::Proceed { presigned_url: presigned, + upgraded: parsed.upgraded == Some(true), }) } @@ -261,11 +336,39 @@ pub async fn upload( metadata: &Metadata<'_>, payload: &Path, ) -> Result { + Ok( + upload_detailed(client, policy, endpoint, app_token, metadata, payload) + .await? + .outcome, + ) +} + +/// [`upload`], also reporting whether it replaced a poorer stored copy and what +/// the server said it already held. +pub async fn upload_detailed( + client: &reqwest::Client, + policy: RetryPolicy, + endpoint: &str, + app_token: &str, + metadata: &Metadata<'_>, + payload: &Path, +) -> Result { match register(client, policy, endpoint, app_token, metadata).await? { - Registration::AlreadyExists => Ok(Outcome::AlreadyExists), - Registration::Proceed { presigned_url } => { + Registration::AlreadyExists { stored } => Ok(UploadDetail { + outcome: Outcome::AlreadyExists, + upgraded: false, + stored, + }), + Registration::Proceed { + presigned_url, + upgraded, + } => { put_payload(client, policy, &presigned_url, payload).await?; - Ok(Outcome::Uploaded) + Ok(UploadDetail { + outcome: Outcome::Uploaded, + upgraded, + stored: None, + }) } } } @@ -288,6 +391,8 @@ mod tests { format: None, uuids: Some(&uuids), overwrite: Some(true), + format_variant: None, + replace_if_richer: None, }; let v = serde_json::to_value(&m).unwrap(); assert_eq!(v["uuids"], serde_json::json!(["aaaa", "bbbb"])); @@ -308,6 +413,8 @@ mod tests { format: None, uuids: None, overwrite: None, + format_variant: None, + replace_if_richer: None, }; let v2 = serde_json::to_value(&m2).unwrap(); assert!(v2.get("uuids").is_none()); @@ -326,6 +433,8 @@ mod tests { format: Some("il2cpp-linemap"), uuids: Some(&uuids), overwrite: Some(true), + format_variant: None, + replace_if_richer: None, }; let v = serde_json::to_value(&m).unwrap(); assert_eq!(v["format"], "il2cpp-linemap"); @@ -333,6 +442,174 @@ mod tests { assert_eq!(v["overwrite"], true); } + fn elf_metadata<'a>(variant: Option<&'a str>, overwrite: Option) -> Metadata<'a> { + Metadata { + uuid: Some("bca64abfec40dbb631bb8f1c37414472"), + version: "1", + build: "1", + hash: Some("h"), + transform: Some("breakpad"), + format: Some("elf"), + uuids: None, + overwrite, + format_variant: variant, + replace_if_richer: Some(true), + } + } + + #[test] + fn metadata_serializes_the_richness_pair_and_omits_it_when_unset() { + let v = serde_json::to_value(elf_metadata(Some("dwarf"), None)).unwrap(); + assert_eq!(v["format_variant"], "dwarf"); + assert_eq!(v["replace_if_richer"], true); + assert!(v.get("overwrite").is_none()); + + // A library with neither debug info nor a symbol table declares no variant. + let v = serde_json::to_value(elf_metadata(None, None)).unwrap(); + assert!(v.get("format_variant").is_none(), "{v}"); + assert_eq!(v["replace_if_richer"], true); + } + + /// Every other flow leaves both fields off the wire entirely. + #[test] + fn metadata_without_the_richness_pair_is_unchanged_on_the_wire() { + let mut m = elf_metadata(None, None); + m.replace_if_richer = None; + let v = serde_json::to_value(&m).unwrap(); + assert!(v.get("format_variant").is_none()); + assert!(v.get("replace_if_richer").is_none()); + } + + async fn post_replying(body: serde_json::Value) -> (MockServer, reqwest::Client) { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/apps/TKN/symbols")) + .respond_with(ResponseTemplate::new(200).set_body_json(body)) + .expect(1) + .mount(&server) + .await; + (server, http::build_client().unwrap()) + } + + #[tokio::test] + async fn register_reports_an_upgrade_when_the_server_says_so() { + let (server, client) = post_replying(serde_json::json!({ + "code": 0, "endpoint": "https://s3.example/put", "upgraded": true + })) + .await; + let reg = register( + &client, + RetryPolicy::none(), + &server.uri(), + "TKN", + &elf_metadata(Some("dwarf"), None), + ) + .await + .unwrap(); + assert!( + matches!(®, Registration::Proceed { upgraded: true, .. }), + "{reg:?}" + ); + } + + #[tokio::test] + async fn register_is_not_an_upgrade_when_the_server_does_not_say_so() { + for body in [ + serde_json::json!({"code": 0, "endpoint": "https://s3.example/put"}), + serde_json::json!({"code": 0, "endpoint": "https://s3.example/put", "upgraded": false}), + ] { + let (server, client) = post_replying(body).await; + let reg = register( + &client, + RetryPolicy::none(), + &server.uri(), + "TKN", + &elf_metadata(Some("dwarf"), None), + ) + .await + .unwrap(); + assert!( + matches!( + ®, + Registration::Proceed { + upgraded: false, + .. + } + ), + "{reg:?}" + ); + } + } + + #[tokio::test] + async fn register_reads_the_stored_variant_off_a_duplicate_reply() { + for (reported, expected) in [ + (Some("dwarf"), Some(Variant::Dwarf)), + (Some("symtab"), Some(Variant::Symtab)), + // A value this client does not know is "not said", never a guess. + (Some("future-kind"), None), + (None, None), + ] { + let mut error = + serde_json::json!({"type": "DuplicateSymbolsFoundError", "code": 16004}); + if let Some(v) = reported { + error["params"] = serde_json::json!({ "format_variant": v }); + } + let (server, client) = + post_replying(serde_json::json!({"ok": false, "error": error})).await; + let reg = register( + &client, + RetryPolicy::none(), + &server.uri(), + "TKN", + &elf_metadata(Some("symtab"), None), + ) + .await + .unwrap(); + assert!( + matches!(®, Registration::AlreadyExists { stored } if *stored == expected), + "reported {reported:?}: {reg:?}" + ); + } + } + + #[tokio::test] + async fn upload_detailed_carries_the_upgrade_and_the_stored_variant() { + let tmp = tempfile::tempdir().unwrap(); + let payload = tmp.path().join("p.zip"); + std::fs::write(&payload, b"zip").unwrap(); + + let server = MockServer::start().await; + let put_url = format!("{}/put", server.uri()); + Mock::given(method("POST")) + .and(path("/apps/TKN/symbols")) + .respond_with(ResponseTemplate::new(200).set_body_json( + serde_json::json!({"code": 0, "endpoint": put_url, "upgraded": true}), + )) + .mount(&server) + .await; + Mock::given(method("PUT")) + .and(path("/put")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + let client = http::build_client().unwrap(); + let detail = upload_detailed( + &client, + RetryPolicy::none(), + &server.uri(), + "TKN", + &elf_metadata(Some("dwarf"), None), + &payload, + ) + .await + .unwrap(); + assert_eq!(detail.outcome, Outcome::Uploaded); + assert!(detail.upgraded); + assert_eq!(detail.stored, None); + } + #[tokio::test] async fn register_sends_il2cpp_linemap_format_on_wire() { let server = MockServer::start().await; @@ -356,12 +633,14 @@ mod tests { format: Some("il2cpp-linemap"), uuids: Some(&uuids), overwrite: None, + format_variant: None, + replace_if_richer: None, }; let client = http::build_client().unwrap(); let reg = register(&client, RetryPolicy::none(), &server.uri(), "TKN", &m) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists)); + assert!(matches!(reg, Registration::AlreadyExists { .. })); let reqs = server.received_requests().await.unwrap(); let body: serde_json::Value = serde_json::from_slice(&reqs[0].body).unwrap(); @@ -392,12 +671,14 @@ mod tests { format: None, uuids: Some(&uuids), overwrite: None, + format_variant: None, + replace_if_richer: None, }; let client = http::build_client().unwrap(); let reg = register(&client, RetryPolicy::none(), &server.uri(), "TKN", &m) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists)); + assert!(matches!(reg, Registration::AlreadyExists { .. })); // The metadata POST carried the declared UUIDs (the dedup key). let reqs = server.received_requests().await.unwrap(); @@ -416,6 +697,8 @@ mod tests { format: Some("sourcemap"), uuids: None, overwrite: None, + format_variant: None, + replace_if_richer: None, } } @@ -455,7 +738,10 @@ mod tests { })) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists), "got {reg:?}"); + assert!( + matches!(reg, Registration::AlreadyExists { .. }), + "got {reg:?}" + ); } #[tokio::test] @@ -466,7 +752,10 @@ mod tests { })) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists), "got {reg:?}"); + assert!( + matches!(reg, Registration::AlreadyExists { .. }), + "got {reg:?}" + ); } #[tokio::test] @@ -477,7 +766,10 @@ mod tests { })) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists), "got {reg:?}"); + assert!( + matches!(reg, Registration::AlreadyExists { .. }), + "got {reg:?}" + ); } #[tokio::test] @@ -533,7 +825,10 @@ mod tests { })) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists), "got {reg:?}"); + assert!( + matches!(reg, Registration::AlreadyExists { .. }), + "got {reg:?}" + ); } /// A payload bigger than the 64 KiB stream chunk and not a multiple of it. @@ -705,7 +1000,10 @@ mod tests { let reg = register_against(serde_json::json!({ "code": 16004 })) .await .unwrap(); - assert!(matches!(reg, Registration::AlreadyExists), "got {reg:?}"); + assert!( + matches!(reg, Registration::AlreadyExists { .. }), + "got {reg:?}" + ); } #[tokio::test] @@ -728,13 +1026,15 @@ mod tests { format: None, uuids: None, overwrite: None, + format_variant: None, + replace_if_richer: None, }; let client = http::build_client().unwrap(); match register(&client, RetryPolicy::none(), &server.uri(), "TKN", &m) .await .unwrap() { - Registration::Proceed { presigned_url } => { + Registration::Proceed { presigned_url, .. } => { assert_eq!(presigned_url, "https://s3.example/put") } other => panic!("expected Proceed, got {other:?}"), diff --git a/tests/elf_upload.rs b/tests/elf_upload.rs index 62eeebd..a9919eb 100644 --- a/tests/elf_upload.rs +++ b/tests/elf_upload.rs @@ -175,6 +175,240 @@ async fn elf_force_overwrites_a_library_already_on_the_server() { assert_eq!(posts[1]["uuid"], FIXTURE_BUILD_ID); } +/// Zip one of the ELF fixtures under `entry_name` (the unstripped library, or the same +/// library with its debug sections removed, which shares its build-id). +fn pack_fixture_zip(dir: &Path, fixture: &str, entry_name: &str) -> PathBuf { + let elf = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/elf") + .join(fixture); + let zip_path = dir.join("native-debug-symbols.zip"); + let mut zw = zip::ZipWriter::new(std::fs::File::create(&zip_path).unwrap()); + zw.start_file(entry_name, zip::write::SimpleFileOptions::default()) + .unwrap(); + zw.write_all(&std::fs::read(elf).unwrap()).unwrap(); + zw.finish().unwrap(); + zip_path +} + +/// Run `debug-files upload --type elf` on `zip` against `endpoint`; returns stderr. +async fn run_elf_upload_of( + endpoint: String, + zip: PathBuf, + extra: &'static [&'static str], +) -> String { + tokio::task::spawn_blocking(move || { + let zip = zip.to_string_lossy().into_owned(); + let inputs = [zip]; + let out = common::cli() + .args(elf_args(&endpoint, &inputs)) + .args(extra) + .assert() + .success(); + String::from_utf8_lossy(&out.get_output().stderr).into_owned() + }) + .await + .unwrap() +} + +/// Drop ANSI colour sequences so a log line can be matched as plain text. +fn strip_ansi(s: &str) -> String { + let mut out = String::new(); + let mut chars = s.chars(); + while let Some(c) = chars.next() { + if c == '\u{1b}' { + for c in chars.by_ref() { + if c.is_ascii_alphabetic() { + break; + } + } + } else { + out.push(c); + } + } + out +} + +fn post_bodies(requests: &[wiremock::Request]) -> Vec { + requests + .iter() + .filter(|r| r.method.as_str() == "POST") + .map(|r| serde_json::from_slice(&r.body).unwrap()) + .collect() +} + +/// Every native upload declares how rich its file is and asks the server to replace a +/// poorer stored copy — no `--force`, so no `overwrite`. The declaration is read off the +/// FILE (debug info vs symbol table), never off its name. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn elf_upload_declares_its_richness_and_asks_for_replace_if_richer() { + for (fixture, entry, expected) in [ + ("libsymbol1.so", "arm64-v8a/libsymbol1.so", "dwarf"), + ( + "libsymbol1.symtab.so", + "arm64-v8a/libsymbol1.so.sym", + "symtab", + ), + // Name and content disagree: the content decides. + ("libsymbol1.symtab.so", "arm64-v8a/libsymbol1.so", "symtab"), + ("libsymbol1.so", "arm64-v8a/libsymbol1.so.sym", "dwarf"), + ] { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let zip = pack_fixture_zip(tmp.path(), fixture, entry); + let put_url = format!("{}/put/sym", server.uri()); + Mock::given(method("POST")) + .respond_with( + ResponseTemplate::new(200).set_body_json(json!({"code": 0, "endpoint": put_url})), + ) + .mount(&server) + .await; + Mock::given(method("PUT")) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + run_elf_upload_of(server.uri(), zip, &[]).await; + + let posts = post_bodies(&server.received_requests().await.unwrap()); + assert_eq!(posts.len(), 1, "{fixture} as {entry}"); + assert_eq!(posts[0]["format_variant"], expected, "{fixture} as {entry}"); + assert_eq!(posts[0]["replace_if_richer"], true, "{fixture} as {entry}"); + assert!(posts[0].get("overwrite").is_none(), "{}", posts[0]); + assert_eq!(posts[0]["uuid"], FIXTURE_BUILD_ID); + assert_eq!(posts[0]["transform"], "breakpad"); + } +} + +/// Acceptance: the server holds a symbol table for the build-id; the unstripped library +/// replaces it with NO `--force`, transfers its bytes once, and the run reports the upgrade. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn elf_upload_reports_an_upgrade_and_transfers_once() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let zip = pack_fixture_zip(tmp.path(), "libsymbol1.so", "arm64-v8a/libsymbol1.so"); + let put_url = format!("{}/put/sym", server.uri()); + Mock::given(method("POST")) + .and(body_string_contains(r#""format_variant":"dwarf""#)) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({"code": 0, "endpoint": put_url, "upgraded": true})), + ) + .expect(1) + .mount(&server) + .await; + Mock::given(method("PUT")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + let stderr = strip_ansi(&run_elf_upload_of(server.uri(), zip, &[]).await); + + assert!(stderr.contains("upgraded SYMBOL_TABLE -> FULL"), "{stderr}"); + assert!(stderr.contains("uploaded=1 upgraded=1"), "{stderr}"); + assert!(!stderr.contains("re-run with --force"), "{stderr}"); +} + +/// Acceptance: re-running with the same file, or offering a symbol table when the server +/// holds DWARF, transfers nothing — the server answers 16004 and says what it keeps — and +/// the old "re-run with --force" advice does not appear, because it would be wrong. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn elf_upload_skips_without_transfer_when_the_server_keeps_the_richer_copy() { + for (fixture, entry, stored) in [ + ("libsymbol1.so", "arm64-v8a/libsymbol1.so", "dwarf"), + ( + "libsymbol1.symtab.so", + "arm64-v8a/libsymbol1.so.sym", + "dwarf", + ), + ( + "libsymbol1.symtab.so", + "arm64-v8a/libsymbol1.so.sym", + "symtab", + ), + ] { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let zip = pack_fixture_zip(tmp.path(), fixture, entry); + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "ok": false, + "error": { + "type": "DuplicateSymbolsFoundError", + "code": 16004, + "params": {"format_variant": stored} + } + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("PUT")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) // nothing is transferred + .mount(&server) + .await; + + let stderr = run_elf_upload_of(server.uri(), zip, &[]).await; + + assert!(stderr.contains("same or better level"), "{stderr}"); + assert!(!stderr.contains("re-run with --force"), "{stderr}"); + } +} + +/// When the server does not say what it holds (a server that predates this, or a copy +/// with no variant), the per-library line must not claim it is "the same or better". +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn elf_upload_does_not_claim_same_or_better_when_the_server_does_not_say() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let zip = pack_fixture_zip(tmp.path(), "libsymbol1.so", "arm64-v8a/libsymbol1.so"); + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "ok": false, + "error": {"type": "DuplicateSymbolsFoundError", "code": 16004} + }))) + .mount(&server) + .await; + + let stderr = run_elf_upload_of(server.uri(), zip, &[]).await; + + assert!(stderr.contains("already on server, skipped"), "{stderr}"); + assert!(!stderr.contains("same or better"), "{stderr}"); + // The summary still tells the operator how to replace a copy this server cannot upgrade. + assert!(stderr.contains("re-run with --force"), "{stderr}"); +} + +/// `--force` still means "always replace": it sends `overwrite` and still declares the +/// file's richness, so the record it creates is labelled for the next comparison. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn elf_force_still_overwrites_and_declares_richness() { + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let zip = pack_fixture_zip( + tmp.path(), + "libsymbol1.symtab.so", + "arm64-v8a/libsymbol1.so.sym", + ); + let put_url = format!("{}/put/sym", server.uri()); + Mock::given(method("POST")) + .respond_with( + ResponseTemplate::new(200).set_body_json(json!({"code": 0, "endpoint": put_url})), + ) + .mount(&server) + .await; + Mock::given(method("PUT")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + + run_elf_upload_of(server.uri(), zip, &["--force"]).await; + + let posts = post_bodies(&server.received_requests().await.unwrap()); + assert_eq!(posts[0]["overwrite"], true); + assert_eq!(posts[0]["format_variant"], "symtab"); +} + /// `--extension` lets a caller pick up a symbol spelling the CLI does not know /// yet: the entry is keyed by its real build-id exactly like a `.so`. Without /// the flag the same archive uploads nothing. diff --git a/tests/fixtures/elf/libsymbol1.symtab.so b/tests/fixtures/elf/libsymbol1.symtab.so new file mode 100644 index 0000000..6abc837 Binary files /dev/null and b/tests/fixtures/elf/libsymbol1.symtab.so differ