From 56b50c755a5651e7be54e86612dcf5bd9c6b5556 Mon Sep 17 00:00:00 2001 From: Till Wegmueller Date: Sat, 26 Jul 2025 13:33:56 +0200 Subject: [PATCH] Add comprehensive project guidelines for Junie, detailing structure, dependencies, error handling, testing, build process, code style, and workflow. --- .junie/guidelines.md | 231 +++++++++++++++++++++++ doc/rust_docs/error_handling.md | 322 ++++++++++++++++++++++++++++++++ 2 files changed, 553 insertions(+) create mode 100644 .junie/guidelines.md create mode 100644 doc/rust_docs/error_handling.md diff --git a/.junie/guidelines.md b/.junie/guidelines.md new file mode 100644 index 0000000..c406a2c --- /dev/null +++ b/.junie/guidelines.md @@ -0,0 +1,231 @@ +# Project Guidelines for Junie + +This document provides guidelines for Junie (JetBrains AI Coding Agent) when working on the IPS (Image Packaging System) Rust codebase. + +## Project Structure + +The IPS project is organized as a Rust workspace with multiple crates: + +### Core Library +- **libips**: The core library for the Image Packaging System. Includes Python bindings. + - Contains the fundamental data structures and algorithms for package management + - Used by all other crates in the project + +### Application Crates +- **pkg6depotd**: Depot daemon for serving packages +- **pkg6dev**: Helper tool for IPS package development +- **pkg6repo**: Repository management utility +- **userland**: Userland components +- **specfile**: For handling spec files +- **ports**: Port management +- **crates/pkg6**: CLI tool for package management + +### Dependencies +The project uses several key dependencies: +- **Error handling**: thiserror (currently), with plans to add miette +- **Serialization**: serde and serde_json +- **Parsing**: pest and pest_derive +- **Compression**: flate2 and lz4 +- **Versioning**: semver +- **Search**: searchy and tantivy +- **CLI**: clap + +## Error Handling Guidelines + +The project is transitioning to use miette and thiserror for error handling. Follow these guidelines when implementing error handling: + +### Dependencies Setup + +**For Application Crates:** +```toml +[dependencies] +miette = { version = "7.6.0", features = ["fancy"] } +thiserror = "1.0.50" +tracing = "0.1.37" +tracing-subscriber = "0.3.17" +``` + +**For Library Crates:** +```toml +[dependencies] +miette = "7.6.0" +thiserror = "1.0.50" +tracing = "0.1.37" +``` + +**Rule:** Only enable the "fancy" feature in top-level application crates, not in library crates. + +### Error Type Definition + +Define error types as enums using thiserror and miette's Diagnostic derive macro: + +```rust +use miette::Diagnostic; +use thiserror::Error; + +#[derive(Error, Debug, Diagnostic)] +#[error("A validation error occurred")] +#[diagnostic( + code(ips::validation_error), + help("Please check the input data and try again.") +)] +pub enum ValidationError { + // Error variants go here +} +``` + +### Error Code Naming Convention + +Use a consistent naming scheme for error codes: +- Top-level errors: `ips::category_error` (e.g., `ips::validation_error`) +- Specific errors: `ips::category_error::specific_error` (e.g., `ips::validation_error::invalid_name`) + +Examples: +- `ips::validation_error` +- `ips::validation_error::invalid_name` + +### Error Handling in Library vs. Application Code + +- In library code (like `libips`), always return specific error types +- In application code, you can use `miette::Result` for convenience + +### Decision Tree for Error Handling + +1. **Is this a library or application crate?** + - **Library**: Use specific error types, don't use miette's "fancy" feature + - **Application**: Can use miette::Result for convenience, enable "fancy" feature + +2. **What type of error is being handled?** + - **Input validation**: Create specific error variants with helpful messages + - **I/O operations**: Wrap std::io::Error with transparent error + - **Parsing**: Include source highlighting with NamedSource and SourceSpan + - **External library errors**: Wrap with transparent error or convert with From + +3. **How should the error be propagated?** + - **Within same error type**: Use ? operator + - **Between different error types**: Use map_err or implement From trait + +4. **What level of diagnostic information is needed?** + - **Basic**: Just use #[error] attribute + - **Medium**: Add #[diagnostic] with code and help + - **Detailed**: Include source_code, label, and related information + +For more detailed guidelines on error handling, refer to: +- `/home/toasty/ws/illumos/ips/doc/rust_docs/error_handling.md` +- `/home/toasty/ws/illumos/ips/doc/rust_docs/error_handling_junie.md` + +## Testing Guidelines + +### Running Tests + +To run tests for the entire project: +```bash +cargo test +``` + +To run tests for a specific crate: +```bash +cargo test -p +``` + +### Setting Up Test Environment + +The project includes a script to set up the test environment for repository tests: +```bash +./setup_test_env.sh +``` + +This script: +1. Creates test directories in `/tmp/pkg6_test` +2. Compiles the applications +3. Creates a prototype directory structure with sample files +4. Creates package manifests for testing + +### Writing Tests + +- Unit tests should be placed in the same file as the code they're testing, in a `mod tests` block +- Integration tests should be placed in the `tests` directory of each crate +- End-to-end tests should use the test environment set up by `setup_test_env.sh` + +## Build Guidelines + +### Building the Project + +To build the entire project: +```bash +cargo build +``` + +To build a specific crate: +```bash +cargo build -p +``` + +To build with optimizations for release: +```bash +cargo build --release +``` + +### Build Order + +The crates are built in the following order (as specified in the workspace Cargo.toml): +1. libips +2. pkg6depotd +3. pkg6dev +4. pkg6repo +5. userland +6. specfile +7. ports +8. crates/* + +This order is important as it reflects the dependency hierarchy, with `libips` being the foundation that other crates build upon. + +## Code Style Guidelines + +### General Guidelines + +- Follow the Rust standard style guide +- Use `cargo fmt` to format code +- Use `cargo clippy` to check for common mistakes and improve code quality + +### Naming Conventions + +- Use snake_case for variables, functions, and modules +- Use CamelCase for types, traits, and enums +- Use SCREAMING_SNAKE_CASE for constants +- Prefix unsafe functions with `unsafe_` + +### Documentation + +- Document all public items with doc comments +- Include examples in doc comments where appropriate +- Document error conditions and return values + +### Error Handling + +- Follow the error handling guidelines above +- Use the ? operator for error propagation where appropriate +- Avoid using `unwrap()` or `expect()` in production code + +### Logging + +- Use the tracing crate for logging +- Use appropriate log levels: + - `trace`: Very detailed information + - `debug`: Useful information for debugging + - `info`: General information about the application's operation + - `warn`: Potentially problematic situations + - `error`: Error conditions + +## Workflow for Junie + +When working on the IPS project, Junie should follow this workflow: + +1. **Understand the Issue**: Thoroughly review the issue description and related code +2. **Plan the Changes**: Create a plan for implementing the changes +3. **Implement the Changes**: Make the necessary changes to the code +4. **Test the Changes**: Run tests to ensure the changes work as expected +5. **Document the Changes**: Update documentation as needed +6. **Submit the Changes**: Submit the changes for review + +When implementing error handling, Junie should follow the error handling guidelines above and use the decision tree to determine the appropriate approach. \ No newline at end of file diff --git a/doc/rust_docs/error_handling.md b/doc/rust_docs/error_handling.md new file mode 100644 index 0000000..610dcd7 --- /dev/null +++ b/doc/rust_docs/error_handling.md @@ -0,0 +1,322 @@ +# Error Handling Guidelines for IPS Rust Code + +This document outlines best practices for error handling in the IPS Rust codebase. It covers how to use the `miette` and `thiserror` crates for robust error handling and reporting, and the `tracing` crate for configurable debug output. + +## Core Principles + +The core idea is to combine: +- `thiserror` for creating custom error types with clear error messages +- `miette` for rich, user-friendly error reporting with diagnostic information +- `tracing` for structured, configurable debug output + +## Project Setup + +### Dependencies + +Add the necessary dependencies to your crate's `Cargo.toml`: + +```toml +[dependencies] +miette = { version = "7.6.0", features = ["fancy"] } +thiserror = "1.0.50" +tracing = "0.1.37" +tracing-subscriber = "0.3.17" +``` + +**Note**: The "fancy" feature for miette enables colorful, detailed error reports. This feature should only be enabled in the top-level crate of your project (application crates) to avoid unnecessary dependencies in library crates. + +For library crates like `libips`, use: + +```toml +[dependencies] +miette = "7.6.0" +thiserror = "1.0.50" +tracing = "0.1.37" +``` + +## Defining Custom Error Types + +Define your error types as enums using `thiserror`. This allows you to create specific error variants for different failure scenarios. Then, use miette's `Diagnostic` derive macro to add rich diagnostic information to your errors. + +### Example + +```rust +use miette::{Diagnostic, NamedSource, SourceSpan}; +use thiserror::Error; + +#[derive(Error, Debug, Diagnostic)] +#[error("A validation error occurred")] +#[diagnostic( + code(ips::validation_error), + help("Please check the input data and try again.") +)] +pub enum ValidationError { + #[error("Invalid package name: {0}")] + #[diagnostic( + code(ips::validation_error::invalid_name), + help("Package names must follow the IPS naming conventions.") + )] + InvalidName(String), + + #[error("Invalid version format")] + #[diagnostic( + code(ips::validation_error::invalid_version), + help("Version must be in the format: major.minor.patch") + )] + InvalidVersion { + #[source_code] + src: NamedSource, + + #[label("the invalid version")] + span: SourceSpan, + }, +} +``` + +In this example: + +- `#[derive(Error, Debug, Diagnostic)]` automatically implements the necessary traits from thiserror and miette. +- `#[error("...")]` provides the main error message. +- `#[diagnostic(...)]` adds diagnostic information like a unique error code and a help message. +- For more specific errors, like `InvalidVersion`, you can provide `source_code` and a `label` with a `SourceSpan` to highlight the exact location of the error. + +### Error Codes + +Use a consistent naming scheme for error codes: + +- Top-level errors: `crate_name::error_category` +- Specific errors: `crate_name::error_category::specific_error` + +For example: +- `ips::validation_error` +- `ips::validation_error::invalid_name` + +## Returning Errors + +### In Library Code + +In library code (like `libips`), always return your specific error types. This makes your library's API clear and easy to use. + +```rust +pub fn validate_package(name: &str, version: &str) -> Result<(), ValidationError> { + if !is_valid_package_name(name) { + return Err(ValidationError::InvalidName(name.to_string())); + } + + if !is_valid_version(version) { + let source = NamedSource::new("input.txt", version.to_string()); + let span = (0, version.len()).into(); + return Err(ValidationError::InvalidVersion { src: source, span }); + } + + Ok(()) +} +``` + +### In Application Code + +In your application's main function and other top-level code, you can use `miette::Result` for convenience. + +```rust +use miette::{IntoDiagnostic, Result}; + +fn main() -> Result<()> { + // Initialize tracing + setup_tracing(); + + // Your application logic here + let config = std::fs::read_to_string("config.json").into_diagnostic()?; + + // Process the config + process_config(&config).map_err(|e| e.into())?; + + Ok(()) +} +``` + +### Converting Between Error Types + +To convert from one error type to another, you can use the `From` trait or the `map_err` method on `Result`. + +```rust +// Using From trait +impl From for MyError { + fn from(err: std::io::Error) -> Self { + MyError::IoError(err) + } +} + +// Using map_err +fn read_config() -> Result { + std::fs::read_to_string("config.json") + .map_err(|e| MyError::IoError(e))? + .parse() + .map_err(|e| MyError::ParseError(e)) +} +``` + +## Using Miette's Diagnostic Features + +Miette provides several features to enhance error reporting: + +### Source Code Highlighting + +You can include the source code that caused the error and highlight the specific part that's problematic: + +```rust +#[error("Invalid syntax in configuration file")] +struct InvalidSyntax { + #[source_code] + src: NamedSource, + + #[label("this syntax is invalid")] + span: SourceSpan, +} +``` + +To use this in your code: + +```rust +let source = NamedSource::new("config.json", config_content.to_string()); +let span = (error_start, error_length).into(); +return Err(ConfigError::InvalidSyntax { src: source, span }); +``` + +### Related Information + +You can add related information to provide context for the error: + +```rust +#[error("Failed to process package")] +#[diagnostic( + code(ips::package_error), + help("Check the package manifest for errors.") +)] +ProcessError { + #[related] + related: Vec, +} +``` + +### Custom Help Messages + +Provide helpful messages to guide users on how to fix the error: + +```rust +#[error("Invalid configuration")] +#[diagnostic( + code(ips::config_error), + help("The configuration file must be valid JSON. Check the syntax and try again.") +)] +InvalidConfig, +``` + +## Configurable Debug Output with Tracing + +The `tracing` crate provides a powerful framework for structured, event-based logging. You can use it to add debug output to your library that can be enabled or disabled at runtime. + +### Setting Up Tracing + +In your application's main function, initialize the tracing-subscriber: + +```rust +use tracing_subscriber::{EnvFilter, FmtSubscriber}; + +fn setup_tracing() { + let subscriber = FmtSubscriber::builder() + .with_env_filter(EnvFilter::from_default_env()) + .finish(); + + tracing::subscriber::set_global_default(subscriber) + .expect("setting default subscriber failed"); +} +``` + +### Instrumenting Your Code + +Use the tracing macros (`trace!`, `debug!`, `info!`, `warn!`, `error!`) to add log statements to your code. You can also use the `#[tracing::instrument]` attribute to automatically log the entry and exit of a function, along with its arguments. + +```rust +#[tracing::instrument] +pub fn process_package(package: &Package) -> Result<(), ProcessError> { + tracing::debug!("Processing package: {}", package.name); + + // Your logic here + + tracing::info!("Package processed successfully"); + Ok(()) +} +``` + +### Controlling Log Levels + +You can control the log level using the `RUST_LOG` environment variable. For example, to enable debug output, you would run your application like this: + +```bash +RUST_LOG=debug cargo run +``` + +Common log levels, from most to least verbose: +- `trace`: Very detailed information, typically only useful for debugging specific issues +- `debug`: Useful information for debugging +- `info`: General information about the application's operation +- `warn`: Potentially problematic situations that don't prevent the application from working +- `error`: Error conditions that prevent some functionality from working + +## Best Practices + +1. **Be Specific**: Create specific error variants for different failure scenarios. +2. **Be Helpful**: Include helpful error messages and diagnostic information. +3. **Be Consistent**: Use a consistent naming scheme for error codes. +4. **Be Transparent**: Use `#[error(transparent)]` for wrapping errors from dependencies. +5. **Be Traceable**: Use tracing to log important events and debug information. + +## Example: Converting Existing Code + +Here's an example of how to convert an existing error type to use miette: + +### Before + +```rust +#[derive(Debug, Error)] +pub enum FmriError { + #[error("invalid FMRI format")] + InvalidFormat, + #[error("invalid version format")] + InvalidVersionFormat, + #[error("invalid release format")] + InvalidReleaseFormat, +} +``` + +### After + +```rust +#[derive(Debug, Error, Diagnostic)] +#[diagnostic(code(ips::fmri_error))] +pub enum FmriError { + #[error("invalid FMRI format")] + #[diagnostic( + help("FMRI must be in the format: pkg://publisher/package@version") + )] + InvalidFormat, + + #[error("invalid version format")] + #[diagnostic( + help("Version must be in the format: major.minor.patch") + )] + InvalidVersionFormat, + + #[error("invalid release format")] + #[diagnostic( + help("Release must be a dot-separated list of numbers") + )] + InvalidReleaseFormat, +} +``` + +## Further Reading + +- [miette documentation](https://docs.rs/miette/7.6.0/miette/) +- [thiserror documentation](https://docs.rs/thiserror/1.0.50/thiserror/) +- [tracing documentation](https://docs.rs/tracing/0.1.37/tracing/) \ No newline at end of file