Skip to content

perf(install): batch nix path-info and prefetch nixpkgs once - #3053

Open
mikeland73 wants to merge 4 commits into
mainfrom
claude/perf-fewer-nix-calls-4c80df
Open

mikeland73 wants to merge 4 commits into
mainfrom
claude/perf-fewer-nix-calls-4c80df

Conversation

@mikeland73

Copy link
Copy Markdown
Collaborator

Implements Tier 3 item 15 and the nix flake prefetch bullet of item 17 from specs/perf-improvements.md.

Batch nix path-info

packagesToInstallInStore used to call Package.GetStorePaths per package, which ran one nix path-info process per installable for every package without store paths in the lockfile (flakes, unresolved packages).

New devpkg.PackagesNotInStore:

  • uses lockfile store paths where available (unchanged),
  • runs one nix path-info --json --impure for all remaining installables (one per allow_insecure setting),
  • checks everything with the existing single nix path-info --offline call.

nix path-info doesn't say which installable produced which path, so a batch's paths can't be split per package. When every path is in the store (the common, steady-state case) that doesn't matter. When the batch fails or a path is missing, that group is retried one package at a time, so packageInstallErrorHandler still names the failing package and only the right packages get installed. Groups with a single package skip the batch call.

The batching logic is generic over the item type and covered by unit tests with a fake nix.

Prefetch nixpkgs once

EnsureNixpkgsPrefetched ran nix flake prefetch to download, then again with --json to get the store path. It now runs it once with --json (progress still goes to stderr) and parses storePath from stdout.

Testing

  • go test ./internal/devpkg ./internal/nix ./internal/devbox
  • Manual: project with two flake-ref packages; DEVBOX_DEBUG=1 devbox install shows a single path-info with both installables followed by one --offline check.
  • Confirmed nix flake prefetch --json output shape on nix 2.30.2.

🤖 Generated with Claude Code

mikeland73 and others added 2 commits November 13, 2026 09:27
Check which packages need installing with one `nix path-info` call per
allow_insecure setting instead of one per installable. path-info output
can't be mapped back to installables, so a batch is retried one package
at a time only if it fails or reports a missing path; that keeps the
per-package error messages.

Run `nix flake prefetch --json` once and read the store path from its
output instead of running prefetch a second time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Nov 13, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@greptile-apps

greptile-apps Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The latest changes appear safe to merge.

Summary

The PR batches nix path-info calls and downloads nixpkgs with one prefetch call.

  • Devbox checks package store paths with fewer Nix calls.
  • Nixpkgs prefetch gets and saves its store path in one call.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Collect package paths] --> B[Query batches by allow_insecure]
  B --> C[Retry failed or single-package groups individually]
  C --> D[Check paths in local store]
  D --> E{Any batch path missing?}
  E -->|Yes| F[Query packages individually and check unknown paths]
  E -->|No| G[Select packages with missing paths]
  F --> G
Loading

Reviews (2) · Last reviewed commit: "style(devpkg): gofumpt -extra storepaths..." · Reviewed by Greptile

mikeland73 and others added 2 commits November 13, 2026 09:30
Split notInStore into helpers to reduce cognitive complexity and use
longer variable names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mikeland73

Copy link
Copy Markdown
Collaborator Author

@greptileai please re-review the latest commits (lint/format fixes since your last review) and approve if there are no issues.


Written by Claude on behalf of / prompted by @mikeland73.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant