Skip to content

feat(build): preserve repository name case for build upload #2777

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.

Sign up for GitHub

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 6 commits into from
Sep 23, 2025
Merged

feat(build): preserve repository name case for build upload #2777

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad1dc4c
feat(build): preserve repository name case for build upload
runningcode Sep 19, 2025
ffd38bb
fix: resolve clippy warnings
runningcode Sep 19, 2025
6f24dd7
refactor: simplify case-preserving VCS URL parsing
runningcode Sep 22, 2025
6e4c6d7
refactor: remove unused preserve_case parameter
runningcode Sep 22, 2025
d0b5024
fix: address clippy warning for wrong_self_convention
runningcode Sep 23, 2025
39346bf
Update src/utils/vcs.rs
runningcode Sep 23, 2025
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
8 changes: 4 additions & 4 deletions src/commands/build/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ 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, git_repo_base_ref, git_repo_base_repo_name, git_repo_head_ref,
git_repo_remote_url,
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 @@ -141,7 +141,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
.or_else(|| {
remote_url
.as_ref()
.map(|url| get_repo_from_remote(url))
.map(|url| get_repo_from_remote_preserve_case(url))
.map(Cow::Owned)
});

Expand Down Expand Up @@ -202,7 +202,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
.or_else(|| {
// Try to get the base repo name from the VCS if not provided
repo_ref
.and_then(|r| match git_repo_base_repo_name(r) {
.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)
Expand Down
77 changes: 66 additions & 11 deletions src/utils/vcs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ fn strip_git_suffix(s: &str) -> &str {

impl VcsUrl {
pub fn parse(url: &str) -> VcsUrl {
Self::parse_preserve_case(url).into_lowercase()
}

pub fn parse_preserve_case(url: &str) -> VcsUrl {
lazy_static! {
static ref GIT_URL_RE: Regex =
Regex::new(r"^(?:(?:git\+)?(?:git|ssh|https?))://(?:[^@]+@)?([^/]+)/(.+)$")
Expand All @@ -129,6 +133,13 @@ impl VcsUrl {
}
}

fn into_lowercase(self) -> VcsUrl {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had these two clippy warning with function you suggested so I adjusted it to what I have in the code.

warning: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference
        --> src/utils/vcs.rs:136:21
         |
     136 |     fn to_lowercase(mut self) -> VcsUrl {
         |                     ^^^^^^^^
         |
         = help: consider choosing a less ambiguous name
         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
         = note: `#[warn(clippy::wrong_self_convention)]` on by default

  warning: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference
        --> src/utils/vcs.rs:136:21
         |
     136 |     fn to_lowercase(self) -> VcsUrl {
         |                     ^^^^
         |
         = help: consider choosing a less ambiguous name
         = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
         = note: `#[warn(clippy::wrong_self_convention)]` on by default

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if clippy complained, then this is the right thing to do!

VcsUrl {
provider: self.provider,
id: self.id.to_lowercase(),
}
}

fn from_git_parts(host: &str, path: &str) -> VcsUrl {
// Azure Devops has multiple domains and multiple URL styles for the
// various different API versions.
Expand Down Expand Up @@ -157,13 +168,13 @@ impl VcsUrl {
if let Some(caps) = VS_GIT_PATH_RE.captures(path) {
return VcsUrl {
provider: host.to_lowercase(),
id: format!("{}/{}", username.to_lowercase(), &caps[1].to_lowercase()),
id: format!("{username}/{}", &caps[1]),
};
}
if let Some(caps) = VS_TRAILING_GIT_PATH_RE.captures(path) {
return VcsUrl {
provider: host.to_lowercase(),
id: caps[1].to_lowercase(),
id: caps[1].to_string(),
};
}
}
Expand All @@ -173,13 +184,13 @@ impl VcsUrl {
if let Some(caps) = AZUREDEV_VERSION_PATH_RE.captures(path) {
return VcsUrl {
provider: hostname.into(),
id: format!("{}/{}", &caps[1].to_lowercase(), &caps[2].to_lowercase()),
id: format!("{}/{}", &caps[1], &caps[2]),
};
}
if let Some(caps) = VS_TRAILING_GIT_PATH_RE.captures(path) {
return VcsUrl {
provider: hostname.to_lowercase(),
id: caps[1].to_lowercase(),
id: caps[1].to_string(),
};
}
}
Expand All @@ -196,13 +207,13 @@ impl VcsUrl {
if let Some(caps) = BITBUCKET_SERVER_PATH_RE.captures(path) {
return VcsUrl {
provider: host.to_lowercase(),
id: format!("{}/{}", &caps[1].to_lowercase(), &caps[2].to_lowercase()),
id: format!("{}/{}", &caps[1], &caps[2]),
};
}

VcsUrl {
provider: host.to_lowercase(),
id: strip_git_suffix(path).to_lowercase(),
id: strip_git_suffix(path).to_owned(),
}
}
}
Expand All @@ -221,6 +232,13 @@ pub fn get_repo_from_remote(repo: &str) -> String {
obj.id
}

/// Like get_repo_from_remote but preserves the original case of the repository name.
/// This is used specifically for build upload where case preservation is important.
pub fn get_repo_from_remote_preserve_case(repo: &str) -> String {
let obj = VcsUrl::parse_preserve_case(repo);
obj.id
}

pub fn get_provider_from_remote(remote: &str) -> String {
let obj = VcsUrl::parse(remote);
extract_provider_name(&obj.provider).to_owned()
Expand Down Expand Up @@ -288,10 +306,9 @@ fn find_merge_base_ref(
Ok(merge_base_sha)
}

/// Attempts to get the base repository name from git remotes.
/// Prefers "upstream" remote if it exists, then "origin", otherwise uses the first available remote.
/// Returns the base repository name if a remote is found.
pub fn git_repo_base_repo_name(repo: &git2::Repository) -> Result<Option<String>> {
/// Like git_repo_base_repo_name but preserves the original case of the repository name.
/// This is used specifically for build upload where case preservation is important.
pub fn git_repo_base_repo_name_preserve_case(repo: &git2::Repository) -> Result<Option<String>> {
let remotes = repo.remotes()?;
let remote_names: Vec<&str> = remotes.iter().flatten().collect();

Expand All @@ -312,7 +329,8 @@ pub fn git_repo_base_repo_name(repo: &git2::Repository) -> Result<Option<String>
match git_repo_remote_url(repo, chosen_remote) {
Ok(remote_url) => {
debug!("Found remote '{}': {}", chosen_remote, remote_url);
Ok(Some(get_repo_from_remote(&remote_url)))
let repo_name = get_repo_from_remote_preserve_case(&remote_url);
Ok(Some(repo_name))
Copy link

Choose a reason for hiding this comment

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

Bug: Git Function Renamed, Case Sensitivity Changed

The git_repo_base_repo_name function was replaced by git_repo_base_repo_name_preserve_case, removing the original lowercasing behavior. This is a breaking change for any code expecting a lowercase repository name. The new function's documentation also incorrectly references the removed original function.

Fix in Cursor Fix in Web

}
Err(e) => {
warn!("Could not get URL for remote '{}': {}", chosen_remote, e);
Expand Down Expand Up @@ -942,6 +960,43 @@ mod tests {
);
}

#[test]
fn test_get_repo_from_remote_preserve_case() {
// Test that case-preserving function maintains original casing
assert_eq!(
get_repo_from_remote_preserve_case("https://github.com/MyOrg/MyRepo"),
"MyOrg/MyRepo"
);
assert_eq!(
get_repo_from_remote_preserve_case("[email protected]:SentryOrg/SentryRepo.git"),
"SentryOrg/SentryRepo"
);
assert_eq!(
get_repo_from_remote_preserve_case("https://gitlab.com/MyCompany/MyProject"),
"MyCompany/MyProject"
);
assert_eq!(
get_repo_from_remote_preserve_case("[email protected]:TeamName/ProjectName.git"),
"TeamName/ProjectName"
);
assert_eq!(
get_repo_from_remote_preserve_case("ssh://[email protected]/MyUser/MyRepo.git"),
"MyUser/MyRepo"
);

// Test that regular function still lowercases
assert_eq!(
get_repo_from_remote("https://github.com/MyOrg/MyRepo"),
"myorg/myrepo"
);

// Test edge cases - should fall back to lowercase when regex doesn't match
assert_eq!(
get_repo_from_remote_preserve_case("invalid-url"),
get_repo_from_remote("invalid-url")
);
}

#[test]
fn test_extract_provider_name() {
// Test basic provider name extraction
Expand Down