From 169598d8c40c4dba28048dc87f19ae220b9671b7 Mon Sep 17 00:00:00 2001 From: Marc Jakobi Date: Wed, 1 Jan 2025 19:44:14 +0100 Subject: [PATCH] refactor!(build): use builder pattern --- rocks-bin/src/build.rs | 18 +++---- rocks-lib/src/build/mod.rs | 65 +++++++++++++++++++++++++- rocks-lib/src/lockfile/mod.rs | 12 +++++ rocks-lib/src/luarocks_installation.rs | 31 +++++------- rocks-lib/src/operations/install.rs | 19 ++++---- rocks-lib/tests/build.rs | 48 +++++++------------ 6 files changed, 119 insertions(+), 74 deletions(-) diff --git a/rocks-bin/src/build.rs b/rocks-bin/src/build.rs index 11f3a34c..d2e132a9 100644 --- a/rocks-bin/src/build.rs +++ b/rocks-bin/src/build.rs @@ -6,9 +6,9 @@ use std::{path::PathBuf, sync::Arc}; use clap::Args; use eyre::Result; use rocks_lib::{ - build::BuildBehaviour, + build::{self, BuildBehaviour}, config::Config, - lockfile::{LockConstraint::Unconstrained, PinnedState}, + lockfile::PinnedState, package::{PackageName, PackageReq}, progress::MultiProgress, remote_package_db::RemotePackageDB, @@ -120,15 +120,11 @@ pub async fn build(data: Build, config: Config) -> Result<()> { ) .await?; - rocks_lib::build::build( - rockspec, - pin, - Unconstrained, - build_behaviour, - &config, - &progress.map(|p| p.new_bar()), - ) - .await?; + build::Build::new(rockspec, &config, &progress.map(|p| p.new_bar())) + .pin(pin) + .behaviour(build_behaviour) + .build() + .await?; Ok(()) } diff --git a/rocks-lib/src/build/mod.rs b/rocks-lib/src/build/mod.rs index f17f1bf3..819e0c77 100644 --- a/rocks-lib/src/build/mod.rs +++ b/rocks-lib/src/build/mod.rs @@ -34,6 +34,63 @@ mod rust_mlua; pub mod external_dependency; pub mod variables; +/// A rocks package builder, providing fine-grained control +/// over how a package should be built. +pub struct Build<'a> { + rockspec: Rockspec, + config: &'a Config, + progress: &'a Progress, + pin: PinnedState, + constraint: LockConstraint, + behaviour: BuildBehaviour, +} + +impl<'a> Build<'a> { + /// Construct a new builder. + pub fn new( + rockspec: Rockspec, + config: &'a Config, + progress: &'a Progress, + ) -> Self { + Self { + rockspec, + config, + progress, + pin: PinnedState::default(), + constraint: LockConstraint::default(), + behaviour: BuildBehaviour::default(), + } + } + + /// Sets the pinned state of the package to be built. + pub fn pin(self, pin: PinnedState) -> Self { + Self { pin, ..self } + } + + /// Sets the lock constraint of the package to be built. + pub fn constraint(self, constraint: LockConstraint) -> Self { + Self { constraint, ..self } + } + + /// Sets the build behaviour of the package to be built. + pub fn behaviour(self, behaviour: BuildBehaviour) -> Self { + Self { behaviour, ..self } + } + + /// Builds the package. + pub async fn build(self) -> Result { + build( + self.rockspec, + self.pin, + self.constraint, + self.behaviour, + self.config, + self.progress, + ) + .await + } +} + #[derive(Error, Debug)] pub enum BuildError { #[error("IO operation failed: {0}")] @@ -77,6 +134,12 @@ pub enum BuildBehaviour { Force, } +impl Default for BuildBehaviour { + fn default() -> Self { + Self::NoForce + } +} + impl From for BuildBehaviour { fn from(value: bool) -> Self { if value { @@ -184,7 +247,7 @@ async fn install( Ok(()) } -pub async fn build( +async fn build( rockspec: Rockspec, pinned: PinnedState, constraint: LockConstraint, diff --git a/rocks-lib/src/lockfile/mod.rs b/rocks-lib/src/lockfile/mod.rs index 970d4997..e8da4c38 100644 --- a/rocks-lib/src/lockfile/mod.rs +++ b/rocks-lib/src/lockfile/mod.rs @@ -21,6 +21,12 @@ pub enum PinnedState { Pinned, } +impl Default for PinnedState { + fn default() -> Self { + Self::Unpinned + } +} + impl From for PinnedState { fn from(value: bool) -> Self { if value { @@ -357,6 +363,12 @@ pub enum LockConstraint { Constrained(PackageVersionReq), } +impl Default for LockConstraint { + fn default() -> Self { + Self::Unconstrained + } +} + impl LockConstraint { pub fn to_string_opt(&self) -> Option { match self { diff --git a/rocks-lib/src/luarocks_installation.rs b/rocks-lib/src/luarocks_installation.rs index 35983db4..66aed70e 100644 --- a/rocks-lib/src/luarocks_installation.rs +++ b/rocks-lib/src/luarocks_installation.rs @@ -11,7 +11,7 @@ use tempdir::TempDir; use thiserror::Error; use crate::{ - build::{build, BuildBehaviour, BuildError}, + build::{Build, BuildBehaviour, BuildError}, config::{Config, LuaVersion, LuaVersionUnset}, lockfile::{LocalPackage, LocalPackageId, LockConstraint, PinnedState}, lua_installation::LuaInstallation, @@ -104,15 +104,12 @@ impl LuaRocksInstallation { PackageReq::new("luarocks".into(), Some(LUAROCKS_VERSION.into())).unwrap(); if self.tree.has_rock(&luarocks_req).is_none() { let rockspec = Rockspec::new(LUAROCKS_ROCKSPEC).unwrap(); - let pkg = build( - rockspec, - PinnedState::Unpinned, - LockConstraint::Constrained(luarocks_req.version_req().clone()), - BuildBehaviour::NoForce, - &self.config, - progress, - ) - .await?; + let pkg = Build::new(rockspec, &self.config, progress) + .constraint(LockConstraint::Constrained( + luarocks_req.version_req().clone(), + )) + .build() + .await?; lockfile.add(&pkg); } lockfile.flush()?; @@ -174,15 +171,11 @@ impl LuaRocksInstallation { let config = self.config.clone(); tokio::spawn(async move { let rockspec = install_spec.rockspec; - let pkg = crate::build::build( - rockspec, - pin, - install_spec.spec.constraint(), - install_spec.build_behaviour, - &config, - &bar, - ) - .await?; + let pkg = Build::new(rockspec, &config, &bar) + .constraint(install_spec.spec.constraint()) + .behaviour(install_spec.build_behaviour) + .build() + .await?; bar.map(|b| b.finish_and_clear()); diff --git a/rocks-lib/src/operations/install.rs b/rocks-lib/src/operations/install.rs index 88d473f6..b67cde53 100644 --- a/rocks-lib/src/operations/install.rs +++ b/rocks-lib/src/operations/install.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, io, sync::Arc}; use crate::{ - build::{BuildBehaviour, BuildError}, + build::{Build, BuildBehaviour, BuildError}, config::{Config, LuaVersion, LuaVersionUnset}, lockfile::{LocalPackage, LocalPackageId, Lockfile, PinnedState}, luarocks_installation::{ @@ -117,16 +117,13 @@ async fn install_impl( .await?; } - let pkg = crate::build::build( - rockspec, - pin, - install_spec.spec.constraint(), - install_spec.build_behaviour, - &config, - &bar, - ) - .await - .map_err(|err| InstallError::BuildError(package, err))?; + let pkg = Build::new(rockspec, &config, &bar) + .pin(pin) + .constraint(install_spec.spec.constraint()) + .behaviour(install_spec.build_behaviour) + .build() + .await + .map_err(|err| InstallError::BuildError(package, err))?; bar.map(|b| b.finish_and_clear()); diff --git a/rocks-lib/tests/build.rs b/rocks-lib/tests/build.rs index c1c3e1e2..9ba05669 100644 --- a/rocks-lib/tests/build.rs +++ b/rocks-lib/tests/build.rs @@ -1,9 +1,8 @@ use std::path::PathBuf; use rocks_lib::{ - build::{self, BuildBehaviour::Force}, + build::{Build, BuildBehaviour::Force}, config::{ConfigBuilder, LuaVersion}, - lockfile::{LockConstraint::Unconstrained, PinnedState::Unpinned}, progress::{MultiProgress, Progress}, rockspec::Rockspec, }; @@ -26,16 +25,11 @@ async fn builtin_build() { let progress = MultiProgress::new(); let bar = progress.new_bar(); - build::build( - rockspec, - Unpinned, - Unconstrained, - Force, - &config, - &Progress::Progress(bar), - ) - .await - .unwrap(); + Build::new(rockspec, &config, &Progress::Progress(bar)) + .behaviour(Force) + .build() + .await + .unwrap(); } #[tokio::test] @@ -56,16 +50,11 @@ async fn make_build() { let progress = MultiProgress::new(); let bar = progress.new_bar(); - build::build( - rockspec, - Unpinned, - Unconstrained, - Force, - &config, - &Progress::Progress(bar), - ) - .await - .unwrap(); + Build::new(rockspec, &config, &Progress::Progress(bar)) + .behaviour(Force) + .build() + .await + .unwrap(); } #[tokio::test] @@ -98,14 +87,9 @@ async fn test_build_rockspec(rockspec_path: PathBuf) { let progress = MultiProgress::new(); let bar = progress.new_bar(); - build::build( - rockspec, - Unpinned, - Unconstrained, - Force, - &config, - &Progress::Progress(bar), - ) - .await - .unwrap(); + Build::new(rockspec, &config, &Progress::Progress(bar)) + .behaviour(Force) + .build() + .await + .unwrap(); }