Repository navigation
perf(devbox): skip the nix recompute when only env, init_hook or scripts change - #3031
Merged
Merged
Conversation
…pts change Implements item 8 of `specs/perf-improvements.md`. `.devbox/state.json` stored one hash of the whole config, so editing a script, an env var or the init hook made the next command do a full recompute: package install checks, narinfo lookups, flake generation, `nix print-dev-env` and a nix profile sync. None of those outputs depend on these fields. - `state.json` gets a second hash, `nix_config_hash`. It's like `config_hash`, except the root config's `env`, `env_from`, `shell` (`init_hook` and `scripts`), `aliases`, `name`, `description`, `$schema` and `devbox_version` are removed from the AST before hashing. Everything else, including unknown fields, still counts as a nix input. Included plugins, package hashes and plugin config hashes are hashed in full, the same as before. - New `lock.File.StateStatus` returns up to date, non-nix stale, or stale. It returns non-nix stale only when `config_hash` is the only field that changed: the lockfile, print-dev-env cache, profile manifest, Devbox version and shell must all still match. `IsUpToDateAndInstalled` wraps it. - When the state is non-nix stale, `ensureStateIsUpToDate` (ensure mode) rewrites the scripts and the `.hooks` file and saves the new hashes. It doesn't print "Ensuring packages are installed". The env is then computed from the cached print-dev-env output. Env vars, `env_from` and aliases need no extra work because they're read from the config every time the env is computed. - `--recompute=false` (shellenv, run) also does this refresh instead of printing the stale state warning, since it doesn't run nix. - install, uninstall and update modes don't change. They always do the full work. - A `state.json` written by an older Devbox has no `nix_config_hash`, so it gets one full recompute. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@claude ptal |
|
Claude finished @mikeland73's task in 38s —— View job Review of #3031: skip the nix recompute when only env, init_hook or scripts changeI read the non-test diff and checked the PR's claims against the code. I didn't run the tests or linters. Overall the change looks correct and I found no blocking issues. What I checked
Minor, non-blocking
Tests LGTM from my side, with the nits above as optional follow-ups. |
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
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.
Summary
Implements item 8 of
specs/perf-improvements.md..devbox/state.jsonstored one hash of the whole config. Editing a script, an env var or the init hook made the nextdevbox run/shell/shellenvdo a full recompute: package install checks, narinfo lookups, flake generation,nix print-dev-envand a nix profile sync. None of those outputs depend on these fields.What counts as a nix input
state.jsongets a second hash,nix_config_hash. It's computed likeconfig_hash, but the root config'senv,env_from,shell(init_hookandscripts),aliases,name,description,$schemaanddevbox_versionare removed from the AST first. I checked each of these. None of them reach the flake template,nix print-dev-envor the profile.env/env_fromare merged incomputeEnvon every run, scripts and the init hook are written byshellgen.WriteScriptsToFiles, and aliases are read when a shell or command starts.devbox_versionis only read by vercheck, and the state file already tracks the running version.packages,include,nixpkgs, the package hashes (which cover local flake contents), and all included plugin configs. Plugins are hashed in full, including their env and init hooks, because they can add packages and create files.Up-to-date check
lock.File.StateStatusreturns up to date, non-nix stale, or stale. It returns non-nix stale only whenconfig_hashis the only thing that changed. The lockfile, print-dev-env cache, profile manifest, Devbox version and shell must all still match.IsUpToDateAndInstallednow wraps it.state.jsonwritten by an older Devbox has nonix_config_hash, so it gets one full recompute.Fast path
installPackagesandrecomputeState. It rewrites the scripts and the.hooksfile and saves the new hashes, and the env is then computed from the cached print-dev-env output. It doesn't print "Ensuring packages are installed". Inside a devbox shell it still prints the "environment may be out of date" warning, as before, because the env vars or init hook may have changed.--recompute=false(shellenv,run) also does this refresh instead of callingOnStaleState, since no nix work is involved. Before, it printed the stale warning and kept the old.hooksfile.Testing
internal/devconfig/configfile:NixHashignores each non-nix field and formatting/comments. It changes forpackages,include,nixpkgsand unknown fields, and doesn't modify the config. There's also a test for the struct fallback used when there is no AST.internal/lock: table test forcompareStateHashes(each non-config field change → stale, old state file → stale), plusFile.StateStatusagainst a realstate.jsonon disk.internal/devbox: after a script, env and init_hook edit,ensureStateIsUpToDatenever calls uncachedprint-dev-env. It writes the new hooks and script files, deletes the removed script, and the computed env has the new value. ASkipRecomputetest checks the same thing for--recompute=false. Package andincludeedits are still stale.run/nonnix_state.test.txtcovers the run → edit → run → edit packages flow. I also ran the existingrun,add,rm,generate direnvandlockfiletestscripts and they pass.go testandgolangci-lint runon the changed packages: pass, 0 issues.helloandjq. After an edit, thedevbox shellenv --init-hookoutput,.devbox/gen/andstate.jsonwere byte-for-byte the same as after deletingstate.jsonand recomputing.DEVBOX_DEBUG=1the fast path logsonly non-nix inputs changedand runs no nix commands.DO_NOT_TRACK=1, averaged over 10 edits of each kind. Each row is the first command after that edit:devbox run saymain
devbox run saythis PR
devbox shellenvmain
devbox shellenvthis PR
jq)Most of the main-branch cost is
nix print-dev-env(~830ms), followed bynix path-infofor the install check (~120ms) andnix profile list(~80ms).🤖 Generated with Claude Code