diff --git a/internal/devbox/devbox.go b/internal/devbox/devbox.go index 1c2aaa3edc4..664030f3dad 100644 --- a/internal/devbox/devbox.go +++ b/internal/devbox/devbox.go @@ -208,12 +208,29 @@ func (d *Devbox) Config() *devconfig.Config { return d.cfg } +// ConfigHash hashes every input that goes into the devbox environment: the +// whole config, the packages and the plugin configs. func (d *Devbox) ConfigHash() (string, error) { h, err := d.cfg.Hash() if err != nil { return "", err } + return d.hashWithPackagesAndPlugins(h) +} +// NixConfigHash is like ConfigHash, but leaves out config fields that are only +// read after the Nix environment is built (env, init_hook, scripts, etc.). If +// it hasn't changed, the generated flake, the print-dev-env output and the +// nix profile are still valid. See configfile.ConfigFile.NixHash. +func (d *Devbox) NixConfigHash() (string, error) { + h, err := d.cfg.NixHash() + if err != nil { + return "", err + } + return d.hashWithPackagesAndPlugins(h) +} + +func (d *Devbox) hashWithPackagesAndPlugins(h string) (string, error) { buf := bytes.Buffer{} buf.WriteString(h) for _, pkg := range d.AllPackages() { @@ -888,11 +905,19 @@ func (d *Devbox) ensureStateIsUpToDateAndComputeEnv( ) (map[string]string, error) { defer debug.FunctionTimer().End() - upToDate, err := d.lockfile.IsUpToDateAndInstalled(isFishShell()) + status, err := d.lockfile.StateStatus(isFishShell()) if err != nil { return nil, err } - if !upToDate { + if envOpts.SkipRecompute && status == lock.StateNonNixStale { + // Refreshing the non-Nix state doesn't run Nix and is cheap, so do it + // even when the caller asked to skip the (expensive) recompute. + if err := d.refreshNonNixState(ctx); err != nil { + return nil, err + } + status = lock.StateUpToDate + } + if status != lock.StateUpToDate { if envOpts.Hooks.OnStaleState != nil { envOpts.Hooks.OnStaleState() } diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index 685cddd2bef..c2349e94974 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -309,10 +309,11 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er defer trace.StartRegion(ctx, "devboxEnsureStateIsUpToDate").End() defer debug.FunctionTimer().End() - upToDate, err := d.lockfile.IsUpToDateAndInstalled(isFishShell()) + status, err := d.lockfile.StateStatus(isFishShell()) if err != nil { return err } + upToDate := status == lock.StateUpToDate // if mode is install or uninstall, then we need to compute some state // like updating the flake or installing packages locally, so must continue @@ -322,6 +323,15 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er if upToDate { return nil } + // Only env, init_hook, scripts, etc. changed. The Nix environment + // is still valid, so skip installing packages and recomputing it. + if status == lock.StateNonNixStale { + if err := d.refreshNonNixState(ctx); err != nil { + return err + } + d.warnIfEnvIsStale(ctx) + return nil + } ux.Finfof(d.stderr, "Ensuring packages are installed.\n") } @@ -348,10 +358,20 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er } } + if !upToDate { + d.warnIfEnvIsStale(ctx) + } + + return d.updateLockfile(mode, recomputeState) +} + +// warnIfEnvIsStale warns that the current shell's environment is out of date. +// It should be called after the project state changed. +func (d *Devbox) warnIfEnvIsStale(ctx context.Context) { // If we're in a devbox shell (global or project), then the environment might // be out of date after the user installs something. If have direnv active // it should reload automatically so we don't need to refresh. - if d.IsEnvEnabled() && !upToDate && !d.IsDirenvActive() { + if d.IsEnvEnabled() && !d.IsDirenvActive() { ux.FHidableWarning( ctx, d.stderr, @@ -359,8 +379,43 @@ func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) er d.RefreshAliasOrCommand(), ) } +} - return d.updateLockfile(mode, recomputeState) +// refreshNonNixState brings the project state up to date when only inputs +// that don't affect the Nix environment changed (lock.StateNonNixStale). The +// generated flake, the print-dev-env cache and the nix profile are still +// valid, so this skips all Nix work. It rewrites the scripts and init hook +// files and saves the new state hashes. Env vars, env_from and aliases need no +// work here because they are read from the config every time the environment +// is computed. +func (d *Devbox) refreshNonNixState(ctx context.Context) error { + defer trace.StartRegion(ctx, "devboxRefreshNonNixState").End() + defer debug.FunctionTimer().End() + slog.Debug("only non-nix inputs changed, skipping nix environment recompute") + + if err := shellgen.WriteScriptsToFiles(d); err != nil { + return err + } + return d.saveStateHashFile() +} + +// saveStateHashFile records the current inputs in .devbox/state.json so that +// later commands know the state is up to date. +func (d *Devbox) saveStateHashFile() error { + configHash, err := d.ConfigHash() + if err != nil { + return err + } + nixConfigHash, err := d.NixConfigHash() + if err != nil { + return err + } + return lock.UpdateAndSaveStateHashFile(lock.UpdateStateHashFileArgs{ + ProjectDir: d.projectDir, + ConfigHash: configHash, + NixConfigHash: nixConfigHash, + IsFish: isFishShell(), + }) } // updateLockfile will ensure devbox.lock is up to date with the current state of the project.update @@ -394,15 +449,7 @@ func (d *Devbox) updateLockfile(mode installMode, recomputeState bool) error { // If not, we leave the local.lock in a stale state, so that state is recomputed // on the next ensureStateIsUpToDate call with mode=ensure. if recomputeState { - configHash, err := d.ConfigHash() - if err != nil { - return err - } - return lock.UpdateAndSaveStateHashFile(lock.UpdateStateHashFileArgs{ - ProjectDir: d.projectDir, - ConfigHash: configHash, - IsFish: isFishShell(), - }) + return d.saveStateHashFile() } return nil } diff --git a/internal/devbox/statehash_test.go b/internal/devbox/statehash_test.go new file mode 100644 index 00000000000..ec891dd13f5 --- /dev/null +++ b/internal/devbox/statehash_test.go @@ -0,0 +1,164 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package devbox + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.jetify.com/devbox/internal/devbox/devopt" + "go.jetify.com/devbox/internal/lock" + "go.jetify.com/devbox/internal/nix" + "go.jetify.com/devbox/internal/shellgen" +) + +// cacheOnlyNix serves print-dev-env from the cache and fails if devbox tries +// to run `nix print-dev-env`. +type cacheOnlyNix struct { + uncachedCalls int +} + +func (n *cacheOnlyNix) PrintDevEnv(_ context.Context, args *nix.PrintDevEnvArgs) (*nix.PrintDevEnvOut, error) { + if !args.UsePrintDevEnvCache { + n.uncachedCalls++ + return nil, errors.New("unexpected uncached nix print-dev-env call") + } + return &nix.PrintDevEnvOut{Variables: map[string]nix.Variable{ + "PATH": {Type: "exported", Value: "/nix/store/fake/bin"}, + }}, nil +} + +const stateTestConfig = `{ + "packages": ["github:NixOS/nixpkgs/nixpkgs-unstable#hello"], + "env": {"MY_VAR": "old"}, + "shell": { + "init_hook": ["echo old-hook"], + "scripts": {"old-script": "echo old"} + } +}` + +func openStateTestProject(t *testing.T, dir, config string) *Devbox { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "devbox.json"), []byte(config), 0o644)) + d, err := Open(&devopt.Opts{Dir: dir, Stderr: os.Stderr}) + require.NoError(t, err) + return d +} + +func stateStatus(t *testing.T, d *Devbox) lock.StateStatus { + t.Helper() + status, err := d.lockfile.StateStatus(isFishShell()) + require.NoError(t, err) + return status +} + +func TestEnsureStateNonNixChangeSkipsNix(t *testing.T) { + dir := t.TempDir() + box := openStateTestProject(t, dir, stateTestConfig) + + // Simulate a previous full recompute. + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".devbox"), 0o755)) + require.NoError(t, box.saveStateHashFile()) + require.Equal(t, lock.StateUpToDate, stateStatus(t, box)) + + // Edit env, the init hook and the scripts. + box = openStateTestProject(t, dir, `{ + "packages": ["github:NixOS/nixpkgs/nixpkgs-unstable#hello"], + "env": {"MY_VAR": "new"}, + "shell": { + "init_hook": ["echo new-hook"], + "scripts": {"new-script": "echo new"} + } +}`) + fakeNix := &cacheOnlyNix{} + box.nix = fakeNix + require.Equal(t, lock.StateNonNixStale, stateStatus(t, box)) + + ctx := t.Context() + require.NoError(t, box.ensureStateIsUpToDate(ctx, ensure)) + assert.Equal(t, 0, fakeNix.uncachedCalls, "ensureStateIsUpToDate should not run nix") + assert.Equal(t, lock.StateUpToDate, stateStatus(t, box)) + + hooks, err := os.ReadFile(shellgen.ScriptPath(dir, shellgen.HooksFilename)) + require.NoError(t, err) + assert.Contains(t, string(hooks), "new-hook") + assert.FileExists(t, shellgen.ScriptPath(dir, "new-script")) + assert.NoFileExists(t, shellgen.ScriptPath(dir, "old-script")) + + env, err := box.computeEnv(ctx, true /*usePrintDevEnvCache*/, devopt.EnvOptions{}) + require.NoError(t, err) + assert.Equal(t, "new", env["MY_VAR"]) + assert.Equal(t, 0, fakeNix.uncachedCalls) +} + +func TestSkipRecomputeRefreshesNonNixState(t *testing.T) { + dir := t.TempDir() + box := openStateTestProject(t, dir, stateTestConfig) + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".devbox"), 0o755)) + require.NoError(t, box.saveStateHashFile()) + + box = openStateTestProject(t, dir, `{ + "packages": ["github:NixOS/nixpkgs/nixpkgs-unstable#hello"], + "env": {"MY_VAR": "new"}, + "shell": {"init_hook": ["echo new-hook"]} +}`) + fakeNix := &cacheOnlyNix{} + box.nix = fakeNix + + staleCalls := 0 + env, err := box.ensureStateIsUpToDateAndComputeEnv(t.Context(), devopt.EnvOptions{ + SkipRecompute: true, + Hooks: devopt.LifecycleHooks{OnStaleState: func() { staleCalls++ }}, + }) + require.NoError(t, err) + assert.Equal(t, 0, staleCalls, "non-nix changes should be refreshed, not reported as stale") + assert.Equal(t, 0, fakeNix.uncachedCalls) + assert.Equal(t, "new", env["MY_VAR"]) + assert.Equal(t, lock.StateUpToDate, stateStatus(t, box)) + + hooks, err := os.ReadFile(shellgen.ScriptPath(dir, shellgen.HooksFilename)) + require.NoError(t, err) + assert.Contains(t, string(hooks), "new-hook") +} + +func TestStateStatusNixChanges(t *testing.T) { + tests := []struct { + name string + config string + }{ + {"packages", `{ + "packages": ["github:NixOS/nixpkgs/nixpkgs-unstable#cowsay"], + "env": {"MY_VAR": "old"}, + "shell": {"init_hook": ["echo old-hook"], "scripts": {"old-script": "echo old"}} +}`}, + {"packages and env", `{ + "packages": ["github:NixOS/nixpkgs/nixpkgs-unstable#hello", "github:NixOS/nixpkgs/nixpkgs-unstable#cowsay"], + "env": {"MY_VAR": "new"}, + "shell": {"init_hook": ["echo old-hook"], "scripts": {"old-script": "echo old"}} +}`}, + {"include", `{ + "packages": ["github:NixOS/nixpkgs/nixpkgs-unstable#hello"], + "env": {"MY_VAR": "old"}, + "shell": {"init_hook": ["echo old-hook"], "scripts": {"old-script": "echo old"}}, + "include": ["plugin:nginx"] +}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + box := openStateTestProject(t, dir, stateTestConfig) + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".devbox"), 0o755)) + require.NoError(t, box.saveStateHashFile()) + + box = openStateTestProject(t, dir, tt.config) + assert.Equal(t, lock.StateStale, stateStatus(t, box)) + }) + } +} diff --git a/internal/devconfig/config.go b/internal/devconfig/config.go index c60b1b8cd00..d1771af646e 100644 --- a/internal/devconfig/config.go +++ b/internal/devconfig/config.go @@ -423,6 +423,26 @@ func (c *Config) Hash() (string, error) { return cachehash.Bytes(data), nil } +// NixHash is like Hash but ignores the root config's fields that don't affect +// the Nix environment (see configfile.ConfigFile.NixHash). Included configs +// (plugins) are hashed in full because they can add packages and create files. +func (c *Config) NixHash() (string, error) { + data := []byte{} + for _, i := range c.included { + hash, err := i.Hash() + if err != nil { + return "", err + } + data = append(data, hash...) + } + hash, err := c.Root.NixHash() + if err != nil { + return "", err + } + data = append(data, hash...) + return cachehash.Bytes(data), nil +} + func (c *Config) IsJetifyCloudEnvFrom() bool { for _, i := range c.included { if i.IsJetifyCloudEnvFrom() { diff --git a/internal/devconfig/config_test.go b/internal/devconfig/config_test.go index 0b36538821f..d9b30d776a4 100644 --- a/internal/devconfig/config_test.go +++ b/internal/devconfig/config_test.go @@ -713,7 +713,8 @@ type testLockProject struct { dir string } -func (p *testLockProject) ConfigHash() (string, error) { return "", nil } +func (p *testLockProject) ConfigHash() (string, error) { return "", nil } +func (p *testLockProject) NixConfigHash() (string, error) { return "", nil } func (p *testLockProject) Stdenv() flake.Ref { return flake.Ref{} } func (p *testLockProject) AllPackageNamesIncludingRemovedTriggerPackages() []string { return nil } diff --git a/internal/devconfig/configfile/file.go b/internal/devconfig/configfile/file.go index 5d1514a085f..baae77e1487 100644 --- a/internal/devconfig/configfile/file.go +++ b/internal/devconfig/configfile/file.go @@ -109,6 +109,58 @@ func (c *ConfigFile) Hash() (string, error) { return cachehash.Bytes(ast.Pack()), nil } +// nonNixFields are the top-level devbox.json fields that devbox only reads +// after the Nix environment has been built. None of them end up in the +// generated flake, the `nix print-dev-env` output, or the Nix profile: +// +// - env, env_from: merged into the environment by Devbox.computeEnv on +// every run. +// - shell (init_hook and scripts): written to .devbox/gen/scripts by +// shellgen.WriteScriptsToFiles. +// - aliases: read when starting a shell or running a command. +// - name, description, $schema: metadata. +// - devbox_version: read by vercheck. The state file already records the +// running Devbox version separately. +// +// Any field not listed here (including unknown fields) is treated as a Nix +// input. +var nonNixFields = []string{ + "$schema", + "aliases", + "description", + "devbox_version", + "env", + "env_from", + "name", + "shell", +} + +// NixHash is like Hash, but it ignores fields that don't affect the Nix +// environment (see nonNixFields). Two configs with the same NixHash produce +// the same flake, print-dev-env output and Nix profile. +func (c *ConfigFile) NixHash() (string, error) { + if c.ast == nil { + nixOnly := *c + nixOnly.Name = "" + nixOnly.Description = "" + nixOnly.DevboxVersion = nil + nixOnly.Env = nil + nixOnly.EnvFrom = "" + nixOnly.Shell = nil + nixOnly.Aliases = nil + return cachehash.JSON(&nixOnly) + } + ast := c.ast.root.Clone() + if obj, ok := ast.Value.(*hujson.Object); ok { + obj.Members = slices.DeleteFunc(obj.Members, func(m hujson.ObjectMember) bool { + name, ok := m.Name.Value.(hujson.Literal) + return ok && slices.Contains(nonNixFields, name.String()) + }) + } + ast.Minimize() + return cachehash.Bytes(ast.Pack()), nil +} + func (c *ConfigFile) Equals(other *ConfigFile) bool { hash1, _ := c.Hash() hash2, _ := other.Hash() diff --git a/internal/devconfig/configfile/hash_test.go b/internal/devconfig/configfile/hash_test.go new file mode 100644 index 00000000000..8bfda64c80d --- /dev/null +++ b/internal/devconfig/configfile/hash_test.go @@ -0,0 +1,162 @@ +package configfile + +import ( + "testing" +) + +const hashTestBase = `{ + "$schema": "https://example.com/schema.json", + "name": "my-project", + "description": "a project", + "devbox_version": ">=0.14.0", + "packages": ["go@1.22", "hello@latest"], + "env": {"FOO": "bar"}, + "env_from": ".env", + "shell": { + "init_hook": ["echo hello"], + "scripts": {"test": "go test ./..."} + }, + "aliases": {"ll": "ls -l"}, + "include": ["plugin:nginx"], + "nixpkgs": {"commit": "0000000000000000000000000000000000000000"} +}` + +func mustLoad(t *testing.T, s string) *ConfigFile { + t.Helper() + cfg, err := LoadBytes([]byte(s)) + if err != nil { + t.Fatalf("LoadBytes: %v", err) + } + return cfg +} + +func mustHashes(t *testing.T, cfg *ConfigFile) (hash, nixHash string) { + t.Helper() + hash, err := cfg.Hash() + if err != nil { + t.Fatalf("Hash: %v", err) + } + nixHash, err = cfg.NixHash() + if err != nil { + t.Fatalf("NixHash: %v", err) + } + return hash, nixHash +} + +func TestNixHash(t *testing.T) { + baseHash, baseNixHash := mustHashes(t, mustLoad(t, hashTestBase)) + + tests := []struct { + name string + config string + nixChanged bool + }{ + { + name: "env", + config: `{"$schema": "https://example.com/schema.json", "name": "my-project", "description": "a project", "devbox_version": ">=0.14.0", + "packages": ["go@1.22", "hello@latest"], "env": {"FOO": "baz", "NEW": "1"}, "env_from": ".env", + "shell": {"init_hook": ["echo hello"], "scripts": {"test": "go test ./..."}}, "aliases": {"ll": "ls -l"}, + "include": ["plugin:nginx"], "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}}`, + }, + { + name: "init_hook, scripts and env_from", + config: `{"$schema": "https://example.com/schema.json", "name": "my-project", "description": "a project", "devbox_version": ">=0.14.0", + "packages": ["go@1.22", "hello@latest"], "env": {"FOO": "bar"}, "env_from": "other.env", + "shell": {"init_hook": ["echo bye"], "scripts": {"test": "go test -v ./...", "new": "true"}}, "aliases": {"ll": "ls -l"}, + "include": ["plugin:nginx"], "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}}`, + }, + { + name: "metadata and aliases removed", + config: `{"packages": ["go@1.22", "hello@latest"], "env": {"FOO": "bar"}, "env_from": ".env", + "shell": {"init_hook": ["echo hello"], "scripts": {"test": "go test ./..."}}, + "include": ["plugin:nginx"], "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}}`, + }, + { + name: "packages", + config: `{"$schema": "https://example.com/schema.json", "name": "my-project", "description": "a project", "devbox_version": ">=0.14.0", + "packages": ["go@1.23", "hello@latest"], "env": {"FOO": "bar"}, "env_from": ".env", + "shell": {"init_hook": ["echo hello"], "scripts": {"test": "go test ./..."}}, "aliases": {"ll": "ls -l"}, + "include": ["plugin:nginx"], "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}}`, + nixChanged: true, + }, + { + name: "include", + config: `{"$schema": "https://example.com/schema.json", "name": "my-project", "description": "a project", "devbox_version": ">=0.14.0", + "packages": ["go@1.22", "hello@latest"], "env": {"FOO": "bar"}, "env_from": ".env", + "shell": {"init_hook": ["echo hello"], "scripts": {"test": "go test ./..."}}, "aliases": {"ll": "ls -l"}, + "include": [], "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}}`, + nixChanged: true, + }, + { + name: "nixpkgs commit", + config: `{"$schema": "https://example.com/schema.json", "name": "my-project", "description": "a project", "devbox_version": ">=0.14.0", + "packages": ["go@1.22", "hello@latest"], "env": {"FOO": "bar"}, "env_from": ".env", + "shell": {"init_hook": ["echo hello"], "scripts": {"test": "go test ./..."}}, "aliases": {"ll": "ls -l"}, + "include": ["plugin:nginx"], "nixpkgs": {"commit": "1111111111111111111111111111111111111111"}}`, + nixChanged: true, + }, + { + name: "unknown field is a nix input", + config: `{"$schema": "https://example.com/schema.json", "name": "my-project", "description": "a project", "devbox_version": ">=0.14.0", + "packages": ["go@1.22", "hello@latest"], "env": {"FOO": "bar"}, "env_from": ".env", + "shell": {"init_hook": ["echo hello"], "scripts": {"test": "go test ./..."}}, "aliases": {"ll": "ls -l"}, + "include": ["plugin:nginx"], "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}, + "some_future_field": true}`, + nixChanged: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hash, nixHash := mustHashes(t, mustLoad(t, tt.config)) + if hash == baseHash { + t.Errorf("Hash didn't change") + } + if gotChanged := nixHash != baseNixHash; gotChanged != tt.nixChanged { + t.Errorf("NixHash changed = %v, want %v", gotChanged, tt.nixChanged) + } + }) + } +} + +func TestNixHashIgnoresFormatting(t *testing.T) { + _, want := mustHashes(t, mustLoad(t, hashTestBase)) + _, got := mustHashes(t, mustLoad(t, `{ + // A comment. + "packages": [ + "go@1.22", + "hello@latest", + ], + "include": ["plugin:nginx"], + "nixpkgs": {"commit": "0000000000000000000000000000000000000000"}, + "env": {"FOO": "something else"} +}`)) + if got != want { + t.Errorf("NixHash changed after reformatting and removing non-nix fields") + } +} + +func TestNixHashDoesNotModifyConfig(t *testing.T) { + cfg := mustLoad(t, hashTestBase) + before := string(cfg.Bytes()) + if _, err := cfg.NixHash(); err != nil { + t.Fatal(err) + } + if after := string(cfg.Bytes()); after != before { + t.Errorf("NixHash modified the config:\n%s", after) + } +} + +func TestNixHashWithoutAST(t *testing.T) { + cfg := &ConfigFile{Env: map[string]string{"FOO": "bar"}} + _, nixHash := mustHashes(t, cfg) + + cfg.Env["FOO"] = "baz" + cfg.Shell = &shellConfig{} + if _, got := mustHashes(t, cfg); got != nixHash { + t.Errorf("NixHash changed after editing env and shell") + } + cfg.Nixpkgs = &NixpkgsConfig{Commit: "1111111111111111111111111111111111111111"} + if _, got := mustHashes(t, cfg); got == nixHash { + t.Errorf("NixHash didn't change after editing nixpkgs") + } +} diff --git a/internal/lock/interfaces.go b/internal/lock/interfaces.go index 96304cbabfd..b981470f869 100644 --- a/internal/lock/interfaces.go +++ b/internal/lock/interfaces.go @@ -7,6 +7,7 @@ import "go.jetify.com/devbox/nix/flake" type devboxProject interface { ConfigHash() (string, error) + NixConfigHash() (string, error) Stdenv() flake.Ref AllPackageNamesIncludingRemovedTriggerPackages() []string ProjectDir() string diff --git a/internal/lock/lockfile.go b/internal/lock/lockfile.go index 7e0e2e08014..caafc9e86d6 100644 --- a/internal/lock/lockfile.go +++ b/internal/lock/lockfile.go @@ -223,19 +223,32 @@ func (f *File) Tidy() { // local hashes match, which generally indicates all packages are correctly // installed and print-dev-env has been computed and cached. func (f *File) IsUpToDateAndInstalled(isFish bool) (bool, error) { + status, err := f.StateStatus(isFish) + return status == StateUpToDate, err +} + +// StateStatus is like IsUpToDateAndInstalled, but also reports whether only +// non-Nix inputs (env, init_hook, scripts, etc.) changed, in which case the +// Nix environment doesn't need to be recomputed. +func (f *File) StateStatus(isFish bool) (StateStatus, error) { if dirty, err := f.isDirty(); err != nil { - return false, err + return StateStale, err } else if dirty { - return false, nil + return StateStale, nil } configHash, err := f.devboxProject.ConfigHash() if err != nil { - return false, err + return StateStale, err + } + nixConfigHash, err := f.devboxProject.NixConfigHash() + if err != nil { + return StateStale, err } - return isStateUpToDate(UpdateStateHashFileArgs{ - ProjectDir: f.devboxProject.ProjectDir(), - ConfigHash: configHash, - IsFish: isFish, + return stateStatus(UpdateStateHashFileArgs{ + ProjectDir: f.devboxProject.ProjectDir(), + ConfigHash: configHash, + NixConfigHash: nixConfigHash, + IsFish: isFish, }) } diff --git a/internal/lock/statehash.go b/internal/lock/statehash.go index 28d33532979..ec4421b7256 100644 --- a/internal/lock/statehash.go +++ b/internal/lock/statehash.go @@ -5,6 +5,7 @@ package lock import ( "errors" + "fmt" "io/fs" "path/filepath" @@ -21,7 +22,13 @@ var ignoreShellMismatch = false // When we do implement a shared lock file, it may contain some shared fields // with this one but not all. type stateHashFile struct { - ConfigHash string `json:"config_hash"` + // ConfigHash covers every input of the devbox environment. + ConfigHash string `json:"config_hash"` + // NixConfigHash covers only the inputs that affect the Nix environment + // (the generated flake, the print-dev-env output and the nix profile). + // When only ConfigHash changed, the Nix work can be skipped. It's empty + // in state files written by older versions of Devbox. + NixConfigHash string `json:"nix_config_hash"` DevboxVersion string `json:"devbox_version"` // fish has different generated scripts so we need to recompute them if user // changes shell. @@ -32,8 +39,9 @@ type stateHashFile struct { } type UpdateStateHashFileArgs struct { - ProjectDir string - ConfigHash string + ProjectDir string + ConfigHash string + NixConfigHash string // IsFish is an arg because in the future we may allow the user // to specify shell in devbox.json which should be passed in here. IsFish bool @@ -54,21 +62,70 @@ func SetIgnoreShellMismatch(ignore bool) { ignoreShellMismatch = ignore } -func isStateUpToDate(args UpdateStateHashFileArgs) (bool, error) { +// StateStatus describes how the local devbox state (.devbox/state.json) +// compares to the current project. +type StateStatus int + +const ( + // StateUpToDate means nothing changed since the state was last saved. + StateUpToDate StateStatus = iota + // StateNonNixStale means only inputs that don't affect the Nix + // environment changed (e.g. env, init_hook or scripts in devbox.json). + // The generated flake, the cached print-dev-env output and the nix + // profile are still valid, so only the non-Nix outputs need regenerating. + StateNonNixStale + // StateStale means the Nix environment needs to be recomputed. + StateStale +) + +func (s StateStatus) String() string { + switch s { + case StateUpToDate: + return "up-to-date" + case StateNonNixStale: + return "non-nix-stale" + case StateStale: + return "stale" + default: + return fmt.Sprintf("StateStatus(%d)", int(s)) + } +} + +func stateStatus(args UpdateStateHashFileArgs) (StateStatus, error) { filesystemStateHash, err := readStateHashFile(args.ProjectDir) if err != nil { - return false, err + return StateStale, err } newStateHash, err := getCurrentStateHash(args) if err != nil { - return false, err + return StateStale, err } if ignoreShellMismatch { filesystemStateHash.IsFish = newStateHash.IsFish } - return *filesystemStateHash == *newStateHash, nil + return compareStateHashes(filesystemStateHash, newStateHash), nil +} + +// compareStateHashes compares the saved state with the current one. +func compareStateHashes(saved, current *stateHashFile) StateStatus { + if *saved == *current { + return StateUpToDate + } + // Everything except ConfigHash must match. A different lockfile, + // print-dev-env cache, nix profile, Devbox version or shell all need a + // full recompute. An empty NixConfigHash means the state was written by + // an older Devbox that didn't record it. + if saved.NixConfigHash == "" { + return StateStale + } + savedWithCurrentConfig := *saved + savedWithCurrentConfig.ConfigHash = current.ConfigHash + if savedWithCurrentConfig == *current { + return StateNonNixStale + } + return StateStale } func readStateHashFile(projectDir string) (*stateHashFile, error) { @@ -101,6 +158,7 @@ func getCurrentStateHash(args UpdateStateHashFileArgs) (*stateHashFile, error) { newLock := &stateHashFile{ ConfigHash: args.ConfigHash, + NixConfigHash: args.NixConfigHash, DevboxVersion: build.Version, IsFish: args.IsFish, LockFileHash: lockfileHash, diff --git a/internal/lock/statehash_test.go b/internal/lock/statehash_test.go new file mode 100644 index 00000000000..2fd6f8e93d9 --- /dev/null +++ b/internal/lock/statehash_test.go @@ -0,0 +1,192 @@ +package lock + +import ( + "os" + "path/filepath" + "testing" + + "go.jetify.com/devbox/nix/flake" +) + +func TestCompareStateHashes(t *testing.T) { + saved := stateHashFile{ + ConfigHash: "config", + NixConfigHash: "nix", + DevboxVersion: "0.1.0", + LockFileHash: "lock", + NixPrintDevEnvHash: "printdevenv", + NixProfileManifestHash: "manifest", + } + + tests := []struct { + name string + modify func(s *stateHashFile) + want StateStatus + }{ + {"unchanged", func(*stateHashFile) {}, StateUpToDate}, + {"config", func(s *stateHashFile) { s.ConfigHash = "config2" }, StateNonNixStale}, + {"config and nix config", func(s *stateHashFile) { + s.ConfigHash = "config2" + s.NixConfigHash = "nix2" + }, StateStale}, + // Can't happen in practice, but if it does the nix inputs changed. + {"nix config only", func(s *stateHashFile) { s.NixConfigHash = "nix2" }, StateStale}, + {"config and lockfile", func(s *stateHashFile) { + s.ConfigHash = "config2" + s.LockFileHash = "lock2" + }, StateStale}, + {"config and devbox version", func(s *stateHashFile) { + s.ConfigHash = "config2" + s.DevboxVersion = "0.2.0" + }, StateStale}, + {"config and print-dev-env cache", func(s *stateHashFile) { + s.ConfigHash = "config2" + s.NixPrintDevEnvHash = "printdevenv2" + }, StateStale}, + {"config and profile", func(s *stateHashFile) { + s.ConfigHash = "config2" + s.NixProfileManifestHash = "manifest2" + }, StateStale}, + {"config and shell", func(s *stateHashFile) { + s.ConfigHash = "config2" + s.IsFish = true + }, StateStale}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + current := saved + tt.modify(¤t) + if got := compareStateHashes(&saved, ¤t); got != tt.want { + t.Errorf("compareStateHashes() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestCompareStateHashesOldStateFile(t *testing.T) { + // State files written by older versions of Devbox don't have a + // NixConfigHash. They should trigger one full recompute. + saved := stateHashFile{ConfigHash: "config"} + current := stateHashFile{ConfigHash: "config2", NixConfigHash: "nix"} + if got := compareStateHashes(&saved, ¤t); got != StateStale { + t.Errorf("compareStateHashes() = %v, want %v", got, StateStale) + } + current.ConfigHash = "config" + if got := compareStateHashes(&saved, ¤t); got != StateStale { + t.Errorf("compareStateHashes() = %v, want %v", got, StateStale) + } +} + +type stateTestProject struct { + dir string + configHash string + nixConfigHash string +} + +func (p *stateTestProject) ConfigHash() (string, error) { return p.configHash, nil } +func (p *stateTestProject) NixConfigHash() (string, error) { return p.nixConfigHash, nil } +func (p *stateTestProject) Stdenv() flake.Ref { return flake.Ref{} } +func (p *stateTestProject) ProjectDir() string { return p.dir } +func (p *stateTestProject) AllPackageNamesIncludingRemovedTriggerPackages() []string { + return nil +} + +func TestFileStateStatus(t *testing.T) { + project := &stateTestProject{ + dir: t.TempDir(), + configHash: "config", + nixConfigHash: "nix", + } + if err := os.MkdirAll(filepath.Join(project.dir, ".devbox"), 0o755); err != nil { + t.Fatal(err) + } + lockfile, err := GetFile(project) + if err != nil { + t.Fatal(err) + } + status := func() StateStatus { + t.Helper() + got, err := lockfile.StateStatus(false) + if err != nil { + t.Fatal(err) + } + return got + } + save := func() { + t.Helper() + err := UpdateAndSaveStateHashFile(UpdateStateHashFileArgs{ + ProjectDir: project.dir, + ConfigHash: project.configHash, + NixConfigHash: project.nixConfigHash, + }) + if err != nil { + t.Fatal(err) + } + } + + if got := status(); got != StateStale { + t.Errorf("got %v with no state file, want %v", got, StateStale) + } + + save() + if got := status(); got != StateUpToDate { + t.Errorf("got %v after saving, want %v", got, StateUpToDate) + } + + project.configHash = "config2" + if got := status(); got != StateNonNixStale { + t.Errorf("got %v after changing non-nix inputs, want %v", got, StateNonNixStale) + } + if upToDate, err := lockfile.IsUpToDateAndInstalled(false); err != nil || upToDate { + t.Errorf("IsUpToDateAndInstalled() = %v, %v, want false, nil", upToDate, err) + } + + save() + project.configHash = "config3" + project.nixConfigHash = "nix3" + if got := status(); got != StateStale { + t.Errorf("got %v after changing nix inputs, want %v", got, StateStale) + } + + // A changed print-dev-env cache means the nix environment changed outside + // of devbox's control, so recompute everything. + save() + project.configHash = "config4" + cache := filepath.Join(project.dir, ".devbox", ".nix-print-dev-env-cache") + if err := os.WriteFile(cache, []byte(`{"variables": {}}`), 0o644); err != nil { + t.Fatal(err) + } + if got := status(); got != StateStale { + t.Errorf("got %v after changing print-dev-env cache, want %v", got, StateStale) + } +} + +func TestFileStateStatusOldStateFile(t *testing.T) { + project := &stateTestProject{ + dir: t.TempDir(), + configHash: "config", + nixConfigHash: "nix", + } + if err := os.MkdirAll(filepath.Join(project.dir, ".devbox"), 0o755); err != nil { + t.Fatal(err) + } + lockfile, err := GetFile(project) + if err != nil { + t.Fatal(err) + } + // Written by a Devbox version that didn't know about nix_config_hash. + err = UpdateAndSaveStateHashFile(UpdateStateHashFileArgs{ + ProjectDir: project.dir, + ConfigHash: "old-config", + }) + if err != nil { + t.Fatal(err) + } + got, err := lockfile.StateStatus(false) + if err != nil { + t.Fatal(err) + } + if got != StateStale { + t.Errorf("got %v, want %v", got, StateStale) + } +} diff --git a/internal/plugin/plugin_test.go b/internal/plugin/plugin_test.go index f2b5e559944..99e757800a7 100644 --- a/internal/plugin/plugin_test.go +++ b/internal/plugin/plugin_test.go @@ -131,9 +131,10 @@ func TestBuildConfigTemplatePlaceholders(t *testing.T) { // lockProjectForTest is a minimal lock.File project rooted at dir. type lockProjectForTest struct{ dir string } -func (p lockProjectForTest) ConfigHash() (string, error) { return "", nil } -func (p lockProjectForTest) Stdenv() flake.Ref { return flake.Ref{} } -func (p lockProjectForTest) ProjectDir() string { return p.dir } +func (p lockProjectForTest) ConfigHash() (string, error) { return "", nil } +func (p lockProjectForTest) NixConfigHash() (string, error) { return "", nil } +func (p lockProjectForTest) Stdenv() flake.Ref { return flake.Ref{} } +func (p lockProjectForTest) ProjectDir() string { return p.dir } func (p lockProjectForTest) AllPackageNamesIncludingRemovedTriggerPackages() []string { return nil } diff --git a/specs/perf-improvements.md b/specs/perf-improvements.md index b55a796d56c..e41ded91189 100644 --- a/specs/perf-improvements.md +++ b/specs/perf-improvements.md @@ -150,7 +150,7 @@ decision isn't remembered between invocations. Fix: ## Tier 2: after any `devbox.json` edit -### 8. Split the state hash into nix-relevant and other inputs +### 8. Split the state hash into nix-relevant and other inputs (done in #3031) **Win:** ~1s or more on every config edit that doesn't touch packages (biggest win for editing workflows). diff --git a/testscripts/run/nonnix_state.test.txt b/testscripts/run/nonnix_state.test.txt new file mode 100644 index 00000000000..b0df4336bce --- /dev/null +++ b/testscripts/run/nonnix_state.test.txt @@ -0,0 +1,57 @@ +# Editing env, init_hook or scripts in devbox.json takes effect without +# recomputing the Nix environment. Editing packages still recomputes it. + +exec devbox run say +stdout 'hook=old var=old script=old' +stderr 'Ensuring packages are installed' + +# Edit env, the init hook and the script. +cp devbox.new.json devbox.json +exec devbox run say +stdout 'hook=new var=new script=new' +! stderr 'Ensuring packages are installed' +! stderr 'Computed the Devbox environment' + +exec devbox shellenv --init-hook +stdout 'MY_VAR="new"' + +# Up to date: nothing to do. +exec devbox run say +stdout 'hook=new var=new script=new' +! stderr 'Ensuring packages are installed' + +# Editing packages recomputes the Nix environment. +cp devbox.packages.json devbox.json +exec devbox run say +stdout 'hook=new var=new script=new' +stderr 'Ensuring packages are installed' + +-- devbox.json -- +{ + "packages": [], + "env": {"MY_VAR": "old"}, + "shell": { + "init_hook": ["export HOOK=old"], + "scripts": {"say": "echo hook=$HOOK var=$MY_VAR script=old"} + } +} + +-- devbox.new.json -- +{ + "packages": [], + "env": {"MY_VAR": "new"}, + "shell": { + "init_hook": ["export HOOK=new"], + "scripts": {"say": "echo hook=$HOOK var=$MY_VAR script=new"} + } +} + +-- devbox.packages.json -- +{ + "packages": ["hello@latest"], + "env": {"MY_VAR": "new"}, + "shell": { + "init_hook": ["export HOOK=new"], + "scripts": {"say": "echo hook=$HOOK var=$MY_VAR script=new"} + } +}