Skip to content

Commit

Permalink
Move packages cli arg into shared flags struct
Browse files Browse the repository at this point in the history 
Signed-off-by: Ryan Bottriell <[email protected]>
  • Loading branch information
@rydrman
rydrman committed Jan 14, 2025
1 parent 0980e04 commit 086dad0
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 106 deletions.
36 changes: 20 additions & 16 deletions crates/spk-cli/cmd-build/src/cmd_build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

use clap::Args;
use miette::Result;
use spk_cli_common::{flags, CommandArgs, Run};
use spk_cmd_make_binary::cmd_make_binary::PackageSpecifier;
use spk_cli_common::flags::{self, PackageSpecifier};
use spk_cli_common::{CommandArgs, Run};

#[cfg(test)]
#[path = "./cmd_build_test/mod.rs"]
Expand Down Expand Up @@ -37,9 +37,8 @@ pub struct Build {
#[clap(long, short)]
env: bool,

/// The package names or yaml spec files to build
#[clap(name = "NAME|SPEC_FILE")]
packages: Vec<String>,
#[clap(flatten)]
packages: flags::Packages,

/// Build only the specified variants
#[clap(flatten)]
Expand Down Expand Up @@ -77,13 +76,13 @@ impl Run for Build {
.await?;

// divide our packages into one for each iteration of mks/mkb
let mut runs: Vec<_> = self.packages.iter().map(|f| vec![f.to_owned()]).collect();
let mut runs: Vec<_> = self.packages.split();
if runs.is_empty() {
runs.push(Vec::new());
runs.push(Default::default());
}

let mut builds_for_summary = spk_cli_common::BuildResult::default();
for packages in runs {
for mut packages in runs {
let mut make_source = spk_cmd_make_source::cmd_make_source::MakeSource {
options: self.options.clone(),
verbose: self.verbose,
Expand All @@ -94,6 +93,17 @@ impl Run for Build {
let idents = make_source.make_source().await?;
builds_for_summary.extend(make_source.created_src);

// add the source ident specifier from the source build to ensure that
// the binary build operates over this exact source package
packages.packages = packages
.packages
.into_iter()
.zip(idents.into_iter())
.map(|(package, ident)| {
PackageSpecifier::WithSourceIdent((package.into_specifier(), ident.into()))
})
.collect();

let mut make_binary = spk_cmd_make_binary::cmd_make_binary::MakeBinary {
verbose: self.verbose,
runtime: self.runtime.clone(),
Expand All @@ -102,13 +112,7 @@ impl Run for Build {
here: self.here,
interactive: self.interactive,
env: self.env,
packages: packages
.into_iter()
.zip(idents.into_iter())
.map(|(package, ident)| {
PackageSpecifier::WithSourceIdent((package, ident.into()))
})
.collect(),
packages,
variant: self.variant.clone(),
formatter_settings: self.formatter_settings.clone(),
allow_circular_dependencies: self.allow_circular_dependencies,
Expand Down Expand Up @@ -139,6 +143,6 @@ impl Run for Build {
impl CommandArgs for Build {
// The important positional args for a build are the packages
fn get_positional_args(&self) -> Vec<String> {
self.packages.clone()
self.packages.get_positional_args()
}
}
57 changes: 10 additions & 47 deletions crates/spk-cli/cmd-make-binary/src/cmd_make_binary.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use miette::{bail, miette, Context, IntoDiagnostic, Report, Result};
use spk_build::{BinaryPackageBuilder, BuildSource};
use spk_cli_common::{flags, spk_exe, BuildArtifact, BuildResult, CommandArgs, Run};
use spk_schema::foundation::format::FormatIdent;
use spk_schema::ident::{PkgRequest, RangeIdent, RequestedBy};
use spk_schema::ident::{PkgRequest, RequestedBy};
use spk_schema::option_map::HOST_OPTIONS;
use spk_schema::prelude::*;
use spk_schema::OptionMap;
Expand All @@ -21,31 +21,6 @@ use spk_storage as storage;
#[path = "./cmd_make_binary_test.rs"]
mod cmd_make_binary_test;

#[derive(Clone, Debug)]
pub enum PackageSpecifier {
Plain(String),
WithSourceIdent((String, RangeIdent)),
}

impl PackageSpecifier {
// Return the package spec or filename string.
fn get_specifier(&self) -> &String {
match self {
PackageSpecifier::Plain(s) => s,
PackageSpecifier::WithSourceIdent((s, _)) => s,
}
}
}

impl std::str::FromStr for PackageSpecifier {
type Err = clap::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
// On the command line, only `Plain` is possible.
Ok(PackageSpecifier::Plain(s.to_owned()))
}
}

/// Build a binary package from a spec file or source package.
#[derive(Args)]
#[clap(visible_aliases = &["mkbinary", "mkbin", "mkb"])]
Expand All @@ -72,9 +47,8 @@ pub struct MakeBinary {
#[clap(long, short)]
pub env: bool,

/// The local yaml spec files or published package/versions to build or rebuild
#[clap(name = "SPEC_FILE|PKG/VER")]
pub packages: Vec<PackageSpecifier>,
#[clap(flatten)]
pub packages: flags::Packages,

/// Build only the specified variants
#[clap(flatten)]
Expand All @@ -96,11 +70,7 @@ pub struct MakeBinary {
impl CommandArgs for MakeBinary {
// The important positional args for a make-binary are the packages
fn get_positional_args(&self) -> Vec<String> {
self.packages
.iter()
.map(|ps| ps.get_specifier())
.cloned()
.collect()
self.packages.get_positional_args()
}
}

Expand Down Expand Up @@ -130,21 +100,12 @@ impl Run for MakeBinary {
.map(|(_, r)| Arc::new(r))
.collect::<Vec<_>>();

let mut packages: Vec<_> = self.packages.iter().cloned().map(Some).collect();
if packages.is_empty() {
packages.push(None)
}

let opt_host_options =
(!self.options.no_host).then(|| HOST_OPTIONS.get().unwrap_or_default());

for package in packages {
let (spec_data, filename) = flags::find_package_recipe_from_template_or_repo(
package.as_ref().map(|p| p.get_specifier()),
&options,
&repos,
)
.await?;
for (package, spec_data, filename) in
self.packages.find_all_recipes(&options, &repos).await?
{
let recipe = spec_data.into_recipe().wrap_err_with(|| {
format!(
"{filename} was expected to contain a recipe",
Expand Down Expand Up @@ -220,7 +181,9 @@ impl Run for MakeBinary {
.into_diagnostic()
.wrap_err("Failed to get current directory")?;
builder.with_source(BuildSource::LocalPath(here));
} else if let Some(PackageSpecifier::WithSourceIdent((_, ref ident))) = package {
} else if let Some(flags::PackageSpecifier::WithSourceIdent((_, ref ident))) =
package
{
// Use the source package `AnyIdent` if the caller supplied one.
builder.with_source(BuildSource::SourcePackage(ident.clone()));
}
Expand Down
56 changes: 14 additions & 42 deletions crates/spk-cli/cmd-make-source/src/cmd_make_source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/spkenv/spk

use std::path::PathBuf;
use std::sync::Arc;

use clap::Args;
Expand All @@ -11,7 +10,7 @@ use spk_build::SourcePackageBuilder;
use spk_cli_common::{flags, BuildArtifact, BuildResult, CommandArgs, Run};
use spk_schema::foundation::format::FormatIdent;
use spk_schema::ident::LocatedBuildIdent;
use spk_schema::{Package, Recipe, SpecTemplate, Template, TemplateExt};
use spk_schema::{Package, Recipe};
use spk_storage as storage;

/// Build a source package from a spec file.
Expand All @@ -27,9 +26,8 @@ pub struct MakeSource {
#[clap(short, long, global = true, action = clap::ArgAction::Count)]
pub verbose: u8,

/// The packages or yaml spec files to collect
#[clap(name = "PKG|SPEC_FILE")]
pub packages: Vec<String>,
#[clap(flatten)]
pub packages: flags::Packages,

/// Populated with the created src to generate a summary from the caller.
#[clap(skip)]
Expand Down Expand Up @@ -60,44 +58,18 @@ impl MakeSource {
.runtime
.ensure_active_runtime(&["make-source", "mksource", "mksrc", "mks"])
.await?;
let local: storage::RepositoryHandle = storage::local_repository().await?.into();
let local = Arc::new(storage::local_repository().await?.into());
let options = self.options.get_options()?;

let mut packages: Vec<_> = self.packages.iter().cloned().map(Some).collect();
if packages.is_empty() {
packages.push(None)
}

let mut idents = Vec::new();

for package in packages.into_iter() {
let template = match flags::find_package_template(package.as_ref())? {
flags::FindPackageTemplateResult::NotFound(name) => {
// TODO:: load from given repos
Arc::new(SpecTemplate::from_file(name.as_ref())?)
}
res => {
let (_, template) = res.must_be_found();
template
}
};
let root = template
.file_path()
.parent()
.map(ToOwned::to_owned)
.unwrap_or_else(|| std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")));
if let Some(name) = template.name() {
tracing::info!("rendering template for {name}");
} else {
tracing::info!("rendering template without a name");
}
let rendered_data = template.render(&options)?;
let recipe = rendered_data.into_recipe().wrap_err_with(|| {
format!(
"{filename} was expected to contain a recipe",
filename = template.file_path().to_string_lossy()
)
})?;
for (_package, spec_data, path) in self
.packages
.find_all_recipes(&options, &[Arc::clone(&local)])
.await?
{
let root = path.parent().unwrap_or_else(|| std::path::Path::new("."));
let recipe = spec_data.into_recipe()?;
let ident = recipe.ident();

tracing::info!("saving package recipe for {}", ident.format_ident());
Expand All @@ -106,12 +78,12 @@ impl MakeSource {
tracing::info!("collecting sources for {}", ident.format_ident());
let (out, _components) =
SourcePackageBuilder::from_recipe(Arc::unwrap_or_clone(recipe))
.build_and_publish(root, &local)
.build_and_publish(root, &*local)
.await
.wrap_err("Failed to collect sources")?;
tracing::info!("created {}", out.ident().format_ident());
self.created_src.push(
template.file_path().display().to_string(),
path.display().to_string(),
BuildArtifact::Source(out.ident().clone()),
);
idents.push(out.ident().clone().into_located(local.name().to_owned()));
Expand All @@ -123,6 +95,6 @@ impl MakeSource {
impl CommandArgs for MakeSource {
fn get_positional_args(&self) -> Vec<String> {
// The important positional args for a make-source are the packages
self.packages.clone()
self.packages.get_positional_args()
}
}
Loading

0 comments on commit 086dad0

Please sign in to comment.