Skip to content

Commit 06cbbda

Browse files
ref(snapshots): Key manifest entries by relative path instead of hash (#3241)
Switch the snapshot manifest dictionary key from content hash to image file name (basename). This enables the backend to use the key directly as the image identifier, removing the redundant `image_file_name` field from manifest values. Changes: - Manifest entries are now keyed by file name instead of content hash - `image_file_name` removed from `ImageMetadata` — derived from key - `content_hash` stored in image metadata `extra` for objectstore lookups - Collision detection skips duplicate file names before upload, with a warning listing all excluded paths - Path separators normalized via `path_as_url` for cross-platform consistency Companion backend PR: getsentry/sentry#111467 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 94a8931 commit 06cbbda

File tree

3 files changed

+42
-29
lines changed

3 files changed

+42
-29
lines changed

.github/CODEOWNERS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
# Files co-owned by Emerge Tools team
1515
/apple-catalog-parsing @getsentry/emerge-tools @getsentry/owners-sentry-cli
16+
/src/api/data_types/snapshots.rs @getsentry/emerge-tools @getsentry/owners-sentry-cli
1617
/src/commands/build @getsentry/emerge-tools @getsentry/owners-sentry-cli
1718
/src/utils/build @getsentry/emerge-tools @getsentry/owners-sentry-cli
1819
/tests/integration/build @getsentry/emerge-tools @getsentry/owners-sentry-cli

src/api/data_types/snapshots.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use serde_json::Value;
77

88
use super::VcsInfo;
99

10-
const IMAGE_FILE_NAME_FIELD: &str = "image_file_name";
1110
const WIDTH_FIELD: &str = "width";
1211
const HEIGHT_FIELD: &str = "height";
1312

@@ -42,16 +41,7 @@ pub struct ImageMetadata {
4241
}
4342

4443
impl ImageMetadata {
45-
pub fn new(
46-
image_file_name: String,
47-
width: u32,
48-
height: u32,
49-
mut extra: HashMap<String, Value>,
50-
) -> Self {
51-
extra.insert(
52-
IMAGE_FILE_NAME_FIELD.to_owned(),
53-
Value::String(image_file_name),
54-
);
44+
pub fn new(width: u32, height: u32, mut extra: HashMap<String, Value>) -> Self {
5545
extra.insert(WIDTH_FIELD.to_owned(), Value::from(width));
5646
extra.insert(HEIGHT_FIELD.to_owned(), Value::from(height));
5747

@@ -68,18 +58,16 @@ mod tests {
6858
#[test]
6959
fn cli_managed_fields_override_sidecar_fields() {
7060
let extra = serde_json::from_value(json!({
71-
(IMAGE_FILE_NAME_FIELD): "from-sidecar.png",
7261
(WIDTH_FIELD): 1,
7362
(HEIGHT_FIELD): 2,
7463
"custom": "keep-me"
7564
}))
7665
.unwrap();
7766

78-
let metadata = ImageMetadata::new("from-cli.png".to_owned(), 100, 200, extra);
67+
let metadata = ImageMetadata::new(100, 200, extra);
7968
let serialized = serde_json::to_value(metadata).unwrap();
8069

8170
let expected = json!({
82-
(IMAGE_FILE_NAME_FIELD): "from-cli.png",
8371
(WIDTH_FIELD): 100,
8472
(HEIGHT_FIELD): 200,
8573
"custom": "keep-me"

src/commands/build/snapshots.rs

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,28 @@ fn upload_images(
331331

332332
let mut many_builder = session.many();
333333
let mut manifest_entries = HashMap::new();
334-
let image_count = images.len();
335-
334+
let mut collisions: HashMap<String, Vec<String>> = HashMap::new();
335+
let mut kept_paths: HashMap<String, String> = HashMap::new();
336336
for image in images {
337337
debug!("Processing image: {}", image.path.display());
338338

339+
let image_file_name = image
340+
.relative_path
341+
.file_name()
342+
.unwrap_or_default()
343+
.to_string_lossy()
344+
.into_owned();
345+
346+
let relative_path = crate::utils::fs::path_as_url(&image.relative_path);
347+
348+
if manifest_entries.contains_key(&image_file_name) {
349+
collisions
350+
.entry(image_file_name)
351+
.or_default()
352+
.push(relative_path);
353+
continue;
354+
}
355+
339356
let hash = compute_sha256_hash(&image.path)?;
340357
let file = runtime
341358
.block_on(tokio::fs::File::open(&image.path))
@@ -353,33 +370,40 @@ fn upload_images(
353370
.expiration_policy(expiration),
354371
);
355372

356-
let image_file_name = image
357-
.relative_path
358-
.file_name()
359-
.unwrap_or_default()
360-
.to_string_lossy()
361-
.into_owned();
362-
363-
let extra = read_sidecar_metadata(&image.path).unwrap_or_else(|err| {
373+
let mut extra = read_sidecar_metadata(&image.path).unwrap_or_else(|err| {
364374
warn!("Error reading sidecar metadata, ignoring it instead: {err:#}");
365375
HashMap::new()
366376
});
377+
extra.insert("content_hash".to_owned(), serde_json::Value::String(hash));
367378

379+
kept_paths.insert(image_file_name.clone(), relative_path);
368380
manifest_entries.insert(
369-
hash,
370-
ImageMetadata::new(image_file_name, image.width, image.height, extra),
381+
image_file_name,
382+
ImageMetadata::new(image.width, image.height, extra),
371383
);
372384
}
373385

386+
if !collisions.is_empty() {
387+
let mut details = String::new();
388+
for (name, excluded_paths) in &collisions {
389+
let mut all_paths = vec![kept_paths[name].as_str()];
390+
all_paths.extend(excluded_paths.iter().map(|s| s.as_str()));
391+
details.push_str(&format!("\n {name}: {}", all_paths.join(", ")));
392+
}
393+
warn!("Some images share identical file names. Only the first occurrence of each is included:{details}");
394+
}
395+
374396
let result = runtime.block_on(async { many_builder.send().error_for_failures().await });
375397

398+
let uploaded_count = manifest_entries.len();
399+
376400
match result {
377401
Ok(()) => {
378402
println!(
379403
"{} Uploaded {} image {}",
380404
style(">").dim(),
381-
style(image_count).yellow(),
382-
if image_count == 1 { "file" } else { "files" }
405+
style(uploaded_count).yellow(),
406+
if uploaded_count == 1 { "file" } else { "files" }
383407
);
384408
Ok(manifest_entries)
385409
}
@@ -391,7 +415,7 @@ fn upload_images(
391415
eprintln!(" {}", style(format!("{error:#}")).red());
392416
error_count += 1;
393417
}
394-
anyhow::bail!("Failed to upload {error_count} out of {image_count} images")
418+
anyhow::bail!("Failed to upload {error_count} out of {uploaded_count} images")
395419
}
396420
}
397421
}

0 commit comments

Comments
 (0)