Skip to content

Commit 6d15ff1

Browse files
authored
feat(preprod): Add VCS parameters to snapshots upload command (#3200)
### Description Adds vcs parameters to preprod snapshots upload command. Shares utilities with existing build upload command. ### Issues <!-- * resolves: #1234 * resolves: LIN-1234 --> <!-- #### Reminders - Add GH Issue ID _&_ Linear ID (if applicable) - PR title should use [conventional commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type) style (`feat:`, `fix:`, `ref:`, `meta:`) - For external contributors: [CONTRIBUTING.md](https://github.com/getsentry/sentry-cli/blob/master/CONTRIBUTING.md), [Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord community](https://discord.gg/Ww9hbqr) --> ### Legal Boilerplate Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
1 parent 3200dfb commit 6d15ff1

File tree

9 files changed

+349
-276
lines changed

9 files changed

+349
-276
lines changed

src/api/data_types/snapshots.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use std::collections::HashMap;
55
use serde::{Deserialize, Serialize};
66
use serde_json::Value;
77

8+
use super::VcsInfo;
9+
810
const IMAGE_FILE_NAME_FIELD: &str = "image_file_name";
911
const WIDTH_FIELD: &str = "width";
1012
const HEIGHT_FIELD: &str = "height";
@@ -21,9 +23,11 @@ pub struct CreateSnapshotResponse {
2123
// Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py
2224
/// Manifest describing a set of snapshot images for an app.
2325
#[derive(Debug, Serialize)]
24-
pub struct SnapshotsManifest {
26+
pub struct SnapshotsManifest<'a> {
2527
pub app_id: String,
2628
pub images: HashMap<String, ImageMetadata>,
29+
#[serde(flatten)]
30+
pub vcs_info: VcsInfo<'a>,
2731
}
2832

2933
// Keep in sync with https://github.com/getsentry/sentry/blob/master/src/sentry/preprod/snapshots/manifest.py

src/commands/build/snapshots.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ use walkdir::WalkDir;
1818
use crate::api::{Api, CreateSnapshotResponse, ImageMetadata, SnapshotsManifest};
1919
use crate::config::{Auth, Config};
2020
use crate::utils::args::ArgExt as _;
21+
use crate::utils::build_vcs::collect_git_metadata;
22+
use crate::utils::ci::is_ci;
2123

2224
const EXPERIMENTAL_WARNING: &str =
2325
"[EXPERIMENTAL] The \"build snapshots\" command is experimental. \
@@ -47,6 +49,7 @@ pub fn make_command(command: Command) -> Command {
4749
.help("The application identifier.")
4850
.required(true),
4951
)
52+
.git_metadata_args()
5053
}
5154

5255
struct ImageInfo {
@@ -80,6 +83,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
8083
anyhow::bail!("Path is not a directory: {}", dir_path.display());
8184
}
8285

86+
// Collect git metadata if running in CI, unless explicitly enabled or disabled.
87+
let should_collect_git_metadata =
88+
matches.get_flag("force_git_metadata") || (!matches.get_flag("no_git_metadata") && is_ci());
89+
90+
// Always collect git metadata, but only perform automatic inference when enabled
91+
let vcs_info = collect_git_metadata(matches, &config, should_collect_git_metadata);
92+
8393
debug!("Scanning for images in: {}", dir_path.display());
8494
debug!("Organization: {org}");
8595
debug!("Project: {project}");
@@ -114,6 +124,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
114124
let manifest = SnapshotsManifest {
115125
app_id: app_id.clone(),
116126
images: manifest_entries,
127+
vcs_info,
117128
};
118129

119130
// POST manifest to API

src/commands/build/upload.rs

Lines changed: 2 additions & 259 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use std::borrow::Cow;
21
use std::io::Write as _;
32
use std::path::Path;
43
use std::thread;
@@ -8,7 +7,6 @@ use anyhow::{anyhow, bail, Context as _, Result};
87
use clap::{Arg, ArgAction, ArgMatches, Command};
98
use indicatif::ProgressStyle;
109
use log::{debug, info, warn};
11-
use sha1_smol::Digest;
1210
use symbolic::common::ByteView;
1311
use zip::write::SimpleFileOptions;
1412
use zip::{DateTime, ZipWriter};
@@ -22,17 +20,13 @@ use crate::utils::build::{handle_asset_catalogs, ipa_to_xcarchive, is_apple_app,
2220
use crate::utils::build::{
2321
is_aab_file, is_apk_file, is_zip_file, normalize_directory, write_version_metadata,
2422
};
23+
use crate::utils::build_vcs::collect_git_metadata;
2524
use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL};
2625
use crate::utils::ci::is_ci;
2726
use crate::utils::fs::get_sha1_checksums;
2827
use crate::utils::fs::TempDir;
2928
use crate::utils::fs::TempFile;
3029
use crate::utils::progress::ProgressBar;
31-
use crate::utils::vcs::{
32-
self, get_github_base_ref, get_github_head_ref, get_github_pr_number, get_provider_from_remote,
33-
get_repo_from_remote_preserve_case, git_repo_base_ref, git_repo_base_repo_name_preserve_case,
34-
git_repo_head_ref, git_repo_remote_url,
35-
};
3630

3731
pub fn make_command(command: Command) -> Command {
3832
#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
@@ -54,51 +48,7 @@ pub fn make_command(command: Command) -> Command {
5448
.action(ArgAction::Append)
5549
.required(true),
5650
)
57-
.arg(
58-
Arg::new("head_sha")
59-
.long("head-sha")
60-
.value_parser(parse_sha_allow_empty)
61-
.help("The VCS commit sha to use for the upload. If not provided, the current commit sha will be used.")
62-
)
63-
.arg(
64-
Arg::new("base_sha")
65-
.long("base-sha")
66-
.value_parser(parse_sha_allow_empty)
67-
.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.")
68-
)
69-
.arg(
70-
Arg::new("vcs_provider")
71-
.long("vcs-provider")
72-
.help("The VCS provider to use for the upload. If not provided, the current provider will be used.")
73-
)
74-
.arg(
75-
Arg::new("head_repo_name")
76-
.long("head-repo-name")
77-
.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.")
78-
)
79-
.arg(
80-
Arg::new("base_repo_name")
81-
.long("base-repo-name")
82-
.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.")
83-
)
84-
.arg(
85-
Arg::new("head_ref")
86-
.long("head-ref")
87-
.help("The reference (branch) to use for the upload. If not provided, the current reference will be used.")
88-
)
89-
.arg(
90-
Arg::new("base_ref")
91-
.long("base-ref")
92-
.help("The base reference (branch) to use for the upload. If not provided, the merge-base with the remote tracking branch will be used.")
93-
)
94-
.arg(
95-
Arg::new("pr_number")
96-
.long("pr-number")
97-
.value_parser(clap::value_parser!(u32))
98-
.help("The pull request number to use for the upload. If not provided and running \
99-
in a pull_request-triggered GitHub Actions workflow, the PR number will be automatically \
100-
detected from GitHub Actions environment variables.")
101-
)
51+
.git_metadata_args()
10252
.arg(
10353
Arg::new("build_configuration")
10454
.long("build-configuration")
@@ -119,22 +69,6 @@ pub fn make_command(command: Command) -> Command {
11969
for each other.",
12070
)
12171
)
122-
.arg(
123-
Arg::new("force_git_metadata")
124-
.long("force-git-metadata")
125-
.action(ArgAction::SetTrue)
126-
.conflicts_with("no_git_metadata")
127-
.help("Force collection and sending of git metadata (branch, commit, etc.). \
128-
If neither this nor --no-git-metadata is specified, git metadata is \
129-
automatically collected when running in most CI environments.")
130-
)
131-
.arg(
132-
Arg::new("no_git_metadata")
133-
.long("no-git-metadata")
134-
.action(ArgAction::SetTrue)
135-
.conflicts_with("force_git_metadata")
136-
.help("Disable collection and sending of git metadata.")
137-
)
13872
}
13973

14074
/// Parse plugin info from SENTRY_PIPELINE environment variable.
@@ -309,181 +243,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
309243
Ok(())
310244
}
311245

312-
/// Collects git metadata from arguments and VCS introspection.
313-
///
314-
/// When `auto_collect` is false, only explicitly provided values are collected;
315-
/// automatic inference from git repository and CI environment is skipped.
316-
fn collect_git_metadata(
317-
matches: &ArgMatches,
318-
config: &Config,
319-
auto_collect: bool,
320-
) -> VcsInfo<'static> {
321-
let head_sha = matches
322-
.get_one::<Option<Digest>>("head_sha")
323-
.map(|d| d.as_ref().cloned())
324-
.or_else(|| auto_collect.then(|| vcs::find_head_sha().ok()))
325-
.flatten();
326-
327-
let cached_remote = config.get_cached_vcs_remote();
328-
let (vcs_provider, head_repo_name, head_ref, base_ref, base_repo_name) = {
329-
let repo = if auto_collect {
330-
git2::Repository::open_from_env().ok()
331-
} else {
332-
None
333-
};
334-
let repo_ref = repo.as_ref();
335-
let remote_url = repo_ref.and_then(|repo| git_repo_remote_url(repo, &cached_remote).ok());
336-
337-
let vcs_provider = matches
338-
.get_one("vcs_provider")
339-
.cloned()
340-
.or_else(|| {
341-
auto_collect
342-
.then(|| remote_url.as_ref().map(|url| get_provider_from_remote(url)))?
343-
})
344-
.unwrap_or_default();
345-
346-
let head_repo_name = matches
347-
.get_one("head_repo_name")
348-
.cloned()
349-
.or_else(|| {
350-
auto_collect.then(|| {
351-
remote_url
352-
.as_ref()
353-
.map(|url| get_repo_from_remote_preserve_case(url))
354-
})?
355-
})
356-
.unwrap_or_default();
357-
358-
let head_ref = matches
359-
.get_one("head_ref")
360-
.cloned()
361-
.or_else(|| auto_collect.then(get_github_head_ref)?)
362-
.or_else(|| {
363-
auto_collect.then(|| {
364-
repo_ref.and_then(|r| match git_repo_head_ref(r) {
365-
Ok(ref_name) => {
366-
debug!("Found current branch reference: {ref_name}");
367-
Some(ref_name)
368-
}
369-
Err(e) => {
370-
debug!("No valid branch reference found (likely detached HEAD): {e}");
371-
None
372-
}
373-
})
374-
})?
375-
})
376-
.unwrap_or_default();
377-
378-
let base_ref = matches
379-
.get_one("base_ref")
380-
.cloned()
381-
.or_else(|| auto_collect.then(get_github_base_ref)?)
382-
.or_else(|| {
383-
auto_collect.then(|| {
384-
repo_ref.and_then(|r| match git_repo_base_ref(r, &cached_remote) {
385-
Ok(base_ref_name) => {
386-
debug!("Found base reference: {base_ref_name}");
387-
Some(base_ref_name)
388-
}
389-
Err(e) => {
390-
info!("Could not detect base branch reference: {e}");
391-
None
392-
}
393-
})
394-
})?
395-
})
396-
.unwrap_or_default();
397-
398-
let base_repo_name = matches
399-
.get_one("base_repo_name")
400-
.cloned()
401-
.or_else(|| {
402-
auto_collect.then(|| {
403-
repo_ref.and_then(|r| match git_repo_base_repo_name_preserve_case(r) {
404-
Ok(Some(base_repo_name)) => {
405-
debug!("Found base repository name: {base_repo_name}");
406-
Some(base_repo_name)
407-
}
408-
Ok(None) => {
409-
debug!("No base repository found - not a fork");
410-
None
411-
}
412-
Err(e) => {
413-
warn!("Could not detect base repository name: {e}");
414-
None
415-
}
416-
})
417-
})?
418-
})
419-
.unwrap_or_default();
420-
421-
(
422-
vcs_provider,
423-
head_repo_name,
424-
head_ref,
425-
base_ref,
426-
base_repo_name,
427-
)
428-
};
429-
430-
let base_sha_from_user = matches.get_one::<Option<Digest>>("base_sha").is_some();
431-
let base_ref_from_user = matches.get_one::<String>("base_ref").is_some();
432-
433-
let mut base_sha = matches
434-
.get_one::<Option<Digest>>("base_sha")
435-
.map(|d| d.as_ref().cloned())
436-
.or_else(|| {
437-
if auto_collect {
438-
Some(
439-
vcs::find_base_sha(&cached_remote)
440-
.inspect_err(|e| debug!("Error finding base SHA: {e}"))
441-
.ok()
442-
.flatten(),
443-
)
444-
} else {
445-
None
446-
}
447-
})
448-
.flatten();
449-
450-
let mut base_ref = base_ref;
451-
452-
// If base_sha equals head_sha and both were auto-inferred, skip setting base_sha and base_ref
453-
if !base_sha_from_user
454-
&& !base_ref_from_user
455-
&& base_sha.is_some()
456-
&& head_sha.is_some()
457-
&& base_sha == head_sha
458-
{
459-
debug!(
460-
"Base SHA equals head SHA ({}), and both were auto-inferred. Skipping base_sha and base_ref, but keeping head_sha.",
461-
base_sha.expect("base_sha is Some at this point")
462-
);
463-
base_sha = None;
464-
base_ref = "".into();
465-
}
466-
467-
let pr_number = matches.get_one("pr_number").copied().or_else(|| {
468-
if auto_collect {
469-
get_github_pr_number()
470-
} else {
471-
None
472-
}
473-
});
474-
475-
VcsInfo {
476-
head_sha,
477-
base_sha,
478-
vcs_provider: Cow::Owned(vcs_provider),
479-
head_repo_name: Cow::Owned(head_repo_name),
480-
base_repo_name: Cow::Owned(base_repo_name),
481-
head_ref: Cow::Owned(head_ref),
482-
base_ref: Cow::Owned(base_ref),
483-
pr_number,
484-
}
485-
}
486-
487246
fn handle_file(
488247
path: &Path,
489248
byteview: &ByteView,
@@ -709,22 +468,6 @@ fn upload_file(
709468
}
710469
}
711470

712-
/// Utility function to parse a SHA1 digest, allowing empty strings.
713-
///
714-
/// Empty strings result in Ok(None), otherwise we return the parsed digest
715-
/// or an error if the SHA is invalid.
716-
fn parse_sha_allow_empty(sha: &str) -> Result<Option<Digest>> {
717-
if sha.is_empty() {
718-
return Ok(None);
719-
}
720-
721-
let digest = sha
722-
.parse()
723-
.with_context(|| format!("{sha} is not a valid SHA1 digest"))?;
724-
725-
Ok(Some(digest))
726-
}
727-
728471
#[cfg(not(windows))]
729472
#[cfg(test)]
730473
mod tests {

0 commit comments

Comments
 (0)