Skip to content

Add Derivation::shouldResolve() method, use in nix-shell#15170

Merged
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:should-resolve
Feb 7, 2026
Merged

Add Derivation::shouldResolve() method, use in nix-shell#15170
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:should-resolve

Conversation

@amaanq
Copy link
Member

@amaanq amaanq commented Feb 6, 2026

Motivation

Extract the logic for determining whether a derivation should be resolved
before building into a dedicated method. Then use that to not resolve
unnecessarily in nix-shell.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Extract the logic for determining whether a derivation should be resolved
before building into a dedicated method. Then use that to not resolve
unnecessarily in `nix-shell`.
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Feb 6, 2026
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Pure refactor for building, improved behavior for old CLI

@Ericson2314 Ericson2314 enabled auto-merge February 6, 2026 23:34
Comment on lines -534 to 535
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
if (drv.shouldResolve()) {
auto resolvedDrv = drv.tryResolve(*store);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this still require the XP feature? Or would that check be redundant and we'd fail earlier anyway?

Copy link
Member

Choose a reason for hiding this comment

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

It would be redundant, yeah. Though I wouldn't mind putting more redundant checks in the body of the function.

@Ericson2314 Ericson2314 added this pull request to the merge queue Feb 7, 2026
Merged via the queue into NixOS:master with commit e29bb23 Feb 7, 2026
14 checks passed
@Ericson2314 Ericson2314 deleted the should-resolve branch February 7, 2026 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants