Skip to content

Commit 314db3d

Browse files
romtsnclaude
andcommitted
ref(bundle-jvm): Simplify strip_source_set_prefix and build_source_files
Address review feedback on #3275: - Replace the component-walking state machine in strip_source_set_prefix with a single regex match. The pattern mirrors the doc comment (`[/\\]src[/\\][^/\\]+[/\\]<lang>[/\\]`) and leftmost-first semantics give 'first src/*/<lang>/ wins' for free. - Split build_source_files into a candidate-filtering pass and a dedup pass, and use a `HashMap<url, file_index>` instead of `HashMap<url, PathBuf>` to drop one PathBuf clone per kept file. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ef8383e commit 314db3d

1 file changed

Lines changed: 50 additions & 50 deletions

File tree

src/commands/debug_files/bundle_jvm.rs

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@ use crate::utils::source_bundle::{self, BundleContext};
99
use anyhow::{bail, Context as _, Result};
1010
use clap::{Arg, ArgAction, ArgMatches, Command};
1111
use log::{debug, warn};
12+
use regex::Regex;
1213
use sentry::types::DebugId;
1314
use std::collections::hash_map::Entry;
1415
use std::collections::{BTreeMap, HashMap};
1516
use std::ffi::OsStr;
1617
use std::fs;
1718
use std::path::{Path, PathBuf};
1819
use std::str::FromStr as _;
19-
use std::sync::Arc;
20+
use std::sync::{Arc, LazyLock};
2021
use symbolic::debuginfo::sourcebundle::SourceFileType;
2122

2223
const JVM_EXTENSIONS: &[&str] = &[
@@ -28,6 +29,14 @@ const JVM_EXTENSIONS: &[&str] = &[
2829
/// `src/<sourceset>/<lang>/<package>/...`.
2930
const SOURCE_SET_LANGS: &[&str] = &["java", "kotlin", "scala", "groovy", "clojure"];
3031

32+
static SOURCE_SET_PREFIX_RE: LazyLock<Regex> = LazyLock::new(|| {
33+
let langs = SOURCE_SET_LANGS.join("|");
34+
Regex::new(&format!(
35+
r"(?:^|[/\\])src[/\\][^/\\]+[/\\](?:{langs})[/\\](.+)$"
36+
))
37+
.expect("valid regex")
38+
});
39+
3140
/// Strips the `[<module>/]src/<sourceset>/<lang>/` prefix from a relative source
3241
/// path so the remaining portion matches what Symbolicator looks up by URL
3342
/// (e.g. `io/sentry/android/core/ANRWatchDog.java`). This is needed because
@@ -36,21 +45,11 @@ const SOURCE_SET_LANGS: &[&str] = &["java", "kotlin", "scala", "groovy", "clojur
3645
///
3746
/// Returns the path unchanged if no `src/<sourceset>/<lang>/` segment is found.
3847
fn strip_source_set_prefix(relative_path: &Path) -> PathBuf {
39-
let mut iter = relative_path.components();
40-
let mut src_two_back = false;
41-
let mut src_one_back = false;
42-
while let Some(curr) = iter.next() {
43-
let curr_is_lang = curr
44-
.as_os_str()
45-
.to_str()
46-
.is_some_and(|s| SOURCE_SET_LANGS.contains(&s));
47-
if src_two_back && curr_is_lang {
48-
return iter.collect();
49-
}
50-
src_two_back = src_one_back;
51-
src_one_back = curr.as_os_str() == "src";
52-
}
53-
relative_path.to_path_buf()
48+
relative_path
49+
.to_str()
50+
.and_then(|s| SOURCE_SET_PREFIX_RE.captures(s))
51+
.map(|caps| PathBuf::from(&caps[1]))
52+
.unwrap_or_else(|| relative_path.to_path_buf())
5453
}
5554

5655
/// Builds the Symbolicator-compatible URL for a relative source path
@@ -77,43 +76,44 @@ struct UrlCollision {
7776
/// After stripping, both map to the same URL — this keeps the first-seen
7877
/// entry and records the rest as collisions so the caller can warn the user.
7978
fn build_source_files(sources: Vec<ReleaseFileMatch>) -> (Vec<SourceFile>, Vec<UrlCollision>) {
80-
let mut seen_urls: HashMap<String, PathBuf> = HashMap::new();
79+
let candidates = sources.into_iter().filter_map(|source| {
80+
let local_path = source.path.strip_prefix(&source.base_path).unwrap();
81+
if is_in_ambiguous_build_dir(local_path) {
82+
debug!("excluding (build output): {}", source.path.display());
83+
return None;
84+
}
85+
let url = build_source_url(local_path);
86+
Some((url, source))
87+
});
88+
89+
let mut seen_urls: HashMap<String, usize> = HashMap::new();
90+
let mut files: Vec<SourceFile> = Vec::new();
8191
let mut collisions: Vec<UrlCollision> = Vec::new();
82-
let files = sources
83-
.into_iter()
84-
.filter_map(|source| {
85-
let local_path = source.path.strip_prefix(&source.base_path).unwrap();
86-
if is_in_ambiguous_build_dir(local_path) {
87-
debug!("excluding (build output): {}", source.path.display());
88-
return None;
92+
93+
for (url, source) in candidates {
94+
match seen_urls.entry(url) {
95+
Entry::Occupied(existing) => {
96+
collisions.push(UrlCollision {
97+
url: existing.key().clone(),
98+
skipped_path: source.path,
99+
kept_path: files[*existing.get()].path.clone(),
100+
});
89101
}
90-
let url = build_source_url(local_path);
91-
92-
match seen_urls.entry(url) {
93-
Entry::Occupied(existing) => {
94-
collisions.push(UrlCollision {
95-
url: existing.key().clone(),
96-
skipped_path: source.path,
97-
kept_path: existing.get().clone(),
98-
});
99-
None
100-
}
101-
Entry::Vacant(slot) => {
102-
let url = slot.key().clone();
103-
slot.insert(source.path.clone());
104-
Some(SourceFile {
105-
url,
106-
path: source.path,
107-
contents: Arc::new(source.contents),
108-
ty: SourceFileType::Source,
109-
headers: BTreeMap::new(),
110-
messages: vec![],
111-
already_uploaded: false,
112-
})
113-
}
102+
Entry::Vacant(slot) => {
103+
let url = slot.key().clone();
104+
slot.insert(files.len());
105+
files.push(SourceFile {
106+
url,
107+
path: source.path,
108+
contents: Arc::new(source.contents),
109+
ty: SourceFileType::Source,
110+
headers: BTreeMap::new(),
111+
messages: vec![],
112+
already_uploaded: false,
113+
});
114114
}
115-
})
116-
.collect();
115+
}
116+
}
117117
(files, collisions)
118118
}
119119

0 commit comments

Comments
 (0)