From 13a786d9d7e140dce90bbe145fbeec49957467ae Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Wed, 6 Aug 2025 15:48:45 -0500 Subject: [PATCH] Rework the build-info format to be JSON External services like GH Actions can read JSON but *not* YAML, and the previous format couldn't simply be written "as JSON" because it relied on having structs as keys. This tweaks the format to be JSON-friendly and uses actual JSON files. --- Cargo.lock | 3 +- Cargo.toml | 1 + README.md | 14 ++++----- obo-core/Cargo.toml | 2 +- obo-core/src/actions.rs | 17 +++++------ obo-core/src/build_meta.rs | 51 ++++++++++++++----------------- obo-tests/src/lib.rs | 12 +++----- obs-gitlab-runner/Cargo.toml | 1 + obs-gitlab-runner/src/handler.rs | 24 +++++++-------- obs-gitlab-runner/src/pipeline.rs | 10 +++--- 10 files changed, 63 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 04f3085..8308f3e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1514,7 +1514,7 @@ dependencies = [ "rfc822-like", "rstest", "serde", - "serde_yaml", + "serde_json", "shell-words", "tempfile", "thiserror 2.0.16", @@ -1568,6 +1568,7 @@ dependencies = [ "open-build-service-mock", "rstest", "serde", + "serde_json", "serde_yaml", "shell-words", "shellexpand", diff --git a/Cargo.toml b/Cargo.toml index 7116534..176b3e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ open-build-service-mock = { git = "https://github.com/collabora/open-build-servi # open-build-service-mock = { path = "../open-build-service-rs/open-build-service-api" } rstest = "0.26" serde = "1.0" +serde_json = "1.0.140" serde_yaml = "0.9" shell-words = "1.1" tempfile = "3.20" diff --git a/README.md b/README.md index 6ff07e5..cb35248 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ In order to connect to OBS, three variables must be set (generally within the ```bash dput PROJECT DSC_FILE [--branch-to BRANCHED_PROJECT] - [--build-info-out BUILD_INFO_FILE=build-info.yml] + [--build-info-out BUILD_INFO_FILE=build-info.json] [--rebuild-if-unchanged] ``` @@ -92,7 +92,7 @@ within, will be removed. Metadata information on the uploaded revision, such as the revision number, project name, and package name, will be saved into the file specified by -`--build-info-out` (default is `build-info.yml`). This file is **required** by +`--build-info-out` (default is `build-info.json`). This file is **required** by the `generate-monitor` and `prune` steps. Do note that, if `--branch-to` is given, the file will be written *immediately* after the branch takes place (i.e. before the upload); that way, if the upload fails, the branched project can still @@ -109,7 +109,7 @@ testing builds on MRs; you can create an OBS branch named after the MR's Git branch, and then builds can take place there without interfering with your main projects. -##### `--build-info-out BUILD_INFO_FILE=build-info.yml` +##### `--build-info-out BUILD_INFO_FILE=build-info.json` Changes the filename that the build info will be written to. @@ -130,7 +130,7 @@ operation, there will *always* be a change to upload. generate-monitor RUNNER_TAG [--rules RULES] [--download-build-results-to BUILD_RESULTS_DIR] - [--build-info BUILD_INFO_FILE=build-info.yml] + [--build-info BUILD_INFO_FILE=build-info.json] [--pipeline-out PIPELINE_FILE=obs.yml] [--job-prefix MONITOR_JOB_PREFIX=obs] [--job-timeout MONITOR_JOB_TIMEOUT] @@ -199,7 +199,7 @@ dput-and-generate: After a monitoring job completes, download the build results from OBS to the given `BUILD_RESULTS_DIR`, and upload it as a GitLab build artifact.. -##### `--build-info BUILD_INFO_FILE=build-info.yml` +##### `--build-info BUILD_INFO_FILE=build-info.json` Specifies the name of the build info file to read. In particular, if a different build info filename was used with `dput` via @@ -233,7 +233,7 @@ Changes the filename each monitoring job will save the build log into. ```bash prune - [--build-info BUILD_INFO_FILE=build-info.yml] + [--build-info BUILD_INFO_FILE=build-info.json] [--ignore-missing-build-info] [--only-if-job-unsuccessful] ``` @@ -242,7 +242,7 @@ If a branch occurred, deletes the branched package and, if now empty, project, using the information from the build info file. (If no branching occurred, this does nothing.) -##### `--build-info BUILD_INFO_FILE=build-info.yml` +##### `--build-info BUILD_INFO_FILE=build-info.json` Specifies the name of the build info file to read. In particular, if a different build info filename was used with `dput` via diff --git a/obo-core/Cargo.toml b/obo-core/Cargo.toml index 146640a..da839db 100644 --- a/obo-core/Cargo.toml +++ b/obo-core/Cargo.toml @@ -20,7 +20,7 @@ open-build-service-api.workspace = true reqwest = "0.12" rfc822-like = "0.2" serde.workspace = true -serde_yaml.workspace = true +serde_json.workspace = true shell-words.workspace = true tempfile.workspace = true thiserror.workspace = true diff --git a/obo-core/src/actions.rs b/obo-core/src/actions.rs index 1dfee2c..11244d3 100644 --- a/obo-core/src/actions.rs +++ b/obo-core/src/actions.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, io::SeekFrom}; +use std::io::SeekFrom; use camino::{Utf8Path, Utf8PathBuf}; use clap::{ArgAction, Parser}; @@ -11,10 +11,7 @@ use tracing::{debug, instrument}; use crate::{ artifacts::{ArtifactDirectory, ArtifactReader, ArtifactWriter, MissingArtifactToNone}, binaries::download_binaries, - build_meta::{ - BuildHistoryRetrieval, BuildMeta, BuildMetaOptions, CommitBuildInfo, DisabledRepos, - RepoArch, - }, + build_meta::{BuildHistoryRetrieval, BuildMeta, BuildMetaOptions, DisabledRepos, EnabledRepo}, monitor::{MonitoredPackage, ObsMonitor, PackageCompletion, PackageMonitoringOptions}, outputln, prune::prune_branch, @@ -22,7 +19,7 @@ use crate::{ upload::ObsDscUploader, }; -pub const DEFAULT_BUILD_INFO: &str = "build-info.yml"; +pub const DEFAULT_BUILD_INFO: &str = "build-info.json"; pub const DEFAULT_BUILD_LOG: &str = "build.log"; // Our flags can all take explicit values, because it makes it easier to @@ -155,7 +152,7 @@ pub struct ObsBuildInfo { pub rev: Option, pub srcmd5: Option, pub is_branched: bool, - pub enabled_repos: HashMap, + pub enabled_repos: Vec, } impl ObsBuildInfo { @@ -164,7 +161,7 @@ impl ObsBuildInfo { artifacts .save_with(path, async |file: &mut ArtifactWriter| { let data = - serde_yaml::to_string(&self).wrap_err("Failed to serialize build info")?; + serde_json::to_string(&self).wrap_err("Failed to serialize build info")?; file.write_all(data.as_bytes()) .await .wrap_err("Failed to write build info file")?; @@ -217,7 +214,7 @@ impl Actions { rev: None, srcmd5: None, is_branched, - enabled_repos: HashMap::new(), + enabled_repos: vec![], }; debug!("Saving initial build info: {:?}", build_info); build_info @@ -414,7 +411,7 @@ impl Actions { artifacts.read_string(&args.build_info).await? }; - let build_info: ObsBuildInfo = serde_yaml::from_str(&build_info_data) + let build_info: ObsBuildInfo = serde_json::from_str(&build_info_data) .wrap_err("Failed to parse provided build info file")?; if build_info.is_branched { diff --git a/obo-core/src/build_meta.rs b/obo-core/src/build_meta.rs index c2330b6..41ebad7 100644 --- a/obo-core/src/build_meta.rs +++ b/obo-core/src/build_meta.rs @@ -14,7 +14,9 @@ pub struct RepoArch { } #[derive(Deserialize, Serialize, Debug, Clone)] -pub struct CommitBuildInfo { +pub struct EnabledRepo { + #[serde(flatten)] + pub repo_arch: RepoArch, pub prev_endtime_for_commit: Option, } @@ -237,26 +239,19 @@ impl BuildMeta { Ok(()) } - pub fn get_commit_build_info(&self, srcmd5: &str) -> HashMap { - let mut repos = HashMap::new(); - - for (repo, jobhist) in &self.repos { - let prev_endtime_for_commit = jobhist - .jobhist - .iter() - .filter(|e| e.srcmd5 == srcmd5) - .next_back() - .map(|e| e.endtime); - - repos.insert( - repo.clone(), - CommitBuildInfo { - prev_endtime_for_commit, - }, - ); - } - - repos + pub fn get_commit_build_info(&self, srcmd5: &str) -> Vec { + self.repos + .iter() + .map(|(repo, jobhist)| EnabledRepo { + repo_arch: repo.clone(), + prev_endtime_for_commit: jobhist + .jobhist + .iter() + .filter(|e| e.srcmd5 == srcmd5) + .next_back() + .map(|e| e.endtime), + }) + .collect() } } @@ -360,10 +355,10 @@ mod tests { let build_info = meta.get_commit_build_info(&srcmd5_1); - let arch_1 = assert_some!(build_info.get(&repo_arch_1)); + let arch_1 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_1)); assert_some_eq!(arch_1.prev_endtime_for_commit, endtime_1); - let arch_2 = assert_some!(build_info.get(&repo_arch_2)); + let arch_2 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_2)); assert_none!(arch_2.prev_endtime_for_commit); let meta = assert_ok!( @@ -385,9 +380,9 @@ mod tests { let build_info = meta.get_commit_build_info(&srcmd5_1); - let arch_1 = assert_some!(build_info.get(&repo_arch_1)); + let arch_1 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_1)); assert_none!(arch_1.prev_endtime_for_commit); - let arch_2 = assert_some!(build_info.get(&repo_arch_2)); + let arch_2 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_2)); assert_none!(arch_2.prev_endtime_for_commit); assert!(meta.repos.contains_key(&repo_arch_2)); @@ -431,7 +426,7 @@ mod tests { let build_info = meta.get_commit_build_info(&srcmd5_2); - let arch_1 = assert_some!(build_info.get(&repo_arch_2)); + let arch_1 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_2)); assert_some_eq!(arch_1.prev_endtime_for_commit, endtime_2); mock.add_job_history( @@ -465,13 +460,13 @@ mod tests { let build_info = meta.get_commit_build_info(&srcmd5_1); - let arch_2 = assert_some!(build_info.get(&repo_arch_2)); + let arch_2 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_2)); assert_some_eq!(arch_2.prev_endtime_for_commit, endtime_1); meta.clear_stored_history(); let build_info = meta.get_commit_build_info(&srcmd5_1); - let arch_2 = assert_some!(build_info.get(&repo_arch_2)); + let arch_2 = assert_some!(build_info.iter().find(|e| e.repo_arch == repo_arch_2)); assert_none!(arch_2.prev_endtime_for_commit); mock.set_package_build_status( diff --git a/obo-tests/src/lib.rs b/obo-tests/src/lib.rs index 77466f4..e2d6e97 100644 --- a/obo-tests/src/lib.rs +++ b/obo-tests/src/lib.rs @@ -323,10 +323,8 @@ pub async fn test_dput( let arch_1 = build_info .enabled_repos - .get(&RepoArch { - repo: TEST_REPO.to_owned(), - arch: TEST_ARCH_1.to_owned(), - }) + .iter() + .find(|e| e.repo_arch.repo == TEST_REPO && e.repo_arch.arch == TEST_ARCH_1) .unwrap(); if test == DputTest::Rebuild { @@ -336,10 +334,8 @@ pub async fn test_dput( let arch_2 = build_info .enabled_repos - .get(&RepoArch { - repo: TEST_REPO.to_owned(), - arch: TEST_ARCH_2.to_owned(), - }) + .iter() + .find(|e| e.repo_arch.repo == TEST_REPO && e.repo_arch.arch == TEST_ARCH_2) .unwrap(); assert_none!(arch_2.prev_endtime_for_commit); } diff --git a/obs-gitlab-runner/Cargo.toml b/obs-gitlab-runner/Cargo.toml index ac14003..7b3e203 100644 --- a/obs-gitlab-runner/Cargo.toml +++ b/obs-gitlab-runner/Cargo.toml @@ -19,6 +19,7 @@ obo-core = { path = "../obo-core" } obo-test-support = { path = "../obo-test-support" } open-build-service-api.workspace = true serde.workspace = true +serde_json.workspace = true serde_yaml.workspace = true shellexpand = "3.1" shell-words.workspace = true diff --git a/obs-gitlab-runner/src/handler.rs b/obs-gitlab-runner/src/handler.rs index 7276334..ae825c0 100644 --- a/obs-gitlab-runner/src/handler.rs +++ b/obs-gitlab-runner/src/handler.rs @@ -228,7 +228,7 @@ impl ObsJobHandler { }; let build_info_data = artifacts.read_string(&args.build_info).await?; - let build_info: ObsBuildInfo = serde_yaml::from_str(&build_info_data) + let build_info: ObsBuildInfo = serde_json::from_str(&build_info_data) .wrap_err("Failed to parse provided build info file")?; let pipeline = generate_monitor_pipeline( @@ -448,7 +448,7 @@ mod tests { use claims::*; use gitlab_runner::{GitlabLayer, Runner, RunnerBuilder}; use gitlab_runner_mock::*; - use obo_core::build_meta::{CommitBuildInfo, RepoArch}; + use obo_core::build_meta::{EnabledRepo, RepoArch}; use obo_test_support::*; use obo_tests::*; use rstest::rstest; @@ -868,10 +868,10 @@ mod tests { assert_eq!(pipeline_map.len(), build_info.enabled_repos.len()); - for repo in build_info.enabled_repos.keys() { + for enabled in &build_info.enabled_repos { let monitor_job_name = format!( "{}-{}-{}", - DEFAULT_PIPELINE_JOB_PREFIX, TEST_REPO, &repo.arch + DEFAULT_PIPELINE_JOB_PREFIX, TEST_REPO, &enabled.repo_arch.arch ); let monitor_map = pipeline_yaml @@ -945,7 +945,7 @@ mod tests { context, dput.clone(), build_info, - repo, + &enabled.repo_arch, &script, success, dput_test, @@ -1130,23 +1130,21 @@ mod tests { rev: Some("1".to_owned()), srcmd5: Some("abc".to_owned()), is_branched: false, - enabled_repos: [( - RepoArch { + enabled_repos: vec![EnabledRepo { + repo_arch: RepoArch { repo: TEST_REPO.to_owned(), arch: TEST_ARCH_1.to_owned(), }, - CommitBuildInfo { - prev_endtime_for_commit: None, - }, - )] - .into(), + + prev_endtime_for_commit: None, + }], }; let build_info = context .inject_artifacts( [( DEFAULT_BUILD_INFO.to_owned(), - serde_yaml::to_string(&build_info).unwrap().into_bytes(), + serde_json::to_string(&build_info).unwrap().into_bytes(), )] .into(), ) diff --git a/obs-gitlab-runner/src/pipeline.rs b/obs-gitlab-runner/src/pipeline.rs index 375bb19..a0295fa 100644 --- a/obs-gitlab-runner/src/pipeline.rs +++ b/obs-gitlab-runner/src/pipeline.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use color_eyre::eyre::{Context, Result}; use obo_core::{ actions::{DownloadBinariesAction, MonitorAction}, - build_meta::{CommitBuildInfo, RepoArch}, + build_meta::{EnabledRepo, RepoArch}, }; use serde::Serialize; use tracing::instrument; @@ -51,7 +51,7 @@ pub fn generate_monitor_pipeline( package: &str, rev: &str, srcmd5: &str, - enabled_repos: &HashMap, + enabled_repos: &[EnabledRepo], options: GeneratePipelineOptions, ) -> Result { let rules: Option = options @@ -62,7 +62,9 @@ pub fn generate_monitor_pipeline( .wrap_err("Failed to parse provided rules")?; let mut jobs = HashMap::new(); - for (RepoArch { repo, arch }, info) in enabled_repos { + for enabled in enabled_repos { + let RepoArch { repo, arch } = &enabled.repo_arch; + let mut script = vec![]; let mut artifact_paths = vec![]; @@ -75,7 +77,7 @@ pub fn generate_monitor_pipeline( rev: rev.to_owned(), srcmd5: srcmd5.to_owned(), build_log_out: options.build_log_out.clone(), - prev_endtime_for_commit: info.prev_endtime_for_commit, + prev_endtime_for_commit: enabled.prev_endtime_for_commit, } .generate_command(), );