github, jobsh: split some of the basic variety code into a reusable crate

Author: jclulow

Cargo.lock

@@ -17,6 +17,7 @@ members = [
 	"github/hooktypes",
 	"github/server",
 	"github/testdata",
+	"jobsh",
 	"server",
 	"sse",
 	"types",

Cargo.toml

@@ -13,7 +13,7 @@ use rand::distributions::Alphanumeric;
 use rand::{thread_rng, Rng};
 use regex::Regex;
 use rusty_ulid::Ulid;
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use slog::{o, Drain, Logger};
 
 pub fn read_toml<P: AsRef<Path>, T>(n: P) -> Result<T>
@@ -207,3 +207,10 @@ pub fn looks_like_a_ulid(s: &str) -> bool {
 pub fn true_if_missing() -> bool {
     true
 }
+
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
+#[serde(untagged)]
+pub enum StringOrBool {
+    String(String),
+    Bool(bool),
+}

common/src/lib.rs

@@ -11,6 +11,7 @@ doctest = false
 buildomat-common = { path = "../../common" }
 buildomat-database = { path = "../../database" }
 buildomat-github-hooktypes = { path = "../hooktypes" }
+buildomat-jobsh = { path = "../../jobsh" }
 
 anyhow = { workspace = true }
 chrono = { workspace = true }

github/database/Cargo.toml

@@ -187,6 +187,14 @@ pub struct JobFileDepend {
     pub config: serde_json::Value,
 }
 
+impl From<buildomat_jobsh::jobfile::JobFileDepend> for JobFileDepend {
+    fn from(value: buildomat_jobsh::jobfile::JobFileDepend) -> Self {
+        let buildomat_jobsh::jobfile::JobFileDepend { job, config } = value;
+
+        Self { job, config }
+    }
+}
+
 pub struct CheckRunDependency(JobFileDepend);
 
 impl CheckRunDependency {

github/database/src/tables/check_run.rs

@@ -2,6 +2,8 @@
  * Copyright 2024 Oxide Computer Company
  */
 
+use buildomat_jobsh::jobfile;
+
 use super::check_run::{CheckRunVariety, JobFileDepend};
 use super::sublude::*;
 
@@ -38,114 +40,30 @@ pub struct JobFile {
     pub dependencies: HashMap<String, JobFileDepend>,
 }
 
-#[derive(Deserialize)]
-struct FrontMatter {
-    name: String,
-    variety: CheckRunVariety,
-    #[serde(default = "true_if_missing")]
-    enable: bool,
-    #[serde(default)]
-    dependencies: HashMap<String, FrontMatterDepend>,
-    #[serde(flatten)]
-    extra: toml::Value,
-}
-
-#[derive(Deserialize)]
-struct FrontMatterDepend {
-    job: String,
-    #[serde(flatten)]
-    extra: toml::Value,
-}
-
-impl JobFile {
-    /**
-     * Lift the TOML frontmatter out of a job file and parse the global values.
-     * Returns None if the file is valid, but not enabled.
-     */
-    pub fn parse_content_at_path(
-        content: &str,
-        path: &str,
-    ) -> Result<Option<JobFile>> {
-        let mut lines = content.lines();
-
-        if let Some(shebang) = lines.next() {
-            /*
-             * For now, we accept any script and assume it is effectively
-             * bourne-compatible, at least with respect to comments.
-             */
-            if !shebang.starts_with("#!") {
-                bail!("{:?} must have an interpreter line", path);
-            }
-        };
-
-        /*
-         * Extract lines after the interpreter line that begin with "#:".  Treat
-         * this as a TOML block wrapped in something that bourne shells will
-         * ignore as a comment.  Allow the use of regular comments interspersed
-         * with TOML lines, as long as there are no blank lines.
-         */
-        let frontmatter = lines
-            .by_ref()
-            .take_while(|l| l.starts_with('#'))
-            .filter(|l| l.starts_with("#:"))
-            .map(|l| l.trim_start_matches("#:"))
-            .collect::<Vec<_>>()
-            .join("\n");
-
-        /*
-         * Parse the front matter as TOML:
-         */
-        let toml =
-            toml::from_str::<FrontMatter>(&frontmatter).map_err(|e| {
-                anyhow!("TOML front matter in {path:?}: {}", e.message())
-            })?;
-
-        if !toml.enable {
-            /*
-             * Skip job files that have been marked as disabled.
-             */
-            return Ok(None);
+impl From<jobfile::JobFile> for JobFile {
+    fn from(value: jobfile::JobFile) -> Self {
+        let jobfile::JobFile {
+            path,
+            name,
+            variety,
+            config,
+            content,
+            dependencies,
+        } = value;
+
+        Self {
+            path,
+            name,
+            variety: match variety {
+                jobfile::Variety::Basic => CheckRunVariety::Basic,
+            },
+            config,
+            content,
+            dependencies: dependencies
+                .into_iter()
+                .map(|(a, b)| (a.into(), b.into()))
+                .collect(),
         }
-
-        /*
-         * Rule out some common mispellings of "enable", before we get
-         * all the way into variety processing:
-         */
-        if toml.extra.get("enabled").is_some()
-            || toml.extra.get("disable").is_some()
-            || toml.extra.get("disabled").is_some()
-        {
-            bail!(
-                "TOML front matter in {path:?}: \
-                use \"enable\" to disable a job"
-            );
-        }
-
-        Ok(Some(JobFile {
-            path: path.to_string(),
-            name: toml.name.to_string(),
-            variety: toml.variety,
-            /*
-             * The use of the flattened "extra" member here is critical, as it
-             * allows varieties to use "deny_unknown_fields" on their subset of
-             * the frontmatter because we have subtracted the global parts here.
-             */
-            config: serde_json::to_value(&toml.extra)?,
-            content: content.to_string(),
-            dependencies: toml
-                .dependencies
-                .iter()
-                .map(|(name, dep)| {
-                    Ok((
-                        name.to_string(),
-                        JobFileDepend {
-                            job: dep.job.to_string(),
-                            config: serde_json::to_value(&dep.extra)?,
-                        },
-                    ))
-                })
-                .collect::<Result<_>>()?,
-        }))
     }
 }
 

github/database/src/tables/check_suite.rs

@@ -7,7 +7,9 @@ mod sublude {
     pub use std::str::FromStr;
 
     pub use crate::itypes::*;
+    #[allow(unused_imports)]
     pub use anyhow::{anyhow, bail, Result};
+    #[allow(unused_imports)]
     pub use buildomat_common::*;
     pub use buildomat_database::{
         rusqlite, sqlite_json_new_type, sqlite_sql_enum, FromRow,

github/database/src/tables/mod.rs

@@ -12,6 +12,7 @@ buildomat-download = { path = "../../download" }
 buildomat-github-client = { path = "../client" }
 buildomat-github-database = { path = "../database" }
 buildomat-github-hooktypes = { path = "../hooktypes" }
+buildomat-jobsh = { path = "../../jobsh" }
 buildomat-sse = { path = "../../sse" }
 
 ansi-to-html = { workspace = true }

github/server/Cargo.toml

@@ -7,7 +7,7 @@ use buildomat_client::prelude::*;
 use buildomat_common::*;
 use buildomat_download::RequestContextEx;
 use buildomat_github_database::types::*;
-use buildomat_sse::{HeaderMapEx, ServerSentEvents};
+use buildomat_sse::HeaderMapEx;
 use chrono::prelude::*;
 use dropshot::{
     endpoint, ConfigDropshot, HttpError, HttpResponseOk, RequestContext,
@@ -309,7 +309,7 @@ async fn details(
          * The <style> tag needs to appear inside the <head>:
          */
         out += "<style>\n";
-        out += &app.templates.load("variety/basic/style.css")?;
+        out += &app.templates.load("variety/basic/www/style.css")?;
         out += "</style>\n";
     }
     out += "</head>\n";
@@ -381,41 +381,6 @@ async fn details_live(
     let path = path.into_inner();
     let query = query.into_inner();
 
-    /*
-     * The "Last-Event-ID" header will be sent by a browser when reconnecting,
-     * with the "id" field of the last event it saw in the previous stream.  The
-     * event stream for this endpoint is a mixture of job events, which have a
-     * well-defined sequence number, and status change events, which do not.
-     *
-     * We include in each ID value the sequence number of the most recently sent
-     * job event, so that we can always seek to the right point in the events
-     * for the job.  This may lead to more than one event with the same sequence
-     * number, but that doesn't appear to be a problem in practice.
-     *
-     * Note that this value must take precedence over the query parameter, as a
-     * resumed stream from the browser will, each time it reconnects, include
-     * the original query string we gave to the EventSource.  It will only
-     * include the header on subsequent retries once it has seen at least one
-     * event.
-     */
-    let mut minseq = None;
-    if let Some(lei) = rc.request.headers().last_event_id() {
-        if let Some(num) = lei.strip_prefix("seq-") {
-            if let Ok(lei_seq) = num.parse::<u32>() {
-                if let Some(lei_seq) = lei_seq.checked_add(1) {
-                    /*
-                     * Resume the event stream from this earlier point.
-                     */
-                    minseq = Some(lei_seq);
-                }
-            }
-        }
-    }
-
-    if minseq.is_none() {
-        minseq = query.minseq;
-    }
-
     let load = match path.load(&rc) {
         Ok(Some(load)) => load,
         Ok(None) => return html_404(false),
@@ -425,28 +390,26 @@ async fn details_live(
         }
     };
 
-    let mut sse = ServerSentEvents::default();
-    let response = match sse.to_response() {
-        Ok(response) => response,
-        Err(e) => {
-            error!(rc.log, "details live: sse: {e}");
-            return html_500(&rc.request_id, false);
-        }
-    };
-
-    let ok = match load.cr.variety {
+    let res = match load.cr.variety {
         CheckRunVariety::Basic => {
-            variety::basic::live(app, &load.cs, &load.cr, minseq, sse).await
+            variety::basic::live(
+                app,
+                &load.cs,
+                &load.cr,
+                rc.request.headers().last_event_id(),
+                query.minseq,
+            )
+            .await
         }
         /*
          * No other variety exposes live details:
          */
-        _ => Ok(false),
+        _ => Ok(None),
     };
 
-    match ok {
-        Ok(true) => Ok(response),
-        Ok(false) => html_404(false),
+    match res {
+        Ok(Some(res)) => Ok(res),
+        Ok(None) => html_404(false),
         Err(e) => {
             error!(rc.log, "details live: {e}");
             html_500(&rc.request_id, false)