feat(code-mappings): Add git inference for repo and default branch#3208
feat(code-mappings): Add git inference for repo and default branch#3208
Conversation
|
684caab to
912da39
Compare
fb49d43 to
029aeaf
Compare
912da39 to
95e8814
Compare
|
Please check and resolve the open comments from the AI reviewers, before I do a full review. If any comments are inaccurate or non-actionable, please indicate that with a |
e405066 to
8f9babe
Compare
src/commands/code_mappings/upload.rs
Outdated
| let (repo_name, default_branch) = match (explicit_repo, explicit_branch) { | ||
| (Some(r), Some(b)) => (r.to_owned(), b.to_owned()), | ||
| _ => { | ||
| let git_repo = git2::Repository::open_from_env(); | ||
|
|
||
| // Resolve the best remote name when we have a git repo. | ||
| // Prefer explicit config (SENTRY_VCS_REMOTE / ini), then inspect | ||
| // the repo for the best remote (upstream > origin > first). | ||
| let remote_name = git_repo.as_ref().ok().and_then(|repo| { | ||
| let config = Config::current(); | ||
| let configured_remote = config.get_cached_vcs_remote(); | ||
| if vcs::git_repo_remote_url(repo, &configured_remote).is_ok() { | ||
| debug!("Using configured VCS remote: {configured_remote}"); | ||
| Some(configured_remote) | ||
| } else { | ||
| match vcs::find_best_remote(repo) { | ||
| Ok(Some(best)) => { | ||
| debug!( | ||
| "Configured remote '{configured_remote}' not found, using: {best}" | ||
| ); | ||
| Some(best) | ||
| } | ||
| _ => None, | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| let repo_name = match explicit_repo { | ||
| Some(r) => r.to_owned(), | ||
| None => { | ||
| let git_repo = git_repo.as_ref().map_err(|e| { | ||
| anyhow::anyhow!( | ||
| "Could not open git repository: {e}. \ | ||
| Use --repo to specify manually." | ||
| ) | ||
| })?; | ||
| let remote_name = remote_name.as_deref().ok_or_else(|| { | ||
| anyhow::anyhow!( | ||
| "No remotes found in the git repository. \ | ||
| Use --repo to specify manually." | ||
| ) | ||
| })?; | ||
| let remote_url = vcs::git_repo_remote_url(git_repo, remote_name)?; | ||
| debug!("Found remote '{remote_name}': {remote_url}"); | ||
| let inferred = vcs::get_repo_from_remote_preserve_case(&remote_url); | ||
| if inferred.is_empty() { | ||
| bail!("Could not parse repository name from remote URL: {remote_url}"); | ||
| } | ||
| println!("Inferred repository: {inferred}"); | ||
| inferred | ||
| } | ||
| }; | ||
|
|
||
| let default_branch = match explicit_branch { | ||
| Some(b) => b.to_owned(), | ||
| None => { | ||
| let inferred = git_repo | ||
| .as_ref() | ||
| .ok() | ||
| .and_then(|repo| { | ||
| remote_name.as_deref().and_then(|name| { | ||
| vcs::git_repo_base_ref(repo, name) | ||
| .map(Some) | ||
| .unwrap_or_else(|e| { | ||
| debug!("Could not infer default branch from remote: {e}"); | ||
| None | ||
| }) | ||
| }) | ||
| }) | ||
| .unwrap_or_else(|| { | ||
| debug!("No git repo or remote available, falling back to 'main'"); | ||
| "main".to_owned() | ||
| }); | ||
| println!("Inferred default branch: {inferred}"); | ||
| inferred | ||
| } | ||
| }; | ||
|
|
||
| (repo_name, default_branch) | ||
| } | ||
| }; |
There was a problem hiding this comment.
m: This logic is quite difficult for me to follow due to the deep nesting, and I suspect we can greatly simplify it.
As written, the match statement is coupling the values of explicit_repo and explicit_branch, behaving differently when they are both defined versus when only one of them is defined. I guess this is done to avoid needing to open the repo with git2::Repository::open_from_env() more than once, and to also avoid opening it altogether when we don't need to open it, but I think the complexity of avoiding this is not worth it.
Conceptually, in Rust-pseudocode, I think this is best written as:
let repo_name = matches.get_one::<String>("repo").unwrap_or_else(|| todo!("compute fallback"));
let default_branch = matches.get_one::<String>("default_branch").unwrap_or_else(|| todo!("compute fallback"));l: Additionally, we might be able to extract some of this logic to utility functions.
There was a problem hiding this comment.
Happy to help here if needed 🙏
There was a problem hiding this comment.
Refactored as suggested — flattened into two independent lookups with resolve_git_remote, infer_repo_name, and infer_default_branch extracted as helper functions.
— Claude Code
There was a problem hiding this comment.
clankers doing their job 😅
_#skip-changelog_ Add the `sentry-cli code-mappings upload` subcommand group and the `upload` subcommand with file parsing and validation. This is the first in a stack of 4 PRs to support bulk uploading code mappings from a JSON file — useful for Java/Android multi-module projects that need dozens of mappings. This PR adds: - Command scaffold following the `repos`/`deploys` pattern - JSON file reading and validation (empty arrays, empty stackRoot/sourceRoot) - CLI args: positional `PATH`, `--repo`, `--default-branch` - Help and no-subcommand trycmd integration tests Stack: **#3207** → #3208 → #3209 → #3210 Backend PRs: getsentry/sentry#109783, getsentry/sentry#109785, getsentry/sentry#109786 Closes getsentry/sentry-android-gradle-plugin#1076 Closes getsentry/sentry-android-gradle-plugin#1077 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Add a new `code-mappings` subcommand group with an `upload` subcommand that reads and validates a JSON file of code mappings. This is the first step toward CLI support for bulk code mapping uploads to Sentry.
Add descriptive help text explaining what code mappings are and how they work. Replace unwrap with expect for the required path argument. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Automatically detect repository name and default branch from the local git repo when --repo or --default-branch are not provided. Respects SENTRY_VCS_REMOTE config, falling back to best-effort remote detection. Extract find_best_remote() into vcs.rs to deduplicate remote selection logic shared with git_repo_base_repo_name_preserve_case().
When --repo is provided but --default-branch is not, the code no longer requires a git remote to be present. Branch inference gracefully falls back to 'main' when no git repo or remote is available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use get_repo_from_remote_preserve_case instead of get_repo_from_remote to avoid lowercasing the repository name, which would cause mismatches with Sentry's case-sensitive API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
70ddd86 to
6dae115
Compare
Replace the deeply nested match with two independent lookups for repo_name and default_branch. Extract resolve_git_remote, infer_repo_name, and infer_default_branch as utility functions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicate output when values are inferred from git
- Removed println! calls from inference helpers so only the summary lines are printed once.
Or push these changes by commenting:
@cursor push 7a276a79c2
Preview (7a276a79c2)
diff --git a/src/commands/code_mappings/upload.rs b/src/commands/code_mappings/upload.rs
--- a/src/commands/code_mappings/upload.rs
+++ b/src/commands/code_mappings/upload.rs
@@ -123,7 +123,6 @@
if inferred.is_empty() {
bail!("Could not parse repository name from remote URL: {remote_url}");
}
- println!("Inferred repository: {inferred}");
Ok(inferred)
}
@@ -143,6 +142,5 @@
debug!("No git repo or remote available, falling back to 'main'");
"main".to_owned()
});
- println!("Inferred default branch: {inferred}");
inferred
}This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| if inferred.is_empty() { | ||
| bail!("Could not parse repository name from remote URL: {remote_url}"); | ||
| } | ||
| println!("Inferred repository: {inferred}"); |
There was a problem hiding this comment.
Duplicate output when values are inferred from git
Low Severity
When repo or branch is inferred from git, infer_repo_name prints "Inferred repository: ..." and infer_default_branch prints "Inferred default branch: ...", and then execute unconditionally prints "Repository: ..." and "Default branch: ..." with the same values. This produces confusing redundant output to the user showing the same information twice.



#skip-changelog
When
--repoor--default-branchare not provided, infer them from thelocal git repository. Uses the configured VCS remote (SENTRY_VCS_REMOTE / ini)
first, then falls back to best-effort remote detection (upstream > origin > first).
Also extracts
find_best_remote()as a shared utility insrc/utils/vcs.rs,replacing the inline logic that was duplicated in
git_repo_base_repo_name_preserve_case.Stack: #3207 → #3208 → #3209 → #3210
Backend PRs: getsentry/sentry#109783, getsentry/sentry#109785, getsentry/sentry#109786
Closes GRADLE-79