feat(preprod): Add VCS parameters to snapshots upload command

src/api/data_types/snapshots.rs

@@ -5,6 +5,8 @@ use std::collections::HashMap;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 
+use super::VcsInfo;
+
 const IMAGE_FILE_NAME_FIELD: &str = "image_file_name";
 const WIDTH_FIELD: &str = "width";
 const HEIGHT_FIELD: &str = "height";
@@ -21,9 +23,11 @@ pub struct CreateSnapshotResponse {
 // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py
 /// Manifest describing a set of snapshot images for an app.
 #[derive(Debug, Serialize)]
-pub struct SnapshotsManifest {
+pub struct SnapshotsManifest<'a> {
     pub app_id: String,
     pub images: HashMap<String, ImageMetadata>,
+    #[serde(flatten)]
+    pub vcs_info: VcsInfo<'a>,
 }
 
 // Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py

src/commands/build/snapshots.rs

@@ -18,6 +18,8 @@ use walkdir::WalkDir;
 use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest};
 use crate::config::{Auth, Config};
 use crate::utils::args::ArgExt as _;
+use crate::utils::build_vcs::collect_git_metadata;
+use crate::utils::ci::is_ci;
 
 const EXPERIMENTAL_WARNING: &str =
     "[EXPERIMENTAL] The \"build snapshots\" command is experimental. \
@@ -47,6 +49,7 @@ pub fn make_command(command: Command) -> Command {
                 .help("The application identifier.")
                 .required(true),
         )
+        .git_metadata_args()
 }
 
 struct ImageInfo {
@@ -80,6 +83,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
         anyhow::bail!("Path is not a directory: {}", dir_path.display());
     }
 
+    // Collect git metadata if running in CI, unless explicitly enabled or disabled.
+    let should_collect_git_metadata =
+        matches.get_flag("force_git_metadata") || (!matches.get_flag("no_git_metadata") && is_ci());
+
+    // Always collect git metadata, but only perform automatic inference when enabled
+    let vcs_info = collect_git_metadata(matches, &config, should_collect_git_metadata);
+
     debug!("Scanning for images in: {}", dir_path.display());
     debug!("Organization: {org}");
     debug!("Project: {project}");
@@ -114,6 +124,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     let manifest = SnapshotsManifest {
         app_id: app_id.clone(),
         images: manifest_entries,
+        vcs_info,
     };
 
     // POST manifest to API

src/commands/build/upload.rs

@@ -1,4 +1,3 @@
-use std::borrow::Cow;
 use std::io::Write as _;
 use std::path::Path;
 use std::thread;
@@ -8,7 +7,6 @@ use anyhow::{anyhow, bail, Context as _, Result};
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use indicatif::ProgressStyle;
 use log::{debug, info, warn};
-use sha1_smol::Digest;
 use symbolic::common::ByteView;
 use zip::write::SimpleFileOptions;
 use zip::{DateTime, ZipWriter};
@@ -22,17 +20,13 @@ use crate::utils::build::{handle_asset_catalogs, ipa_to_xcarchive, is_apple_app,
 use crate::utils::build::{
     is_aab_file, is_apk_file, is_zip_file, normalize_directory, write_version_metadata,
 };
+use crate::utils::build_vcs::collect_git_metadata;
 use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL};
 use crate::utils::ci::is_ci;
 use crate::utils::fs::get_sha1_checksums;
 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_head_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,
