web: navigate from unresolved to resolved CA build steps#1682
Draft
amaanq wants to merge 8 commits intoNixOS:masterfrom
Draft
web: navigate from unresolved to resolved CA build steps#1682amaanq wants to merge 8 commits intoNixOS:masterfrom
amaanq wants to merge 8 commits intoNixOS:masterfrom
Conversation
Member
|
Compute URLs and title strings upfront instead of repeating inline
ternaries throughout the template. Use consistent `${...}` variable
interpolation style.
No behavior changes.
Soon, we're going to make a new build step page which will share a number of things with the build page. In order to get ready for that, extract the following into a new `build-common.tt` for reuse: - `renderOutputs` - `renderStepStatus` - `renderStepDuration` - `renderStepMachine` - `renderLogButtons` Likewise, move the `showLog` helper out of `Build.pm` into `Hydra::Helper::LogEndpoints`. Both are needed by the BuildStep controller introduced in a later commit. As a bonus, the new `renderLogButtons` is already used twice within `build.tt` itself (for the build log and the runcommand log), deduplicating previously identical button markup. No behavior changes.
Add a `renderOutputsTable` block in `build-common.tt` that renders outputs as a nested info-table with name and path columns. Use it on the build detail page, replacing the old inline `renderOutputs` which only showed paths. I like this much better because I want to see the output names at a glance, not just the store paths. This layout is especially good for CA derivations where we don't know the output paths in advanced of building them.
Build steps are, in my view, an important concept that Hydra doesn't yet give enough attention. This is my attempt to rectify that. A new detail page at `/build/:id/step/:stepnr` shows output paths, derivation, system, machine, duration, status, and log links, reusing the shared blocks extracted in the previous commit. Clicking anywhere in the build steps table row now navigates to this page, instead of the step log page. The step log pages also have a link back to this page. Also reflecting giving build steps more status, introduce `Hydra::Controller::BuildStep`, chained off `/build/buildChain`, giving them a first-class controller. For a bit of back story, note that in the future, we might switch associating build steps more with derivations than builds. This reflects that we don't really care *why* something was scheduled (the build/root derivation) as much as *what* was scheduled, especially when multiple new builds would "race" to schedule the same derivation. It also bodes well for a future where Hydra can act as a Nix derivation that receives ad-hoc build requests, so the "build" in this case would be rather lacking in metadata. Both these scenarios point to a world where `BuildStep`s become more important than `Build`s, building (ahem) atop this refactor. The step log handling is now better suited to live as part of this controller. Accordingly, it is moved from `/build/:id/nixlog/:stepnr[/raw|/tail]` to `/build/:id/step/:stepnr/log[/raw|/tail]`. The old `nixlog` URLs are preserved as 301 redirects in `Build.pm`. All templates updated to generate the new canonical URLs.
Instead of resolving at StepInfo construction and carrying two drv identities through the gRPC layer, resolve in realise_drv_on_valid_machine once all deps are built. If resolution yields a different drv, the original step is marked Resolved and a new DB step is created for the resolved drv with a resolvedTo FK linking them. The builder only ever sees one drv. We create a new Step for that resolution and bunt it back to the scheduler. This grants us more flexibility in execution and the method can be used in the future for dynamic derivations, which won't map 1:1 with the original derivations. We also now only create database steps when we are sure they are necessary, reducing the number of duplicate `BuildStep` rows. In order to make tests more consistent, CA derivations will fail if they cannot be fully resolved. Otherwise, there could be inconsistent successes depending on which builder a step was performed on. As part of this, add local outputs to resolution table With the current queue-runner design, all dependency outputs of a CAFloating derivation must be recorded in the hydra database. This is true for things built or substituted by hydra, but until now not by things found on the local nix store. This may occur for outputs that are part of the system configuration. Therefore, add all local outputs that are not already in the database to the resolution table upon creating a step. This makes it possible to build derivations from `contentAddressedByDefault` nixpkgs. Co-Authored-By: Artemis Tosini <artemis.tosini@obsidian.systems> Co-Authored-By: John Ericson <John.Ericson@Obsidian.Systems> Co-Authored-By: Amaan Qureshi <git@amaanq.com>
wip Add dynamic derivation resolution Do dynamic derivation resolution, still doesn't splice tree Revert "Do dynamic derivation resolution, still doesn't splice tree" This reverts commit 389cc05. Simpler initial dynamic resolution dynamic dep prototype, needs testing Remove dynamic_deps, we can store relation in dynamic_rdeps instead enable dyn-drv test Let Claude fix dynamic derivation building in queue runner This was basically unsupervised, except for me running the test and feeding it the test logs. Four bugs prevented dynamic derivations from being built: - `get_dependents` only walked `rdeps`, not `dynamic_rdeps`, so steps connected via dynamic dependencies appeared orphaned and were "maybe cancelled" instead of dispatched. - `succeed_step` used `get_direct_builds` to find the owning build for dynamic rdeps processing. Intermediate steps (like `hello.drv.drv`) have no direct builds, so we now fall back to `get_dependents` to walk the dependency chain. - `make_rdeps_runnable` only processed `rdeps`, never removing the finished step from `dynamic_rdeps` dependents' forward `deps`. This left the wrapper permanently stuck with an unsatisfied dependency. - `resolve_dynamic_input` used `1..outputs.len()` which produces an empty range for single-element chains (the common case). Changed to `1..=outputs.len()` so the chain is actually resolved and the produced `.drv` file is discovered and built as a new step. Test updated from 2 to 5 expected build steps now that Hydra properly tracks the full dynamic derivation chain. Combine `rdeps` and `dynamic_rdeps` into a single `rdeps` list Both were reverse dependency lists differing only in whether they carried a `relation` (output name chain for dynamic derivations). Having two separate lists meant every operation that walked one had to remember to also walk the other — the exact class of bug fixed in the previous commit. Now `StepState::rdeps` is `Vec<ReverseDep>` where an empty `relation` signifies a regular (non-dynamic) reverse dependency. `get_dependents`, `make_rdeps_runnable`, and `add_referring_data` all operate on the single unified list. The `dynamic_rdeps_len` atomic counter, `clone_dyn_rdeps` method, and `DynamicReverseDep` type are removed in favor of the renamed `ReverseDep`. Resolve dynamic derivation chains one level at a time Instead of resolving the entire chain at once via a recursive SQL query in `resolve_dynamic_input`, resolve one level per step completion: when a step finishes, `pop_dynamic_rdeps` pops the next output name from the relation stack, and `succeed_step` looks it up directly in the build output — no DB round-trip needed. The relation vector is now stored in reverse order so that `pop()` gives the next level to resolve in O(1). `resolve_dynamic_input` is removed from both `State` and `db::Connection` since it is no longer needed. The `create_step` input processing no longer attempts eager resolution either; the dynamic rdeps mechanism in `succeed_step` handles it naturally as each step completes. TEMP add some asserts, see failure Fix CA resolution for intermediate steps, extract `DynDrvRelation` type Three changes: - **Fix premature build completion**: `realise_drv_on_valid_machine` passed `referring_build: Some(build)` when creating resolved steps for ALL CA derivations, not just the toplevel. This caused `succeed_step` to mark the entire build as finished when an intermediate resolved step completed (e.g. resolved `build-b`), orphaning downstream steps (d, e, wrapper). Now `referring_build` is only set when the unresolved step was the build's toplevel. - **`DynDrvRelation` newtype**: Extracts the reversed output-name stack into its own type, documenting the reversal invariant in the type name and providing `is_empty()`/`pop()` methods. Used throughout `ReverseDep`, `add_referring_data`, `create_step`, and `pop_dynamic_rdeps`. - **`input_drvs` free function**: Walks `SingleDerivedPath` directly instead of going through `DerivationInputs::from`. Skips `Opaque` (source) inputs — only `Built` derivation dependencies are returned. The outermost output name (what we consume) is discarded; intermediate names form the `DynDrvRelation`. Called inline in `create_step` before `set_drv` consumes the derivation. Non-trivial dyn-drv test now expects 12 build steps covering the full chain: makeDerivations → a → b,c (resolve+build) → d (resolve+build) → e (resolve+build) → wrapper (resolve+build). Extract `state::drv` module with `OutputNameChain` and `SingleDerivedPath` helpers Move `OutputNameChain` (renamed from `DynDrvRelation`), `input_drvs`, and `flatten_chain` into a new `state::drv` module. Add `flatten_path` as the base case for mutual recursion with `flatten_chain`. `flatten_path`/`flatten_chain` now return `OutputNameChain` directly in stack order (outermost first), eliminating the separate reverse step. `input_drvs` calls `flatten_path` on the inner `drv_path` of each `Built` input, naturally discarding the outermost output name. `flatten_chain` (previously in `step_info.rs`) and `input_drvs` (previously in `step.rs`) are unified in one place. The SQL caller in `step_info.rs` reverses to forward order where needed.
Build step pages branch on status=13 to show the original drv, resolved basename, and the terminal step link instead of the Build/Substitution info table; the log endpoint 302s to the terminal's log or back to the step page when no terminal log exists yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This depends on #1629
Resolved CA steps do not own the log users are trying to inspect, so
leaving them as terminal pages makes the build view look stuck or failed
in the wrong place. The log page now follows the recorded resolution chain
to the step that actually performed the work, while preserving a pending
page for unresolved, running, or malformed chains.
The redirect banner is reconstructed from BuildSteps instead of session
flash so copied log URLs and refreshes keep the same context. The JSON
summary exposes the same durable resolution data for API clients that
need to follow CA build steps without scraping HTML.