fix(queue-runner): don't parse buildproducts.path as a StorePath

`buildproducts.path` is a full filesystem path, not a bare store path:
Hydra's `$out/nix-support/hydra-build-products` lets a product point at
a sub-path of an output (e.g. `doc manual $doc/share/doc/nix/manual
index.html`), so the column legitimately contains a trailing `/...`
after the `<hash>-<name>`.

`OwnedBuildProduct::parse_paths` fed the whole string to
`StoreDir::parse`, which died with `invalid store path / symbol at 50`
on any such value. That wedged every cached-build fast path touching a
product with a subpath — notably the `nix-manual-2.31.4/share/doc/nix/manual`
products from `roots.nix` builds, leaving ~200 builds per eval stuck
as 'unfinished' even though every output was already in the store.

Even had the parse succeeded, `BuildProduct::from_db` was hard-coding
`relative_path: \"\".into()`, so the sibling constructors
(`from_grpc`, `from_shared`) and `from_db` disagreed on what the field
meant.

Fix by:
- dropping the `StorePath` generic + `parse_paths` from
  `OwnedBuildProduct` (path stays as `Option<String>` — it's opaque
  to the db crate), and
- routing `BuildProduct::from_db` through the same
  `RelativeStorePath::from_path` splitter `from_grpc`/`from_shared`
  already use. The db-side write path calls `RelativeStorePath::print`,
  so `from_path` is now its exact inverse.

Adds unit tests covering the subpath product, a bare store path, and
the print/from_path round-trip.

Author: hamishmack

subprojects/crates/db/src/connection.rs

