Skip to content
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ default-members = ["moss"]
resolver = "2"

[workspace.package]
version = "0.25.6"
version = "0.25.7"
edition = "2024"
rust-version = "1.85"

Expand Down
2 changes: 2 additions & 0 deletions boulder/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@
use std::io::BufRead;

thread::spawn(move || {
let pgo = is_pgo.then_some("│").unwrap_or_default().dim();

Check warning on line 334 in boulder/src/build.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: this method chain can be written more clearly with `if .. else ..` --> boulder/src/build.rs:334:19 | 334 | let pgo = is_pgo.then_some("│").unwrap_or_default().dim(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if is_pgo { "│" } else { Default::default() }` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else = note: `#[warn(clippy::obfuscated_if_else)]` on by default Raw Output: boulder/src/build.rs:334:19:w:warning: this method chain can be written more clearly with `if .. else ..` --> boulder/src/build.rs:334:19 | 334 | let pgo = is_pgo.then_some("│").unwrap_or_default().dim(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `if is_pgo { "│" } else { Default::default() }` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else = note: `#[warn(clippy::obfuscated_if_else)]` on by default __END__
let kind = phase.styled(format!("{}│", phase.abbrev()));
let tag = format!("{}{pgo}{kind}", "│".dim());

Expand Down Expand Up @@ -462,4 +462,6 @@
RecreateArtefactsDir(#[source] io::Error),
#[error("git upstream processing")]
Git(#[from] git::GitError),
#[error("invalid repo in mv-to-repo flag")]
InvalidRepo,
}
300 changes: 297 additions & 3 deletions boulder/src/cli.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-FileCopyrightText: Copyright © 2020-2025 Serpent OS Developers
//
// SPDX-License-Identifier: MPL-2.0
use std::path::PathBuf;
use std::{collections::HashMap, path::PathBuf};

use boulder::{Env, env};
use clap::{Args, CommandFactory, Parser};
Expand All @@ -12,6 +12,7 @@
use clap_mangen::Man;
use fs_err::{self as fs, File};
use thiserror::Error;
use tui::Styled;

mod build;
mod chroot;
Expand Down Expand Up @@ -51,6 +52,21 @@
pub generate_manpages: Option<PathBuf>,
#[arg(long, global = true, hide = true)]
pub generate_completions: Option<PathBuf>,
#[arg(
long,
require_equals = true,
global = true,
help = "Move newly built .stone package files to the given repo"
)]
pub mv_to_repo: Option<String>,
#[arg(
long,
default_value_t = false,
requires = "mv-to-repo",
global = true,
help = "Auto re-index the repo after a successful build and move"
)]
pub re_index: bool,
}

#[derive(Debug, clap::Subcommand)]
Expand Down Expand Up @@ -106,6 +122,35 @@
return Ok(());
}

match subcommand {
Some(Subcommand::Build(_)) | Some(Subcommand::Recipe(_)) => { /* do nothing, the flags were passed in appropriately. */
}
_ => match (global.mv_to_repo.clone(), global.re_index) {
(Some(_), false) => {
eprintln!(
"{}: The `--mv-to-repo` flag cannot be used with anything but the `build` or `recipe` subcommands",
"Error".red()
);
std::process::exit(1);
}
(None, true) => {
eprintln!(
"{}: The `--re-index` cannot be used with anything but the `build` or `recipe` subcommands and requires `--mv-to-repo`",
"Error".red()
);
std::process::exit(1);
}
(Some(_), true) => {
eprintln!(
"{}: The ``--mv-to-repo` and ``--re-index` flags can only be used with the `build` or `recipe` subcommands",
"Error".red()
);
std::process::exit(1);
}
(None, false) => { /* do nothing, the flags weren't passed in. */ }
},
}

let env = Env::new(global.cache_dir, global.config_dir, global.data_dir, global.moss_root)?;

if global.verbose {
Expand All @@ -120,10 +165,166 @@
}

