From 36e3aa6bca8b2f74e05fe2b7374d2d0efa629cd8 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 28 May 2026 15:00:04 +0200 Subject: [PATCH 1/4] sess: serialize concurrent git/db/checkout via filesystem locks Modeled on memora-rs's RAII flock pattern: introduces an `FsLock` guard in `src/lock.rs` backed by `fs4`, and takes a per-`(name, url)` exclusive lock under `/git/locks/` around `git_database` and `checkout_git`. Locks released automatically on drop (and by the kernel on crash). `clone` and `snapshot` subcommands intentionally remain unguarded. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 11 +++++++ Cargo.toml | 1 + src/lib.rs | 1 + src/lock.rs | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/sess.rs | 46 +++++++++++++++++++++++++-- 5 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 src/lock.rs diff --git a/Cargo.lock b/Cargo.lock index 46090621..7cdc97a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -121,6 +121,7 @@ dependencies = [ "dirs", "dunce", "env_logger", + "fs4", "futures", "glob", "indexmap", @@ -581,6 +582,16 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" +[[package]] +name = "fs4" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e72ed92b67c146290f88e9c89d60ca163ea417a446f61ffd7b72df3e7f1dfd5" +dependencies = [ + "rustix", + "windows-sys", +] + [[package]] name = "futures" version = "0.3.32" diff --git a/Cargo.toml b/Cargo.toml index d5e6b50e..b511d047 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,7 @@ indicatif = "0.18.3" regex = "1.12.2" log = "0.4" env_logger = { version = "0.11", default-features = false, features = ["auto-color"] } +fs4 = "1.1" [target.'cfg(windows)'.dependencies] dunce = "1.0.4" diff --git a/src/lib.rs b/src/lib.rs index 6830c177..ff29a241 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod cmd; pub mod config; pub mod diagnostic; pub mod git; +pub mod lock; pub mod lockfile; pub mod progress; pub mod resolver; diff --git a/src/lock.rs b/src/lock.rs new file mode 100644 index 00000000..e08f9664 --- /dev/null +++ b/src/lock.rs @@ -0,0 +1,92 @@ +// Copyright (c) 2025 ETH Zurich + +//! Cross-process filesystem advisory locks. +//! +//! Bender uses these to serialize concurrent invocations against the same git +//! database and checkout. The lock is taken on a sentinel file in +//! `/git/locks/-.lock` and released automatically when +//! the [`FsLock`] guard is dropped. + +#![deny(missing_docs)] + +use std::fs::{File, OpenOptions}; +use std::path::{Path, PathBuf}; + +use fs4::{FileExt, TryLockError}; +use miette::{Context as _, IntoDiagnostic as _}; + +use crate::Result; + +/// An exclusive, cross-process advisory lock held on a sentinel file. +/// +/// The lock is released when this guard is dropped (or when the process exits). +pub struct FsLock { + file: Option, + path: PathBuf, +} + +impl FsLock { + /// Acquire an exclusive lock on `path`, creating the file if missing. + /// + /// If the lock is contended, an info message is logged so the user can see + /// why bender is waiting, and the call then blocks until the lock is + /// available. The actual lock acquisition runs on a blocking worker so it + /// does not stall the tokio runtime. + pub async fn acquire_exclusive(path: PathBuf) -> Result { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .into_diagnostic() + .wrap_err_with(|| format!("Failed to create lock directory {:?}.", parent))?; + } + let file = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .truncate(false) + .open(&path) + .into_diagnostic() + .wrap_err_with(|| format!("Failed to open lock file {:?}.", path))?; + + let path_for_blocking = path.clone(); + let file = tokio::task::spawn_blocking(move || -> Result { + match FileExt::try_lock(&file) { + Ok(()) => Ok(file), + Err(TryLockError::WouldBlock) => { + log::info!("waiting for lock on {:?}", path_for_blocking); + FileExt::lock(&file) + .into_diagnostic() + .wrap_err_with(|| { + format!("Failed to acquire lock on {:?}.", path_for_blocking) + })?; + Ok(file) + } + Err(TryLockError::Error(e)) => Err(e).into_diagnostic().wrap_err_with(|| { + format!("Failed to try-lock {:?}.", path_for_blocking) + }), + } + }) + .await + .into_diagnostic() + .wrap_err("Lock acquisition task panicked.")??; + + Ok(FsLock { + file: Some(file), + path, + }) + } + + /// The path of the lock file. + pub fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for FsLock { + fn drop(&mut self) { + if let Some(file) = self.file.take() { + // Best-effort: errors here are not actionable, and the OS releases + // the lock automatically when the file handle closes. + let _ = FileExt::unlock(&file); + } + } +} diff --git a/src/sess.rs b/src/sess.rs index 43c76e65..ec680230 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -37,6 +37,7 @@ use crate::cli::read_manifest; use crate::config::{self, Config, Manifest, PartialManifest}; use crate::diagnostic::{Diagnostics, Errors, Warnings}; use crate::git::Git; +use crate::lock::FsLock; use crate::progress::{GitProgressOps, ProgressHandler}; use crate::src::{SourceFile, SourceGroup, SourceType}; use crate::target::TargetSpec; @@ -560,6 +561,19 @@ impl<'ctx> Session<'ctx> { let db_name = format!("{}-{}", name, hash); db_name } + + /// Path of the cross-process advisory lock file for a git dependency. + /// + /// The same file guards both the bare database under `git/db/` and the + /// working checkout under `git/checkouts/` for this dependency. + pub fn dep_lock_path(&self, name: &str, url: &str) -> PathBuf { + let key = self.git_db_name(name, url); + self.config + .database + .join("git") + .join("locks") + .join(format!("{}.lock", key)) + } } /// A wrapper around a `Session` that provides async IO operations. @@ -642,9 +656,28 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { /// Access the git database for a dependency. /// + /// Acquires the per-dependency filesystem lock and delegates to + /// [`git_database_locked`]. Callers that already hold the lock for this + /// dependency should call [`git_database_locked`] directly to avoid + /// deadlocking. + async fn git_database( + &'io self, + name: &str, + url: &str, + force_fetch: Option, + fetch_ref: Option<&str>, + ) -> Result> { + let _lock = FsLock::acquire_exclusive(self.sess.dep_lock_path(name, url)).await?; + self.git_database_locked(name, url, force_fetch, fetch_ref) + .await + } + + /// Access the git database for a dependency, assuming the per-dependency + /// filesystem lock is already held by the caller. + /// /// If the database does not exist, it is created. If the database has not /// been updated recently, the remote is fetched. - async fn git_database( + async fn git_database_locked( &'io self, name: &str, url: &str, @@ -929,6 +962,11 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { force: bool, update_list: &[String], ) -> Result<&'ctx Path> { + // Serialize concurrent bender invocations operating on the same + // dependency. The lock covers both the bare database (touched via the + // `git_database_locked` calls below) and the working checkout. + let _lock = FsLock::acquire_exclusive(self.sess.dep_lock_path(name, url)).await?; + #[derive(Eq, PartialEq)] enum CheckoutState { Clean, @@ -944,7 +982,9 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { log::debug!("currently `{}` (want `{}`)", current, revision); if current == revision { CheckoutState::Clean - } else if let Ok(db) = self.git_database(name, url, Some(false), None).await { + } else if let Ok(db) = + self.git_database_locked(name, url, Some(false), None).await + { if let Ok(remote) = local_git.clone().remote_url("origin").await { if remote == db.path.to_str().unwrap() { CheckoutState::ToCheckout @@ -1010,7 +1050,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { let tag_name_1 = tag_name_0.clone(); let tag_name_2 = tag_name_0.clone(); let git = self - .git_database(name, url, Some(false), Some(revision)) + .git_database_locked(name, url, Some(false), Some(revision)) .await?; match git .clone() From 759337b447fbcb2a785d5a379d3bac3912063677 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 28 May 2026 15:59:36 +0200 Subject: [PATCH 2/4] config: add `db_dir` for sharing bare repos and locks across projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an optional `db_dir` config field that overrides only the bare-repo and lock-file locations (`/git/db/` and `/git/locks/`), while working-tree checkouts and everything else continue to derive from `database`. This lets a single shared config enable cross-project cache sharing without forcing same-named projects to commit a `workspace.checkout_dir` in their `Bender.yml`. The field is purely additive and unknown to older Bender versions, which silently ignore it and fall back to their per-project default — strictly safer than today's shared-`database:` recipe in mixed-version setups. Docs updated: `book/src/configuration.md` documents the new field; `book/src/workflow/ci.md` rewrites the shared-cache recipe around `db_dir` and retires the now-fixed concurrent-runs caveat; `book/src/local.md` mentions it in the example block. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 2 ++ book/src/configuration.md | 8 ++++++++ book/src/local.md | 5 +++++ book/src/workflow/ci.md | 22 +++++++++++++++------- src/cli.rs | 1 + src/config.rs | 13 +++++++++++++ src/lock.rs | 14 ++++++-------- src/sess.rs | 23 ++++++++++++++--------- 8 files changed, 64 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4ae7714..43c0457c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ### Added - Add new `crates/bender-slang` crate that integrates the vendored Slang parser via a Rust/C++ bridge. - Add new `pickle` command (behind feature `slang`) to parse and re-emit SystemVerilog sources. +- Add cross-process filesystem locks around git database and checkout operations so concurrent `bender` invocations against the same dependency serialize safely. +- Add `db_dir` config field to share bare-repo and lock storage across projects without relocating per-project checkouts; older Bender versions silently ignore the field and fall back to their per-project default. ## 0.31.0 - 2026-03-03 ### Fixed diff --git a/book/src/configuration.md b/book/src/configuration.md index ffac90c9..bd759619 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -27,6 +27,14 @@ The directory where Bender stores cloned and checked-out dependencies. - **Default:** `.bender` in the project root. - **Example:** `database: /var/cache/bender_dependencies` +### `db_dir` +Optional override for the directory that holds bare git repositories and their lock files (i.e. `/git/db/` and `/git/locks/`). When set, it takes precedence over `database` (whether explicitly configured or left at its default) for these two paths only; the working-tree checkouts continue to follow `database` (or [`workspace.checkout_dir`](./manifest.md) in the project manifest). This makes it possible to share the heavy git data across projects on a persistent runner without also relocating per-project checkouts. See [Continuous Integration › Sharing the Database](./workflow/ci.md#sharing-the-database-across-runs-and-projects) for the recommended setup. +- **Config Key:** `db_dir` +- **Default:** unset (falls back to `database`). +- **Example:** `db_dir: /var/cache/bender_shared` + +> **Note:** Older Bender versions (pre-`db_dir`) silently ignore this field and fall back to their per-project default, so it is safe to ship in a shared configuration that mixed bender versions may read. + ### `git` The command or path used to invoke Git. - **Config Key:** `git` diff --git a/book/src/local.md b/book/src/local.md index edab51b0..a2c2239a 100644 --- a/book/src/local.md +++ b/book/src/local.md @@ -40,6 +40,11 @@ For a detailed guide on using these commands for multi-package development, see # Change the directory where dependencies are stored (default is .bender) database: my_deps_cache +# Share only the bare git repos and lock files across projects, while +# keeping per-project checkouts under each project's own .bender/. +# Older Bender versions silently ignore this field. +db_dir: /var/cache/bender_shared + # Use a custom git binary or wrapper git: /usr/local/bin/git-wrapper.sh ``` diff --git a/book/src/workflow/ci.md b/book/src/workflow/ci.md index 9c55307e..8b2a2c66 100644 --- a/book/src/workflow/ci.md +++ b/book/src/workflow/ci.md @@ -84,16 +84,24 @@ cache: ## Sharing the Database Across Runs and Projects -When jobs run on a persistent runner, the `database` directory (where Bender stores cloned repositories) can be shared across CI runs and even across projects to drastically reduce fetch times and disk usage. +When jobs run on a persistent runner, the bare git repositories Bender clones can be shared across CI runs and even across projects to drastically reduce fetch times and disk usage. The recommended way is the [`db_dir`](../configuration.md#db_dir) setting, which relocates *only* the bare repos and lock files; per-project working-tree checkouts stay under each project's own `.bender/` directory and cannot collide. -To enable this, point Bender at a shared location via a [`Bender.local`](../local.md) file (for example placed in a parent directory of the project so it is picked up automatically): +Place a [`Bender.local`](../local.md) in a parent directory of your projects so it is picked up automatically: ```yaml -database: /var/cache/bender_shared +db_dir: /var/cache/bender_shared ``` -This way, every job that runs in the runner reuses the already-fetched Git data instead of re-cloning from scratch. See [Configuration](../configuration.md) for more on the `database` setting. +Every job on the runner now reuses the already-fetched Git data and serializes safely against concurrent jobs via per-dependency filesystem locks living next to the bare repos (`/git/locks/`). + +> **Older Bender versions** silently ignore `db_dir` and fall back to their normal per-project `.bender/` cache, so the shared config above is safe to deploy on a runner that still hosts pinned-to-old-bender projects — those projects simply won't benefit from the shared cache, but they won't misbehave either. + +### Legacy `database:` recipe + +Earlier docs recommended sharing via [`database`](../configuration.md#database), which relocates both the bare repos *and* the checkouts: + +```yaml +database: /var/cache/bender_shared +``` -> **Caveats:** -> - **Checkout collisions:** When sharing the database, checkouts from different projects can collide if the top-level [`Bender.yml`](../manifest.md) does not specify a project-specific `workspace.checkout_dir`. Make sure each top-level project sets its own `checkout_dir` so that checkouts remain isolated even when the database is shared. -> - **Concurrent runs:** Bender does not currently take a lock before performing Git operations on the shared database. Concurrent jobs that touch the same database may occasionally fail with Git errors. This is unlikely, but worth keeping in mind when sizing parallelism. +This still works, but it has a known footgun: two top-level projects that share the same name will collide on the same `/git/checkouts/-/` directory. If you keep this recipe, give each top-level project its own `workspace.checkout_dir` in [`Bender.yml`](../manifest.md) so the checkouts remain isolated. Prefer `db_dir` for new setups. diff --git a/src/cli.rs b/src/cli.rs index ece76a5c..dd8b0427 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -508,6 +508,7 @@ fn load_config(from: &Path, warn_config_loaded: bool) -> Result { // Assemble and merge the default configuration. let default_cfg = PartialConfig { database: Some(from.join(".bender").to_str().unwrap().to_string()), + db_dir: None, git: Some("git".into()), overrides: None, plugins: None, diff --git a/src/config.rs b/src/config.rs index 3a803eb1..dfb9ca01 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1623,6 +1623,11 @@ where pub struct Config { /// The path to the database directory. pub database: PathBuf, + /// Optional override for the directory containing bare git repositories + /// and their lock files. When set, takes precedence over `database` for + /// these two specific paths; checkouts and everything else still derive + /// from `database`. + pub db_dir: Option, /// The git command or path to the binary. pub git: String, /// The dependency overrides. @@ -1640,6 +1645,8 @@ pub struct Config { pub struct PartialConfig { /// The path to the database directory. pub database: Option, + /// Optional override for the bare-repo and lock directory. + pub db_dir: Option, /// The git command or path to the binary. pub git: Option, /// The dependency overrides. @@ -1663,6 +1670,7 @@ impl PrefixPaths for PartialConfig { fn prefix_paths(self, prefix: &Path) -> Result { Ok(PartialConfig { database: self.database.prefix_paths(prefix)?, + db_dir: self.db_dir.prefix_paths(prefix)?, overrides: self.overrides.prefix_paths(prefix)?, plugins: self.plugins.prefix_paths(prefix)?, ..self @@ -1674,6 +1682,7 @@ impl Merge for PartialConfig { fn merge(self, other: PartialConfig) -> PartialConfig { PartialConfig { database: self.database.or(other.database), + db_dir: self.db_dir.or(other.db_dir), git: self.git.or(other.git), overrides: match (self.overrides, other.overrides) { (Some(o), None) | (None, Some(o)) => Some(o), @@ -1710,6 +1719,10 @@ impl Validate for PartialConfig { Some(db) => env_path_from_string(&db)?, None => bail!("Database directory not configured"), }, + db_dir: match self.db_dir { + Some(d) => Some(env_path_from_string(&d)?), + None => None, + }, git: match self.git { Some(git) => git, None => bail!("Git command or path to binary not configured"), diff --git a/src/lock.rs b/src/lock.rs index e08f9664..3eec90d2 100644 --- a/src/lock.rs +++ b/src/lock.rs @@ -53,16 +53,14 @@ impl FsLock { Ok(()) => Ok(file), Err(TryLockError::WouldBlock) => { log::info!("waiting for lock on {:?}", path_for_blocking); - FileExt::lock(&file) - .into_diagnostic() - .wrap_err_with(|| { - format!("Failed to acquire lock on {:?}.", path_for_blocking) - })?; + FileExt::lock(&file).into_diagnostic().wrap_err_with(|| { + format!("Failed to acquire lock on {:?}.", path_for_blocking) + })?; Ok(file) } - Err(TryLockError::Error(e)) => Err(e).into_diagnostic().wrap_err_with(|| { - format!("Failed to try-lock {:?}.", path_for_blocking) - }), + Err(TryLockError::Error(e)) => Err(e) + .into_diagnostic() + .wrap_err_with(|| format!("Failed to try-lock {:?}.", path_for_blocking)), } }) .await diff --git a/src/sess.rs b/src/sess.rs index ec680230..55f004df 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -568,12 +568,23 @@ impl<'ctx> Session<'ctx> { /// working checkout under `git/checkouts/` for this dependency. pub fn dep_lock_path(&self, name: &str, url: &str) -> PathBuf { let key = self.git_db_name(name, url); - self.config - .database + self.db_parent() .join("git") .join("locks") .join(format!("{}.lock", key)) } + + /// The directory under which bare git databases and lock files live. + /// + /// Uses `db_dir` if configured (the recommended way to share bare repos + /// across projects without relocating checkouts), otherwise falls back to + /// `database`. + pub fn db_parent(&self) -> &Path { + self.config + .db_dir + .as_deref() + .unwrap_or(&self.config.database) + } } /// A wrapper around a `Session` that provides async IO operations. @@ -694,13 +705,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { // Determine the location of the git database and create it if its does // not yet exist. - let db_dir = self - .sess - .config - .database - .join("git") - .join("db") - .join(db_name); + let db_dir = self.sess.db_parent().join("git").join("db").join(db_name); let db_dir = self.sess.intern_path(db_dir); std::fs::create_dir_all(db_dir) .into_diagnostic() From 37836f77836d81868e99b90f710414d724e1e7f1 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Thu, 28 May 2026 15:59:46 +0200 Subject: [PATCH 3/4] chore: address pre-existing clippy warnings - src/progress.rs: collapse nested `if self.git_op == Submodule { ... }` blocks into match guards on the relevant arms; the existing `_ => {}` wildcard now handles the non-submodule case. - src/config.rs: collapse the inner glob/non-glob `if` in `glob_file` into a match guard; the pre-existing `_ => Ok(vec![self])` arm covers the fall-through. - Drop redundant `.into_iter()` on values passed to `.chain()` in `src/sess.rs`, `src/src.rs`, and `src/config.rs`. - tests/cli_regression.rs: pass an array directly to `Command::args` instead of borrowing it. `cargo clippy --all-targets -- -D warnings` is clean; `cargo test` unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/config.rs | 68 +++++++++++-------------- src/progress.rs | 108 ++++++++++++++++++++-------------------- src/sess.rs | 3 +- src/src.rs | 4 +- tests/cli_regression.rs | 2 +- 5 files changed, 87 insertions(+), 98 deletions(-) diff --git a/src/config.rs b/src/config.rs index dfb9ca01..1f22aadb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1177,7 +1177,7 @@ impl Validate for PartialSources { let post_env_files: Vec = if let Some(fls) = files { fls.into_iter() - .chain(external_flist_groups?.into_iter()) + .chain(external_flist_groups?) .filter_map(|file| match file { PartialSourceFile::File(ref filename) | PartialSourceFile::SvFile(ref filename) @@ -1447,46 +1447,38 @@ impl GlobFile for PartialSourceFile { PartialSourceFile::File(ref path) | PartialSourceFile::SvFile(ref path) | PartialSourceFile::VerilogFile(ref path) - | PartialSourceFile::VhdlFile(ref path) => { - // Check if glob patterns used - if path.contains("*") || path.contains("?") { - let glob_matches = glob(path) - .into_diagnostic() - .wrap_err_with(|| format!("Invalid glob pattern for {:?}", path))?; - let out = glob_matches - .map(|glob_match| { - let file_str = glob_match - .into_diagnostic() - .wrap_err_with(|| format!("Glob match failed for {:?}", path))? - .to_str() - .unwrap() - .to_string(); - Ok(match self { - PartialSourceFile::File(_) => PartialSourceFile::File(file_str), - PartialSourceFile::SvFile(_) => PartialSourceFile::SvFile(file_str), - PartialSourceFile::VerilogFile(_) => { - PartialSourceFile::VerilogFile(file_str) - } - PartialSourceFile::VhdlFile(_) => { - PartialSourceFile::VhdlFile(file_str) - } - _ => unreachable!(), - }) + | PartialSourceFile::VhdlFile(ref path) + if path.contains("*") || path.contains("?") => + { + let glob_matches = glob(path) + .into_diagnostic() + .wrap_err_with(|| format!("Invalid glob pattern for {:?}", path))?; + let out = glob_matches + .map(|glob_match| { + let file_str = glob_match + .into_diagnostic() + .wrap_err_with(|| format!("Glob match failed for {:?}", path))? + .to_str() + .unwrap() + .to_string(); + Ok(match self { + PartialSourceFile::File(_) => PartialSourceFile::File(file_str), + PartialSourceFile::SvFile(_) => PartialSourceFile::SvFile(file_str), + PartialSourceFile::VerilogFile(_) => { + PartialSourceFile::VerilogFile(file_str) + } + PartialSourceFile::VhdlFile(_) => PartialSourceFile::VhdlFile(file_str), + _ => unreachable!(), }) - .collect::>>()?; - if out.is_empty() { - Warnings::NoFilesForGlobPattern { path: path.clone() }.emit(); - } - Ok(out) - } else { - // Return self if not a glob pattern - Ok(vec![self]) + }) + .collect::>>()?; + if out.is_empty() { + Warnings::NoFilesForGlobPattern { path: path.clone() }.emit(); } + Ok(out) } - _ => { - // Return self if not a glob pattern - Ok(vec![self]) - } + // Return self if not a glob pattern (also matches non-file variants). + _ => Ok(vec![self]), } } } diff --git a/src/progress.rs b/src/progress.rs index 3a8691df..9a37530c 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -209,69 +209,67 @@ impl ProgressHandler { match progress { // This case is only relevant for submodule operations i.e. `git submodule update` // It indicates that a new submodule has been registered, and we create a new progress bar for it. - GitProgress::SubmoduleRegistered { name } => { - if self.git_op == GitProgressOps::Submodule { - // The main bar simply becomes a spinner since the sub-bar will show progress - // on the subsequent line. - state.pb.set_style( - ProgressStyle::with_template("{spinner:>3.cyan} {prefix:<40!}").unwrap(), - ); - - // The submodule style is similar to the main bar, but indented and without spinner - let style = ProgressStyle::with_template( - " {prefix:<24!} {bar:40.cyan/blue} {percent:>3}% {msg}", - ) + GitProgress::SubmoduleRegistered { name } + if self.git_op == GitProgressOps::Submodule => + { + // The main bar simply becomes a spinner since the sub-bar will show progress + // on the subsequent line. + state.pb.set_style( + ProgressStyle::with_template("{spinner:>3.cyan} {prefix:<40!}").unwrap(), + ); + + // The submodule style is similar to the main bar, but indented and without spinner + let style = ProgressStyle::with_template( + " {prefix:<24!} {bar:40.cyan/blue} {percent:>3}% {msg}", + ) + .unwrap() + .progress_chars("-- "); + + // We can have multiple sub-bars, and we insert them after the last one. + // In order to maintain proper tree-like structure, we need to update the previous last bar + // to have a "T" connector (├─) instead of an "L" + let prev_bar = match state.sub_bars.last() { + Some((last_name, last_pb)) => { + let prev_prefix = format!("{} {}", fmt_dim!("├─"), last_name); + last_pb.set_prefix(prev_prefix); + last_pb // Insert the new one after this one + } + None => &state.pb, // Insert the first one after the main bar + }; + + // Create the new sub-bar and insert it in the multi-progress *after* the previous sub-bar + let sub_pb = self + .multiprogress + .as_ref() .unwrap() - .progress_chars("-- "); - - // We can have multiple sub-bars, and we insert them after the last one. - // In order to maintain proper tree-like structure, we need to update the previous last bar - // to have a "T" connector (├─) instead of an "L" - let prev_bar = match state.sub_bars.last() { - Some((last_name, last_pb)) => { - let prev_prefix = format!("{} {}", fmt_dim!("├─"), last_name); - last_pb.set_prefix(prev_prefix); - last_pb // Insert the new one after this one - } - None => &state.pb, // Insert the first one after the main bar - }; - - // Create the new sub-bar and insert it in the multi-progress *after* the previous sub-bar - let sub_pb = self - .multiprogress - .as_ref() - .unwrap() - .insert_after(prev_bar, ProgressBar::new(100).with_style(style)); - // Set the prefix and initial message - let sub_prefix = format!("{} {}", fmt_dim!("╰─"), &name); - sub_pb.set_prefix(sub_prefix); - sub_pb.set_message(format!("{}", fmt_dim!("Waiting..."))); - - // Store the sub-bar in the state for later updates - state.sub_bars.insert(name, sub_pb); - } + .insert_after(prev_bar, ProgressBar::new(100).with_style(style)); + // Set the prefix and initial message + let sub_prefix = format!("{} {}", fmt_dim!("╰─"), &name); + sub_pb.set_prefix(sub_prefix); + sub_pb.set_message(format!("{}", fmt_dim!("Waiting..."))); + + // Store the sub-bar in the state for later updates + state.sub_bars.insert(name, sub_pb); } // This indicates that we are starting to clone a submodule. // Again, it is only relevant for submodule operations. For normal // clones, we just update the main bar. - GitProgress::CloningInto { name } => { - if self.git_op == GitProgressOps::Submodule { - // Logic to handle missing 'checked out' lines: - // If we are activating 'bar', but 'foo' was active, assume 'foo' is done. - if let Some(prev) = &state.active_sub { - if prev != &name { - if let Some(b) = state.sub_bars.get(prev) { - b.finish_and_clear(); - } + GitProgress::CloningInto { name } if self.git_op == GitProgressOps::Submodule => { + // Logic to handle missing 'checked out' lines: + // If we are activating 'bar', but 'foo' was active, assume 'foo' is done. + if let Some(prev) = &state.active_sub { + if prev != &name { + if let Some(b) = state.sub_bars.get(prev) { + b.finish_and_clear(); } } - // Set the new bar to active - if let Some(bar) = state.sub_bars.get(&name) { - // Switch style to the active progress bar style - bar.set_message(format!("{}", fmt_dim!("Cloning..."))); - } - state.active_sub = Some(name); } + // Set the new bar to active + if let Some(bar) = state.sub_bars.get(&name) { + // Switch style to the active progress bar style + bar.set_message(format!("{}", fmt_dim!("Cloning..."))); + } + state.active_sub = Some(name); } // Indicates that we have finished processing a submodule. GitProgress::SubmoduleEnd { name } => { diff --git a/src/sess.rs b/src/sess.rs index 55f004df..7501e561 100644 --- a/src/sess.rs +++ b/src/sess.rs @@ -873,8 +873,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> { versions.sort_by(|a, b| b.cmp(a)); // Merge tags and branches. - let refs: IndexMap<&str, &str> = - branches.into_iter().chain(tags.into_iter()).collect(); + let refs: IndexMap<&str, &str> = branches.into_iter().chain(tags).collect(); let git_path = git.path; diff --git a/src/src.rs b/src/src.rs index cfdcd116..7f57b55f 100644 --- a/src/src.rs +++ b/src/src.rs @@ -359,13 +359,13 @@ impl<'ctx> SourceGroup<'ctx> { .include_dirs .iter() .cloned() - .chain(grp.include_dirs.into_iter()) + .chain(grp.include_dirs) .collect(); grp.defines = self .defines .iter() .map(|(k, v)| (k.clone(), v.clone())) - .chain(grp.defines.into_iter()) + .chain(grp.defines) .collect(); grp.flatten_into(into); } diff --git a/tests/cli_regression.rs b/tests/cli_regression.rs index 6ced2304..551b35ba 100644 --- a/tests/cli_regression.rs +++ b/tests/cli_regression.rs @@ -66,7 +66,7 @@ fn get_test_env() -> &'static (PathBuf, PathBuf) { std::fs::create_dir_all(&install_root).expect("Failed to create install dir"); let status = SysCommand::new("cargo") - .args(&[ + .args([ "install", "--git", "https://github.com/pulp-platform/bender", From ad37a690dc59811d4d9bae644d1fab69f59d0353 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Fri, 29 May 2026 19:55:25 +0200 Subject: [PATCH 4/4] config: read `db_dir` from `BENDER_DB_DIR` env var Honoured only when no configuration file sets `db_dir`; any `Bender.local`/`.bender.yml`/user/system config still wins. Empty strings are treated as unset. Lets a CI runner export the shared cache path once instead of maintaining a `Bender.local` next to every project. Co-Authored-By: Claude Opus 4.7 (1M context) --- book/src/configuration.md | 1 + book/src/workflow/ci.md | 4 +++- src/cli.rs | 8 ++++++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/book/src/configuration.md b/book/src/configuration.md index bd759619..5e2888ce 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -30,6 +30,7 @@ The directory where Bender stores cloned and checked-out dependencies. ### `db_dir` Optional override for the directory that holds bare git repositories and their lock files (i.e. `/git/db/` and `/git/locks/`). When set, it takes precedence over `database` (whether explicitly configured or left at its default) for these two paths only; the working-tree checkouts continue to follow `database` (or [`workspace.checkout_dir`](./manifest.md) in the project manifest). This makes it possible to share the heavy git data across projects on a persistent runner without also relocating per-project checkouts. See [Continuous Integration › Sharing the Database](./workflow/ci.md#sharing-the-database-across-runs-and-projects) for the recommended setup. - **Config Key:** `db_dir` +- **Env Var:** `BENDER_DB_DIR` (used only when no configuration file sets `db_dir`; configuration files always take precedence). - **Default:** unset (falls back to `database`). - **Example:** `db_dir: /var/cache/bender_shared` diff --git a/book/src/workflow/ci.md b/book/src/workflow/ci.md index 8b2a2c66..2a6216e6 100644 --- a/book/src/workflow/ci.md +++ b/book/src/workflow/ci.md @@ -94,7 +94,9 @@ db_dir: /var/cache/bender_shared Every job on the runner now reuses the already-fetched Git data and serializes safely against concurrent jobs via per-dependency filesystem locks living next to the bare repos (`/git/locks/`). -> **Older Bender versions** silently ignore `db_dir` and fall back to their normal per-project `.bender/` cache, so the shared config above is safe to deploy on a runner that still hosts pinned-to-old-bender projects — those projects simply won't benefit from the shared cache, but they won't misbehave either. +If you'd rather not place a `Bender.local` on the runner at all, exporting `BENDER_DB_DIR=/var/cache/bender_shared` in the job environment has the same effect — any project that explicitly sets `db_dir` in its own configuration overrides the env var. + +> **Older Bender versions** silently ignore both `db_dir` and `BENDER_DB_DIR` and fall back to their normal per-project `.bender/` cache, so the shared config above is safe to deploy on a runner that still hosts pinned-to-old-bender projects — those projects simply won't benefit from the shared cache, but they won't misbehave either. ### Legacy `database:` recipe diff --git a/src/cli.rs b/src/cli.rs index dd8b0427..e9b5584c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -505,10 +505,14 @@ fn load_config(from: &Path, warn_config_loaded: bool) -> Result { out = out.merge(cfg); } - // Assemble and merge the default configuration. + // Assemble and merge the default configuration. Env-var-supplied `db_dir` + // lives here so that any configuration file value still wins via + // `PartialConfig::merge` (which uses `self.or(other)`). let default_cfg = PartialConfig { database: Some(from.join(".bender").to_str().unwrap().to_string()), - db_dir: None, + db_dir: std::env::var("BENDER_DB_DIR") + .ok() + .filter(|s| !s.is_empty()), git: Some("git".into()), overrides: None, plugins: None,