Record builder host name in build provenance#341
Conversation
📝 WalkthroughWalkthroughThis PR adds hostname tracking to build provenance by introducing a configurable hostName setting in Settings, implementing a hostname getter with caching, extending the BuildProvenance struct to include an optional buildHost field, and updating JSON serialization/deserialization and display logic to surface the build host information. Changes
Sequence DiagramsequenceDiagram
participant Settings
participant DerivationBuilder as Derivation Builder
participant BuildProvenance
participant JSON as JSON Serialization
participant Display as Provenance Display
DerivationBuilder->>Settings: getHostName()
Settings->>Settings: Check cached hostname
alt Cached
Settings-->>DerivationBuilder: Return cached hostname
else Not cached
Settings->>Settings: Call gethostname() via POSIX
Settings-->>DerivationBuilder: Cache and return hostname
end
DerivationBuilder->>BuildProvenance: construct(drvPath, output, buildHost, next)
BuildProvenance->>BuildProvenance: Initialize buildHost field
BuildProvenance-->>DerivationBuilder: BuildProvenance created
BuildProvenance->>JSON: to_json()
JSON->>JSON: Serialize buildHost field
JSON-->>BuildProvenance: JSON with buildHost
JSON->>Display: Read buildHost from JSON
Display->>Display: Format "on <buildHost>"
Display-->>Display: Output provenance with host info
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/functional/common/init.sh (1)
46-56:⚠️ Potential issue | 🟠 MajorSet
host-namefor the NixOS test config too to keep provenance deterministic.Line 55 adds a fixed host name for the non‑NixOS path, but the NixOS branch’s
test_nix_confdoes not set it. If tests run on NixOS,buildHostwill vary by machine and can make provenance assertions flaky.🔧 Suggested fix
@@ cat > "$test_nix_conf" <<EOF # TODO: this is not needed for all tests and prevents stable commands from be tested in isolation experimental-features = ${experimental_features:-} flake-registry = $TEST_ROOT/registry.json show-trace = true +host-name = test-host EOF
🤖 Fix all issues with AI agents
In `@src/libstore/globals.cc`:
- Around line 270-279: The getHostName implementation uses POSIX APIs
(gethostname and HOST_NAME_MAX) but lacks the required headers and platform
guards; update the file to include <unistd.h> and <limits.h> (or <climits>) when
not on Windows and wrap the POSIX-specific code with an `#ifndef` _WIN32 (or `#if`
defined(__unix__) || defined(__APPLE__)) guard so Windows builds skip or provide
an alternative, and ensure the Settings::getHostName() implementation references
gethostname and HOST_NAME_MAX only inside that guarded section.
b5b7c85 to
f9c08f8
Compare
f9c08f8 to
0a59bf7
Compare
cole-h
left a comment
There was a problem hiding this comment.
LGTM.
$ ./result/bin/nix provenance show .#nix-store-static --store /tmp/foostore --extra-experimental-features provenance
/nix/store/kxdn5ql8wbq9kgdzinkcr48x32557xm8-determinate-nix-store-static-x86_64-unknown-linux-musl-3.15.2
← built from derivation /nix/store/d5nkhxqk3ppmxi3jr13a1ap82dly3wv5-determinate-nix-store-static-x86_64-unknown-linux-musl-3.15.2.drv (output out) on scadrial
← instantiated from flake output git+file:///home/vin/workspace/detsys/nix-src?rev=6cae017ca69d0b95b9e8a60ebc3f4ed0ce8b64b5#packages.x86_64-linux.nix-store-static
| if (auto p = optionalValueAt(obj, "next"); p && !p->is_null()) | ||
| next = Provenance::from_json(*p); | ||
| std::optional<std::string> buildHost; | ||
| if (auto p = optionalValueAt(obj, "buildHost")) |
There was a problem hiding this comment.
I wonder if it would also be a good idea to include the native architecture, if that's easy to get...
There was a problem hiding this comment.
Related to that, we should probably record the system attribute of the derivation, so we can query what system a store path is for. Currently there is no way to get that info unless you have the deriver around.
0a59bf7 to
97c517d
Compare
Motivation
This keeps track where a build was done.
Depends on #340.
Context
Summary by CodeRabbit