match subcommand {
Some(Subcommand::Build(command)) => build::handle(command, env)?,
Some(Subcommand::Build(command)) => {
match build::handle(command, env) {
Ok(_) => {
if let Some(repo) = global.mv_to_repo {
// Check to see if the repo is in moss
let moss_cmd = std::process::Command::new("moss")
.args(["repo", "list"])
.stdout(std::process::Stdio::piped())
.output()
.expect("Couldn't get a list of moss repos");
Comment on lines +172 to +177
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use moss APIs directly from Rust, we shouldn't need to invoke it as a separate executable.

Copy link
Author

@bhh32 bhh32 Sep 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought here is I'd like to refactor both boulder and moss to have a shared library crate. This way they both can call shared logic and not rely directly upon each other. To me, and I could be wrong, invoking moss directly here reduces the refactoring later if that were to become a thing. It also marks where the refactoring needs to happen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactoring is much easier w/ the help of rustc / rust-analyzer :) If we broke these interfaces, we'd get no compilation errors here. We can also build a much better API if we handle this natively in moss lib, such as moss::repository::Manager::get(repo: &str) -> Option<Repository>.


// Convert the output to a String
let repos = String::from_utf8(moss_cmd.stdout).expect("Could get the repo list from moss");

let mv_repo = repos

Check warning on line 182 in boulder/src/cli.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator --> boulder/src/cli.rs:182:39 | 182 | let mv_repo = repos | _______________________________________^ 183 | | .lines() 184 | | .filter_map(|line| { 185 | | if line.contains(&repo) { ... | 206 | | }) 207 | | .last() | |______________________________-----^ | | | help: try: `next_back()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last = note: `#[warn(clippy::double_ended_iterator_last)]` on by default Raw Output: boulder/src/cli.rs:182:39:w:warning: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator --> boulder/src/cli.rs:182:39 | 182 | let mv_repo = repos | _______________________________________^ 183 | | .lines() 184 | | .filter_map(|line| { 185 | | if line.contains(&repo) { ... | 206 | | }) 207 | | .last() | |______________________________-----^ | | | help: try: `next_back()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last = note: `#[warn(clippy::double_ended_iterator_last)]` on by default __END__
.lines()
.filter_map(|line| {
if line.contains(&repo) {
let mut ret_map = HashMap::new();
let uri = line

Check warning on line 187 in boulder/src/cli.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator --> boulder/src/cli.rs:187:47 | 187 | ... let uri = line | _________________________________^ 188 | | ... .split_whitespace() 189 | | ... .filter(|line| line.contains("//")) 190 | | ... .last() | |____________________________-----^ | | | help: try: `next_back()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last Raw Output: boulder/src/cli.rs:187:47:w:warning: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator --> boulder/src/cli.rs:187:47 | 187 | ... let uri = line | _________________________________^ 188 | | ... .split_whitespace() 189 | | ... .filter(|line| line.contains("//")) 190 | | ... .last() | |____________________________-----^ | | | help: try: `next_back()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last __END__

Check warning on line 187 in boulder/src/cli.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` --> boulder/src/cli.rs:187:47 | 187 | ... let uri = line | _________________________________^ 188 | | ... .split_whitespace() 189 | | ... .filter(|line| line.contains("//")) 190 | | ... .last() ... | 197 | | ... }) | |____________________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map = note: `#[warn(clippy::bind_instead_of_map)]` on by default help: use `map` instead | 191 ~ .map(|uri| { 192 | if uri.contains("file:///") { 193 ~ uri.to_string().replace("file://", "") 194 | } else { 195 ~ uri.to_string() | Raw Output: boulder/src/cli.rs:187:47:w:warning: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` --> boulder/src/cli.rs:187:47 | 187 | ... let uri = line | _________________________________^ 188 | | ... .split_whitespace() 189 | | ... .filter(|line| line.contains("//")) 190 | | ... .last() ... | 197 | | ... }) | |____________________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map = note: `#[warn(clippy::bind_instead_of_map)]` on by default help: use `map` instead | 191 ~ .map(|uri| { 192 | if uri.contains("file:///") { 193 ~ uri.to_string().replace("file://", "") 194 | } else { 195 ~ uri.to_string() | __END__
.split_whitespace()
.filter(|line| line.contains("//"))
.last()
.and_then(|uri| {
if uri.contains("file:///") {
Some(uri.to_string().replace("file://", ""))
} else {
Some(uri.to_string())
}
})
.expect("Couldn't get URI from repo string".red().to_string().as_str());

Check warning on line 198 in boulder/src/cli.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: function call inside of `expect` --> boulder/src/cli.rs:198:42 | 198 | ... .expect("Couldn't get URI from repo string".red().to_string().as_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("{}", "Couldn't get URI from repo string".red().to_string()))` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call Raw Output: boulder/src/cli.rs:198:42:w:warning: function call inside of `expect` --> boulder/src/cli.rs:198:42 | 198 | ... .expect("Couldn't get URI from repo string".red().to_string().as_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| panic!("{}", "Couldn't get URI from repo string".red().to_string()))` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call __END__

let _ = ret_map.insert(&repo, Some(uri.clone()));

Some(ret_map)
} else {
None
}
})
.last()
.unwrap_or_else(|| HashMap::new());

Check warning on line 208 in boulder/src/cli.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: redundant closure --> boulder/src/cli.rs:208:45 | 208 | ... .unwrap_or_else(|| HashMap::new()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `HashMap::new` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure = note: `#[warn(clippy::redundant_closure)]` on by default Raw Output: boulder/src/cli.rs:208:45:w:warning: redundant closure --> boulder/src/cli.rs:208:45 | 208 | ... .unwrap_or_else(|| HashMap::new()); | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `HashMap::new` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure = note: `#[warn(clippy::redundant_closure)]` on by default __END__

// Check to ensure that the repo has a URI;
// return Err if there isn't.
if mv_repo.get(&repo).is_none() {
eprintln!("{} {}", &repo, "is not a valid repo registered with moss");
return Err(Error::Build(build::Error::Build(boulder::build::Error::InvalidRepo)));
}

// Move the newly built .stone files
match mv_to_repo(&repo, &mv_repo) {
Ok(repo) => {
if global.re_index && repo.is_some() {

Check warning on line 220 in boulder/src/cli.rs

View workflow job for this annotation

GitHub Actions / Build & Test Project

[clippy] reported by reviewdog 🐶 warning: called `expect` on `repo` after checking its variant with `is_some` --> boulder/src/cli.rs:221:70 | 220 | ... if global.re_index && repo.is_some() { | -------------- the check is happening here 221 | ... if let Err(err) = re_index_repo(&repo.expect("Repo was supposed to be Some")) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try using `if let` or `match` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap = note: `#[warn(clippy::unnecessary_unwrap)]` on by default Raw Output: boulder/src/cli.rs:220:55:w:warning: called `expect` on `repo` after checking its variant with `is_some` --> boulder/src/cli.rs:221:70 | 220 | ... if global.re_index && repo.is_some() { | -------------- the check is happening here 221 | ... if let Err(err) = re_index_repo(&repo.expect("Repo was supposed to be Some")) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try using `if let` or `match` = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap = note: `#[warn(clippy::unnecessary_unwrap)]` on by default __END__
if let Err(err) = re_index_repo(&repo.expect("Repo was supposed to be Some")) {
eprintln!("Error: {err}");
return Err(err);
}
} else if global.re_index && repo.is_none() {
eprintln!("Error: Cannot re-index, returned repo name was empty!");
return Err(Error::Reindex(
"Cannot re-index, move operation returned an invalid repo name".to_string(),
));
}
}
Err(err) => {
eprintln!("Error: {err}");
return Err(err);
}
}
}
}
Err(e) => {
eprintln!("{e}");
return Err(Error::Build(e));
}
};
}
Some(Subcommand::Chroot(command)) => chroot::handle(command, env)?,
Some(Subcommand::Profile(command)) => profile::handle(command, env)?,
Some(Subcommand::Recipe(command)) => recipe::handle(command, env)?,
// Recipe takes into account the global.build flag
Some(Subcommand::Recipe(command)) => {
// Give an error message and exit without running the command
// if the --mv-to-repo flag was give without the --build flag.
if global.mv_to_repo.is_some() && !command.build {
eprintln!("Error: Cannot use `--mv-to-repo` without the `--build` flag");
std::process::exit(1);
}
if let Some(repo) = global.mv_to_repo {
// Check to see if the repo is in moss
let moss_cmd = std::process::Command::new("moss")
.args(["repo", "list"])
.output()
.expect("Couldn't get a list of moss repos");

let repos = String::from_utf8(moss_cmd.stdout).expect("Could not get the repo list from moss");

let mv_repo = repos
.lines()
.filter_map(|line| {
Comment on lines +262 to +266
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we factor out the mv-to-repo / re-index logic and call the single function from both build and recipe branches? This can all just be handled within cli::build since its a side-effect of the build and when calling cli::build::handle from recipe, we can just forward these arguments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, absolutely. I can make this change for sure.

if line.contains(&repo) {
let mut ret_map = HashMap::new();

let uri = line
.split_whitespace()
.filter(|line| line.contains("//"))
.last()
.and_then(|uri| {
if uri.contains("file:///") {
Some(uri.to_string().replace("file://", ""))
} else {
Some(uri.to_string())
}
})
.expect("Could not get URI from repo string");

let _ = ret_map.insert(&repo, Some(uri.clone()));

Some(ret_map)
} else {
None
}
})
.last()
.unwrap_or_else(|| HashMap::new());

if mv_repo.get(&repo).is_none() {
eprintln!("{} is not a valid repo registered with moss", &repo);
return Err(Error::Build(build::Error::Build(boulder::build::Error::InvalidRepo)));
}

recipe::handle(command, env)?;

match mv_to_repo(&repo, &mv_repo) {
Ok(repo) => {
// Ok to re-index as there is a value use
if global.re_index && repo.is_some() {
if let Err(err) =
re_index_repo(&repo.clone().expect("Error: Returned repo should've been Some"))
{
eprintln!("Error {err}");
return Err(Error::Reindex(
"Cannot re-index, move operation returned an invalid repo name".to_string(),
));
}
} else if global.re_index && repo.is_none() {
eprintln!("Error: Cannot re-index, returned repo name was empty!");
return Err(Error::Reindex(
"Cannot re-index, move operation returned an invalid repo name".to_string(),
));
}
}
Err(err) => {
eprintln!("Error: {err}");
return Err(err);
}
}
} else {
recipe::handle(command, env)?;
}
}
Some(Subcommand::Version(command)) => version::handle(command),
None => (),
}
Expand Down Expand Up @@ -160,6 +361,97 @@
args
}

fn mv_to_repo(repo_key: &String, repo_map: &HashMap<&String, Option<String>>) -> Result<Option<String>, Error> {
let repo_path = repo_map.get(repo_key).unwrap_or_else(|| &None);
if let Some(repo_path) = repo_path {
let cwd = PathBuf::from(".");
let manifest_ext = "stone";

let repo_path = PathBuf::from(if repo_path.contains("file://") {
repo_path.replacen("file://", "", 1).replacen("stone.index", "", 1)
} else {
repo_path.to_string().replacen("stone.index", "", 1)
});

// Create repo directory if it doesn't exist
if !repo_path.exists() {
fs::create_dir_all(&repo_path).expect("Failed to create {repo_key} repo directories");
}

match fs::read_dir(&cwd) {
Ok(dir) => {
for (_, pkg_file) in dir.enumerate() {
let pkg_file = pkg_file.expect("Failed to get package file to move");
let path = pkg_file.path();

if path.is_file()
&& let Some(ext) = path.extension().and_then(|ext| ext.to_str())
{
if ext == manifest_ext {
let file_name = path
.file_name()
.ok_or("Invalid package file name")
.expect("Failed to get package file name");
let dest_path = PathBuf::from(&repo_path).join(file_name);

println!(
"Moving {} to {}",
&path.to_string_lossy().to_string(),
&dest_path.to_string_lossy().to_string()
);
match fs::rename(&path, &dest_path) {
Ok(_) => {
println!(
"Successfully moved {} to {}",
&path.to_string_lossy().to_string(),
&dest_path.to_string_lossy().to_string()
);
}
Err(e) => {
eprintln!(
"Failed to move {} to {}: {e}",
&path.to_string_lossy().to_string(),
&dest_path.to_string_lossy().to_string(),
);
}
}
}
}
}

return Ok(Some(repo_path.to_string_lossy().to_string()));
}
Err(e) => {
eprintln!("Failed to read directory: {e}");
return Err(Error::Io(e));
}
}
}

if let Some(repo_path) = repo_path {
Ok(Some(repo_path.clone()))
} else {
Err(Error::Io(std::io::Error::new(
std::io::ErrorKind::NotFound,
format!("Error: {repo_key} doesn't have a valid path").as_str(),
)))
}
}

fn re_index_repo(repo: &str) -> Result<(), Error> {
use std::process::{Command as Cmd, Stdio};

let mut moss_cmd = Cmd::new("moss")
.args(["index", repo])
.stdout(Stdio::inherit())
.stderr(Stdio::inherit())
.spawn()?;

let _index_status = moss_cmd.wait()?;

Ok(())
}

#[derive(Debug, Error)]
pub enum Error {
#[error("build")]
Expand All @@ -174,4 +466,6 @@
Recipe(#[from] recipe::Error),
#[error("io error")]
Io(#[from] std::io::Error),
#[error("reindex")]
Reindex(String),
}
Loading
Loading