Introduce Pkg6DevError for improved error handling across pkg6dev, replace anyhow with custom error types, enable structured diagnostics with miette, update dependencies, and add tracing for logging.

This commit is contained in:
Till Wegmueller 2025-07-26 21:20:50 +02:00
parent b451099b54
commit cb75c045e5
No known key found for this signature in database
7 changed files with 292 additions and 65 deletions

82
Cargo.lock generated
View file

@ -960,6 +960,16 @@ dependencies = [
"tempfile",
]
[[package]]
name = "nu-ansi-term"
version = "0.46.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "77a8165726e8236064dbb45459242600304b42a5ea24ee2948e18e023bf7ba84"
dependencies = [
"overload",
"winapi",
]
[[package]]
name = "num"
version = "0.4.3"
@ -1140,6 +1150,12 @@ version = "6.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ceedf44fb00f2d1984b0bc98102627ce622e083e49a5bacdb3e514fa4238e267"
[[package]]
name = "overload"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39"
[[package]]
name = "owo-colors"
version = "4.2.2"
@ -1232,8 +1248,12 @@ name = "pkg6dev"
version = "0.1.1"
dependencies = [
"anyhow",
"clap 3.2.23",
"clap 4.5.41",
"libips",
"miette",
"thiserror 1.0.69",
"tracing",
"tracing-subscriber",
"userland",
]
@ -1246,7 +1266,9 @@ dependencies = [
"miette",
"serde",
"serde_json",
"thiserror 1.0.69",
"thiserror 2.0.12",
"tracing",
"tracing-subscriber",
]
[[package]]
@ -1589,6 +1611,15 @@ dependencies = [
"opaque-debug",
]
[[package]]
name = "sharded-slab"
version = "0.1.7"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6"
dependencies = [
"lazy_static",
]
[[package]]
name = "shellexpand"
version = "2.1.2"
@ -1613,6 +1644,12 @@ dependencies = [
"autocfg",
]
[[package]]
name = "smallvec"
version = "1.15.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03"
[[package]]
name = "socket2"
version = "0.4.9"
@ -1798,6 +1835,15 @@ dependencies = [
"syn 2.0.104",
]
[[package]]
name = "thread_local"
version = "1.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185"
dependencies = [
"cfg-if",
]
[[package]]
name = "tinyvec"
version = "1.6.0"
@ -1900,6 +1946,32 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "24eb03ba0eab1fd845050058ce5e616558e8f8d8fca633e6b163fe25c797213a"
dependencies = [
"once_cell",
"valuable",
]
[[package]]
name = "tracing-log"
version = "0.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3"
dependencies = [
"log",
"once_cell",
"tracing-core",
]
[[package]]
name = "tracing-subscriber"
version = "0.3.19"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e8189decb5ac0fa7bc8b96b7cb9b2701d60d48805aca84a238004d665fcc4008"
dependencies = [
"nu-ansi-term",
"sharded-slab",
"smallvec",
"thread_local",
"tracing-core",
"tracing-log",
]
[[package]]
@ -1996,6 +2068,12 @@ version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821"
[[package]]
name = "valuable"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65"
[[package]]
name = "vcpkg"
version = "0.2.15"

View file

