Fallback store by Dragyx · Pull Request #50 · faukah/dix

Dragyx · 2026-01-29T19:08:45Z

At the moment, it is not possible to fall back to e.g. an immutable DB connection should something go wrong. This PR attempts to fix this by creating a wrapper that combines multiple database connection implementations into one and falls back should something fail.

Attempts to fix #47 .

Copilot

Pull request overview

This PR introduces a fallback store mechanism to handle database connection failures gracefully. It refactors the existing database connection code into a trait-based abstraction that allows multiple database connection backends to be combined with automatic fallback behavior.

Changes:

Introduces StoreFrontend trait to abstract database operations
Refactors Connection into DBConnection implementing the new trait
Adds CombinedStoreFrontend wrapper that attempts multiple backends with fallback logic
Changes query return types from impl Iterator to Box<dyn Iterator> to support trait objects

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.

File	Description
src/store.rs	Introduces StoreFrontend trait, DBConnection struct, CombinedStoreFrontend wrapper for fallback behavior, and refactors query methods to return boxed iterators
src/diff.rs	Updates to use new CombinedStoreFrontend::default() instead of store::connect()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/store.rs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/store.rs

src/diff.rs

src/store.rs

Dragyx · 2026-01-29T21:01:35Z

I also plan to add a fallback to traditional nix shell commands to ensure that there is a worst case fallback option should something change with the database location / schema. Although I am not sure if I will accomplish this in this PR.

src/store.rs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-30T00:09:27Z

src/store.rs

+  /// Closes all connected frontends.
+  ///
+  /// If some fail to close, the combined error is returned.
+  fn close(&mut self) -> Result<()> {
+    let mut combined_err: Option<anyhow::Error> = None;
+    for (i, frontend) in self.frontends.iter_mut().enumerate() {
+      if frontend.connected() {
+        if let Err(err) = frontend.close() {
+          warn!(
+            "Unable to close store frontend {i}: {frontend}. (error: {err})"
+          );
+          combined_err = match combined_err {
+            Some(combined) => Some(combined.context(err)),
+            None => Some(err),
+          };
+        }
+      }
+    }
+    if let Some(err) = combined_err {
+      Err(err.context("One or more frontends failed to close."))
+    } else {
+      Ok(())
+    }
+  }


The close functionality of CombinedStoreFrontend lacks test coverage. Consider adding a test to verify that: (1) all connected frontends are closed, (2) errors from individual close operations are properly collected and combined, and (3) the close method handles cases where some frontends are not connected.

src/store.rs

Copilot · 2026-01-30T00:09:28Z

src/store.rs

+/// A backup database connection that can access the database
+/// even in a read-only environment
+///
+/// This might produce incorrect results as the connection is not guaranteed
+/// to be the only one accessing the database. (There might be e.g. a
+/// `nixos-rebuild` modifying the database)


The documentation comment could be clearer. Consider rephrasing to: "A fallback database connection for read-only environments (e.g., when the filesystem is mounted read-only). Uses the immutable flag which tells SQLite the database won't change, enabling optimizations. However, if other processes modify the database while this connection is active, queries may return stale or incorrect results." This makes it clearer that immutable mode is a fallback for read-only environments, not a preferred mode.

Suggested change

/// A backup database connection that can access the database

/// even in a read-only environment

///

/// This might produce incorrect results as the connection is not guaranteed

/// to be the only one accessing the database. (There might be e.g. a

/// `nixos-rebuild` modifying the database)

/// A fallback database connection for read-only environments

/// (e.g., when the filesystem is mounted read-only).

///

/// Uses the SQLite `immutable=1` flag, which tells SQLite the database

/// will not change, enabling certain optimizations. However, if other

/// processes modify the database while this connection is active, queries

/// may return stale or incorrect results.

Copilot · 2026-01-30T00:09:28Z

src/store.rs

+/// combines multiple store frontends by falling back to the next one if the
+/// current one fails.
+///
+/// Currently, the first frontend that works when connecting is used.


The documentation comment is misleading. The implementation attempts to connect to ALL frontends, not just the first one that works. During queries, it tries each connected frontend in order until one succeeds. Consider updating the comment to: "Attempts to connect to all frontends during initialization. During queries, frontends are tried in order until one succeeds."

Suggested change

/// combines multiple store frontends by falling back to the next one if the

/// current one fails.

///

/// Currently, the first frontend that works when connecting is used.

/// Combines multiple store frontends by falling back to the next one if the

/// current one fails.

///

/// Attempts to connect to all frontends during initialization. During

/// queries, frontends are tried in order until one succeeds.

src/store.rs

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 14 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-30T00:51:24Z

src/store.rs

+        combined_err = match combined_err {
+          Some(combined) => Some(combined.context(err)),
+          None => Some(err),
+        }


The error combination logic uses .context(err) to chain errors, but this may not produce the most useful error messages. When multiple backends fail, the error chain will show errors in reverse order (most recent first), which could be confusing. Consider using a different approach like collecting all errors into a list and formatting them clearly, or using something like anyhow::Error::new() with a custom message that lists all failures.

Copilot · 2026-01-30T00:51:24Z

src/store.rs

+      .arg("--closure-size")
+      .arg(path.join("sw"))
+      .output()
+      .context(anyhow!("Encountered error while executing nix command"))?;


The error handling pattern .context(anyhow!("message")) is redundant. The .context() method from anyhow already creates an anyhow::Error with the provided message. Using anyhow!() inside .context() is unnecessary. This should be simplified to .context("Encountered error while executing nix command")?.

Suggested change

.context(anyhow!("Encountered error while executing nix command"))?;

.context("Encountered error while executing nix command")?;

Copilot · 2026-01-30T00:51:25Z

src/store.rs

+      .arg("--closure-size")
+      .arg(path.join("sw"))
+      .output()
+      .context(anyhow!("Encountered error while executing nix command"))?;


The shell command execution does not check the exit status of the command. If nix or nix-store commands fail (non-zero exit code), the function still attempts to parse stdout, which could lead to silent failures or misleading error messages. The implementation should check cmd_res.status.success() before attempting to parse the output and provide a clear error message if the command failed.

Suggested change

.context(anyhow!("Encountered error while executing nix command"))?;

.context(anyhow!("Encountered error while executing nix command"))?;

if !cmd_res.status.success() {

let stderr = String::from_utf8_lossy(&cmd_res.stderr);

return Err(anyhow!(

"nix path-info --closure-size failed with status {:?}: {}",

cmd_res.status,

stderr

));

}

Copilot · 2026-01-30T00:51:25Z

src/store.rs

+) -> Result<Box<dyn Iterator<Item = StorePath>>> {
+  let references = Command::new("nix-store").args(args).output();
+
+  let query = references?;


The shell command execution in shell_query does not check the exit status of the command. If nix-store fails (non-zero exit code), the function will attempt to parse stdout anyway, which could lead to silent failures or unexpected errors. The implementation should check query.status.success() before attempting to parse the output and provide a clear error message if the command failed.

Suggested change

let query = references?;

let query = references?;

if !query.status.success() {

let stderr = String::from_utf8_lossy(&query.stderr);

return Err(anyhow!(

"nix-store command failed with status {:?}: {}",

query.status.code(),

stderr

));

}

Copilot · 2026-01-30T00:51:25Z

src/store.rs

+  pub(crate) fn execute_row_query_with_path<T, M>(
    &self,
    query: &str,
    path: &Path,
    map: M,
-  ) -> Result<impl Iterator<Item = T>>
+  ) -> Result<Box<dyn Iterator<Item = T> + '_>>
  where
    T: 'static,
-    M: Fn(&Row) -> rusqlite::Result<T>,
+    M: Fn(&Row) -> rusqlite::Result<T> + 'static,
  {
    let path = path_to_canonical_string(path)?;
-    let stmt = self.prepare_cached(query)?;
-    QueryIterator::try_new(stmt, [path], map)
+    let stmt = self.get_inner()?.prepare_cached(query)?;
+    let iter = QueryIterator::try_new(stmt, [path], map)?;
+    Ok(Box::new(iter))


The return type has changed from Result<impl Iterator<Item = T>> to Result<Box<dyn Iterator<Item = T> + '_>>, introducing heap allocation and dynamic dispatch where it wasn't needed before. This has a performance cost. While boxing is necessary for trait object returns (as used in the StoreBackend trait), this internal helper function could still use the concrete type to avoid the overhead. Consider keeping the original return type for this internal method and only boxing when implementing the trait methods.

Copilot · 2026-01-30T00:51:26Z

src/store.rs

+      .arg(path.join("sw"))
+      .output()
+      .context(anyhow!("Encountered error while executing nix command"))?;
+    let text = str::from_utf8(&cmd_res.stdout)?;
+    if let Some(bytes_text) = text.split_whitespace().last()
+      && let Ok(bytes) = bytes_text.parse::<u64>()
+    {
+      Ok(Size::from_bytes(bytes))
+    } else {
+      Err(anyhow!("Unable to parse closure size from nix output"))
+    }
+  }
+
+  fn query_system_derivations(
+    &self,
+    system: &Path,
+  ) -> Result<Box<dyn Iterator<Item = StorePath> + '_>> {
+    shell_query(&[
+      "--query",
+      "--references",
+      &*system.join("sw").to_string_lossy(),


The code assumes that system paths should have "/sw" appended to them when querying derivations and closure size. This assumption is hard-coded in the ShellBackend implementation but may not be universally true. If a caller provides a path that already includes the "/sw" component or doesn't need it, this could lead to incorrect results. Consider documenting this assumption or validating that the path structure matches expectations before appending "/sw".

Copilot · 2026-01-30T00:51:26Z

src/store.rs

+    let path = StorePath::try_from(PathBuf::from(line))
+      .context(anyhow!("encountered invalid path in shell output: {line}"))?;


The error handling pattern .context(anyhow!("message")) is redundant. The .context() method from anyhow already creates an anyhow::Error with the provided message. Using anyhow!() inside .context() is unnecessary. This should be simplified to .context(format!("encountered invalid path in shell output: {line}"))? or use a string literal if the variable interpolation isn't needed in the context.

Copilot · 2026-01-30T00:51:26Z

src/store.rs

+  }
+}
+
+impl<'a> StoreBackend<'a> for DBConnection<'_> {


The trait implementation has a lifetime mismatch. The trait StoreBackend<'a> is parameterized with lifetime 'a, but the implementation impl<'a> StoreBackend<'a> for DBConnection<'_> uses a wildcard lifetime for DBConnection while declaring a separate 'a for the trait. These lifetimes should be aligned. The implementation should be impl<'a> StoreBackend<'a> for DBConnection<'a> to properly bind the lifetimes.

Suggested change

impl<'a> StoreBackend<'a> for DBConnection<'_> {

impl<'a> StoreBackend<'a> for DBConnection<'a> {

Copilot · 2026-01-30T00:51:27Z

src/store.rs

+/// The normal database connection
+pub const DATABASE_PATH: &str = "file:/nix/var/nix/db/db.sqlite";
+/// A backup database connection that can access the database
+/// even in a read-only environment
+///
+/// This might produce incorrect results as the connection is not guaranteed
+/// to be the only one accessing the database. (There might be e.g. a
+/// `nixos-rebuild` modifying the database)


The database path constants have changed. The old code used DATABASE_PATH with the immutable=1 parameter, but now DATABASE_PATH refers to the normal (non-immutable) connection and DATABASE_PATH_IMMUTABLE is the immutable one. This is a breaking change in behavior: the old connect() function always used the immutable mode, but now the default CombinedStoreBackend tries the normal connection first. This could lead to connection failures in read-only environments that worked before. Consider whether the order should be reversed, or document this breaking change clearly.

Suggested change

/// The normal database connection

pub const DATABASE_PATH: &str = "file:/nix/var/nix/db/db.sqlite";

/// A backup database connection that can access the database

/// even in a read-only environment

///

/// This might produce incorrect results as the connection is not guaranteed

/// to be the only one accessing the database. (There might be e.g. a

/// `nixos-rebuild` modifying the database)

/// The normal database connection (read/write, when permitted by the OS).

///

/// Note: In older versions, the default `connect()` implementation used the

/// immutable connection by default. The current implementation treats this

/// constant as the primary database path, and code such as

/// `CombinedStoreBackend` may try this path first before falling back to an

/// immutable connection (if it does so at all).

///

/// This change is a **breaking behavior change** for users running in

/// read-only environments: if the directory is not writable, attempts to use

/// this path may now fail where previous versions succeeded. In such cases,

/// callers should explicitly use `DATABASE_PATH_IMMUTABLE`.

pub const DATABASE_PATH: &str = "file:/nix/var/nix/db/db.sqlite";

