diff options
author | Florian Klink <flokli@flokli.de> | 2024-10-01T14·07+0300 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-10-01T14·27+0000 |
commit | f0d5ed7074097ca44a86865ef82cdbd1d1bddff7 (patch) | |
tree | b6c8eb2be12385c64b1cea572d5dfba5618e0dba /tvix/docs | |
parent | 284c1eb45aa1a7d8436a3e8edb20bb0fbbced5c7 (diff) |
docs(tvix/TODO): add PathInfo data types and ca reference items r/8746
With https://cl.tvl.fyi/12533 in, we still need to lookup references to properly populate `BuildRequest`. It currently fails as the reference to h9lc1dpi14z7is86ffhl3ld569138595-audit-tmpdir.sh is not propagated. We should prevent Frankenbuilds from the go, so let's update our PathInfo type to accomodate for that. Change-Id: I26f9215312c258bba222efd390bc135f1a3a3d6d Reviewed-on: https://cl.tvl.fyi/c/depot/+/12560 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
Diffstat (limited to 'tvix/docs')
-rw-r--r-- | tvix/docs/src/TODO.md | 43 |
1 files changed, 43 insertions, 0 deletions
diff --git a/tvix/docs/src/TODO.md b/tvix/docs/src/TODO.md index af558c9580fe..9f8feef9e3ac 100644 --- a/tvix/docs/src/TODO.md +++ b/tvix/docs/src/TODO.md @@ -136,6 +136,49 @@ Similarly, we also don't properly populate the build environment for `fetchClosure` yet. (Note there already is `ExportedPathInfo`, so once `structuredAttrs` is there this should be easy. +### PathInfo Data types +Similar to the refactors done in tvix-castore, we want a stricter type for +PathInfo, and use the `tvix_castore::nodes::Node` type we now have as the root +node. + +This allows removing some checks, conversions and handling for invalid data in +many different places in different store implementations. + +Steps: + + - Define the stricter `PathInfo` type + - Update the `PathInfoService` trait to use the stricter types + - Update the grpc client impl to convert from the proto types to the + stricter types (and reject invalid ones) + - Update the grpc server wrapper to convert to the proto types + +### PathInfo: include references by content +In the PathInfo struct, we currently only store references by their names and +store path hash. Getting the castore node for the content at that store path +requires another lookup to the PathInfoService. + +Due to this information missing, this also means we currently cannot preserve +information necessary to detect/prevent +[Frankenbuilds](https://tvl.fyi/blog/tvix-update-february-24#builder-protocol-drv-builder). + +We should extend/change the `PathInfo` type to maintain references in a +`BTreeMap` from store path basename to castore node. Should probably be done +after PathInfo uses its own data type. + +The `NixHTTPPathInfoService` needs to get some more logic to populate the ca +bits of the references: + + - If the referenced store path if not present in our PathInfoService hierarchy, + it needs to be ingested too, and persisted. + - If the referenced store path is present, we can use the castore root from there. + In an optional mode, we should parse the .narinfo file for the reference, and + compare the nar hash with our local one, to detect Frankenbuilds in a binary +cache. + +As `NixHTTPPathInfoService` now needs to interact with "the PathInfoService" +hierarchy, we need to accept a pointer to a PathInfoService (hierarchy) that's +used to check for presence, and PathInfos are inserted into. + ### Builders Once builds are proven to work with real-world builds, and the corner cases there are ruled out, adding other types of builders might be interesting. |