-
-
Notifications
You must be signed in to change notification settings - Fork 245
fix(releases): handle partial SHAs correctly in commit resolution #2734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
d0c6e74
fix partial sha & add tests
srest2021 eeadf4f
update tests
srest2021 501e432
linter
srest2021 2355ec6
linter
srest2021 cdd97cc
make git test consistent with other tests
srest2021 713678c
move git test out of mod
srest2021 b662ada
use find_matching_rev
srest2021 04df02d
preserve partial sha when sending to api
srest2021 6dad2ac
linter
srest2021 fd030f2
rename
srest2021 651c7c2
clean up comments
srest2021 e5983dc
fix logic & tests
srest2021 43830ac
fix test
srest2021 7c165ff
linter
srest2021 898af8a
Merge branch 'master' into srest2021/REPLAY-413
szokeasaurusrex 6cc111f
only valid full & partial SHAs accepted
srest2021 8b900ad
Merge branch 'master' into srest2021/REPLAY-413
srest2021 acf91a5
revert prior changes
srest2021 40b1f39
linteR
srest2021 9859a9c
Update src/commands/releases/set_commits.rs
srest2021 85d334c
soft validation
srest2021 8ddca69
linter
srest2021 6ecf8b0
Merge branch 'master' into srest2021/REPLAY-413
srest2021 116ce99
update validaton & help
srest2021 2c4f96e
Update src/commands/releases/set_commits.rs
srest2021 4dd3548
Update src/commands/releases/set_commits.rs
srest2021 6d3fa1c
add 64 char constant
srest2021 c515e78
Update src/constants.rs
srest2021 307ed46
Make the example SHA have hex chars only
srest2021 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| #![expect(clippy::unwrap_used, reason = "contains legacy code which uses unwrap")] | ||
|
|
||
| use std::sync::LazyLock; | ||
|
|
||
| use anyhow::{bail, Result}; | ||
| use clap::{Arg, ArgAction, ArgMatches, Command}; | ||
| use lazy_static::lazy_static; | ||
| use regex::Regex; | ||
|
|
||
| use crate::api::{Api, NewRelease, NoneReleaseInfo, OptionalReleaseInfo, UpdatedRelease}; | ||
| use crate::api::{Api, NewRelease, NoneReleaseInfo, OptionalReleaseInfo, Ref, UpdatedRelease}; | ||
| use crate::config::Config; | ||
| use crate::utils::args::ArgExt as _; | ||
| use crate::utils::formatting::Table; | ||
|
|
@@ -55,17 +57,13 @@ pub fn make_command(command: Command) -> Command { | |
| .action(ArgAction::Append) | ||
| .help("Defines a single commit for a repo as \ | ||
| identified by the repo name in the remote Sentry config. \ | ||
| If no commit has been specified sentry-cli will attempt \ | ||
| to auto discover that repository in the local git repo \ | ||
| and then use the HEAD commit. This will either use the \ | ||
| current git repository or attempt to auto discover a \ | ||
| submodule with a compatible URL.\n\n\ | ||
| The value can be provided as `REPO` in which case sentry-cli \ | ||
| will auto-discover the commit based on reachable repositories. \ | ||
| Alternatively it can be provided as `REPO#PATH` in which case \ | ||
| the current commit of the repository at the given PATH is \ | ||
| assumed. To override the revision `@REV` can be appended \ | ||
| which will force the revision to a certain value.")) | ||
| The value must be provided as `REPO@SHA` where SHA is a \ | ||
| Git commit SHA. To specify a range, use `REPO@PREV_SHA..SHA` \ | ||
| format.\n\n\ | ||
| Examples:\n\ | ||
| - `my-repo@abc123` (partial SHA)\n\ | ||
| - `my-repo@abc123def456789...` (full SHA)\n\ | ||
| - `my-repo@abc123..def456` (commit range)")) | ||
| // Legacy flag that has no effect, left hidden for backward compatibility | ||
| .arg(Arg::new("ignore-empty") | ||
| .long("ignore-empty") | ||
|
|
@@ -83,14 +81,21 @@ fn strip_sha(sha: &str) -> &str { | |
| sha | ||
| } | ||
| } | ||
|
|
||
| /// Validates that a string is a valid Git SHA (full or partial) | ||
| fn is_valid_sha(sha: &str) -> bool { | ||
| static SHA_RE: LazyLock<Regex> = | ||
| LazyLock::new(|| Regex::new(r"^[a-fA-F0-9]{4,40}$").expect("Regex is valid")); | ||
| SHA_RE.is_match(sha) | ||
| } | ||
|
|
||
| pub fn execute(matches: &ArgMatches) -> Result<()> { | ||
| let config = Config::current(); | ||
| let api = Api::current(); | ||
| let authenticated_api = api.authenticated()?; | ||
| let version = matches.get_one::<String>("version").unwrap(); | ||
| let org = config.get_org(matches)?; | ||
| let repos = authenticated_api.list_organization_repos(&org)?; | ||
| let mut commit_specs = vec![]; | ||
|
|
||
| let heads = if repos.is_empty() { | ||
| None | ||
|
|
@@ -105,30 +110,44 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { | |
| Some(vec![]) | ||
| } else if matches.get_flag("local") { | ||
| None | ||
| } else { | ||
| if let Some(commits) = matches.get_many::<String>("commits") { | ||
| for spec in commits { | ||
| let commit_spec = CommitSpec::parse(spec)?; | ||
| if repos | ||
| .iter() | ||
| .any(|r| r.name.to_lowercase() == commit_spec.repo.to_lowercase()) | ||
| { | ||
| commit_specs.push(commit_spec); | ||
| } else { | ||
| bail!("Unknown repo '{}'", commit_spec.repo); | ||
| } else if let Some(commits) = matches.get_many::<String>("commits") { | ||
| let mut refs = vec![]; | ||
| for spec in commits { | ||
| let commit_spec = CommitSpec::parse(spec)?; | ||
|
|
||
| if !repos | ||
| .iter() | ||
| .any(|r| r.name.to_lowercase() == commit_spec.repo.to_lowercase()) | ||
| { | ||
| bail!("Unknown repo '{}'", commit_spec.repo); | ||
| } | ||
|
|
||
| if !is_valid_sha(&commit_spec.rev) { | ||
| eprintln!( | ||
| "Warning: Invalid commit SHA '{}'. Only Git SHAs (full or partial) are supported. Proceeding anyway.", | ||
| commit_spec.rev | ||
| ); | ||
| } | ||
|
|
||
| if let Some(ref prev_rev) = commit_spec.prev_rev { | ||
| if !is_valid_sha(prev_rev) { | ||
| eprintln!("Warning: Invalid previous commit SHA '{prev_rev}'. Only Git SHAs (full or partial) are supported. Proceeding anyway."); | ||
| } | ||
| } | ||
|
srest2021 marked this conversation as resolved.
Outdated
|
||
|
|
||
| refs.push(Ref { | ||
| repo: commit_spec.repo, | ||
| rev: commit_spec.rev, | ||
| prev_rev: commit_spec.prev_rev, | ||
| }); | ||
| } | ||
| let commits = find_heads( | ||
| Some(commit_specs), | ||
| &repos, | ||
| Some(config.get_cached_vcs_remote()), | ||
| )?; | ||
| if commits.is_empty() { | ||
| if refs.is_empty() { | ||
| None | ||
| } else { | ||
| Some(commits) | ||
| Some(refs) | ||
| } | ||
| } else { | ||
| None | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Command Change Breaks Implicit Commit DiscoveryThe |
||
| }; | ||
|
|
||
| // make sure the release exists if projects are given | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.