/// A backup database connection that can access the database

/// even in a read-only environment using SQLite's `immutable=1` mode.

///

/// This might produce incorrect results as the connection is not guaranteed

/// to be the only one accessing the database. (There might be e.g. a

/// `nixos-rebuild` modifying the database.)

///

/// Historically, code like `connect()` used this immutable connection by

/// default. If you are upgrading from such a version and rely on read-only

/// behavior, ensure that you explicitly select this constant rather than

/// relying on the default `DATABASE_PATH`.

Copilot · 2026-01-30T00:51:27Z

src/store.rs

+fn shell_query<'a>(
+  args: &'a [&'a str],


The shell_query function signature declares an unused lifetime parameter 'a. The function signature is fn shell_query<'a>(args: &'a [&'a str]) but the returned iterator doesn't use this lifetime - it owns the data (Vec). The lifetime parameter should be removed for clarity: fn shell_query(args: &[&str]).

Suggested change

fn shell_query<'a>(

args: &'a [&'a str],

fn shell_query(

args: &[&str],

Co-authored-by: faukah <fau@faukah.com>

The initial motivation for the name `StoreFrontend` was that the trait provides a unified interface / frontend to the actual backend querying the data from the database. While this is what the trait is doing, it is still more a backend for querying data that is then served by the code in `diff.rs`. Therefor, this commit changes the naming to backend.

prepare for shell backend.

If for some reason the direct SQL queries to the sqlite database of the Nix store fail, we fall back to `nix` and `nix-store` shell commands (and pray).

Focuses on correctness over speed.

This flag is added in preparation for a later inclusion of machine-readable output. It allows the user to specify that the output should prioritize correctness at all costs. This rules out the use of immutable database connections or lazy queries. The former have no correctness guarantees and lazy queries could (theoretically) raise an error for a later row e.g. because the path returned from the store is not a valid `StorePath`.

This query was never actually used by dix. We can include it again if we ever need it.

Dragyx · 2026-01-30T17:21:18Z

There is not really much of a noticeable speed difference when switching between the EagerDBConnection and the LazyDBConnection (the eager one collects the results, the lazy one doesn't). I would still argue to keep both of them since the lazy connection might be useful when using dix as a connection to the nix DB in a different crate.

Copilot AI review requested due to automatic review settings January 29, 2026 19:08

Copilot started reviewing on behalf of Dragyx January 29, 2026 19:09 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

faukah requested a review from Copilot January 29, 2026 19:53

Copilot started reviewing on behalf of faukah January 29, 2026 19:54 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

Dragyx added 8 commits January 29, 2026 21:39

Abstract store code in preparation for fallbacks

7ad8a6c

finish initial implementation of CombinedStoreFrontend

d4d794b

Skip frontends that are not connected

0567a31

remove redundand connection information

96a4224

retain old connection on failed close

ce32d90

Make close function more robust against failure

5857f98

Fix typo

bcab7d8

Add documentation to DBConnection drop impl

226c86a

Dragyx force-pushed the fallback-store branch from 5de3f1d to 7d0e1d6 Compare January 29, 2026 20:57

faukah reviewed Jan 29, 2026

View reviewed changes

Dragyx force-pushed the fallback-store branch from c894457 to fffac74 Compare January 29, 2026 22:45

faukah requested a review from Copilot January 30, 2026 00:03

Copilot started reviewing on behalf of faukah January 30, 2026 00:04 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

Dragyx force-pushed the fallback-store branch from aec8d01 to 476f927 Compare January 30, 2026 00:43

Dragyx requested review from Copilot and faukah January 30, 2026 00:44

Copilot started reviewing on behalf of Dragyx January 30, 2026 00:45 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

Dragyx force-pushed the fallback-store branch 2 times, most recently from a233c4d to cbe1d85 Compare January 30, 2026 14:04

Dragyx mentioned this pull request Jan 30, 2026

Json output #51

Open

Dragyx force-pushed the fallback-store branch from 9400add to edaf486 Compare January 30, 2026 16:56

Dragyx and others added 11 commits January 30, 2026 18:13

switch to expect(dead_code) in StoreFrontend

29a6920

Make StoreFrontend trait public

be6eb9f

Add some (very, very) basic tests for the combined store

f0c9662

Fix spelling mistakes and add proper punctuation

25f4105

Co-authored-by: faukah <fau@faukah.com>

Remove DerivationIds from the query output to

38e8545

prepare for shell backend.

Implement shell backend fallback

5ece675

If for some reason the direct SQL queries to the sqlite database of the Nix store fail, we fall back to `nix` and `nix-store` shell commands (and pray).

Split up store.rs into multiple files.

405b4ac

Implement eager store backend.

c01b695

Focuses on correctness over speed.

Remove dependency graph query.

424c915

This query was never actually used by dix. We can include it again if we ever need it.

Dragyx force-pushed the fallback-store branch from edaf486 to 424c915 Compare January 30, 2026 17:13

faukah approved these changes Jan 30, 2026

View reviewed changes

faukah merged commit c260c5d into main Jan 30, 2026
4 checks passed

faukah deleted the fallback-store branch January 30, 2026 20:33

-/// A backup database connection that can access the database
-/// even in a read-only environment
-///
-/// This might produce incorrect results as the connection is not guaranteed
-/// to be the only one accessing the database. (There might be e.g. a
-/// `nixos-rebuild` modifying the database)
+/// A fallback database connection for read-only environments
+/// (e.g., when the filesystem is mounted read-only).
+///
+/// Uses the SQLite `immutable=1` flag, which tells SQLite the database
+/// will not change, enabling certain optimizations. However, if other
+/// processes modify the database while this connection is active, queries
+/// may return stale or incorrect results.

	.context(anyhow!("Encountered error while executing nix command"))?;
	.context("Encountered error while executing nix command")?;

-      .context(anyhow!("Encountered error while executing nix command"))?;
+      .context(anyhow!("Encountered error while executing nix command"))?;
+    if !cmd_res.status.success() {
+      let stderr = String::from_utf8_lossy(&cmd_res.stderr);
+      return Err(anyhow!(
+        "nix path-info --closure-size failed with status {:?}: {}",
+        cmd_res.status,
+        stderr
+      ));
+    }

-  let query = references?;
+  let query = references?;
+  if !query.status.success() {
+    let stderr = String::from_utf8_lossy(&query.stderr);
+    return Err(anyhow!(
+      "nix-store command failed with status {:?}: {}",
+      query.status.code(),
+      stderr
+    ));
+  }

		let path = StorePath::try_from(PathBuf::from(line))
		.context(anyhow!("encountered invalid path in shell output: {line}"))?;

	impl<'a> StoreBackend<'a> for DBConnection<'_> {
	impl<'a> StoreBackend<'a> for DBConnection<'a> {

-/// The normal database connection
-pub const DATABASE_PATH: &str = "file:/nix/var/nix/db/db.sqlite";
-/// A backup database connection that can access the database
-/// even in a read-only environment
-///
-/// This might produce incorrect results as the connection is not guaranteed
-/// to be the only one accessing the database. (There might be e.g. a
-/// `nixos-rebuild` modifying the database)
+/// The normal database connection (read/write, when permitted by the OS).
+///
+/// Note: In older versions, the default `connect()` implementation used the
+/// immutable connection by default. The current implementation treats this
+/// constant as the primary database path, and code such as
+/// `CombinedStoreBackend` may try this path first before falling back to an
+/// immutable connection (if it does so at all).
+///
+/// This change is a **breaking behavior change** for users running in
+/// read-only environments: if the directory is not writable, attempts to use
+/// this path may now fail where previous versions succeeded. In such cases,
+/// callers should explicitly use `DATABASE_PATH_IMMUTABLE`.
+pub const DATABASE_PATH: &str = "file:/nix/var/nix/db/db.sqlite";
+/// A backup database connection that can access the database
+/// even in a read-only environment using SQLite's `immutable=1` mode.
+///
+/// This might produce incorrect results as the connection is not guaranteed
+/// to be the only one accessing the database. (There might be e.g. a
+/// `nixos-rebuild` modifying the database.)
+///
+/// Historically, code like `connect()` used this immutable connection by
+/// default. If you are upgrading from such a version and rely on read-only
+/// behavior, ensure that you explicitly select this constant rather than
+/// relying on the default `DATABASE_PATH`.

Conversation

Dragyx commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dragyx commented Jan 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Dragyx commented Jan 29, 2026 •

edited

Loading