-};
 
 pub fn make_command(command: Command) -> Command {
     #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
@@ -54,51 +48,7 @@ pub fn make_command(command: Command) -> Command {
                 .action(ArgAction::Append)
                 .required(true),
         )
-        .arg(
-            Arg::new("head_sha")
-                .long("head-sha")
-                .value_parser(parse_sha_allow_empty)
-                .help("The VCS commit sha to use for the upload. If not provided, the current commit sha will be used.")
-        )
-        .arg(
-            Arg::new("base_sha")
-                .long("base-sha")
-                .value_parser(parse_sha_allow_empty)
-                .help("The VCS commit's base sha to use for the upload. If not provided, the merge-base of the current and remote branch will be used.")
-        )
-        .arg(
-            Arg::new("vcs_provider")
-                .long("vcs-provider")
-                .help("The VCS provider to use for the upload. If not provided, the current provider will be used.")
-        )
-        .arg(
-            Arg::new("head_repo_name")
-                .long("head-repo-name")
-                .help("The name of the git repository to use for the upload (e.g. organization/repository). If not provided, the current repository will be used.")
-        )
-        .arg(
-            Arg::new("base_repo_name")
-                .long("base-repo-name")
-                .help("The name of the git repository to use for the upload (e.g. organization/repository). If not provided, the current repository will be used.")
-        )
-        .arg(
-            Arg::new("head_ref")
-                .long("head-ref")
-                .help("The reference (branch) to use for the upload. If not provided, the current reference will be used.")
-        )
-        .arg(
-            Arg::new("base_ref")
-                .long("base-ref")
-                .help("The base reference (branch) to use for the upload. If not provided, the merge-base with the remote tracking branch will be used.")
-        )
-        .arg(
-            Arg::new("pr_number")
-                .long("pr-number")
-                .value_parser(clap::value_parser!(u32))
-                .help("The pull request number to use for the upload. If not provided and running \
-                    in a pull_request-triggered GitHub Actions workflow, the PR number will be automatically \
-                    detected from GitHub Actions environment variables.")
-        )
+        .git_metadata_args()
         .arg(
             Arg::new("build_configuration")
                 .long("build-configuration")
@@ -119,22 +69,6 @@ pub fn make_command(command: Command) -> Command {
                     for each other.",
                 )
         )
-        .arg(
-            Arg::new("force_git_metadata")
-                .long("force-git-metadata")
-                .action(ArgAction::SetTrue)
-                .conflicts_with("no_git_metadata")
-                .help("Force collection and sending of git metadata (branch, commit, etc.). \
-                    If neither this nor --no-git-metadata is specified, git metadata is \
-                    automatically collected when running in most CI environments.")
-        )
-        .arg(
-            Arg::new("no_git_metadata")
-                .long("no-git-metadata")
-                .action(ArgAction::SetTrue)
-                .conflicts_with("force_git_metadata")
-                .help("Disable collection and sending of git metadata.")
-        )
 }
 
 /// Parse plugin info from SENTRY_PIPELINE environment variable.
@@ -309,181 +243,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     Ok(())
 }
 
-/// Collects git metadata from arguments and VCS introspection.
-///
-/// When `auto_collect` is false, only explicitly provided values are collected;
-/// automatic inference from git repository and CI environment is skipped.
-fn collect_git_metadata(
-    matches: &ArgMatches,
-    config: &Config,
-    auto_collect: bool,
-) -> VcsInfo<'static> {
-    let head_sha = matches
-        .get_one::<Option<Digest>>("head_sha")
-        .map(|d| d.as_ref().cloned())
-        .or_else(|| auto_collect.then(|| vcs::find_head_sha().ok()))
-        .flatten();
-
-    let cached_remote = config.get_cached_vcs_remote();
-    let (vcs_provider, head_repo_name, head_ref, base_ref, base_repo_name) = {
-        let repo = if auto_collect {
-            git2::Repository::open_from_env().ok()
-        } else {
-            None
-        };
-        let repo_ref = repo.as_ref();
-        let remote_url = repo_ref.and_then(|repo| git_repo_remote_url(repo, &cached_remote).ok());
-
-        let vcs_provider = matches
-            .get_one("vcs_provider")
-            .cloned()
-            .or_else(|| {
-                auto_collect
-                    .then(|| remote_url.as_ref().map(|url| get_provider_from_remote(url)))?
-            })
-            .unwrap_or_default();
-
-        let head_repo_name = matches
-            .get_one("head_repo_name")
-            .cloned()
-            .or_else(|| {
-                auto_collect.then(|| {
-                    remote_url
-                        .as_ref()
-                        .map(|url| get_repo_from_remote_preserve_case(url))
-                })?
-            })
-            .unwrap_or_default();
-
-        let head_ref = matches
-            .get_one("head_ref")
-            .cloned()
-            .or_else(|| auto_collect.then(get_github_head_ref)?)
-            .or_else(|| {
-                auto_collect.then(|| {
-                    repo_ref.and_then(|r| match git_repo_head_ref(r) {
-                        Ok(ref_name) => {
-                            debug!("Found current branch reference: {ref_name}");
-                            Some(ref_name)
-                        }
-                        Err(e) => {
-                            debug!("No valid branch reference found (likely detached HEAD): {e}");
-                            None
-                        }
-                    })
-                })?
-            })
-            .unwrap_or_default();
-
-        let base_ref = matches
-            .get_one("base_ref")
-            .cloned()
-            .or_else(|| auto_collect.then(get_github_base_ref)?)
-            .or_else(|| {
-                auto_collect.then(|| {
-                    repo_ref.and_then(|r| match git_repo_base_ref(r, &cached_remote) {
-                        Ok(base_ref_name) => {
-                            debug!("Found base reference: {base_ref_name}");
-                            Some(base_ref_name)
-                        }
-                        Err(e) => {
-                            info!("Could not detect base branch reference: {e}");
-                            None
-                        }
-                    })
-                })?
-            })
-            .unwrap_or_default();
-
-        let base_repo_name = matches
-            .get_one("base_repo_name")
-            .cloned()
-            .or_else(|| {
-                auto_collect.then(|| {
-                    repo_ref.and_then(|r| match git_repo_base_repo_name_preserve_case(r) {
-                        Ok(Some(base_repo_name)) => {
-                            debug!("Found base repository name: {base_repo_name}");
-                            Some(base_repo_name)
-                        }
-                        Ok(None) => {
-                            debug!("No base repository found - not a fork");
-                            None
-                        }
-                        Err(e) => {
-                            warn!("Could not detect base repository name: {e}");
-                            None
-                        }
-                    })
-                })?
-            })
-            .unwrap_or_default();
-
-        (
-            vcs_provider,
-            head_repo_name,
-            head_ref,
-            base_ref,
-            base_repo_name,
-        )
-    };
-
-    let base_sha_from_user = matches.get_one::<Option<Digest>>("base_sha").is_some();
-    let base_ref_from_user = matches.get_one::<String>("base_ref").is_some();
-
-    let mut base_sha = matches
-        .get_one::<Option<Digest>>("base_sha")
-        .map(|d| d.as_ref().cloned())
-        .or_else(|| {
-            if auto_collect {
-                Some(
-                    vcs::find_base_sha(&cached_remote)
-                        .inspect_err(|e| debug!("Error finding base SHA: {e}"))
-                        .ok()
-                        .flatten(),
-                )
-            } else {
-                None
-            }
-        })
-        .flatten();
-
-    let mut base_ref = base_ref;
-
-    // If base_sha equals head_sha and both were auto-inferred, skip setting base_sha and base_ref
-    if !base_sha_from_user
-        && !base_ref_from_user
-        && base_sha.is_some()
-        && head_sha.is_some()
-        && base_sha == head_sha
-    {
-        debug!(
-            "Base SHA equals head SHA ({}), and both were auto-inferred. Skipping base_sha and base_ref, but keeping head_sha.",
-            base_sha.expect("base_sha is Some at this point")
-        );
-        base_sha = None;
-        base_ref = "".into();
-    }
-
-    let pr_number = matches.get_one("pr_number").copied().or_else(|| {
-        if auto_collect {
-            get_github_pr_number()
-        } else {
-            None
-        }
-    });
-
-    VcsInfo {
-        head_sha,
-        base_sha,
-        vcs_provider: Cow::Owned(vcs_provider),
-        head_repo_name: Cow::Owned(head_repo_name),
-        base_repo_name: Cow::Owned(base_repo_name),
-        head_ref: Cow::Owned(head_ref),
-        base_ref: Cow::Owned(base_ref),
-        pr_number,
-    }
-}
-
 fn handle_file(
     path: &Path,
     byteview: &ByteView,
@@ -709,22 +468,6 @@ fn upload_file(
     }
 }
 
-/// Utility function to parse a SHA1 digest, allowing empty strings.
-///
-/// Empty strings result in Ok(None), otherwise we return the parsed digest
-/// or an error if the SHA is invalid.
-fn parse_sha_allow_empty(sha: &str) -> Result<Option<Digest>> {
-    if sha.is_empty() {
-        return Ok(None);
-    }
-
-    let digest = sha
-        .parse()
-        .with_context(|| format!("{sha} is not a valid SHA1 digest"))?;
-
-    Ok(Some(digest))
-}
-
 #[cfg(not(windows))]
 #[cfg(test)]
 mod tests {