@@ -261,10 +261,9 @@ impl Connection {
     pub async fn get_build_products_for_build_id(
         &mut self,
         build_id: i32,
-        store_dir: &StoreDir,
-    ) -> anyhow::Result<Vec<crate::models::OwnedBuildProduct>> {
-        let rows = sqlx::query_as!(
-            crate::models::OwnedBuildProduct::<String>,
+    ) -> sqlx::Result<Vec<crate::models::OwnedBuildProduct>> {
+        sqlx::query_as!(
+            crate::models::OwnedBuildProduct,
             r#"
             SELECT
               type,
@@ -279,10 +278,7 @@ impl Connection {
             build_id
         )
         .fetch_all(&mut *self.conn)
-        .await?;
-        rows.into_iter()
-            .map(|r| Ok(r.parse_paths(store_dir)?))
-            .collect()
+        .await
     }
 
     pub async fn get_build_metrics_for_build_id(

subprojects/crates/db/src/models.rs

@@ -221,34 +221,22 @@ pub struct BuildOutput {
     pub size: Option<i64>,
 }
 
+// `buildproducts.path` is an opaque filesystem path, not a store path: Hydra's
+// `$out/nix-support/hydra-build-products` lets a product point at a sub-path of
+// an output (e.g. `doc manual $doc/share/doc/nix/manual index.html`), so the
+// value can legitimately contain a trailing `/...` after the store path. Keep
+// it as a raw `String` here; splitting into base + relative is the caller's job.
 #[derive(Debug)]
-pub struct OwnedBuildProduct<StorePath = harmonia_store_core::store_path::StorePath> {
+pub struct OwnedBuildProduct {
     pub r#type: String,
     pub subtype: String,
     pub filesize: Option<i64>,
     pub sha256hash: Option<String>,
-    pub path: Option<StorePath>,
+    pub path: Option<String>,
     pub name: String,
     pub defaultpath: Option<String>,
 }
 
-impl OwnedBuildProduct<String> {
-    pub fn parse_paths(
-        self,
-        store_dir: &StoreDir,
-    ) -> Result<OwnedBuildProduct, ParseStorePathError> {
-        Ok(OwnedBuildProduct {
-            r#type: self.r#type,
-            subtype: self.subtype,
-            filesize: self.filesize,
-            sha256hash: self.sha256hash,
-            path: self.path.map(|p| store_dir.parse(&p)).transpose()?,
-            name: self.name,
-            defaultpath: self.defaultpath,
-        })
-    }
-}
-
 #[derive(Debug)]
 pub struct BuildProduct<'a> {
     pub r#type: &'a str,

subprojects/hydra-queue-runner/src/state/build.rs

@@ -388,12 +388,15 @@ pub struct BuildProduct {
 }
 
 impl BuildProduct {
-    pub fn from_db(v: db::models::OwnedBuildProduct) -> Self {
-        Self {
-            path: v.path.map(|p| RelativeStorePath {
-                base_path: p,
-                relative_path: "".into(),
-            }),
+    pub fn from_db(
+        store_dir: &nix_utils::StoreDir,
+        v: db::models::OwnedBuildProduct,
+    ) -> anyhow::Result<Self> {
+        Ok(Self {
+            path: v
+                .path
+                .map(|p| RelativeStorePath::from_path(store_dir, &p))
+                .transpose()?,
             default_path: v.defaultpath,
             r#type: v.r#type,
             subtype: v.subtype,
@@ -402,7 +405,7 @@ impl BuildProduct {
             sha256hash: v.sha256hash,
             #[allow(clippy::cast_sign_loss)]
             file_size: v.filesize.map(|v| v as u64),
-        }
+        })
     }
 
     pub fn from_grpc(
@@ -739,3 +742,63 @@ impl Builds {
         builds.remove(&id);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn test_store_dir() -> nix_utils::StoreDir {
+        nix_utils::StoreDir::new("/nix/store").unwrap()
+    }
+
+    // Regression: `buildproducts.path` produced by e.g. a `doc manual …` entry in
+    // `$out/nix-support/hydra-build-products` points at a sub-path of an output.
+    // Parsing the full value as a bare `StorePath` used to fail with
+    // `invalid store path / symbol at 50`, wedging every "cached" Hydra build
+    // whose outputs included a doc/manual or similar sub-path product.
+    #[test]
+    fn relative_store_path_splits_product_subpath() {
+        let dir = test_store_dir();
+        let rel = RelativeStorePath::from_path(
+            &dir,
+            "/nix/store/bwqqp42xqn37z31dapi7jrhy8iwc2zsx-nix-manual-2.31.4/share/doc/nix/manual",
+        )
+        .expect("subpath product must parse");
+        assert_eq!(
+            rel.base_path.to_string(),
+            "bwqqp42xqn37z31dapi7jrhy8iwc2zsx-nix-manual-2.31.4"
+        );
+        assert_eq!(&*rel.relative_path, "share/doc/nix/manual");
+    }
+
+    #[test]
+    fn relative_store_path_accepts_bare_store_path() {
+        let dir = test_store_dir();
+        let rel = RelativeStorePath::from_path(
+            &dir,
+            "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-example-1.0",
+        )
+        .expect("bare store path must parse");
+        assert_eq!(
+            rel.base_path.to_string(),
+            "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-example-1.0"
+        );
+        assert!(rel.relative_path.is_empty());
+    }
+
+    // The DB write path calls `RelativeStorePath::print` and the DB read path
+    // (after this fix) calls `RelativeStorePath::from_path`. They must be inverses
+    // for both shapes of `buildproducts.path`.
+    #[test]
+    fn relative_store_path_roundtrips() {
+        let dir = test_store_dir();
+        for original in [
+            "/nix/store/bwqqp42xqn37z31dapi7jrhy8iwc2zsx-nix-manual-2.31.4/share/doc/nix/manual",
+            "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-example-1.0",
+        ] {
+            let rel = RelativeStorePath::from_path(&dir, original)
+                .unwrap_or_else(|e| panic!("parse {original}: {e}"));
+            assert_eq!(rel.print(&dir), original);
+        }
+    }
+}

subprojects/hydra-queue-runner/src/state/mod.rs

@@ -2057,11 +2057,11 @@ impl State {
                 };
 
                 res.products = db
-                    .get_build_products_for_build_id(build_id, self.store.store_dir())
+                    .get_build_products_for_build_id(build_id)
                     .await?
                     .into_iter()
-                    .map(build::BuildProduct::from_db)
-                    .collect();
+                    .map(|p| build::BuildProduct::from_db(self.store.store_dir(), p))
+                    .collect::<anyhow::Result<_>>()?;
                 res.metrics = db
                     .get_build_metrics_for_build_id(build_id)
                     .await?