@ -17,6 +17,7 @@ use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::time::{SystemTime, UNIX_EPOCH};
use tracing::{debug, error, info, trace, warn};
use crate::actions::{File as FileAction, Manifest};
use crate::digest::Digest;
@ -423,53 +424,75 @@ impl Transaction {
}
}
// Generate a timestamp for the manifest version
let timestamp = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_secs();
// Move the manifest to its final location in the repository
// Store in both the pkg directory and the trans directory as required
// Extract package name from manifest
let mut package_name = String::from("unknown");
for attr in &self.manifest.attributes {
if attr.key == "pkg.fmri" && !attr.values.is_empty() {
if let Ok(fmri) = Fmri::parse(&attr.values[0]) {
package_name = fmri.name.to_string();
debug!("Extracted package name from FMRI: {}", package_name);
break;
}
}
}
// Determine the pkg directory path based on publisher
let pkg_manifest_path = if let Some(publisher) = &self.publisher {
// Determine the publisher to use
let publisher = match &self.publisher {
Some(pub_name) => {
debug!("Using specified publisher: {}", pub_name);
pub_name.clone()
},
None => {
debug!("No publisher specified, trying to use default publisher");
// If no publisher is specified, use the default publisher from the repository config
let config_path = self.repo.join(REPOSITORY_CONFIG_FILENAME);
if config_path.exists() {
let config_content = fs::read_to_string(&config_path)?;
let config: RepositoryConfig = serde_json::from_str(&config_content)?;
match config.default_publisher {
Some(default_pub) => {
debug!("Using default publisher: {}", default_pub);
default_pub
},
None => {
debug!("No default publisher set in repository");
return Err(RepositoryError::Other(
"No publisher specified and no default publisher set in repository".to_string()
))
}
}
} else {
debug!("Repository configuration not found");
return Err(RepositoryError::Other(
"No publisher specified and repository configuration not found".to_string()
));
}
}
};
// Create publisher directory if it doesn't exist
let publisher_dir = self.repo.join("pkg").join(publisher);
let publisher_dir = self.repo.join("pkg").join(&publisher);
debug!("Publisher directory: {}", publisher_dir.display());
if !publisher_dir.exists() {
debug!("Creating publisher directory");
fs::create_dir_all(&publisher_dir)?;
}
// Store in publisher-specific directory with package name
publisher_dir.join(format!("{}.manifest", package_name))
} else {
// Store in root pkg directory (legacy behavior)
self.repo.join("pkg").join("manifest")
};
let trans_manifest_path = self
.repo
.join("trans")
.join(format!("manifest_{}", timestamp));
let pkg_manifest_path = publisher_dir.join(format!("{}.manifest", package_name));
debug!("Manifest path: {}", pkg_manifest_path.display());
// Copy to pkg directory
debug!("Copying manifest from {} to {}", manifest_path.display(), pkg_manifest_path.display());
fs::copy(&manifest_path, &pkg_manifest_path)?;
// Move to trans directory
fs::rename(manifest_path, trans_manifest_path)?;
// Also create a copy in the root pkg directory for backward compatibility
// This is temporary and should be removed once all clients are updated
debug!("Also creating a copy in the root pkg directory for backward compatibility");
let root_manifest_path = self.repo.join("pkg").join("manifest");
fs::copy(&manifest_path, &root_manifest_path)?;
// Clean up the transaction directory (except for the manifest which was moved)
// Clean up the transaction directory
fs::remove_dir_all(self.path)?;
Ok(())
@ -1201,15 +1224,15 @@ impl WritableRepository for FileBackend {
// For each publisher, rebuild metadata
for pub_name in publishers {
println!("Rebuilding metadata for publisher: {}", pub_name);
info!("Rebuilding metadata for publisher: {}", pub_name);
if !no_catalog {
println!("Rebuilding catalog...");
info!("Rebuilding catalog...");
// In a real implementation, we would rebuild the catalog
}
if !no_index {
println!("Rebuilding search index...");
info!("Rebuilding search index...");
self.build_search_index(&pub_name)?;
}
}
@ -1231,15 +1254,15 @@ impl WritableRepository for FileBackend {
// For each publisher, refresh metadata
for pub_name in publishers {
println!("Refreshing metadata for publisher: {}", pub_name);
info!("Refreshing metadata for publisher: {}", pub_name);
if !no_catalog {
println!("Refreshing catalog...");
info!("Refreshing catalog...");
// In a real implementation, we would refresh the catalog
}
if !no_index {
println!("Refreshing search index...");
info!("Refreshing search index...");
// Check if the index exists
let index_path = self.path.join("index").join(&pub_name).join("search.json");
@ -1319,7 +1342,7 @@ impl FileBackend {
/// Generate catalog parts for a publisher
fn generate_catalog_parts(&mut self, publisher: &str, create_update_log: bool) -> Result<()> {
println!("Generating catalog parts for publisher: {}", publisher);
info!("Generating catalog parts for publisher: {}", publisher);
// Collect package data first
let repo_path = self.path.clone();
@ -1533,7 +1556,7 @@ impl FileBackend {
/// Build a search index for a publisher
fn build_search_index(&self, publisher: &str) -> Result<()> {
println!("Building search index for publisher: {}", publisher);
info!("Building search index for publisher: {}", publisher);
// Create a new search index
let mut index = SearchIndex::new();
@ -1700,7 +1723,7 @@ impl FileBackend {
let index_path = self.path.join("index").join(publisher).join("search.json");
index.save(&index_path)?;
println!("Search index built for publisher: {}", publisher);
info!("Search index built for publisher: {}", publisher);
Ok(())
}
@ -1718,10 +1741,11 @@ impl FileBackend {
#[cfg(test)]
pub fn test_publish_files(&mut self, test_dir: &Path) -> Result<()> {
println!("Testing file publishing...");
debug!("Testing file publishing...");
// Create a test publisher
self.add_publisher("test")?;
let publisher = "test";
self.add_publisher(publisher)?;
// Create a nested directory structure
let nested_dir = test_dir.join("nested").join("dir");
@ -1734,6 +1758,9 @@ impl FileBackend {
// Begin a transaction
let mut transaction = self.begin_transaction()?;
// Set the publisher for the transaction
transaction.set_publisher(publisher);
// Create a FileAction from the test file path
let mut file_action = FileAction::read_from_path(&test_file_path)?;
@ -1773,14 +1800,21 @@ impl FileBackend {
return Err(RepositoryError::Other("File was not stored correctly".to_string()));
}
// Verify the manifest was updated
let manifest_path = self.path.join("pkg").join("manifest");
// Verify the manifest was updated in the publisher-specific directory
// The manifest should be named "unknown.manifest" since we didn't set a package name
let manifest_path = self.path.join("pkg").join(publisher).join("unknown.manifest");
if !manifest_path.exists() {
return Err(RepositoryError::Other("Manifest was not created".to_string()));
return Err(RepositoryError::Other(format!(
"Manifest was not created at the expected location: {}",
manifest_path.display()
)));
}
println!("File publishing test passed!");
// Regenerate catalog and search index
self.rebuild(Some(publisher), false, false)?;
debug!("File publishing test passed!");
Ok(())
}
@ -1810,12 +1844,18 @@ impl FileBackend {
// Begin a transaction
let mut transaction = self.begin_transaction()?;
// Set the publisher for the transaction
transaction.set_publisher(publisher);
// Walk the prototype directory and add files to the transaction
self.add_files_to_transaction(&mut transaction, proto_dir, proto_dir)?;
// Commit the transaction
transaction.commit()?;
// Regenerate catalog and search index
self.rebuild(Some(publisher), false, false)?;
Ok(())
}

View file

@ -12,7 +12,11 @@ keywords = ["packaging", "illumos"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
libips = {path = "../libips", version = "0.1.1"}
userland = {path = "../userland", version = "0.1.1"}
anyhow = "1.0.59"
clap = {version = "3.2.16", features = [ "derive" ] }
libips = {path = "../libips", version = "0.1"}
userland = {path = "../userland", version = "*"}
clap = {version = "4", features = [ "derive" ] }
tracing = "0.1"
tracing-subscriber = "0.3"
miette = { version = "7.6.0", features = ["fancy"] }
thiserror = "1.0.50"
anyhow = "1.0"

64
pkg6dev/src/error.rs Normal file
View file

@ -0,0 +1,64 @@
use libips::actions::ActionError;
use libips::repository;
use miette::Diagnostic;
use thiserror::Error;
/// Result type for pkg6dev operations
pub type Result<T> = std::result::Result<T, Pkg6DevError>;
/// Errors that can occur in pkg6dev operations
#[derive(Debug, Error, Diagnostic)]
pub enum Pkg6DevError {
#[error("I/O error: {0}")]
#[diagnostic(
code(ips::pkg6dev_error::io),
help("Check system resources and permissions")
)]
IoError(#[from] std::io::Error),
#[error("action error: {0}")]
#[diagnostic(
code(ips::pkg6dev_error::action),
help("Check the action format and try again")
)]
ActionError(#[from] ActionError),
#[error("repository error: {0}")]
#[diagnostic(transparent)]
RepositoryError(#[from] repository::RepositoryError),
#[error("userland error: {0}")]
#[diagnostic(
code(ips::pkg6dev_error::userland),
help("Check the userland component and try again")
)]
UserlandError(String),
#[error("{0}")]
#[diagnostic(
code(ips::pkg6dev_error::custom),
help("See error message for details")
)]
Custom(String),
}
/// Convert a string to a Pkg6DevError::Custom
impl From<String> for Pkg6DevError {
fn from(s: String) -> Self {
Pkg6DevError::Custom(s)
}
}
/// Convert a &str to a Pkg6DevError::Custom
impl From<&str> for Pkg6DevError {
fn from(s: &str) -> Self {
Pkg6DevError::Custom(s.to_string())
}
}
/// Convert an anyhow::Error to a Pkg6DevError::UserlandError
impl From<anyhow::Error> for Pkg6DevError {
fn from(e: anyhow::Error) -> Self {
Pkg6DevError::UserlandError(e.to_string())
}
}

View file

@ -1,12 +1,16 @@
mod error;
use clap::{Parser, Subcommand};
use libips::actions::{ActionError, File, Manifest};
use libips::repository::{FileBackend, ReadableRepository, WritableRepository};
use anyhow::{anyhow, Result};
use error::{Pkg6DevError, Result};
use std::collections::HashMap;
use std::fs::{read_dir, OpenOptions};
use std::io::Write;
use std::path::{Path, PathBuf};
use tracing::{debug, error, info, warn};
use tracing_subscriber::fmt;
use userland::repology::find_newest_version;
use userland::{Component, Makefile};
@ -53,6 +57,15 @@ enum Commands {
}
fn main() -> Result<()> {
// Initialize the tracing subscriber with default log level as warning and no decorations
fmt::Subscriber::builder()
.with_max_level(tracing::Level::WARN)
.without_time()
.with_target(false)
.with_ansi(false)
.with_writer(std::io::stderr)
.init();
let cli = App::parse();
match &cli.command {
@ -100,7 +113,7 @@ fn diff_component(
None
};
let files = read_dir(&component_path)?;
let files = read_dir(&component_path).map_err(|e| Pkg6DevError::IoError(e))?;
let manifest_files: Vec<String> = files
.filter_map(std::result::Result::ok)
@ -118,7 +131,8 @@ fn diff_component(
.as_ref()
.join("manifests/sample-manifest.p5m");
let manifests_res: Result<Vec<Manifest>, ActionError> =
// Use std::result::Result here to avoid confusion with our custom Result type
let manifests_res: std::result::Result<Vec<Manifest>, ActionError> =
manifest_files.iter().map(Manifest::parse_file).collect();
let sample_manifest = Manifest::parse_file(sample_manifest_file)?;
@ -147,9 +161,9 @@ fn diff_component(
.write(true)
.truncate(true)
.create(true)
.open(output_manifest)?;
.open(output_manifest).map_err(|e| Pkg6DevError::IoError(e))?;
for action in missing_files {
writeln!(&mut f, "file path={}", action.path)?;
writeln!(&mut f, "file path={}", action.path).map_err(|e| Pkg6DevError::IoError(e))?;
}
}
@ -159,6 +173,8 @@ fn diff_component(
fn show_component_info<P: AsRef<Path>>(component_path: P) -> Result<()> {
let makefile_path = component_path.as_ref().join("Makefile");
// For userland functions, we'll use the From trait implementation for anyhow::Error
// that we added to Pkg6DevError
let initial_makefile = Makefile::parse_single_file(makefile_path)?;
let makefile = initial_makefile.parse_all()?;
@ -325,18 +341,18 @@ fn publish_package(
) -> Result<()> {
// Check if the manifest file exists
if !manifest_path.exists() {
return Err(anyhow!(
return Err(Pkg6DevError::Custom(format!(
"Manifest file does not exist: {}",
manifest_path.display()
));
)));
}
// Check if the prototype directory exists
if !prototype_dir.exists() {
return Err(anyhow!(
return Err(Pkg6DevError::Custom(format!(
"Prototype directory does not exist: {}",
prototype_dir.display()
));
)));
}
// Parse the manifest file
@ -358,14 +374,19 @@ fn publish_package(
let publisher_name = if let Some(pub_name) = publisher {
// Use the explicitly specified publisher
if !repo.config.publishers.contains(pub_name) {
return Err(anyhow!("Publisher '{}' does not exist in the repository. Please add it first using pkg6repo add-publisher.", pub_name));
return Err(Pkg6DevError::Custom(format!(
"Publisher '{}' does not exist in the repository. Please add it first using pkg6repo add-publisher.",
pub_name
)));
}
pub_name.clone()
} else {
// Use the default publisher
match &repo.config.default_publisher {
Some(default_pub) => default_pub.clone(),
None => return Err(anyhow!("No default publisher set in the repository. Please specify a publisher using the --publisher option or set a default publisher."))
None => return Err(Pkg6DevError::Custom(
"No default publisher set in the repository. Please specify a publisher using the --publisher option or set a default publisher.".to_string()
))
}
};
@ -373,6 +394,9 @@ fn publish_package(
println!("Beginning transaction for publisher: {}", publisher_name);
let mut transaction = repo.begin_transaction()?;
// Set the publisher for the transaction
transaction.set_publisher(&publisher_name);
// Add files from the prototype directory to the transaction
println!(
"Adding files from prototype directory: {}",
@ -404,6 +428,10 @@ fn publish_package(
println!("Committing transaction...");
transaction.commit()?;
// Regenerate catalog and search index
println!("Regenerating catalog and search index...");
repo.rebuild(Some(&publisher_name), false, false)?;
println!("Package published successfully!");
Ok(())
}

View file

@ -12,12 +12,14 @@ keywords = ["packaging", "illumos"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
clap = { version = "4.4", features = ["derive"] }
miette = { version = "7.6.0", features = ["fancy"] }
thiserror = "1.0.50"
clap = { version = "4", features = ["derive"] }
miette = { version = "7", features = ["fancy"] }
thiserror = "2"
tracing = "0.1"
tracing-subscriber = "0.3"
libips = { path = "../libips" }
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
[[test]]
name = "e2e_tests"

View file

@ -5,6 +5,8 @@ use clap::{Parser, Subcommand};
use serde::Serialize;
use std::convert::TryFrom;
use std::path::PathBuf;
use tracing::{debug, error, info, warn};
use tracing_subscriber::fmt;
use libips::repository::{FileBackend, ReadableRepository, RepositoryVersion, WritableRepository};
@ -306,6 +308,15 @@ enum Commands {
}
fn main() -> Result<()> {
// Initialize the tracing subscriber with default log level as warning and no decorations
fmt::Subscriber::builder()
.with_max_level(tracing::Level::WARN)
.without_time()
.with_target(false)
.with_ansi(false)
.with_writer(std::io::stderr)
.init();
let cli = App::parse();
match &cli.command {