Skip to content

Don't pass resolved_drv to the builder#1620

Closed
artemist wants to merge 1 commit intoNixOS:masterfrom
obsidiansystems:remove-builder-resolution
Closed

Don't pass resolved_drv to the builder#1620
artemist wants to merge 1 commit intoNixOS:masterfrom
obsidiansystems:remove-builder-resolution

Conversation

@artemist
Copy link
Copy Markdown
Member

@artemist artemist commented Mar 30, 2026

Whenever possible, only a resolved derivation should be passed around. Remove all the separation between "resolved" and "unresolved" derivation past Job, including in the builder.

pub async fn new(store: &nix_utils::LocalStore, step: Arc<Step>) -> Self {
pub async fn new(step: Arc<Step>) -> Self {
Self {
resolved_drv_path: store.try_resolve_drv(step.get_drv_path()).await,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't want to lose this. We can just monkey patch the step to have the right drv?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patching Step is somewhat challenging. It would be easier to patch Job, which is the first part of sending to a builder.

@artemist artemist force-pushed the remove-builder-resolution branch from 757329b to ec8f54f Compare March 30, 2026 16:19
The builder should only ever be given a resolved derivation, and we can
name that drv.
@artemist artemist force-pushed the remove-builder-resolution branch from ec8f54f to 7d17df2 Compare March 30, 2026 16:36
@artemist
Copy link
Copy Markdown
Member Author

Strange, the tests worked locally on x86_64. Content addressed is what changed here, but I expected it would work.

@artemist
Copy link
Copy Markdown
Member Author

I think the issue is that we made Job always have a resolved path, and a lot of code assumes that it doesn't change. I can change this, but my final design may depend on how we plan to store resolved derivations in the db.

@Ericson2314
Copy link
Copy Markdown
Member

Superceded by #1629, I believe.

@Ericson2314 Ericson2314 closed this Apr 7, 2026
@Ericson2314 Ericson2314 deleted the remove-builder-resolution branch April 7, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants