Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions src/commands/build/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ use crate::utils::fs::TempDir;
use crate::utils::fs::TempFile;
use crate::utils::progress::ProgressBar;
use crate::utils::vcs::{
self, get_github_base_ref, get_github_pr_number, get_provider_from_remote,
get_repo_from_remote_preserve_case, git_repo_base_ref, git_repo_base_repo_name_preserve_case,
git_repo_head_ref, git_repo_remote_url,
self, get_github_pr_number, get_provider_from_remote, get_repo_from_remote_preserve_case,
git_repo_base_ref, git_repo_base_repo_name_preserve_case, git_repo_head_ref,
git_repo_remote_url,
};

pub fn make_command(command: Command) -> Command {
Expand Down Expand Up @@ -176,11 +176,8 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
.map(String::as_str)
.map(Cow::Borrowed)
.or_else(|| {
// First try GitHub Actions environment variables
get_github_base_ref().map(Cow::Owned)
})
.or_else(|| {
// Fallback to git repository introspection
// Try to get the base ref from the VCS if not provided
// This attempts to find the merge-base with the remote tracking branch
repo_ref
.and_then(|r| match git_repo_base_ref(r, &cached_remote) {
Ok(base_ref_name) => {
Comment on lines 176 to 183

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: Reverting the use of GITHUB_BASE_REF causes base branch detection to fail in GitHub Actions PRs, as the git_repo_base_ref() fallback is unreliable in that environment.
  • Description: The removal of logic that reads the GITHUB_BASE_REF environment variable introduces a functional regression. The fallback mechanism, git_repo_base_ref(), attempts to find the base branch by introspecting the Git repository. However, this function often fails in GitHub Actions pull request workflows because they typically operate in a detached HEAD state and may use shallow clones, which lack the necessary refs/remotes/{remote_name}/HEAD reference. When git_repo_base_ref() fails, it returns None, resulting in incomplete VCS context being sent to Sentry. This breaks build tracking and pull request analysis features for a common CI/CD use case.

  • Suggested fix: Restore the logic that reads the GITHUB_BASE_REF environment variable as the primary method for detecting the base branch within GitHub Actions environments. This ensures reliable base branch detection in CI, falling back to git_repo_base_ref() only when the environment variable is not present.
    severity: 0.7, confidence: 0.95

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
41 changes: 0 additions & 41 deletions src/utils/vcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,21 +359,6 @@ pub fn get_github_pr_number() -> Option<u32> {
Some(pr_number)
}

/// Attempts to get the base branch from GitHub Actions environment variables.
/// Returns the base branch name if running in a GitHub Actions pull request environment.
pub fn get_github_base_ref() -> Option<String> {
let event_name = std::env::var("GITHUB_EVENT_NAME").ok()?;

if event_name != "pull_request" {
debug!("Not running in pull_request event, got: {}", event_name);
return None;
}

let base_ref = std::env::var("GITHUB_BASE_REF").ok()?;
debug!("Auto-detected base ref from GitHub Actions: {}", base_ref);
Some(base_ref)
}

fn find_reference_url(repo: &str, repos: &[Repo]) -> Result<Option<String>> {
let mut non_git = false;
for configured_repo in repos {
Expand Down Expand Up @@ -1481,30 +1466,4 @@ mod tests {
std::env::remove_var("GITHUB_EVENT_NAME");
std::env::remove_var("GITHUB_REF");
}

#[test]
fn test_get_github_base_ref() {
std::env::set_var("GITHUB_EVENT_NAME", "pull_request");
std::env::set_var("GITHUB_BASE_REF", "main");
let base_ref = get_github_base_ref();
assert_eq!(base_ref, Some("main".to_owned()));

// Test with different base branch
std::env::set_var("GITHUB_BASE_REF", "develop");
let base_ref = get_github_base_ref();
assert_eq!(base_ref, Some("develop".to_owned()));

// Test when not in pull_request event
std::env::set_var("GITHUB_EVENT_NAME", "push");
let base_ref = get_github_base_ref();
assert_eq!(base_ref, None);

// Test when GITHUB_BASE_REF is not set
std::env::set_var("GITHUB_EVENT_NAME", "pull_request");
std::env::remove_var("GITHUB_BASE_REF");
let base_ref = get_github_base_ref();
assert_eq!(base_ref, None);

std::env::remove_var("GITHUB_EVENT_NAME");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
$ sentry-cli build upload tests/integration/_fixtures/build/apk.apk
? success
[..]WARN[..]EXPERIMENTAL: The build subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release.
WARN [..] Could not detect base branch reference: [..]
Successfully uploaded 1 file to Sentry
- tests/integration/_fixtures/build/apk.apk (http[..]/wat-org/preprod/wat-project/42)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
$ sentry-cli build upload tests/integration/_fixtures/build/apk.apk
? failed
[..]WARN[..]EXPERIMENTAL: The build subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release.
WARN [..] Could not detect base branch reference: [..]
error: Auth token is required for this request. Please run `sentry-cli login` and try again!

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Expand Down
1 change: 1 addition & 0 deletions tests/integration/_cases/build/build-upload-apk.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
$ sentry-cli build upload tests/integration/_fixtures/build/apk.apk --head-sha test_head_sha
? success
[..]WARN[..]EXPERIMENTAL: The build subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release.
WARN [..] Could not detect base branch reference: [..]
Successfully uploaded 1 file to Sentry
- tests/integration/_fixtures/build/apk.apk (http[..]/wat-org/preprod/wat-project/42)

Expand Down
1 change: 1 addition & 0 deletions tests/integration/_cases/build/build-upload-ipa.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
$ sentry-cli build upload tests/integration/_fixtures/build/ipa.ipa --head-sha test_head_sha
? success
[..]WARN[..]EXPERIMENTAL: The build subcommand is experimental. The command is subject to breaking changes and may be removed without notice in any release.
WARN [..] Could not detect base branch reference: [..]
Successfully uploaded 1 file to Sentry
- tests/integration/_fixtures/build/ipa.ipa (http[..]/wat-org/preprod/wat-project/some-text-id)

Expand Down