From 905715df267536afc041217eabc974093caeb783 Mon Sep 17 00:00:00 2001 From: Hector Date: Wed, 29 Oct 2025 13:32:44 +0000 Subject: [PATCH] fix(vcs): Fix base_ref / base_sha for non-github actions --- CHANGELOG.md | 4 ++ src/commands/build/upload.rs | 2 +- src/utils/vcs.rs | 101 ++++++++++++++++++++--------------- 3 files changed, 62 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b01b39b475..80bc34cfbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Deprecated the `upload-proguard` subcommand's `--platform` flag ([#2863](https://github.com/getsentry/sentry-cli/pull/2863)). This flag appears to have been a no-op for some time, so we will remove it in the next major. +### Fixes + +- Fix auto filled git metadata when using the `build upload` subcommand in non-github workflow contexts ([#2888](https://github.com/getsentry/sentry-cli/pull/2888)) + ## 2.57.0 ### New Features diff --git a/src/commands/build/upload.rs b/src/commands/build/upload.rs index 923dda5f48..f449890799 100644 --- a/src/commands/build/upload.rs +++ b/src/commands/build/upload.rs @@ -233,7 +233,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { .map(String::as_str) .map(Cow::Borrowed) .or_else(|| { - vcs::find_base_sha() + vcs::find_base_sha(&cached_remote) .inspect_err(|e| debug!("Error finding base SHA: {e}")) .ok() .flatten() diff --git a/src/utils/vcs.rs b/src/utils/vcs.rs index 1c52678141..161c5b9089 100644 --- a/src/utils/vcs.rs +++ b/src/utils/vcs.rs @@ -3,7 +3,7 @@ use std::fmt; use std::path::PathBuf; -use anyhow::{bail, format_err, Context as _, Error, Result}; +use anyhow::{bail, format_err, Error, Result}; use chrono::{DateTime, FixedOffset, TimeZone as _}; use git2::{Commit, Repository, Time}; use if_chain::if_chain; @@ -272,30 +272,27 @@ pub fn git_repo_head_ref(repo: &git2::Repository) -> Result { } pub fn git_repo_base_ref(repo: &git2::Repository, remote_name: &str) -> Result { - // Get the current HEAD commit - let head_commit = repo.head()?.peel_to_commit()?; - - // Try to find the remote tracking branch let remote_branch_name = format!("refs/remotes/{remote_name}/HEAD"); let remote_ref = repo.find_reference(&remote_branch_name).map_err(|e| { anyhow::anyhow!("Could not find remote tracking branch for {remote_name}: {e}") })?; - find_merge_base_ref(repo, &head_commit, &remote_ref) -} - -fn find_merge_base_ref( - repo: &git2::Repository, - head_commit: &git2::Commit, - remote_ref: &git2::Reference, -) -> Result { - let remote_commit = remote_ref.peel_to_commit()?; - let merge_base_oid = repo.merge_base(head_commit.id(), remote_commit.id())?; - - // Return the merge-base commit SHA as the base reference - let merge_base_sha = merge_base_oid.to_string(); - debug!("Found merge-base commit as base reference: {merge_base_sha}"); - Ok(merge_base_sha) + let name = remote_ref + .resolve()? + .shorthand() + .ok_or(anyhow::anyhow!( + "Could not find remote tracking branch for {remote_name}" + ))? + .to_owned(); + + let expected_prefix = format!("{remote_name}/"); + if let Some(branch_name) = name.strip_prefix(&expected_prefix) { + Ok(branch_name.to_owned()) + } else { + Err(anyhow::anyhow!( + "Remote branch name '{name}' does not start with expected prefix '{expected_prefix}'" + )) + } } /// Like git_repo_base_repo_name but preserves the original case of the repository name. @@ -569,13 +566,31 @@ pub fn find_head_sha() -> Result { Ok(head.id().to_string()) } -pub fn find_base_sha() -> Result> { - let github_event = std::env::var("GITHUB_EVENT_PATH") - .map_err(Error::from) - .and_then(|event_path| std::fs::read_to_string(event_path).map_err(Error::from)) - .context("Failed to read GitHub event path")?; +pub fn find_base_sha(remote_name: &str) -> Result> { + if let Some(pr_base_sha) = std::env::var("GITHUB_EVENT_PATH") + .ok() + .and_then(|event_path| std::fs::read_to_string(event_path).ok()) + .and_then(|content| extract_pr_base_sha_from_event(&content)) + { + debug!("Using GitHub Actions PR base SHA from event payload: {pr_base_sha}"); + return Ok(Some(pr_base_sha)); + } + + let repo = git2::Repository::open_from_env()?; + + let head_commit = repo.head()?.peel_to_commit()?; - extract_pr_base_sha_from_event(&github_event) + // Try to find the remote tracking branch + let remote_branch_name = format!("refs/remotes/{remote_name}/HEAD"); + let remote_ref = repo.find_reference(&remote_branch_name).map_err(|e| { + anyhow::anyhow!("Could not find remote tracking branch for {remote_name}: {e}") + })?; + + let remote_commit = remote_ref.peel_to_commit()?; + let merge_base_oid = repo.merge_base(head_commit.id(), remote_commit.id())?; + let merge_base_sha = merge_base_oid.to_string(); + debug!("Found merge-base commit as base reference: {merge_base_sha}"); + Ok(Some(merge_base_sha)) } /// Extracts the PR head SHA from GitHub Actions event payload JSON. @@ -595,15 +610,19 @@ fn extract_pr_head_sha_from_event(json_content: &str) -> Option { } /// Extracts the PR base SHA from GitHub Actions event payload JSON. -/// Returns Ok(None) if not a PR event or if SHA cannot be extracted. -/// Returns an error if we cannot parse the JSON. -fn extract_pr_base_sha_from_event(json_content: &str) -> Result> { - let v: Value = serde_json::from_str(json_content) - .context("Failed to parse GitHub event payload as JSON")?; +/// Returns None if not a PR event or if SHA cannot be extracted. +fn extract_pr_base_sha_from_event(json_content: &str) -> Option { + let v: Value = match serde_json::from_str(json_content) { + Ok(v) => v, + Err(_) => { + debug!("Failed to parse GitHub event payload as JSON"); + return None; + } + }; - Ok(v.pointer("/pull_request/base/sha") + v.pointer("/pull_request/base/sha") .and_then(|s| s.as_str()) - .map(|s| s.to_owned())) + .map(|s| s.to_owned()) } /// Given commit specs, repos and remote_name this returns a list of head @@ -1705,7 +1724,7 @@ mod tests { .to_string(); assert_eq!( - extract_pr_base_sha_from_event(&pr_json).unwrap(), + extract_pr_base_sha_from_event(&pr_json), Some("55e6bc8c264ce95164314275d805f477650c440d".to_owned()) ); @@ -1719,10 +1738,7 @@ mod tests { }) .to_string(); - assert_eq!(extract_pr_base_sha_from_event(&push_json).unwrap(), None); - - // Test with malformed JSON - assert!(extract_pr_base_sha_from_event("invalid json {").is_err()); + assert_eq!(extract_pr_base_sha_from_event(&push_json), None); // Test with missing base SHA let incomplete_json = r#"{ @@ -1733,10 +1749,7 @@ mod tests { } }"#; - assert_eq!( - extract_pr_base_sha_from_event(incomplete_json).unwrap(), - None - ); + assert_eq!(extract_pr_base_sha_from_event(incomplete_json), None); } #[test] @@ -1765,7 +1778,7 @@ mod tests { fs::write(&event_file, pr_json).expect("Failed to write event file"); std::env::set_var("GITHUB_EVENT_PATH", event_file.to_str().unwrap()); - let result = find_base_sha(); + let result = find_base_sha("origin"); assert_eq!( result.unwrap().unwrap(), "55e6bc8c264ce95164314275d805f477650c440d" @@ -1773,7 +1786,7 @@ mod tests { // Test without GITHUB_EVENT_PATH std::env::remove_var("GITHUB_EVENT_PATH"); - let result = find_base_sha(); + let result = find_base_sha("origin"); assert!(result.is_err()); } }