From d9450a287d7457b95f5fed5013c80c2cf207156e Mon Sep 17 00:00:00 2001 From: Henrique Ferreiro Date: Wed, 15 Jan 2025 11:09:06 +0100 Subject: [PATCH 1/6] Add a git hosting provider for Chromium Add an implementation of GitHostingProvider for repositories hosted on https://chromium.googlesource.com. Pull requests target the Gerrit instance at https://chromium-review.googlesource.com and avatar images are fetched using the Gerrit REST API. Release Notes: - Support repositories hosted on https://chromium.googlesource.com --- .../src/git_hosting_providers.rs | 1 + crates/git_hosting_providers/src/providers.rs | 2 + .../src/providers/chromium.rs | 301 ++++++++++++++++++ .../src/providers/codeberg.rs | 6 +- .../src/providers/github.rs | 4 +- 5 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 crates/git_hosting_providers/src/providers/chromium.rs diff --git a/crates/git_hosting_providers/src/git_hosting_providers.rs b/crates/git_hosting_providers/src/git_hosting_providers.rs index 8916d7a42e3cf2..6dc846d9f30cb8 100644 --- a/crates/git_hosting_providers/src/git_hosting_providers.rs +++ b/crates/git_hosting_providers/src/git_hosting_providers.rs @@ -12,6 +12,7 @@ pub use crate::providers::*; pub fn init(cx: &App) { let provider_registry = GitHostingProviderRegistry::global(cx); provider_registry.register_hosting_provider(Arc::new(Bitbucket)); + provider_registry.register_hosting_provider(Arc::new(Chromium)); provider_registry.register_hosting_provider(Arc::new(Codeberg)); provider_registry.register_hosting_provider(Arc::new(Gitee)); provider_registry.register_hosting_provider(Arc::new(Github)); diff --git a/crates/git_hosting_providers/src/providers.rs b/crates/git_hosting_providers/src/providers.rs index 68541541cf7108..e106c04355c2c9 100644 --- a/crates/git_hosting_providers/src/providers.rs +++ b/crates/git_hosting_providers/src/providers.rs @@ -4,6 +4,7 @@ mod gitee; mod github; mod gitlab; mod sourcehut; +mod chromium; pub use bitbucket::*; pub use codeberg::*; @@ -11,3 +12,4 @@ pub use gitee::*; pub use github::*; pub use gitlab::*; pub use sourcehut::*; +pub use chromium::*; diff --git a/crates/git_hosting_providers/src/providers/chromium.rs b/crates/git_hosting_providers/src/providers/chromium.rs new file mode 100644 index 00000000000000..78872376977487 --- /dev/null +++ b/crates/git_hosting_providers/src/providers/chromium.rs @@ -0,0 +1,301 @@ +use std::str::FromStr; +use std::sync::{Arc, LazyLock}; + +use anyhow::{bail, Context, Result}; +use async_trait::async_trait; +use futures::AsyncReadExt; +use http_client::{AsyncBody, HttpClient, HttpRequestExt, Request}; +use regex::Regex; +use serde::Deserialize; +use url::Url; + +use git::{ + BuildCommitPermalinkParams, BuildPermalinkParams, GitHostingProvider, Oid, ParsedGitRemote, + PullRequest, RemoteUrl, +}; + +static CHROMIUM_REVIEW_URL: &str = "https://chromium-review.googlesource.com"; + +// Parse Gerrit URLs like +// https://chromium-review.googlesource.com/c/chromium/src/+/3310961. +fn pull_request_regex() -> &'static Regex { + static PULL_REQUEST_NUMBER_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(&format!( + r#"Reviewed-on: ({CHROMIUM_REVIEW_URL}/c/(.*)/\+/(\d+))"# + )) + .unwrap() + }); + &PULL_REQUEST_NUMBER_REGEX +} + +// https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html +#[derive(Debug, Deserialize)] +struct ChangeInfo { + owner: AccountInfo, +} + +#[derive(Debug, Deserialize)] +pub struct AccountInfo { + #[serde(rename = "_account_id")] + id: u64, +} + +pub struct Chromium; + +impl Chromium { + async fn fetch_chromium_commit_author( + &self, + _repo: &str, + commit: &str, + client: &Arc, + ) -> Result> { + let url = format!("{CHROMIUM_REVIEW_URL}/changes/{commit}"); + + let request = Request::get(&url) + .header("Content-Type", "application/json") + .follow_redirects(http_client::RedirectPolicy::FollowAll); + + let mut response = client + .send(request.body(AsyncBody::default())?) + .await + .with_context(|| format!("error fetching Gerrit commit details at {:?}", url))?; + + let mut body = Vec::new(); + response.body_mut().read_to_end(&mut body).await?; + + if response.status().is_client_error() { + let text = String::from_utf8_lossy(body.as_slice()); + bail!( + "status error {}, response: {text:?}", + response.status().as_u16() + ); + } + + // Remove XSSI protection prefix + let body_str = std::str::from_utf8(&body)?.trim_start_matches(")]}'"); + + serde_json::from_str::(body_str) + .map(|change| Some(change.owner)) + .context("failed to deserialize Gerrit change info") + } +} + +#[async_trait] +impl GitHostingProvider for Chromium { + fn name(&self) -> String { + "Chromium".to_string() + } + + fn base_url(&self) -> Url { + Url::parse("https://chromium.googlesource.com").unwrap() + } + + fn supports_avatars(&self) -> bool { + true + } + + fn format_line_number(&self, line: u32) -> String { + format!("{line}") + } + + fn format_line_numbers(&self, start_line: u32, _end_line: u32) -> String { + format!("{start_line}") + } + + fn parse_remote_url(&self, url: &str) -> Option { + let url = RemoteUrl::from_str(url).ok()?; + + let host = url.host_str()?; + if host != self.base_url().host_str()? { + return None; + } + + let path_segments = url.path_segments()?.collect::>(); + let joined_path = path_segments.join("/"); + let repo = joined_path.trim_end_matches(".git"); + + Some(ParsedGitRemote { + owner: Arc::from(""), + repo: repo.into(), + }) + } + + fn build_commit_permalink( + &self, + remote: &ParsedGitRemote, + params: BuildCommitPermalinkParams, + ) -> Url { + let BuildCommitPermalinkParams { sha } = params; + let ParsedGitRemote { owner: _, repo } = remote; + + self.base_url().join(&format!("{repo}/+/{sha}")).unwrap() + } + + fn build_permalink(&self, remote: ParsedGitRemote, params: BuildPermalinkParams) -> Url { + let ParsedGitRemote { owner: _, repo } = remote; + let BuildPermalinkParams { + sha, + path, + selection, + } = params; + + let mut permalink = self + .base_url() + .join(&format!("{repo}/+/{sha}/{path}")) + .unwrap(); + permalink.set_fragment( + selection + .map(|selection| self.line_fragment(&selection)) + .as_deref(), + ); + permalink + } + + fn extract_pull_request(&self, remote: &ParsedGitRemote, message: &str) -> Option { + let capture = pull_request_regex().captures(message)?; + let url = Url::parse(capture.get(1)?.as_str()).unwrap(); + let repo = capture.get(2)?.as_str(); + if repo != remote.repo.as_ref() { + return None; + } + + let number = capture.get(3)?.as_str().parse::().ok()?; + + Some(PullRequest { number, url }) + } + + async fn commit_author_avatar_url( + &self, + _repo_owner: &str, + repo: &str, + commit: Oid, + http_client: Arc, + ) -> Result> { + let commit = commit.to_string(); + let avatar_url = self + .fetch_chromium_commit_author(repo, &commit, &http_client) + .await? + .map(|author| -> Result { + let mut url = Url::parse(&format!( + "{CHROMIUM_REVIEW_URL}/accounts/{}/avatar", + &author.id + ))?; + url.set_query(Some("size=128")); + Ok(url) + }) + .transpose()?; + Ok(avatar_url) + } +} + +#[cfg(test)] +mod tests { + use indoc::indoc; + use pretty_assertions::assert_eq; + + use super::*; + + #[test] + fn test_parse_remote_url_given_https_url() { + let parsed_remote = Chromium + .parse_remote_url("https://chromium.googlesource.com/chromium/src") + .unwrap(); + + assert_eq!( + parsed_remote, + ParsedGitRemote { + owner: Arc::from(""), + repo: "chromium/src".into(), + } + ); + } + + #[test] + fn test_build_chromium_permalink() { + let permalink = Chromium.build_permalink( + ParsedGitRemote { + owner: Arc::from(""), + repo: "chromium/src".into(), + }, + BuildPermalinkParams { + sha: "fea5080b182fc92e3be0c01c5dece602fe70b588", + path: "ui/base/cursor/cursor.h", + selection: None, + }, + ); + + let expected_url = "https://chromium.googlesource.com/chromium/src/+/fea5080b182fc92e3be0c01c5dece602fe70b588/ui/base/cursor/cursor.h"; + assert_eq!(permalink.to_string(), expected_url.to_string()) + } + + #[test] + fn test_build_chromium_permalink_with_single_line_selection() { + let permalink = Chromium.build_permalink( + ParsedGitRemote { + owner: Arc::from(""), + repo: "chromium/src".into(), + }, + BuildPermalinkParams { + sha: "fea5080b182fc92e3be0c01c5dece602fe70b588", + path: "ui/base/cursor/cursor.h", + selection: Some(18..18), + }, + ); + + let expected_url = "https://chromium.googlesource.com/chromium/src/+/fea5080b182fc92e3be0c01c5dece602fe70b588/ui/base/cursor/cursor.h#19"; + assert_eq!(permalink.to_string(), expected_url.to_string()) + } + + #[test] + fn test_build_chromium_permalink_with_multi_line_selection() { + let permalink = Chromium.build_permalink( + ParsedGitRemote { + owner: Arc::from(""), + repo: "chromium/src".into(), + }, + BuildPermalinkParams { + sha: "fea5080b182fc92e3be0c01c5dece602fe70b588", + path: "ui/base/cursor/cursor.h", + selection: Some(18..30), + }, + ); + + let expected_url = "https://chromium.googlesource.com/chromium/src/+/fea5080b182fc92e3be0c01c5dece602fe70b588/ui/base/cursor/cursor.h#19"; + assert_eq!(permalink.to_string(), expected_url.to_string()) + } + + #[test] + fn test_chromium_pull_requests() { + let remote = ParsedGitRemote { + owner: Arc::from(""), + repo: "chromium/src".into(), + }; + + let message = "This does not contain a pull request"; + assert!(Chromium.extract_pull_request(&remote, message).is_none()); + + // Pull request number at end of "Reviewed-on:" line + let message = indoc! {r#" + Test commit header + + Test commit description with multiple + lines. + + Bug: 1193775, 1270302 + Change-Id: Id15e9b4d75cce43ebd5fe34f0fb37d5e1e811b66 + Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3310961 + Reviewed-by: Test reviewer + Cr-Commit-Position: refs/heads/main@{#1054973} + "# + }; + + assert_eq!( + Chromium + .extract_pull_request(&remote, &message) + .unwrap() + .url + .as_str(), + "https://chromium-review.googlesource.com/c/chromium/src/+/3310961" + ); + } +} diff --git a/crates/git_hosting_providers/src/providers/codeberg.rs b/crates/git_hosting_providers/src/providers/codeberg.rs index 0e01331278a6f2..b88585388b2e2c 100644 --- a/crates/git_hosting_providers/src/providers/codeberg.rs +++ b/crates/git_hosting_providers/src/providers/codeberg.rs @@ -34,9 +34,9 @@ struct Author { #[derive(Debug, Deserialize)] struct User { - pub login: String, - pub id: u64, - pub avatar_url: String, + login: String, + id: u64, + avatar_url: String, } pub struct Codeberg; diff --git a/crates/git_hosting_providers/src/providers/github.rs b/crates/git_hosting_providers/src/providers/github.rs index f86b586ea8c701..e98f8eaee71b2a 100644 --- a/crates/git_hosting_providers/src/providers/github.rs +++ b/crates/git_hosting_providers/src/providers/github.rs @@ -39,8 +39,8 @@ struct Author { #[derive(Debug, Deserialize)] struct User { - pub id: u64, - pub avatar_url: String, + id: u64, + avatar_url: String, } pub struct Github; From 756150b8aa2e351a31b721b883c868a406b8473c Mon Sep 17 00:00:00 2001 From: Henrique Ferreiro Date: Fri, 14 Feb 2025 22:56:29 +0100 Subject: [PATCH 2/6] Sort providers.rs and update commit_author_avatar_url argument type --- crates/git_hosting_providers/src/providers.rs | 4 ++-- crates/git_hosting_providers/src/providers/chromium.rs | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/git_hosting_providers/src/providers.rs b/crates/git_hosting_providers/src/providers.rs index e106c04355c2c9..c94b830f582779 100644 --- a/crates/git_hosting_providers/src/providers.rs +++ b/crates/git_hosting_providers/src/providers.rs @@ -1,15 +1,15 @@ mod bitbucket; +mod chromium; mod codeberg; mod gitee; mod github; mod gitlab; mod sourcehut; -mod chromium; pub use bitbucket::*; +pub use chromium::*; pub use codeberg::*; pub use gitee::*; pub use github::*; pub use gitlab::*; pub use sourcehut::*; -pub use chromium::*; diff --git a/crates/git_hosting_providers/src/providers/chromium.rs b/crates/git_hosting_providers/src/providers/chromium.rs index 78872376977487..1f7f92ef7ab75a 100644 --- a/crates/git_hosting_providers/src/providers/chromium.rs +++ b/crates/git_hosting_providers/src/providers/chromium.rs @@ -4,13 +4,14 @@ use std::sync::{Arc, LazyLock}; use anyhow::{bail, Context, Result}; use async_trait::async_trait; use futures::AsyncReadExt; +use gpui::SharedString; use http_client::{AsyncBody, HttpClient, HttpRequestExt, Request}; use regex::Regex; use serde::Deserialize; use url::Url; use git::{ - BuildCommitPermalinkParams, BuildPermalinkParams, GitHostingProvider, Oid, ParsedGitRemote, + BuildCommitPermalinkParams, BuildPermalinkParams, GitHostingProvider, ParsedGitRemote, PullRequest, RemoteUrl, }; @@ -168,7 +169,7 @@ impl GitHostingProvider for Chromium { &self, _repo_owner: &str, repo: &str, - commit: Oid, + commit: SharedString, http_client: Arc, ) -> Result> { let commit = commit.to_string(); From 4077e5c715e970e642b80f0012c6daa7b0be0855 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 20 Feb 2025 18:23:22 -0500 Subject: [PATCH 3/6] Revert changes to other providers --- crates/git_hosting_providers/src/providers/codeberg.rs | 6 +++--- crates/git_hosting_providers/src/providers/github.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/git_hosting_providers/src/providers/codeberg.rs b/crates/git_hosting_providers/src/providers/codeberg.rs index b88585388b2e2c..0e01331278a6f2 100644 --- a/crates/git_hosting_providers/src/providers/codeberg.rs +++ b/crates/git_hosting_providers/src/providers/codeberg.rs @@ -34,9 +34,9 @@ struct Author { #[derive(Debug, Deserialize)] struct User { - login: String, - id: u64, - avatar_url: String, + pub login: String, + pub id: u64, + pub avatar_url: String, } pub struct Codeberg; diff --git a/crates/git_hosting_providers/src/providers/github.rs b/crates/git_hosting_providers/src/providers/github.rs index e98f8eaee71b2a..f86b586ea8c701 100644 --- a/crates/git_hosting_providers/src/providers/github.rs +++ b/crates/git_hosting_providers/src/providers/github.rs @@ -39,8 +39,8 @@ struct Author { #[derive(Debug, Deserialize)] struct User { - id: u64, - avatar_url: String, + pub id: u64, + pub avatar_url: String, } pub struct Github; From 03becd8d109ec3d2eb31b9a6db6ee04e74f7353c Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 20 Feb 2025 18:44:15 -0500 Subject: [PATCH 4/6] Do some cleanup --- .../src/providers/chromium.rs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/git_hosting_providers/src/providers/chromium.rs b/crates/git_hosting_providers/src/providers/chromium.rs index 1f7f92ef7ab75a..8f4e10480e476d 100644 --- a/crates/git_hosting_providers/src/providers/chromium.rs +++ b/crates/git_hosting_providers/src/providers/chromium.rs @@ -17,8 +17,8 @@ use git::{ static CHROMIUM_REVIEW_URL: &str = "https://chromium-review.googlesource.com"; -// Parse Gerrit URLs like -// https://chromium-review.googlesource.com/c/chromium/src/+/3310961. +/// Parses Gerrit URLs like +/// https://chromium-review.googlesource.com/c/chromium/src/+/3310961. fn pull_request_regex() -> &'static Regex { static PULL_REQUEST_NUMBER_REGEX: LazyLock = LazyLock::new(|| { Regex::new(&format!( @@ -29,7 +29,7 @@ fn pull_request_regex() -> &'static Regex { &PULL_REQUEST_NUMBER_REGEX } -// https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html +/// https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html #[derive(Debug, Deserialize)] struct ChangeInfo { owner: AccountInfo, @@ -72,7 +72,7 @@ impl Chromium { ); } - // Remove XSSI protection prefix + // Remove XSSI protection prefix. let body_str = std::str::from_utf8(&body)?.trim_start_matches(")]}'"); serde_json::from_str::(body_str) @@ -173,19 +173,20 @@ impl GitHostingProvider for Chromium { http_client: Arc, ) -> Result> { let commit = commit.to_string(); - let avatar_url = self + let Some(author) = self .fetch_chromium_commit_author(repo, &commit, &http_client) .await? - .map(|author| -> Result { - let mut url = Url::parse(&format!( - "{CHROMIUM_REVIEW_URL}/accounts/{}/avatar", - &author.id - ))?; - url.set_query(Some("size=128")); - Ok(url) - }) - .transpose()?; - Ok(avatar_url) + else { + return Ok(None); + }; + + let mut avatar_url = Url::parse(&format!( + "{CHROMIUM_REVIEW_URL}/accounts/{}/avatar", + &author.id + ))?; + avatar_url.set_query(Some("size=128")); + + Ok(Some(avatar_url)) } } From 81577c4d90788c8782dba92741fada1b5477b853 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 20 Feb 2025 18:54:21 -0500 Subject: [PATCH 5/6] Use a `const` instead of a `static` --- crates/git_hosting_providers/src/providers/chromium.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/git_hosting_providers/src/providers/chromium.rs b/crates/git_hosting_providers/src/providers/chromium.rs index 8f4e10480e476d..bfbc49a22267cd 100644 --- a/crates/git_hosting_providers/src/providers/chromium.rs +++ b/crates/git_hosting_providers/src/providers/chromium.rs @@ -15,7 +15,7 @@ use git::{ PullRequest, RemoteUrl, }; -static CHROMIUM_REVIEW_URL: &str = "https://chromium-review.googlesource.com"; +const CHROMIUM_REVIEW_URL: &str = "https://chromium-review.googlesource.com"; /// Parses Gerrit URLs like /// https://chromium-review.googlesource.com/c/chromium/src/+/3310961. From f5fd5a4397d8c6c96e01c33a6241264068ed6b6a Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 20 Feb 2025 18:54:40 -0500 Subject: [PATCH 6/6] Organize imports --- crates/git_hosting_providers/src/providers/chromium.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/git_hosting_providers/src/providers/chromium.rs b/crates/git_hosting_providers/src/providers/chromium.rs index bfbc49a22267cd..b651144b666b00 100644 --- a/crates/git_hosting_providers/src/providers/chromium.rs +++ b/crates/git_hosting_providers/src/providers/chromium.rs @@ -4,17 +4,16 @@ use std::sync::{Arc, LazyLock}; use anyhow::{bail, Context, Result}; use async_trait::async_trait; use futures::AsyncReadExt; +use git::{ + BuildCommitPermalinkParams, BuildPermalinkParams, GitHostingProvider, ParsedGitRemote, + PullRequest, RemoteUrl, +}; use gpui::SharedString; use http_client::{AsyncBody, HttpClient, HttpRequestExt, Request}; use regex::Regex; use serde::Deserialize; use url::Url; -use git::{ - BuildCommitPermalinkParams, BuildPermalinkParams, GitHostingProvider, ParsedGitRemote, - PullRequest, RemoteUrl, -}; - const CHROMIUM_REVIEW_URL: &str = "https://chromium-review.googlesource.com"; /// Parses Gerrit URLs like