wip! wip! wip! HEAD of work

delete commit status, make it compile, etc.

Signed-off-by: Raito Bezarius <masterancpp@gmail.com>

ofborg/src/tasks/eval/mod.rs

@@ -4,7 +4,8 @@ pub mod stdenvs;
 pub use self::nixpkgs::NixpkgsStrategy;
 pub use self::stdenvs::Stdenvs;
 use crate::message::buildjob::BuildJob;
-use crate::vcs::generic::{CheckRun, CommitStatusError};
+use crate::vcs::generic::{ChangeStatusError, CheckRun};
+use thiserror::Error;
 
 pub type StepResult<T> = Result<T, Error>;
 
@@ -14,15 +15,18 @@ pub struct EvaluationComplete {
     pub checks: Vec<CheckRun>,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
 pub enum Error {
-    CommitStatusWrite(CommitStatusError),
+    #[error("While updating the status of this change, encountered: {0}")]
+    ChangeStatusWrite(ChangeStatusError),
+    #[error("Step failed: {0}")]
     Fail(String),
+    #[error("Step failed: {0} - details will be available in a pastebin")]
     FailWithPastebin(String, String, String),
 }
 
-impl From<CommitStatusError> for Error {
+impl From<ChangeStatusError> for Error {
-    fn from(e: CommitStatusError) -> Error {
+    fn from(e: ChangeStatusError) -> Error {
-        Error::CommitStatusWrite(e)
+        Error::ChangeStatusWrite(e)
     }
 }

ofborg/src/tasks/eval/nixpkgs.rs

@@ -259,7 +259,7 @@ impl<'a> NixpkgsStrategy<'a> {
                 if !attrs.is_empty() {
                     overall_status
                         .set_status_link(self.gist_changed_paths(&attrs).await)
-                        .await;
+                        .await?;
                     self.record_impacted_maintainers(dir, &attrs).await?;
                 }
 
@@ -305,7 +305,7 @@ impl<'a> NixpkgsStrategy<'a> {
 
             status
                 .create(self.vcs_api.clone(), CheckRunState::Running)
-                .await;
+                .await?;
 
             let m =
                 ImpactedMaintainers::calculate(&self.nix, dir, changed_paths, &changed_attributes)
@@ -335,7 +335,7 @@ impl<'a> NixpkgsStrategy<'a> {
                         CheckRunState::Completed,
                         gist_url,
                     )
-                    .await;
+                    .await?;
                 return Ok(());
             }
 
@@ -345,7 +345,7 @@ impl<'a> NixpkgsStrategy<'a> {
                     CheckRunState::Completed,
                     gist_url,
                 )
-                .await;
+                .await?;
 
             if let Ok(ref maint) = m {
                 self.request_reviews(maint).await;
@@ -370,7 +370,7 @@ impl<'a> NixpkgsStrategy<'a> {
                 .label_name("Verified")
                 .build();
 
-            status.update_status(CheckRunState::Running).await;
+            status.update_status(CheckRunState::Running).await?;
 
             let nixenv = HydraNixEnv::new(self.nix.clone(), dir.to_path_buf(), true);
             match nixenv.execute_with_stats().await {
@@ -384,7 +384,7 @@ impl<'a> NixpkgsStrategy<'a> {
                     try_build.sort();
                     try_build.dedup();
 
-                    status.update_status(CheckRunState::Completed).await;
+                    status.update_status(CheckRunState::Completed).await?;
 
                     if !try_build.is_empty() && try_build.len() <= 20 {
                         // In the case of trying to merge master in to
@@ -418,7 +418,7 @@ impl<'a> NixpkgsStrategy<'a> {
                             .ok()
                             .map(|pp| pp.uri),
                         )
-                        .await;
+                        .await?;
                     // TODO: add a failed result with the details.
                     Err(Error::Fail(String::from(
                         "Failed to validate package metadata.",
@@ -446,7 +446,7 @@ impl<'a> NixpkgsStrategy<'a> {
                     "The branch you have targeted is a read-only mirror for channels. \
                     Please target release-* or master.",
                 )
-                .await;
+                .await?;
 
             info!("PR targets a nixos-* or nixpkgs-* branch");
             return Ok(false);
@@ -466,7 +466,7 @@ impl<'a> NixpkgsStrategy<'a> {
                 CheckRunState::Scheduled,
                 None,
             )
-            .await;
+            .await?;
         self.check_stdenvs_before(dir).await;
 
         status
@@ -475,7 +475,7 @@ impl<'a> NixpkgsStrategy<'a> {
                 CheckRunState::Scheduled,
                 None,
             )
-            .await;
+            .await?;
         self.check_outpaths_before(dir).await?;
 
         Ok(())
@@ -504,7 +504,7 @@ impl<'a> NixpkgsStrategy<'a> {
 
         status
             .update_status_with_description("Checking new stdenvs", CheckRunState::Scheduled, None)
-            .await;
+            .await?;
         self.check_stdenvs_after().await;
 
         status
@@ -513,7 +513,7 @@ impl<'a> NixpkgsStrategy<'a> {
                 CheckRunState::Scheduled,
                 None,
             )
-            .await;
+            .await?;
         self.check_outpaths_after().await?;
 
         Ok(())
@@ -674,7 +674,7 @@ impl<'a> NixpkgsStrategy<'a> {
                 CheckRunState::Scheduled,
                 None,
             )
-            .await;
+            .await?;
 
         self.update_new_package_labels().await;
         self.update_rebuild_labels(dir, status).await?;

ofborg/src/tasks/evaluate.rs

@@ -268,37 +268,41 @@ impl<'a, E: stats::SysEvents + 'static> OneEval<'a, E> {
         let job = self.job;
         let auto_schedule_build_archs: Vec<systems::System>;
 
-        // TODO: determine if the job refers to a change:
+        // 1. Fetch information about this change again via the API.
-        // - already merged
+        match self
-        // - currently a work in progress
+            .vcs_api
-        // let _issue: Issue = match issue_ref {
+            .get_change(&self.job.repo, self.job.change.number)
-        //     Ok(iss) => {
+            .await
-        //         if matches!(iss.state, IssueState::Closed) {
+        {
-        //             self.events.notify(Event::IssueAlreadyClosed).await;
+            Some(change) => {
-        //             info!("Skipping {} because it is closed", job.change.number);
+                if matches!(change.state, ChangeState::Merged)
-        //             return Ok(Actions::skip(job));
+                    || matches!(change.state, ChangeState::Abandoned)
-        //         }
+                {
+                    self.events.notify(Event::IssueAlreadyClosed).await;
+                    info!(
+                        "Skipping {} because it is either merged or abandoned",
+                        job.change.number
+                    );
+                    return Ok(Actions::skip(job));
+                }
 
-        //         if iss.is_wip() {
+                if matches!(change.state, ChangeState::WorkInProgress) {
-        //             self.events.notify(Event::CurrentlyWorkInProgress).await;
+                    self.events.notify(Event::CurrentlyWorkInProgress).await;
-        //             auto_schedule_build_archs = vec![];
+                    auto_schedule_build_archs = vec![];
-        //         } else {
+                } else {
-        //             auto_schedule_build_archs = self.acl.build_job_architectures_for_user_repo(
+                    auto_schedule_build_archs = self.acl.build_job_architectures_for_user_repo(
-        //                 &iss.created_by.username,
+                        &change.owner.username,
-        //                 &job.repo.full_name,
+                        &job.repo.full_name,
-        //             );
+                    );
-        //         }
+                }
-
+            }
-        //         iss
+            None => {
-        //     }
+                // NOTE(legacy): Issue meant PR here.
-
+                self.events.notify(Event::IssueFetchFailed).await;
-        //     Err(e) => {
+                error!("Could not find {}!", job.change.number);
-        //         self.events.notify(Event::IssueFetchFailed).await;
+                return Ok(Actions::skip(job));
-        //         error!("Error fetching {}!", job.change.number);
+            }
-        //         error!("E: {:?}", e);
+        }
-        //         return Ok(Actions::skip(job));
-        //     }
-        // };
 
         let mut evaluation_strategy = eval::NixpkgsStrategy::new(
             chan.clone(),

ofborg/src/vcs/generic/api.rs

@@ -62,7 +62,7 @@ pub trait VersionControlSystemAPI: Sync + Send + MinimalVersionControlSystemAPI
         context: String,
         description: String,
         target_url: String,
-    ) -> BoxFuture<Result<(), CommitStatusError>>;
+    ) -> BoxFuture<Result<(), ChangeStatusError>>;
     fn create_check_statuses(
         &self,
         repo: &crate::message::Repo,
@@ -142,7 +142,7 @@ impl<A: MinimalVersionControlSystemAPI> VersionControlSystemAPI for AugmentedVCS
         _context: String,
         _description: String,
         _target_url: String,
-    ) -> BoxFuture<Result<(), CommitStatusError>> {
+    ) -> BoxFuture<Result<(), ChangeStatusError>> {
         // Create the commit status.
         todo!();
     }

ofborg/src/vcs/generic/change_status.rs

@@ -5,10 +5,12 @@ use std::sync::Arc;
 use chrono::NaiveDateTime;
 
 use crate::message::Change;
+use thiserror::Error;
 
 use super::{CheckResult, CheckRunState, VersionControlSystemAPI};
 
 /// This is a structure to control a specific check run and its results.
+#[allow(dead_code)]
 pub struct ChangeStatus {
     // Internal information for the status server.
     change: u64,
@@ -35,8 +37,6 @@ pub struct ChangeStatus {
     results: Vec<CheckResult>,
 }
 
-impl ChangeStatus {}
-
 /// Builder for ChangeStatus.
 pub struct ChangeStatusBuilder {
     change: u64,
@@ -127,38 +127,39 @@ impl ChangeStatus {
     /// This creates the change status over the API, making it possibly visible to VCSes.
     pub async fn create(
         &mut self,
-        api: Arc<dyn VersionControlSystemAPI>,
+        _api: Arc<dyn VersionControlSystemAPI>,
         initial_state: CheckRunState,
-    ) -> Self {
+    ) -> Result<Self> {
         self.status = initial_state;
         todo!();
     }
 
-    pub async fn set_started(&self) {
+    pub async fn set_started(&self) -> Result<()> {
         // Update the started timestamp.
         todo!();
     }
 
-    pub async fn update_description(&self, description: &str) {
+    pub async fn update_description(&self, description: &str) -> Result<()> {
-        todo!();
+        self.update_status_with_description(description, self.status, None)
+            .await
     }
 
     /// This updates the current status of this check with a description.
     pub async fn update_status_with_description(
         &self,
-        description: &str,
+        _description: &str,
-        status: CheckRunState,
+        _status: CheckRunState,
-        link: Option<String>,
+        _link: Option<String>,
-    ) {
+    ) -> Result<()> {
         todo!();
     }
 
-    pub async fn set_status_link(&mut self, link: Option<String>) {
+    pub async fn set_status_link(&mut self, link: Option<String>) -> Result<()> {
         self.status_link = link;
         todo!();
     }
 
-    pub async fn update_status(&self, status: CheckRunState) {
+    pub async fn update_status(&self, _status: CheckRunState) -> Result<()> {
         todo!();
     }
 
@@ -170,7 +171,19 @@ impl ChangeStatus {
     }
 
     /// This sends the results via the API and make them visible to servers.
-    pub async fn send_results(&mut self) -> Self {
+    pub async fn send_results(&mut self) -> Result<Self> {
         todo!();
     }
 }
+
+#[derive(Debug, Error)]
+pub enum ChangeStatusError {
+    #[error("Unauthorized to update the status on a change")]
+    InvalidCredentials,
+    #[error("Credentials expired to update the status on a change")]
+    ExpiredCredentials,
+    #[error("Unexpected error during update")]
+    UnexpectedError(#[from] Box<dyn std::error::Error + Send + Sync>),
+}
+
+type Result<T> = std::result::Result<T, ChangeStatusError>;

ofborg/src/vcs/generic/commit_status.rs

@@ -1,85 +0,0 @@
-use std::sync::Arc;
-
-use tracing::warn;
-
-use super::{CheckRunState, VersionControlSystemAPI};
-
-pub struct CommitStatus {
-    api: Arc<dyn VersionControlSystemAPI>,
-    repo: crate::message::Repo,
-    sha: String,
-    context: String,
-    description: String,
-    url: String,
-}
-
-impl CommitStatus {
-    pub fn new(
-        api: Arc<dyn VersionControlSystemAPI>,
-        repo: crate::message::Repo,
-        sha: String,
-        context: String,
-        description: String,
-        url: Option<String>,
-    ) -> CommitStatus {
-        let mut stat = CommitStatus {
-            api,
-            repo,
-            sha,
-            context,
-            description,
-            url: String::new(),
-        };
-
-        stat.set_url(url);
-
-        stat
-    }
-
-    pub fn set_url(&mut self, url: Option<String>) {
-        self.url = url.unwrap_or_default();
-    }
-
-    pub async fn set_with_description(
-        &mut self,
-        description: &str,
-        state: CheckRunState,
-    ) -> Result<(), CommitStatusError> {
-        self.set_description(description.to_owned());
-        self.set(state).await
-    }
-
-    pub fn set_description(&mut self, description: String) {
-        self.description = description;
-    }
-
-    pub async fn set(&self, state: CheckRunState) -> Result<(), CommitStatusError> {
-        let desc = if self.description.len() >= 140 {
-            warn!(
-                "description is over 140 char; truncating: {:?}",
-                &self.description
-            );
-            self.description.chars().take(140).collect()
-        } else {
-            self.description.clone()
-        };
-
-        self.api
-            .create_commit_statuses(
-                &self.repo,
-                self.sha.clone(),
-                state,
-                self.context.clone(),
-                desc,
-                self.url.clone(),
-            )
-            .await
-    }
-}
-
-#[derive(Debug)]
-pub enum CommitStatusError {
-    ExpiredCreds(()),
-    MissingSha(()),
-    Error(()),
-}

ofborg/src/vcs/generic/mod.rs

@@ -1,10 +1,8 @@
 pub mod api;
 pub mod change_status;
 pub mod checks;
-pub mod commit_status;
 pub mod http;
 
 pub use api::*;
 pub use change_status::*;
 pub use checks::*;
-pub use commit_status::*;