From 6edfc92bb1935ba4a1209b9a4b22cb6ff9b34224 Mon Sep 17 00:00:00 2001 From: Nico Mattes Date: Thu, 12 Mar 2026 18:50:21 +0100 Subject: [PATCH 1/3] improvements to overall workflow --- .github/workflows/docs.yml | 44 +++++++++++++++++++++ .github/workflows/release.yml | 5 +++ .github/workflows/rust-ci.yml | 41 +++++++++++++++++++ .github/workflows/test.yml | 25 ++---------- .github/workflows/validate-packages.yml | 10 +++++ .gitlab-ci.yml | 3 ++ docs/workflows/automation.md | 4 ++ docs/workflows/triggers.md | 35 +++++++++++++---- src/cli/doctor.rs | 23 +++++++++-- src/cli/install.rs | 23 +++++++---- src/cli/list.rs | 7 +++- src/cli/search.rs | 7 +++- src/cli/uninstall.rs | 4 +- src/cli/upgrade.rs | 11 +++++- src/core/database.rs | 11 ++++-- src/core/registry.rs | 3 +- src/core/resolver.rs | 9 ++++- src/ops/build.rs | 52 ++++++++++--------------- src/ops/fetch.rs | 13 ++++--- src/ops/install.rs | 19 +++------ src/platform/mod.rs | 6 ++- src/ui/table.rs | 5 +-- tests/cli_help.rs | 6 ++- tests/database.rs | 15 +++++-- tests/package_parse.rs | 5 ++- tests/platform.rs | 6 ++- 26 files changed, 277 insertions(+), 115 deletions(-) create mode 100644 .github/workflows/docs.yml create mode 100644 .github/workflows/rust-ci.yml diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml new file mode 100644 index 0000000..a0b86c5 --- /dev/null +++ b/.github/workflows/docs.yml @@ -0,0 +1,44 @@ +name: Documentation + +on: + push: + branches: [ main, develop ] + paths: + - 'docs/**' + - 'mkdocs.yml' + - 'requirements-docs.txt' + - '.github/workflows/docs.yml' + pull_request: + branches: [ main, develop ] + paths: + - 'docs/**' + - 'mkdocs.yml' + - 'requirements-docs.txt' + - '.github/workflows/docs.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + build: + name: Build documentation + runs-on: ubuntu-latest + defaults: + run: + shell: bash + + steps: + - name: Checkout repository + uses: actions/checkout@v5.0.1 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.x' + + - name: Install dependencies + run: pip install -r requirements-docs.txt + + - name: Build documentation + run: mkdocs build --strict diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 66d65ac..7ae9c05 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -41,6 +41,11 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@stable + - name: Cache Cargo + uses: Swatinem/rust-cache@v2 + with: + key: ${{ matrix.platform }}-cargo-${{ hashFiles('Cargo.lock') }} + - name: Build run: cargo build --release diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml new file mode 100644 index 0000000..517c1c8 --- /dev/null +++ b/.github/workflows/rust-ci.yml @@ -0,0 +1,41 @@ +name: Rust CI + +on: + workflow_call: + inputs: + runner: + description: 'Runner (e.g. ubuntu-latest, macos-latest, windows-latest)' + required: true + type: string + +permissions: + contents: read + +jobs: + rust: + name: Rust (build, test, clippy, fmt) + runs-on: ${{ inputs.runner }} + + steps: + - name: Checkout code + uses: actions/checkout@v5.0.1 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + + - name: Cache Cargo + uses: Swatinem/rust-cache@v2 + with: + key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock') }} + + - name: Build + run: cargo build --release + + - name: Run tests + run: cargo test + + - name: Run clippy + run: cargo clippy -- -D warnings + + - name: Check formatting + run: cargo fmt --check diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d5f7608..73a6b32 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,28 +22,11 @@ permissions: jobs: test: - name: Test Rust - runs-on: ${{ matrix.os }} + name: Test Rust (${{ matrix.os }}) + uses: ./.github/workflows/rust-ci.yml + with: + runner: ${{ matrix.os }} strategy: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] - - steps: - - name: Checkout code - uses: actions/checkout@v5.0.1 - - - name: Install Rust - uses: dtolnay/rust-toolchain@stable - - - name: Build - run: cargo build --release - - - name: Run tests - run: cargo test - - - name: Run clippy - run: cargo clippy -- -D warnings - - - name: Check formatting - run: cargo fmt --check diff --git a/.github/workflows/validate-packages.yml b/.github/workflows/validate-packages.yml index 26e522f..2cd4724 100644 --- a/.github/workflows/validate-packages.yml +++ b/.github/workflows/validate-packages.yml @@ -187,6 +187,11 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@stable + - name: Cache Cargo + uses: Swatinem/rust-cache@v2 + with: + key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock') }} + - name: Test package parsing run: cargo test test_parse_all_packages @@ -262,6 +267,11 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@stable + - name: Cache Cargo + uses: Swatinem/rust-cache@v2 + with: + key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock') }} + - name: Build TSI run: cargo build --release diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fc4098c..74691dc 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,3 +1,6 @@ +# GitLab CI: mirrors the Rust CI steps from .github/workflows/rust-ci.yml. +# Primary CI is GitHub Actions; this file is for GitLab-based mirrors and forks. + stages: - test - lint diff --git a/docs/workflows/automation.md b/docs/workflows/automation.md index 39795f8..7a2e67b 100644 --- a/docs/workflows/automation.md +++ b/docs/workflows/automation.md @@ -229,6 +229,10 @@ The workflows integrate with your CI/CD pipeline: - Package tests run on new versions - Automated checks ensure quality +### GitLab CI + +**Primary CI is GitHub Actions.** A [`.gitlab-ci.yml`](../../.gitlab-ci.yml) file is provided for GitLab-based mirrors and forks. It mirrors the Rust build, test, clippy, and fmt steps from [`.github/workflows/rust-ci.yml`](../../.github/workflows/rust-ci.yml). When updating Rust CI behavior, keep both in sync so GitLab users get the same checks. + ## See Also - [Version Discovery](version-discovery.md) - Detailed version discovery documentation diff --git a/docs/workflows/triggers.md b/docs/workflows/triggers.md index b7443b3..58cc341 100644 --- a/docs/workflows/triggers.md +++ b/docs/workflows/triggers.md @@ -20,10 +20,28 @@ This document explains when each workflow runs and what triggers them. - Only other workflow files change **Jobs:** -- `test`: Tests Rust build and functionality (matrix: ubuntu-latest, macos-latest, windows-latest); runs `cargo build --release`, `cargo test`, `cargo clippy`, `cargo fmt --check` +- `test`: Calls the reusable [Rust CI](.github/workflows/rust-ci.yml) workflow (matrix: ubuntu-latest, macos-latest, windows-latest); runs build, test, clippy, fmt with Cargo caching. **Manual Trigger:** Yes, can be triggered manually via `workflow_dispatch` +## Documentation Workflow + +**File:** `.github/workflows/docs.yml` + +**Purpose:** Builds MkDocs documentation so doc issues are caught on PRs before release. + +**Triggers:** +- ✅ **Runs when:** + - `docs/**` - Documentation source + - `mkdocs.yml` - MkDocs config + - `requirements-docs.txt` - Doc dependencies + - `.github/workflows/docs.yml` - The workflow file itself + +**Jobs:** +- `build`: Sets up Python, installs doc dependencies, runs `mkdocs build --strict`. + +**Manual Trigger:** Yes, via `workflow_dispatch` + ## Validate Packages Workflow **File:** `.github/workflows/validate-packages.yml` @@ -89,13 +107,14 @@ This document explains when each workflow runs and what triggers them. ## Summary -| Workflow | Triggers on Source Code | Triggers on Packages | Triggers on Tag | Scheduled | -|----------|-------------------------|---------------------|-----------------|-----------| -| TSI Tests | ✅ Yes | ✅ Yes | ❌ No | ❌ No | -| Validate Packages | ❌ No | ✅ Yes | ❌ No | ❌ No | -| Release (binaries + docs) | ❌ No | ❌ No | ✅ Yes | ❌ No | -| Discover Versions | ❌ No | ❌ No | ❌ No | ✅ Weekly | -| Sync External | ❌ No | ❌ No | ❌ No | ❌ No | +| Workflow | Triggers on Source Code | Triggers on Packages | Triggers on Docs | Triggers on Tag | Scheduled | +|----------|-------------------------|---------------------|------------------|-----------------|-----------| +| TSI Tests | ✅ Yes | ✅ Yes | ❌ No | ❌ No | ❌ No | +| Documentation | ❌ No | ❌ No | ✅ Yes | ❌ No | ❌ No | +| Validate Packages | ❌ No | ✅ Yes | ❌ No | ❌ No | ❌ No | +| Release (binaries + docs) | ❌ No | ❌ No | ❌ No | ✅ Yes | ❌ No | +| Discover Versions | ❌ No | ❌ No | ❌ No | ❌ No | ✅ Weekly | +| Sync External | ❌ No | ❌ No | ❌ No | ❌ No | ❌ No | ## Benefits diff --git a/src/cli/doctor.rs b/src/cli/doctor.rs index 9a13176..45bc1b3 100644 --- a/src/cli/doctor.rs +++ b/src/cli/doctor.rs @@ -18,14 +18,22 @@ pub fn run(args: DoctorArgs) -> Result<()> { let mut warnings = 0; let cc = if cfg!(windows) { "cl" } else { "cc" }; - if std::process::Command::new(cc).arg("--version").output().is_ok() { + if std::process::Command::new(cc) + .arg("--version") + .output() + .is_ok() + { ui::output::success("C compiler found"); } else { ui::output::warning("C compiler not found -- required for building"); warnings += 1; } - if std::process::Command::new("make").arg("--version").output().is_ok() { + if std::process::Command::new("make") + .arg("--version") + .output() + .is_ok() + { ui::output::success("make found"); } else { ui::output::warning("make not found -- required for most packages"); @@ -34,7 +42,10 @@ pub fn run(args: DoctorArgs) -> Result<()> { if packages_dir.exists() { let registry = Registry::load_from_dir(&packages_dir).unwrap_or_else(|_| Registry::new()); - ui::output::success(format!("Package definitions: {} packages available", registry.count())); + ui::output::success(format!( + "Package definitions: {} packages available", + registry.count() + )); } else { ui::output::warning("No package definitions -- run 'tsi update'"); warnings += 1; @@ -55,7 +66,11 @@ pub fn run(args: DoctorArgs) -> Result<()> { } } - if std::process::Command::new("git").arg("--version").output().is_ok() { + if std::process::Command::new("git") + .arg("--version") + .output() + .is_ok() + { ui::output::success("git found"); } else { ui::output::warning("git not found -- some packages require git sources"); diff --git a/src/cli/install.rs b/src/cli/install.rs index a369e0f..762ac6b 100644 --- a/src/cli/install.rs +++ b/src/cli/install.rs @@ -22,7 +22,10 @@ pub fn run(args: InstallArgs) -> Result<()> { if !packages_dir.exists() { ui::output::error("No package definitions found. Run 'tsi update' first."); - return Err(anyhow::anyhow!("Package directory not found: {}", packages_dir.display())); + return Err(anyhow::anyhow!( + "Package directory not found: {}", + packages_dir.display() + )); } let registry = Registry::load_from_dir(&packages_dir)?; @@ -41,7 +44,11 @@ pub fn run(args: InstallArgs) -> Result<()> { ui::output::section(format!( "Installing {} packages: {}", order.len(), - order.iter().map(|p| p.name.as_str()).collect::>().join(", ") + order + .iter() + .map(|p| p.name.as_str()) + .collect::>() + .join(", ") )); for pkg in order.iter() { @@ -51,14 +58,16 @@ pub fn run(args: InstallArgs) -> Result<()> { ui::output::section(format!("Building {} {}", pkg.name, pkg.version)); ops_install::install_package(pkg, &prefix, &mut db, args.force)?; - ui::output::section(format!("Linking {} {} into {}", pkg.name, pkg.version, prefix.display())); + ui::output::section(format!( + "Linking {} {} into {}", + pkg.name, + pkg.version, + prefix.display() + )); } ui::output::section("Summary"); - ui::output::detail(format!( - "{} packages installed successfully.", - order.len() - )); + ui::output::detail(format!("{} packages installed successfully.", order.len())); if let Some(last) = order.last() { ui::output::detail(format!("{} {} is ready to use.", last.name, last.version)); } diff --git a/src/cli/list.rs b/src/cli/list.rs index fe71d12..8045e46 100644 --- a/src/cli/list.rs +++ b/src/cli/list.rs @@ -23,7 +23,12 @@ pub fn run(args: ListArgs) -> Result<()> { } ui::output::section("Installed packages:"); - let max_name = packages.iter().map(|p| p.name.len()).max().unwrap_or(0).max(20); + let max_name = packages + .iter() + .map(|p| p.name.len()) + .max() + .unwrap_or(0) + .max(20); for pkg in packages { ui::output::detail(format!( "{} {}", diff --git a/src/cli/search.rs b/src/cli/search.rs index 963ee2a..11d109e 100644 --- a/src/cli/search.rs +++ b/src/cli/search.rs @@ -28,7 +28,12 @@ pub fn run(args: SearchArgs) -> Result<()> { return Ok(()); } - let max_name = results.iter().map(|p| p.name.len()).max().unwrap_or(0).max(20); + let max_name = results + .iter() + .map(|p| p.name.len()) + .max() + .unwrap_or(0) + .max(20); for pkg in results { ui::output::detail(format!( "{} {} {}", diff --git a/src/cli/uninstall.rs b/src/cli/uninstall.rs index adea9d9..408fef8 100644 --- a/src/cli/uninstall.rs +++ b/src/cli/uninstall.rs @@ -13,7 +13,9 @@ pub struct UninstallArgs { pub fn run(args: UninstallArgs) -> Result<()> { if args.packages.is_empty() { ui::output::error("No packages specified"); - return Err(anyhow::anyhow!("Usage: tsi uninstall [package...]")); + return Err(anyhow::anyhow!( + "Usage: tsi uninstall [package...]" + )); } let prefix = platform::resolve_prefix(args.prefix.as_deref()); diff --git a/src/cli/upgrade.rs b/src/cli/upgrade.rs index da9507a..90142dd 100644 --- a/src/cli/upgrade.rs +++ b/src/cli/upgrade.rs @@ -48,13 +48,20 @@ pub fn run(args: UpgradeArgs) -> Result<()> { let mut db_mut = Database::new(&db_dir)?; crate::ops::uninstall::uninstall_package(name, &prefix, &mut db_mut)?; let installed_set = db_mut.installed_set(); - let packages = resolver::resolve(®istry, &format!("{}@{}", name, latest.version), &installed_set)?; + let packages = resolver::resolve( + ®istry, + &format!("{}@{}", name, latest.version), + &installed_set, + )?; let order = resolver::get_build_order(&packages); for pkg in &order { ops_install::install_package(pkg, &prefix, &mut db_mut, true)?; } } else { - ui::output::detail(format!("{} {} is already up to date", name, installed.version)); + ui::output::detail(format!( + "{} {} is already up to date", + name, installed.version + )); } } } else { diff --git a/src/core/database.rs b/src/core/database.rs index c38f894..62c5a15 100644 --- a/src/core/database.rs +++ b/src/core/database.rs @@ -32,8 +32,7 @@ impl Database { std::fs::create_dir_all(db_dir).context("Failed to create database directory")?; let path = db_dir.join("installed.json"); let packages = if path.exists() { - let json = std::fs::read_to_string(&path) - .context("Failed to read database")?; + let json = std::fs::read_to_string(&path).context("Failed to read database")?; let db: DatabaseFile = serde_json::from_str(&json).unwrap_or(DatabaseFile { schema_version: SCHEMA_VERSION, installed: vec![], @@ -71,7 +70,13 @@ impl Database { self.packages.iter().any(|p| p.name == name) } - pub fn add(&mut self, name: &str, version: &str, install_path: &Path, deps: &[String]) -> Result<()> { + pub fn add( + &mut self, + name: &str, + version: &str, + install_path: &Path, + deps: &[String], + ) -> Result<()> { if self.is_installed(name) { return Ok(()); } diff --git a/src/core/registry.rs b/src/core/registry.rs index 122942a..a411afe 100644 --- a/src/core/registry.rs +++ b/src/core/registry.rs @@ -75,8 +75,7 @@ impl Registry { .values() .filter_map(|versions| versions.first()) .filter(|p| { - p.name.to_lowercase().contains(&q) - || p.description.to_lowercase().contains(&q) + p.name.to_lowercase().contains(&q) || p.description.to_lowercase().contains(&q) }) .collect(); results.sort_by(|a, b| a.name.cmp(&b.name)); diff --git a/src/core/resolver.rs b/src/core/resolver.rs index 05d5395..59cd6de 100644 --- a/src/core/resolver.rs +++ b/src/core/resolver.rs @@ -11,7 +11,14 @@ pub fn resolve( let mut result = Vec::new(); let mut visited = HashSet::new(); let mut stack = HashSet::new(); - resolve_recursive(registry, spec, installed, &mut result, &mut visited, &mut stack)?; + resolve_recursive( + registry, + spec, + installed, + &mut result, + &mut visited, + &mut stack, + )?; Ok(result) } diff --git a/src/ops/build.rs b/src/ops/build.rs index 9007b3b..19595fb 100644 --- a/src/ops/build.rs +++ b/src/ops/build.rs @@ -54,12 +54,7 @@ fn build_env_base(prefix: &Path) -> Vec<(String, String)> { let path = std::env::var("PATH").unwrap_or_default(); let path_sep = if cfg!(windows) { ";" } else { ":" }; - let new_path = format!( - "{}{}{}", - bin.display(), - path_sep, - path - ); + let new_path = format!("{}{}{}", bin.display(), path_sep, path); vec![ ("PATH".to_string(), new_path), @@ -71,14 +66,8 @@ fn build_env_base(prefix: &Path) -> Vec<(String, String)> { "LD_LIBRARY_PATH".to_string(), lib.to_string_lossy().to_string(), ), - ( - "CPPFLAGS".to_string(), - format!("-I{}", include.display()), - ), - ( - "LDFLAGS".to_string(), - format!("-L{}", lib.display()), - ), + ("CPPFLAGS".to_string(), format!("-I{}", include.display())), + ("LDFLAGS".to_string(), format!("-L{}", lib.display())), ] } @@ -114,7 +103,9 @@ fn build_autotools( run_cmd(Command::new("make").current_dir(source_dir), env)?; run_cmd( - Command::new("make").args(["install"]).current_dir(source_dir), + Command::new("make") + .args(["install"]) + .current_dir(source_dir), env, )?; Ok(()) @@ -139,13 +130,11 @@ fn build_cmake( run_cmd(Command::new("cmake").args(&cmake_args), env)?; run_cmd( - Command::new("cmake") - .args(["--build", build_dir.to_string_lossy().as_ref()]), + Command::new("cmake").args(["--build", build_dir.to_string_lossy().as_ref()]), env, )?; run_cmd( - Command::new("cmake") - .args(["--install", build_dir.to_string_lossy().as_ref()]), + Command::new("cmake").args(["--install", build_dir.to_string_lossy().as_ref()]), env, )?; Ok(()) @@ -160,24 +149,21 @@ fn build_meson( ) -> Result<()> { let prefix = install_dir.to_string_lossy(); run_cmd( - Command::new("meson") - .args([ - "setup", - build_dir.to_string_lossy().as_ref(), - source_dir.to_string_lossy().as_ref(), - "--prefix", - &prefix, - ]), + Command::new("meson").args([ + "setup", + build_dir.to_string_lossy().as_ref(), + source_dir.to_string_lossy().as_ref(), + "--prefix", + &prefix, + ]), env, )?; run_cmd( - Command::new("meson") - .args(["compile", "-C", build_dir.to_string_lossy().as_ref()]), + Command::new("meson").args(["compile", "-C", build_dir.to_string_lossy().as_ref()]), env, )?; run_cmd( - Command::new("meson") - .args(["install", "-C", build_dir.to_string_lossy().as_ref()]), + Command::new("meson").args(["install", "-C", build_dir.to_string_lossy().as_ref()]), env, )?; Ok(()) @@ -194,7 +180,9 @@ fn build_make( let mut make_args = pkg.make_args.clone(); make_args.push(format!("PREFIX={}", prefix)); run_cmd( - Command::new("make").args(&make_args).current_dir(source_dir), + Command::new("make") + .args(&make_args) + .current_dir(source_dir), env, )?; run_cmd( diff --git a/src/ops/fetch.rs b/src/ops/fetch.rs index 0616983..8df0539 100644 --- a/src/ops/fetch.rs +++ b/src/ops/fetch.rs @@ -14,7 +14,10 @@ pub fn fetch(pkg: &Package, dest_dir: &Path, force: bool) -> Result fetch_archive(pkg, dest_dir, force), "git" => fetch_git(pkg, dest_dir, force), "local" => fetch_local(pkg, dest_dir), - _ => Err(anyhow::anyhow!("Unknown source type: {}", pkg.source.source_type)), + _ => Err(anyhow::anyhow!( + "Unknown source type: {}", + pkg.source.source_type + )), } } @@ -63,7 +66,8 @@ fn fetch_archive(pkg: &Package, dest_dir: &Path, force: bool) -> Result Result<()> { } fn extract_archive(archive: &Path, dest: &Path) -> Result<()> { - let ext = archive - .extension() - .and_then(|e| e.to_str()) - .unwrap_or(""); + let ext = archive.extension().and_then(|e| e.to_str()).unwrap_or(""); let path_str = archive.to_string_lossy(); if path_str.ends_with(".zip") { diff --git a/src/ops/install.rs b/src/ops/install.rs index 18b5658..6fc903c 100644 --- a/src/ops/install.rs +++ b/src/ops/install.rs @@ -6,14 +6,11 @@ use crate::ops::link; use anyhow::{Context, Result}; use std::path::Path; -pub fn install_package( - pkg: &Package, - prefix: &Path, - db: &mut Database, - force: bool, -) -> Result<()> { +pub fn install_package(pkg: &Package, prefix: &Path, db: &mut Database, force: bool) -> Result<()> { let sources_dir = prefix.join("sources"); - let build_dir = prefix.join("build").join(format!("{}-{}", pkg.name, pkg.version)); + let build_dir = prefix + .join("build") + .join(format!("{}-{}", pkg.name, pkg.version)); let install_dir = prefix .join("install") .join(format!("{}-{}", pkg.name, pkg.version)); @@ -23,13 +20,7 @@ pub fn install_package( let source_dir = fetch::fetch(pkg, &sources_dir, force)?; - build::build( - pkg, - &source_dir, - &build_dir, - &install_dir, - &main_install, - )?; + build::build(pkg, &source_dir, &build_dir, &install_dir, &main_install)?; link::create_symlinks(&install_dir, &main_install)?; diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 860f944..d55af4a 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -61,7 +61,11 @@ pub fn resolve_prefix(user_prefix: Option<&str>) -> PathBuf { fn detect_prefix_from_binary() -> Option { let exe = std::env::current_exe().ok()?; let exe_str = exe.to_string_lossy(); - let bin_tsi = if cfg!(windows) { r"\bin\tsi.exe" } else { "/bin/tsi" }; + let bin_tsi = if cfg!(windows) { + r"\bin\tsi.exe" + } else { + "/bin/tsi" + }; if let Some(pos) = exe_str.find(bin_tsi) { let prefix = exe_str[..pos].to_string(); if !prefix.is_empty() { diff --git a/src/ui/table.rs b/src/ui/table.rs index e1f9025..0023ee3 100644 --- a/src/ui/table.rs +++ b/src/ui/table.rs @@ -9,10 +9,7 @@ pub fn pad_right(s: &str, width: usize) -> String { } } -pub fn format_list_table( - rows: &[(T, U)], - name_width: usize, -) -> String { +pub fn format_list_table(rows: &[(T, U)], name_width: usize) -> String { let mut out = String::new(); for (name, version) in rows { out.push_str(&pad_right(&name.to_string(), name_width)); diff --git a/tests/cli_help.rs b/tests/cli_help.rs index d66228b..55e5bcb 100644 --- a/tests/cli_help.rs +++ b/tests/cli_help.rs @@ -10,7 +10,11 @@ fn tsi_cmd(args: &[&str]) -> (std::process::Output, bool) { #[test] fn test_tsi_help() { let (output, success) = tsi_cmd(&["--help"]); - assert!(success, "tsi --help should succeed: {}", String::from_utf8_lossy(&output.stderr)); + assert!( + success, + "tsi --help should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); assert!(String::from_utf8_lossy(&output.stdout).contains("TSI")); assert!(String::from_utf8_lossy(&output.stdout).contains("install")); } diff --git a/tests/database.rs b/tests/database.rs index 852075e..4b863fb 100644 --- a/tests/database.rs +++ b/tests/database.rs @@ -1,4 +1,3 @@ -use std::path::Path; use tsi::core::database::Database; #[test] @@ -9,7 +8,8 @@ fn test_database_create_and_add() { let mut db = Database::new(db_dir).unwrap(); assert!(!db.is_installed("foo")); - db.add("foo", "1.0.0", &db_dir.join("install/foo"), &[]).unwrap(); + db.add("foo", "1.0.0", &db_dir.join("install/foo"), &[]) + .unwrap(); assert!(db.is_installed("foo")); let pkg = db.get("foo").unwrap(); @@ -23,7 +23,13 @@ fn test_database_remove() { let db_dir = temp.path(); let mut db = Database::new(db_dir).unwrap(); - db.add("bar", "2.0.0", &db_dir.join("install/bar"), &["dep1".into()]).unwrap(); + db.add( + "bar", + "2.0.0", + &db_dir.join("install/bar"), + &["dep1".into()], + ) + .unwrap(); assert!(db.is_installed("bar")); let removed = db.remove("bar").unwrap(); @@ -41,7 +47,8 @@ fn test_database_persistence() { { let mut db = Database::new(db_dir).unwrap(); - db.add("baz", "3.0.0", &db_dir.join("install/baz"), &[]).unwrap(); + db.add("baz", "3.0.0", &db_dir.join("install/baz"), &[]) + .unwrap(); } let db = Database::new(db_dir).unwrap(); diff --git a/tests/package_parse.rs b/tests/package_parse.rs index 9b4de95..8960eb8 100644 --- a/tests/package_parse.rs +++ b/tests/package_parse.rs @@ -7,5 +7,8 @@ fn test_parse_all_packages() { return; } let registry = tsi::core::registry::Registry::load_from_dir(&packages_dir).unwrap(); - assert!(registry.count() > 0, "Should have parsed at least one package"); + assert!( + registry.count() > 0, + "Should have parsed at least one package" + ); } diff --git a/tests/platform.rs b/tests/platform.rs index 8028acf..0e6f8a2 100644 --- a/tests/platform.rs +++ b/tests/platform.rs @@ -19,7 +19,11 @@ fn test_os_name_is_non_empty() { fn test_default_prefix_contains_tsi() { let prefix = platform::default_prefix(); let s = prefix.to_string_lossy(); - assert!(s.contains(".tsi"), "default_prefix should contain .tsi: {}", s); + assert!( + s.contains(".tsi"), + "default_prefix should contain .tsi: {}", + s + ); } #[test] From 6da709f91a5267f6d70799e26ad57d7f73b8af7e Mon Sep 17 00:00:00 2001 From: Nico Mattes Date: Thu, 12 Mar 2026 19:05:49 +0100 Subject: [PATCH 2/3] implemented better coding practices --- .github/workflows/validate-packages.yml | 12 +++- docs/developer-guide/architecture.md | 2 +- mkdocs.yml | 1 + scripts/discover-versions.py | 96 ++++++++++--------------- scripts/lib_json.py | 18 +++++ scripts/merge-external-package.py | 20 ++---- scripts/validate-package-versions.py | 20 ++++-- src/cli/info.rs | 9 +-- src/cli/install.rs | 12 +--- src/cli/mod.rs | 18 +++++ src/cli/search.rs | 9 +-- src/cli/update.rs | 37 +++++----- src/cli/upgrade.rs | 9 +-- src/core/config.rs | 28 +++++--- src/core/database.rs | 43 ++++++----- src/core/mod.rs | 2 + src/core/package.rs | 3 + src/core/registry.rs | 3 +- src/core/resolver.rs | 2 + src/ops/fetch.rs | 23 +++--- src/ops/install.rs | 1 + src/ops/mod.rs | 2 + src/platform/unix.rs | 9 +-- src/ui/output.rs | 2 +- src/ui/progress.rs | 6 +- tests/cli_help.rs | 69 ++++++------------ 26 files changed, 219 insertions(+), 237 deletions(-) create mode 100644 scripts/lib_json.py diff --git a/.github/workflows/validate-packages.yml b/.github/workflows/validate-packages.yml index 2cd4724..87aba8d 100644 --- a/.github/workflows/validate-packages.yml +++ b/.github/workflows/validate-packages.yml @@ -85,10 +85,16 @@ jobs: fi done - # Validate source object - if ! python3 -c "import json, sys; data = json.load(open('$pkg')); sys.exit(0 if isinstance(data.get('source'), dict) and 'type' in data.get('source', {}) and 'url' in data.get('source', {}) else 1)" 2>/dev/null; then + # Validate source object (url for git/tarball/zip; path for type local) + if ! python3 -c " +import json, sys +data = json.load(open('$pkg')) +s = data.get('source', {}) +ok = isinstance(s, dict) and 'type' in s and ('url' in s or (s.get('type') == 'local' and 'path' in s)) +sys.exit(0 if ok else 1) +" 2>/dev/null; then echo "❌ Invalid or missing 'source' object in $pkg" - echo " Source must be an object with 'type' and 'url' fields" + echo " Source must have 'type' and either 'url' (git/tarball/zip) or 'path' (local)" FAILED=1 fi diff --git a/docs/developer-guide/architecture.md b/docs/developer-guide/architecture.md index 46fe5ba..30a9a6d 100644 --- a/docs/developer-guide/architecture.md +++ b/docs/developer-guide/architecture.md @@ -18,7 +18,7 @@ TSI is implemented in Rust with a single static binary and zero runtime dependen - **Language**: Rust (edition 2021) - **Build System**: Cargo -- **Dependencies**: Minimal crates (serde, clap, reqwest, etc.) +- **Dependencies**: Minimal crates (serde, clap, ureq, etc.) - **Output**: Single executable (`tsi` or `tsi.exe`) ### Module Layout diff --git a/mkdocs.yml b/mkdocs.yml index 481bf8b..c4064ad 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -71,6 +71,7 @@ nav: - Reference: - CLI Reference: reference/cli-reference.md - Package Repository: reference/package-repository.md + - Basic Packages: Basic_Packages.md - Scripts: reference/scripts.md - Bootstrap Options: reference/bootstrap-options.md diff --git a/scripts/discover-versions.py b/scripts/discover-versions.py index 6d5081a..d0b8e82 100755 --- a/scripts/discover-versions.py +++ b/scripts/discover-versions.py @@ -26,18 +26,27 @@ from typing import List, Dict, Optional, Tuple from datetime import datetime +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from lib_json import load_json, save_json -def load_json(filepath): - """Load and parse a JSON file.""" - with open(filepath, 'r') as f: - return json.load(f) + +def get_latest_version_def(pkg: dict) -> Optional[dict]: + """Return the latest version definition (first in versions array or single-version pkg).""" + if "versions" in pkg and pkg["versions"]: + return pkg["versions"][0] + if "version" in pkg: + return pkg + return None -def save_json(filepath, data): - """Save data as formatted JSON.""" - with open(filepath, 'w') as f: - json.dump(data, f, indent=2) - f.write('\n') +def normalize_version_tag(tag: str) -> Optional[str]: + """Normalize a tag (e.g. v1.2.3 or pkg-1.2.3) to a version string, or None if not version-like.""" + version = tag.lstrip("v") if tag.startswith("v") else tag + if "-" in version or "_" in version: + parts = version.replace("_", "-").split("-") + if parts and re.match(r"^\d+\.\d+", parts[-1]): + version = parts[-1] + return version if (version and re.match(r"^\d+", version)) else None def fetch_url(url: str, headers: Optional[Dict] = None, token: Optional[str] = None) -> Optional[str]: @@ -104,18 +113,8 @@ def discover_github_versions(repo: str, max_versions: Optional[int] = None, toke break for release in releases: - tag = release.get('tag_name', '') - # Remove 'v' prefix if present - version = tag.lstrip('v') if tag.startswith('v') else tag - # Remove package name prefix if present (e.g., "pcre2-10.43" -> "10.43") - # Common patterns: package-name-version, package_version - if '-' in version or '_' in version: - # Try to extract just the version part (assume version is at the end) - parts = version.replace('_', '-').split('-') - # Check if last part looks like a version (contains digits and dots) - if parts and re.match(r'^\d+\.\d+', parts[-1]): - version = parts[-1] - if version and re.match(r'^\d+', version): # Must start with digit + version = normalize_version_tag(release.get("tag_name", "")) + if version: versions.append(version) # Check if we've reached max_versions limit @@ -142,26 +141,18 @@ def discover_github_versions(repo: str, max_versions: Optional[int] = None, toke break for tag in tags: - name = tag.get('name', '') - version = name.lstrip('v') if name.startswith('v') else name - # Remove package name prefix if present - if '-' in version or '_' in version: - parts = version.replace('_', '-').split('-') - if parts and re.match(r'^\d+\.\d+', parts[-1]): - version = parts[-1] - if version and re.match(r'^\d+', version): # Must start with digit - # Filter out development versions with very high patch numbers - # (e.g., 9.1.1924 is likely a development version, not a release) - parts = version.split('.') - if len(parts) >= 3: - try: - patch = int(parts[2]) - # Skip versions with patch number >= 1000 (likely development versions) - if patch >= 1000: - continue - except ValueError: - pass - versions.append(version) + version = normalize_version_tag(tag.get("name", "")) + if not version: + continue + # Filter out development versions with very high patch numbers + parts = version.split(".") + if len(parts) >= 3: + try: + if int(parts[2]) >= 1000: + continue + except ValueError: + pass + versions.append(version) # Check if we've reached max_versions limit if max_versions and len(versions) >= max_versions: @@ -318,16 +309,11 @@ def discover_package_versions(package_file: Path, max_versions: Optional[int] = return [] pkg = load_json(package_file) - - # Get the latest version as template - if 'versions' in pkg and pkg['versions']: - latest = pkg['versions'][0] - elif 'version' in pkg: - latest = pkg - else: + latest = get_latest_version_def(pkg) + if not latest: return [] - source = latest.get('source', {}) + source = latest.get("source", {}) source_type = source.get('type', 'tarball') source_url = source.get('url', '') @@ -443,18 +429,12 @@ def check_and_add_version(package_file: Path, target_version: str, token: Option if not package_file.exists(): return False - # Load package to get source info pkg = load_json(package_file) - - # Get the latest version as template - if 'versions' in pkg and pkg['versions']: - latest = pkg['versions'][0] - elif 'version' in pkg: - latest = pkg - else: + latest = get_latest_version_def(pkg) + if not latest: return False - source = latest.get('source', {}) + source = latest.get("source", {}) source_url = source.get('url', '') # Check if target version matches filtering rules (same as discover_github_versions) diff --git a/scripts/lib_json.py b/scripts/lib_json.py new file mode 100644 index 0000000..e08a05b --- /dev/null +++ b/scripts/lib_json.py @@ -0,0 +1,18 @@ +"""Shared JSON read/write helpers for TSI scripts.""" + +import json +from pathlib import Path +from typing import Any, Union + + +def load_json(filepath: Union[str, Path]) -> Any: + """Load and parse a JSON file.""" + with open(filepath, "r") as f: + return json.load(f) + + +def save_json(filepath: Union[str, Path], data: Any) -> None: + """Save data as formatted JSON.""" + with open(filepath, "w") as f: + json.dump(data, f, indent=2) + f.write("\n") diff --git a/scripts/merge-external-package.py b/scripts/merge-external-package.py index 69fc450..19f609b 100755 --- a/scripts/merge-external-package.py +++ b/scripts/merge-external-package.py @@ -16,18 +16,8 @@ import os from pathlib import Path - -def load_json(filepath): - """Load and parse a JSON file.""" - with open(filepath, 'r') as f: - return json.load(f) - - -def save_json(filepath, data): - """Save data as formatted JSON.""" - with open(filepath, 'w') as f: - json.dump(data, f, indent=2) - f.write('\n') +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from lib_json import load_json, save_json def merge_package_version(external_pkg, packages_dir, package_name=None): @@ -124,7 +114,11 @@ def main(): try: # Load external package - external_pkg = load_json(external_file) + try: + external_pkg = load_json(external_file) + except json.JSONDecodeError as e: + print(f"Error: Invalid JSON in {external_file}: {e}", file=sys.stderr) + sys.exit(1) # Merge into packages repository package_file, was_created, was_updated = merge_package_version( diff --git a/scripts/validate-package-versions.py b/scripts/validate-package-versions.py index 67fbe0d..c0d388a 100755 --- a/scripts/validate-package-versions.py +++ b/scripts/validate-package-versions.py @@ -5,13 +5,20 @@ import sys def validate_versions(pkg_file): - """Validate all versions in a multi-version package file.""" - with open(pkg_file, 'r') as f: - data = json.load(f) + """Validate structure and required fields of each version in a multi-version package file.""" + try: + with open(pkg_file, "r") as f: + data = json.load(f) + except FileNotFoundError: + print(f"Error: File not found: {pkg_file}") + sys.exit(1) + except json.JSONDecodeError as e: + print(f"Error: Invalid JSON in {pkg_file}: {e}") + sys.exit(1) versions = data.get('versions', []) valid_types = ['git', 'tarball', 'zip', 'local'] - valid_build_systems = ['autotools', 'cmake', 'meson', 'make', 'cargo', 'custom'] + valid_build_systems = ['autotools', 'cmake', 'meson', 'make', 'custom'] array_fields = ['dependencies', 'build_dependencies', 'configure_args', 'cmake_args', 'make_args', 'patches'] failed = False @@ -28,7 +35,10 @@ def validate_versions(pkg_file): continue source = version.get('source', {}) - if not isinstance(source, dict) or 'type' not in source or 'url' not in source: + has_url = isinstance(source, dict) and 'type' in source and ( + 'url' in source or (source.get('type') == 'local' and 'path' in source) + ) + if not has_url: print(f'❌ Version {i+1} has invalid or missing source object in {pkg_file}') failed = True continue diff --git a/src/cli/info.rs b/src/cli/info.rs index 1c5cef3..c67166d 100644 --- a/src/cli/info.rs +++ b/src/cli/info.rs @@ -1,5 +1,4 @@ use crate::core::registry::Registry; -use crate::platform; use crate::ui; use anyhow::Result; use clap::Args; @@ -11,13 +10,7 @@ pub struct InfoArgs { } pub fn run(args: InfoArgs) -> Result<()> { - let prefix = platform::resolve_prefix(args.prefix.as_deref()); - let packages_dir = prefix.join("packages"); - - if !packages_dir.exists() { - ui::output::error("No package definitions found. Run 'tsi update' first."); - return Err(anyhow::anyhow!("Package directory not found")); - } + let (_prefix, packages_dir) = crate::cli::resolve_packages_dir(args.prefix.as_deref())?; let registry = Registry::load_from_dir(&packages_dir)?; let pkg = registry diff --git a/src/cli/install.rs b/src/cli/install.rs index 762ac6b..284ba11 100644 --- a/src/cli/install.rs +++ b/src/cli/install.rs @@ -2,7 +2,6 @@ use crate::core::database::Database; use crate::core::registry::Registry; use crate::core::resolver; use crate::ops::install as ops_install; -use crate::platform; use crate::ui; use anyhow::Result; use clap::Args; @@ -16,18 +15,9 @@ pub struct InstallArgs { } pub fn run(args: InstallArgs) -> Result<()> { - let prefix = platform::resolve_prefix(args.prefix.as_deref()); - let packages_dir = prefix.join("packages"); + let (prefix, packages_dir) = crate::cli::resolve_packages_dir(args.prefix.as_deref())?; let db_dir = prefix.join("db"); - if !packages_dir.exists() { - ui::output::error("No package definitions found. Run 'tsi update' first."); - return Err(anyhow::anyhow!( - "Package directory not found: {}", - packages_dir.display() - )); - } - let registry = Registry::load_from_dir(&packages_dir)?; let mut db = Database::new(&db_dir)?; let installed = db.installed_set(); diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 345009d..e3a33bd 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -9,6 +9,24 @@ mod upgrade; use anyhow::Result; use clap::{Parser, Subcommand}; +use std::path::PathBuf; + +use crate::platform; +use crate::ui; + +/// Resolves prefix and packages directory; errors if packages dir does not exist. +pub fn resolve_packages_dir(prefix: Option<&str>) -> Result<(PathBuf, PathBuf)> { + let prefix = platform::resolve_prefix(prefix); + let packages_dir = prefix.join("packages"); + if !packages_dir.exists() { + ui::output::error("No package definitions found. Run 'tsi update' first."); + return Err(anyhow::anyhow!( + "Package directory not found: {}", + packages_dir.display() + )); + } + Ok((prefix, packages_dir)) +} #[derive(Parser)] #[command(name = "tsi")] diff --git a/src/cli/search.rs b/src/cli/search.rs index 11d109e..c4dda50 100644 --- a/src/cli/search.rs +++ b/src/cli/search.rs @@ -1,5 +1,4 @@ use crate::core::registry::Registry; -use crate::platform; use crate::ui; use anyhow::Result; use clap::Args; @@ -11,13 +10,7 @@ pub struct SearchArgs { } pub fn run(args: SearchArgs) -> Result<()> { - let prefix = platform::resolve_prefix(args.prefix.as_deref()); - let packages_dir = prefix.join("packages"); - - if !packages_dir.exists() { - ui::output::error("No package definitions found. Run 'tsi update' first."); - return Err(anyhow::anyhow!("Package directory not found")); - } + let (_prefix, packages_dir) = crate::cli::resolve_packages_dir(args.prefix.as_deref())?; let registry = Registry::load_from_dir(&packages_dir)?; let results = registry.search(&args.query); diff --git a/src/cli/update.rs b/src/cli/update.rs index 2e6768b..b65121d 100644 --- a/src/cli/update.rs +++ b/src/cli/update.rs @@ -1,11 +1,23 @@ use crate::platform; use crate::ui; -use anyhow::Result; +use anyhow::{Context, Result}; use clap::Args; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; const DEFAULT_REPO: &str = "https://github.com/PanterSoft/tsi.git"; +fn copy_package_jsons(from_dir: &Path, packages_dir: &Path) -> Result<()> { + for entry in std::fs::read_dir(from_dir).context("Read package source dir")? { + let entry = entry?; + let path = entry.path(); + if path.extension().is_some_and(|e| e == "json") { + let dest = packages_dir.join(entry.file_name()); + std::fs::copy(&path, &dest).context("Copy package file")?; + } + } + Ok(()) +} + #[derive(Args)] pub struct UpdateArgs { #[arg(long)] @@ -27,14 +39,7 @@ pub fn run(args: UpdateArgs) -> Result<()> { if !src.is_dir() { return Err(anyhow::anyhow!("Local path is not a directory: {}", local)); } - for entry in std::fs::read_dir(&src)? { - let entry = entry?; - let path = entry.path(); - if path.extension().is_some_and(|e| e == "json") { - let dest = packages_dir.join(entry.file_name()); - std::fs::copy(&path, &dest)?; - } - } + copy_package_jsons(&src, &packages_dir)?; ui::output::detail("Package definitions updated"); return Ok(()); } @@ -53,7 +58,8 @@ pub fn run(args: UpdateArgs) -> Result<()> { } if !tmp.exists() { let status = std::process::Command::new("git") - .args(["clone", "--depth", "1", repo, tmp.to_str().unwrap()]) + .args(["clone", "--depth", "1", repo]) + .arg(&tmp) .status()?; if !status.success() { return Err(anyhow::anyhow!("git clone failed")); @@ -62,14 +68,7 @@ pub fn run(args: UpdateArgs) -> Result<()> { let src_packages = tmp.join("packages"); if src_packages.exists() { - for entry in std::fs::read_dir(&src_packages)? { - let entry = entry?; - let path = entry.path(); - if path.extension().is_some_and(|e| e == "json") { - let dest = packages_dir.join(entry.file_name()); - std::fs::copy(&path, &dest)?; - } - } + copy_package_jsons(&src_packages, &packages_dir)?; } ui::output::detail("Package definitions updated"); Ok(()) diff --git a/src/cli/upgrade.rs b/src/cli/upgrade.rs index 90142dd..88b3ce7 100644 --- a/src/cli/upgrade.rs +++ b/src/cli/upgrade.rs @@ -2,7 +2,6 @@ use crate::core::database::Database; use crate::core::registry::Registry; use crate::core::resolver; use crate::ops::install as ops_install; -use crate::platform; use crate::ui; use anyhow::Result; use clap::Args; @@ -14,15 +13,9 @@ pub struct UpgradeArgs { } pub fn run(args: UpgradeArgs) -> Result<()> { - let prefix = platform::resolve_prefix(args.prefix.as_deref()); - let packages_dir = prefix.join("packages"); + let (prefix, packages_dir) = crate::cli::resolve_packages_dir(args.prefix.as_deref())?; let db_dir = prefix.join("db"); - if !packages_dir.exists() { - ui::output::error("No package definitions found. Run 'tsi update' first."); - return Err(anyhow::anyhow!("Package directory not found")); - } - let registry = Registry::load_from_dir(&packages_dir)?; let db = Database::new(&db_dir)?; diff --git a/src/core/config.rs b/src/core/config.rs index 0c88f36..068334f 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -16,16 +16,26 @@ struct ConfigFile { impl Config { pub fn load(prefix: &Path) -> Self { let path = prefix.join("tsi.toml"); - if path.exists() { - if let Ok(toml) = std::fs::read_to_string(&path) { - if let Ok(cfg) = toml::from_str::(&toml) { - return Self { - strict_isolation: cfg.strict_isolation.unwrap_or(false), - log_level: cfg.log_level.unwrap_or_else(|| "info".to_string()), - }; - } + if !path.exists() { + return Self::default(); + } + let toml = match std::fs::read_to_string(&path) { + Ok(t) => t, + Err(e) => { + log::warn!("Failed to read config {}: {}", path.display(), e); + return Self::default(); + } + }; + let cfg = match toml::from_str::(&toml) { + Ok(c) => c, + Err(e) => { + log::warn!("Failed to parse config {}: {}", path.display(), e); + return Self::default(); } + }; + Self { + strict_isolation: cfg.strict_isolation.unwrap_or(false), + log_level: cfg.log_level.unwrap_or_else(|| "info".to_string()), } - Self::default() } } diff --git a/src/core/database.rs b/src/core/database.rs index 62c5a15..c95c87d 100644 --- a/src/core/database.rs +++ b/src/core/database.rs @@ -27,32 +27,28 @@ pub struct Database { packages: Vec, } +fn read_db_file(path: &Path) -> Result> { + if !path.exists() { + return Ok(vec![]); + } + let json = std::fs::read_to_string(path).context("Failed to read database")?; + let db: DatabaseFile = serde_json::from_str(&json).unwrap_or(DatabaseFile { + schema_version: SCHEMA_VERSION, + installed: vec![], + }); + Ok(db.installed) +} + impl Database { pub fn new(db_dir: &Path) -> Result { std::fs::create_dir_all(db_dir).context("Failed to create database directory")?; let path = db_dir.join("installed.json"); - let packages = if path.exists() { - let json = std::fs::read_to_string(&path).context("Failed to read database")?; - let db: DatabaseFile = serde_json::from_str(&json).unwrap_or(DatabaseFile { - schema_version: SCHEMA_VERSION, - installed: vec![], - }); - db.installed - } else { - vec![] - }; + let packages = read_db_file(&path)?; Ok(Self { path, packages }) } pub fn load(&mut self) -> Result<()> { - if self.path.exists() { - let json = std::fs::read_to_string(&self.path).context("Failed to read database")?; - let db: DatabaseFile = serde_json::from_str(&json).unwrap_or(DatabaseFile { - schema_version: SCHEMA_VERSION, - installed: vec![], - }); - self.packages = db.installed; - } + self.packages = read_db_file(&self.path)?; Ok(()) } @@ -80,14 +76,17 @@ impl Database { if self.is_installed(name) { return Ok(()); } + let installed_at = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .ok() + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + self.packages.push(InstalledPackage { name: name.to_string(), version: version.to_string(), install_path: install_path.to_string_lossy().to_string(), - installed_at: std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() as i64, + installed_at, dependencies: deps.to_vec(), }); self.save() diff --git a/src/core/mod.rs b/src/core/mod.rs index 7431f6d..a0c8bad 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1,3 +1,5 @@ +//! Core types and logic: packages, registry, resolver, database, config. + pub mod config; pub mod database; pub mod package; diff --git a/src/core/package.rs b/src/core/package.rs index 4199c53..eab45df 100644 --- a/src/core/package.rs +++ b/src/core/package.rs @@ -1,6 +1,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; +/// Source location and type (git, tarball, zip, local). #[derive(Debug, Clone, Deserialize, Serialize)] pub struct PackageSource { #[serde(rename = "type")] @@ -12,6 +13,7 @@ pub struct PackageSource { pub path: Option, } +/// Single version definition within a package. #[derive(Debug, Clone, Deserialize, Serialize)] pub struct PackageVersion { pub version: String, @@ -70,6 +72,7 @@ pub struct MultiVersionPackageFile { pub versions: Vec, } +/// Resolved package with a single version (name, version, source, deps, build config). #[derive(Debug, Clone)] pub struct Package { pub name: String, diff --git a/src/core/registry.rs b/src/core/registry.rs index a411afe..892bf6e 100644 --- a/src/core/registry.rs +++ b/src/core/registry.rs @@ -14,6 +14,7 @@ impl Registry { Self::default() } + /// Loads all package JSON files from a directory. Parse errors are logged and skipped (best-effort). pub fn load_from_dir(dir: &Path) -> Result { let mut registry = Self::new(); for entry in WalkDir::new(dir) @@ -36,7 +37,7 @@ impl Registry { } } Err(e) => { - log::warn!("Failed to parse {}: {}", path.display(), e); + log::warn!("Skipping {} (parse error): {}", path.display(), e); } } } diff --git a/src/core/resolver.rs b/src/core/resolver.rs index 59cd6de..29daa13 100644 --- a/src/core/resolver.rs +++ b/src/core/resolver.rs @@ -3,6 +3,7 @@ use crate::core::registry::Registry; use anyhow::{anyhow, Result}; use std::collections::{HashMap, HashSet}; +/// Resolves a package spec (name or name@version) and its dependencies into a build order. pub fn resolve( registry: &Registry, spec: &str, @@ -60,6 +61,7 @@ fn resolve_recursive( Ok(()) } +/// Returns packages in topological build order (dependencies before dependents). pub fn get_build_order(packages: &[Package]) -> Vec { let name_to_pkg: HashMap = packages.iter().map(|p| (p.name.clone(), p)).collect(); diff --git a/src/ops/fetch.rs b/src/ops/fetch.rs index 8df0539..98a8515 100644 --- a/src/ops/fetch.rs +++ b/src/ops/fetch.rs @@ -4,6 +4,7 @@ use std::fs::File; use std::io::{BufReader, Read, Write}; use std::path::Path; +/// Fetches package sources (archive, git, or local) into dest_dir and returns the source tree path. pub fn fetch(pkg: &Package, dest_dir: &Path, force: bool) -> Result { let source_dir = dest_dir.join(format!("{}-{}", pkg.name, pkg.version)); if source_dir.exists() && !force { @@ -126,35 +127,33 @@ fn extract_archive(archive: &Path, dest: &Path) -> Result<()> { Ok(()) } +fn extract_tar_with(reader: R, dest: &Path) -> Result<()> { + let mut tar = tar::Archive::new(reader); + tar.unpack(dest).context("Extract tar")?; + Ok(()) +} + fn extract_tar_gz(archive: &Path, dest: &Path) -> Result<()> { let file = File::open(archive).context("Open archive")?; let dec = flate2::read::GzDecoder::new(BufReader::new(file)); - let mut tar = tar::Archive::new(dec); - tar.unpack(dest).context("Extract tar.gz")?; - Ok(()) + extract_tar_with(dec, dest) } fn extract_tar_xz(archive: &Path, dest: &Path) -> Result<()> { let file = File::open(archive).context("Open archive")?; let dec = xz2::read::XzDecoder::new(BufReader::new(file)); - let mut tar = tar::Archive::new(dec); - tar.unpack(dest).context("Extract tar.xz")?; - Ok(()) + extract_tar_with(dec, dest) } fn extract_tar_bz2(archive: &Path, dest: &Path) -> Result<()> { let file = File::open(archive).context("Open archive")?; let dec = bzip2::read::BzDecoder::new(BufReader::new(file)); - let mut tar = tar::Archive::new(dec); - tar.unpack(dest).context("Extract tar.bz2")?; - Ok(()) + extract_tar_with(dec, dest) } fn extract_tar(archive: &Path, dest: &Path) -> Result<()> { let file = File::open(archive).context("Open archive")?; - let mut tar = tar::Archive::new(file); - tar.unpack(dest).context("Extract tar")?; - Ok(()) + extract_tar_with(BufReader::new(file), dest) } fn extract_zip(archive: &Path, dest: &Path) -> Result<()> { diff --git a/src/ops/install.rs b/src/ops/install.rs index 6fc903c..1d97b02 100644 --- a/src/ops/install.rs +++ b/src/ops/install.rs @@ -6,6 +6,7 @@ use crate::ops::link; use anyhow::{Context, Result}; use std::path::Path; +/// Fetches, builds, links, and records a package under prefix. pub fn install_package(pkg: &Package, prefix: &Path, db: &mut Database, force: bool) -> Result<()> { let sources_dir = prefix.join("sources"); let build_dir = prefix diff --git a/src/ops/mod.rs b/src/ops/mod.rs index 8174ad5..5b9064b 100644 --- a/src/ops/mod.rs +++ b/src/ops/mod.rs @@ -1,3 +1,5 @@ +//! Operations: fetch sources, build, install, link, uninstall. + pub mod build; pub mod fetch; pub mod install; diff --git a/src/platform/unix.rs b/src/platform/unix.rs index f02eb0d..639227d 100644 --- a/src/platform/unix.rs +++ b/src/platform/unix.rs @@ -16,12 +16,5 @@ pub fn create_symlink(src: &Path, dst: &Path) -> Result<()> { } pub fn create_dir_symlink(src: &Path, dst: &Path) -> Result<()> { - if dst.exists() { - fs::remove_file(dst).context("Failed to remove existing symlink target")?; - } - if let Some(parent) = dst.parent() { - fs::create_dir_all(parent).context("Failed to create parent directory")?; - } - symlink(src, dst).context("Failed to create symlink")?; - Ok(()) + create_symlink(src, dst) } diff --git a/src/ui/output.rs b/src/ui/output.rs index 7eb2011..d45a16f 100644 --- a/src/ui/output.rs +++ b/src/ui/output.rs @@ -5,7 +5,7 @@ const ARROW: &str = "==>"; const INDENT: &str = " "; const OK_MARKER: &str = "[ok]"; const WARN_MARKER: &str = "[!!]"; -const ERROR_MARKER: &str = "[!!]"; +const ERROR_MARKER: &str = "[XX]"; fn is_tty() -> bool { console::Term::stderr().features().is_attended() diff --git a/src/ui/progress.rs b/src/ui/progress.rs index f56f977..1d91e84 100644 --- a/src/ui/progress.rs +++ b/src/ui/progress.rs @@ -5,7 +5,7 @@ pub fn create_download_progress(total: u64) -> ProgressBar { pb.set_style( ProgressStyle::default_bar() .template("{spinner:.green} [{bar:40.cyan/blue}] {bytes}/{total_bytes} ({eta})") - .unwrap() + .expect("valid progress template") .progress_chars("##-"), ); pb @@ -17,7 +17,7 @@ pub fn create_spinner(message: &str) -> ProgressBar { pb.set_style( ProgressStyle::default_spinner() .template("{spinner:.green} {msg}") - .unwrap(), + .expect("valid progress template"), ); pb } @@ -27,7 +27,7 @@ pub fn create_simple_progress_bar(total: u64) -> ProgressBar { pb.set_style( ProgressStyle::default_bar() .template("{spinner:.green} [{bar:60.cyan/blue}] {pos}/{len} ({per_sec})") - .unwrap() + .expect("valid progress template") .progress_chars("##-"), ); pb diff --git a/tests/cli_help.rs b/tests/cli_help.rs index 55e5bcb..90fb002 100644 --- a/tests/cli_help.rs +++ b/tests/cli_help.rs @@ -26,51 +26,26 @@ fn test_tsi_version() { assert!(String::from_utf8_lossy(&output.stdout).contains("tsi")); } -#[test] -fn test_tsi_install_help() { - let (output, success) = tsi_cmd(&["install", "--help"]); - assert!(success, "tsi install --help should succeed"); - assert!(String::from_utf8_lossy(&output.stdout).contains("install")); -} - -#[test] -fn test_tsi_list_help() { - let (_, success) = tsi_cmd(&["list", "--help"]); - assert!(success, "tsi list --help should succeed"); -} - -#[test] -fn test_tsi_uninstall_help() { - let (_, success) = tsi_cmd(&["uninstall", "--help"]); - assert!(success, "tsi uninstall --help should succeed"); -} - -#[test] -fn test_tsi_search_help() { - let (_, success) = tsi_cmd(&["search", "--help"]); - assert!(success, "tsi search --help should succeed"); -} - -#[test] -fn test_tsi_info_help() { - let (_, success) = tsi_cmd(&["info", "--help"]); - assert!(success, "tsi info --help should succeed"); -} - -#[test] -fn test_tsi_update_help() { - let (_, success) = tsi_cmd(&["update", "--help"]); - assert!(success, "tsi update --help should succeed"); -} - -#[test] -fn test_tsi_doctor_help() { - let (_, success) = tsi_cmd(&["doctor", "--help"]); - assert!(success, "tsi doctor --help should succeed"); -} - -#[test] -fn test_tsi_upgrade_help() { - let (_, success) = tsi_cmd(&["upgrade", "--help"]); - assert!(success, "tsi upgrade --help should succeed"); +const SUBCOMMANDS: &[&str] = &[ + "install", + "list", + "uninstall", + "search", + "info", + "update", + "doctor", + "upgrade", +]; + +#[test] +fn test_tsi_subcommand_help() { + for subcmd in SUBCOMMANDS { + let (output, success) = tsi_cmd(&[subcmd, "--help"]); + assert!( + success, + "tsi {} --help should succeed: {}", + subcmd, + String::from_utf8_lossy(&output.stderr) + ); + } } From d3cbe48034a6d2db2d888a6428497647b2a7be0c Mon Sep 17 00:00:00 2001 From: Nico Mattes Date: Thu, 12 Mar 2026 19:10:12 +0100 Subject: [PATCH 3/3] fixed workflow --- ...{validate-packages.yml => package-validation.yml} | 5 ++--- docs/workflows/triggers.md | 12 ++++++------ 2 files changed, 8 insertions(+), 9 deletions(-) rename .github/workflows/{validate-packages.yml => package-validation.yml} (99%) diff --git a/.github/workflows/validate-packages.yml b/.github/workflows/package-validation.yml similarity index 99% rename from .github/workflows/validate-packages.yml rename to .github/workflows/package-validation.yml index 87aba8d..9ac386b 100644 --- a/.github/workflows/validate-packages.yml +++ b/.github/workflows/package-validation.yml @@ -1,10 +1,10 @@ -name: Validate Packages +name: Package Validation on: push: paths: - 'packages/**/*.json' - - '.github/workflows/validate-packages.yml' + - '.github/workflows/package-validation.yml' pull_request: paths: - 'packages/**/*.json' @@ -301,4 +301,3 @@ sys.exit(0 if ok else 1) echo "Testing dependency resolution for zlib..." tsi install zlib --force 2>&1 | head -30 || echo "Install test completed (may have build errors, but resolution worked)" - diff --git a/docs/workflows/triggers.md b/docs/workflows/triggers.md index 58cc341..3a264a9 100644 --- a/docs/workflows/triggers.md +++ b/docs/workflows/triggers.md @@ -42,16 +42,16 @@ This document explains when each workflow runs and what triggers them. **Manual Trigger:** Yes, via `workflow_dispatch` -## Validate Packages Workflow +## Package Validation Workflow -**File:** `.github/workflows/validate-packages.yml` +**File:** `.github/workflows/package-validation.yml` **Purpose:** Validates package JSON files and ensures TSI can parse them **Triggers:** - ✅ **Only runs when package files change:** - `packages/**/*.json` - Package definition files - - `.github/workflows/validate-packages.yml` - The workflow file itself + - `.github/workflows/package-validation.yml` - The workflow file itself - ❌ **Does NOT run when:** - TSI source code changes @@ -111,7 +111,7 @@ This document explains when each workflow runs and what triggers them. |----------|-------------------------|---------------------|------------------|-----------------|-----------| | TSI Tests | ✅ Yes | ✅ Yes | ❌ No | ❌ No | ❌ No | | Documentation | ❌ No | ❌ No | ✅ Yes | ❌ No | ❌ No | -| Validate Packages | ❌ No | ✅ Yes | ❌ No | ❌ No | ❌ No | +| Package Validation | ❌ No | ✅ Yes | ❌ No | ❌ No | ❌ No | | Release (binaries + docs) | ❌ No | ❌ No | ❌ No | ✅ Yes | ❌ No | | Discover Versions | ❌ No | ❌ No | ❌ No | ❌ No | ✅ Weekly | | Sync External | ❌ No | ❌ No | ❌ No | ❌ No | ❌ No | @@ -134,7 +134,7 @@ git commit -am "test: source code change" git push ``` -**Expected:** TSI Tests workflow runs, Validate Packages does NOT run +**Expected:** TSI Tests workflow runs, Package Validation does NOT run ### Test 2: Package File Change @@ -145,7 +145,7 @@ git commit -am "test: package change" git push ``` -**Expected:** Both TSI Tests and Validate Packages workflows run (both trigger on `packages/**`) +**Expected:** Both TSI Tests and Package Validation workflows run (both trigger on `packages/**`) ### Test 3: Documentation Change