feat(checkouts): generalize change fetching via git references
This way, we can plug any VCS and just leave it to the Git reference converter. We implement a very simple one for Gerrit. Signed-off-by: Raito Bezarius <masterancpp@gmail.com>
This commit is contained in:
parent
9e46987a27
commit
debb1abcd7
7 changed files with 62 additions and 13 deletions
|
@ -29,6 +29,8 @@ async fn main() -> Result<(), Box<dyn Error>> {
|
|||
};
|
||||
|
||||
let pr_msg = Change {
|
||||
provenance: ofborg::tasks::evaluate::SupportedVCS::Gerrit,
|
||||
r#ref: "refs/pull/42/head:pr".to_string(),
|
||||
number: 42,
|
||||
head_sha: "6dd9f0265d52b946dd13daf996f30b64e4edb446".to_owned(),
|
||||
target_branch: Some("scratch".to_owned()),
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
use crate::clone::{self, GitClonable};
|
||||
|
||||
use std::ffi::{OsStr, OsString};
|
||||
use std::fmt::Debug;
|
||||
use std::fs;
|
||||
use std::io::{Error, ErrorKind};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
@ -31,6 +32,11 @@ pub struct CachedProjectCo {
|
|||
local_reference: PathBuf,
|
||||
}
|
||||
|
||||
pub trait ToGitRef {
|
||||
/// This transforms the current change representation to a git reference that you can fetch.
|
||||
fn to_git_ref(&self) -> String;
|
||||
}
|
||||
|
||||
impl CachedCloner {
|
||||
#[must_use]
|
||||
pub fn project(&self, name: &str, clone_url: String) -> CachedProject {
|
||||
|
@ -96,14 +102,14 @@ impl CachedProjectCo {
|
|||
Ok(self.clone_to().to_str().unwrap().to_string())
|
||||
}
|
||||
|
||||
pub fn fetch_pr(&self, pr_id: u64) -> Result<(), Error> {
|
||||
pub fn fetch_ref(&self, git_ref: &str) -> Result<(), Error> {
|
||||
let mut lock = self.lock()?;
|
||||
|
||||
info!("Fetching PR #{}", pr_id);
|
||||
info!("Fetching ref #{}", git_ref);
|
||||
let result = Command::new("git")
|
||||
.arg("fetch")
|
||||
.arg("origin")
|
||||
.arg(format!("+refs/pull/{pr_id}/head:pr"))
|
||||
.arg(format!("+{git_ref}"))
|
||||
.current_dir(self.clone_to())
|
||||
.stdout(Stdio::null())
|
||||
.status()?;
|
||||
|
@ -113,10 +119,18 @@ impl CachedProjectCo {
|
|||
if result.success() {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(Error::new(ErrorKind::Other, "Failed to fetch PR"))
|
||||
Err(Error::new(
|
||||
ErrorKind::Other,
|
||||
format!("Failed to fetch reference {git_ref}"),
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
pub fn fetch_change<C: ToGitRef + Debug>(&self, change: &C) -> Result<(), Error> {
|
||||
tracing::debug!("Fetching change {:?}", change);
|
||||
self.fetch_ref(&change.to_git_ref())
|
||||
}
|
||||
|
||||
pub fn commit_exists(&self, commit: &OsStr) -> bool {
|
||||
let mut lock = self.lock().expect("Failed to lock");
|
||||
|
||||
|
|
|
@ -1,3 +1,7 @@
|
|||
use std::fmt::Display;
|
||||
|
||||
use crate::{checkout::ToGitRef, tasks::evaluate::SupportedVCS};
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone)]
|
||||
pub struct Repo {
|
||||
pub owner: String,
|
||||
|
@ -8,7 +12,25 @@ pub struct Repo {
|
|||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone)]
|
||||
pub struct Change {
|
||||
pub provenance: SupportedVCS,
|
||||
pub target_branch: Option<String>,
|
||||
pub number: u64,
|
||||
pub head_sha: String,
|
||||
// Git reference for this change
|
||||
pub r#ref: String,
|
||||
}
|
||||
|
||||
impl ToGitRef for Change {
|
||||
fn to_git_ref(&self) -> String {
|
||||
self.r#ref.clone()
|
||||
}
|
||||
}
|
||||
|
||||
impl Display for Change {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_fmt(format_args!(
|
||||
"VCS change (prov: {:?}) #{} targeting {:?} on {}",
|
||||
self.provenance, self.number, self.target_branch, self.r#ref
|
||||
))
|
||||
}
|
||||
}
|
||||
|
|
|
@ -335,8 +335,8 @@ impl notifyworker::SimpleNotifyWorker for BuildWorker {
|
|||
|
||||
let refpath = co.checkout_origin_ref(target_branch.as_ref()).unwrap();
|
||||
|
||||
if co.fetch_pr(job.change.number).is_err() {
|
||||
info!("Failed to fetch {}", job.change.number);
|
||||
if co.fetch_change(&job.change).is_err() {
|
||||
info!("Failed to fetch {}", job.change);
|
||||
actions.pr_head_missing().await;
|
||||
return;
|
||||
}
|
||||
|
|
|
@ -21,6 +21,7 @@ use std::time::Instant;
|
|||
use async_trait::async_trait;
|
||||
use tracing::{debug_span, error, info, warn};
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize)]
|
||||
pub enum SupportedVCS {
|
||||
Gerrit,
|
||||
}
|
||||
|
@ -373,8 +374,7 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> {
|
|||
.set_with_description("Fetching PR", State::Pending)
|
||||
.await?;
|
||||
|
||||
// TODO: generalize fetch change
|
||||
co.fetch_pr(job.change.number).unwrap();
|
||||
co.fetch_change(&job.change).unwrap();
|
||||
|
||||
if !co.commit_exists(job.change.head_sha.as_ref()) {
|
||||
overall_status
|
||||
|
|
|
@ -1,5 +1,6 @@
|
|||
/// This transforms a Gerrit event into a `OfBorg` event.
|
||||
use crate::message::vcs::VCSEvent;
|
||||
use crate::tasks::evaluate::SupportedVCS;
|
||||
use crate::vcs::gerrit::data_structures::GerritStreamEvent;
|
||||
use crate::worker;
|
||||
use async_trait::async_trait;
|
||||
|
@ -60,11 +61,13 @@ impl worker::SimpleWorker for GerritEventFilterWorker {
|
|||
GerritStreamEvent::ChangeDeleted { change, .. }
|
||||
| GerritStreamEvent::ChangeAbandoned { change, .. } => Some(VCSEvent::ChangeDeleted {
|
||||
change: crate::message::Change {
|
||||
provenance: SupportedVCS::Gerrit,
|
||||
target_branch: Some(change.branch.clone()),
|
||||
number: change
|
||||
.change_number
|
||||
.expect("Change deletion payload should contain a CL number"),
|
||||
head_sha: change.current_patch_set.as_ref().unwrap().r#ref.clone(),
|
||||
head_sha: change.current_patch_set.as_ref().unwrap().revision.clone(),
|
||||
r#ref: change.current_patch_set.as_ref().unwrap().r#ref.clone(),
|
||||
},
|
||||
repo: crate::message::Repo {
|
||||
owner: "no one really".to_string(),
|
||||
|
@ -76,6 +79,7 @@ impl worker::SimpleWorker for GerritEventFilterWorker {
|
|||
GerritStreamEvent::ChangeRestored { change, .. }
|
||||
| GerritStreamEvent::PatchSetCreated { change, .. } => Some(VCSEvent::ChangeCreated {
|
||||
change: crate::message::Change {
|
||||
provenance: SupportedVCS::Gerrit,
|
||||
target_branch: Some(change.branch.clone()),
|
||||
number: change
|
||||
.change_number
|
||||
|
@ -84,8 +88,9 @@ impl worker::SimpleWorker for GerritEventFilterWorker {
|
|||
.current_patch_set
|
||||
.as_ref()
|
||||
.expect("Patch set creation payload should contain a current patch set")
|
||||
.r#ref
|
||||
.revision
|
||||
.clone(),
|
||||
r#ref: change.current_patch_set.as_ref().unwrap().r#ref.clone(),
|
||||
},
|
||||
repo: crate::message::Repo {
|
||||
owner: "no one really".to_string(),
|
||||
|
@ -98,9 +103,11 @@ impl worker::SimpleWorker for GerritEventFilterWorker {
|
|||
change, patch_set, ..
|
||||
} => Some(VCSEvent::WIPStatusChange {
|
||||
change: crate::message::Change {
|
||||
provenance: SupportedVCS::Gerrit,
|
||||
target_branch: Some(change.branch.clone()),
|
||||
number: change.change_number.unwrap(),
|
||||
head_sha: patch_set.r#ref.clone(),
|
||||
head_sha: patch_set.revision.clone(),
|
||||
r#ref: patch_set.r#ref.clone(),
|
||||
},
|
||||
wip: change.wip.unwrap(),
|
||||
}),
|
||||
|
|
|
@ -2,6 +2,8 @@ use std::collections::HashMap;
|
|||
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::tasks::evaluate::SupportedVCS;
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug)]
|
||||
pub struct Account {
|
||||
pub name: Option<String>, // User's full name, if configured
|
||||
|
@ -212,8 +214,9 @@ pub struct Change {
|
|||
}
|
||||
|
||||
impl From<Change> for crate::message::Change {
|
||||
fn from(value: Change) -> Self {
|
||||
fn from(mut value: Change) -> Self {
|
||||
Self {
|
||||
provenance: SupportedVCS::Gerrit,
|
||||
target_branch: Some(value.branch),
|
||||
// While the change number is deprecated, we actually need it.
|
||||
// FIXME: enforce type level checking of this.
|
||||
|
@ -222,7 +225,8 @@ impl From<Change> for crate::message::Change {
|
|||
// basically, some events like `comment-added` does not include the current patch set
|
||||
// because it's already included in another field, we could probably do another
|
||||
// conversion pass to improve the type safety.
|
||||
head_sha: value.current_patch_set.unwrap().revision,
|
||||
head_sha: std::mem::take(&mut value.current_patch_set.as_mut().unwrap().revision),
|
||||
r#ref: std::mem::take(&mut value.current_patch_set.as_mut().unwrap().r#ref),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue