[lockfile] add lockfile.Tidy and call from ensurePackagesAreInstalled

internal/impl/packages.go

@@ -199,6 +199,9 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 		return err
 	}
 
+	// Ensure we clean out packages that are no longer needed.
+	d.lockfile.Tidy()
+
 	if err = d.lockfile.Save(); err != nil {
 		return err
 	}

internal/lock/interfaces.go

@@ -6,6 +6,7 @@ package lock
 type devboxProject interface {
 	ConfigHash() (string, error)
 	NixPkgsCommitHash() string
+	Packages() []string
 	ProjectDir() string
 }
 
@@ -14,7 +15,7 @@ type resolver interface {
 }
 
 type Locker interface {
-	devboxProject
 	resolver
 	LegacyNixpkgsPath(string) string
+	ProjectDir() string
 }

internal/lock/lockfile.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/samber/lo"
 	"go.jetpack.io/devbox/internal/boxcli/featureflag"
 	"go.jetpack.io/devbox/internal/cuecfg"
 )
@@ -99,7 +100,7 @@ func (l *File) Save() error {
 		return nil
 	}
 
-	return cuecfg.WriteFile(lockFilePath(l), l)
+	return cuecfg.WriteFile(lockFilePath(l.devboxProject), l)
 }
 
 func (l *File) LegacyNixpkgsPath(pkg string) string {
@@ -121,12 +122,18 @@ func IsLegacyPackage(pkg string) bool {
 	return !IsVersionedPackage(pkg) &&
 		!strings.Contains(pkg, ":") &&
 		// We don't support absolute paths without "path:" prefix, but adding here
-		// just inc ase we ever do.
+		// just in case we ever do.
 		// Landau note: I don't think we should support it, it's hard to read and a
 		// bit ambiguous.
 		!strings.HasPrefix(pkg, "/")
 }
 
+// Tidy ensures that the lockfile has the set of packages corresponding to the devbox.json config.
+// It gets rid of older packages that are no longer needed.
+func (l *File) Tidy() {
+	l.Packages = lo.PickByKeys(l.Packages, l.devboxProject.Packages())
+}
+
 func lockFilePath(project devboxProject) string {
 	return filepath.Join(project.ProjectDir(), "devbox.lock")
 }

internal/nix/input_test.go

@@ -106,14 +106,6 @@ type lockfile struct {
 	projectDir string
 }
 
-func (lockfile) ConfigHash() (string, error) {
-	return "", nil
-}
-
-func (lockfile) NixPkgsCommitHash() string {
-	return ""
-}
-
 func (l *lockfile) ProjectDir() string {
 	return l.projectDir
 }

testscripts/lockfile/lockfile_tidy.test.txt

@@ -0,0 +1,28 @@
+# Testscript to ensure lockfile is updated to remove the older version of a package
+
+# start with a devbox.json having [email protected]
+cp devbox_original.json devbox.json
+exec devbox install
+devboxlock.packages.contains devbox.lock [email protected]
+
+# change devbox.json to instead have [email protected]
+cp devbox_changed.json devbox.json
+exec devbox install
+devboxlock.packages.contains devbox.lock [email protected]
+! devboxlock.packages.contains devbox.lock [email protected]
+
+
+-- devbox_original.json --
+ {
+   "packages": [
+     "[email protected]"
+   ]
+ }
+
+-- devbox_changed.json --
+{
+  "packages": [
+    "[email protected]"
+  ]
+}
+

testscripts/testrunner/assert.go

@@ -10,6 +10,7 @@ import (
 
 	"go.jetpack.io/devbox/internal/devconfig"
 	"go.jetpack.io/devbox/internal/envir"
+	"go.jetpack.io/devbox/internal/lock"
 )
 
 // Usage: env.path.len <number>
@@ -36,7 +37,7 @@ func assertPathLength(script *testscript.TestScript, neg bool, args []string) {
 
 func assertDevboxJSONPackagesContains(script *testscript.TestScript, neg bool, args []string) {
 	if len(args) != 2 {
-		script.Fatalf("usage: json.list.contains list.json value")
+		script.Fatalf("usage: devboxjson.packages.contains devbox.json value")
 	}
 
 	data := script.ReadFile(args[0])
@@ -59,6 +60,28 @@ func assertDevboxJSONPackagesContains(script *testscript.TestScript, neg bool, a
 	}
 }
 
+func assertDevboxLockPackagesContains(script *testscript.TestScript, neg bool, args []string) {
+	if len(args) != 2 {
+		script.Fatalf("usage: devboxlock.packages.contains devbox.lock value")
+	}
+
+	data := script.ReadFile(args[0])
+	lockfile := lock.File{}
+	err := json.Unmarshal([]byte(data), &lockfile)
+	script.Check(err)
+
+	expected := args[1]
+	if _, ok := lockfile.Packages[expected]; ok {
+		if neg {
+			script.Fatalf("value '%s' found in %s", expected, args[0])
+		}
+	} else {
+		if !neg {
+			script.Fatalf("value '%s' not found in '%s'", expected, args[0])
+		}
+	}
+}
+
 // Usage: json.superset superset.json subset.json
 // Checks that the JSON in superset.json contains all the keys and values
 // present in subset.json.

testscripts/testrunner/testrunner.go

@@ -67,15 +67,29 @@ func globDirs(pattern string) []string {
 	return directories
 }
 
+// copyFileCmd enables copying files within the WORKDIR
+func copyFileCmd(script *testscript.TestScript, neg bool, args []string) {
+	if len(args) < 2 {
+		script.Fatalf("usage: cp <from-file> <to-file>")
+	}
+	if neg {
+		script.Fatalf("neg does not make sense for this command")
+	}
+	err := script.Exec("cp", script.MkAbs(args[0]), script.MkAbs(args[1]))
+	script.Check(err)
+}
+
 func getTestscriptParams(t *testing.T, dir string) testscript.Params {
 	return testscript.Params{
 		Dir:                 dir,
 		RequireExplicitExec: true,
 		TestWork:            false, // Set to true if you're trying to debug a test.
 		Setup:               func(env *testscript.Env) error { return setupTestEnv(t, env) },
 		Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
-			"env.path.len":                 assertPathLength,
+			"cp":                           copyFileCmd,
 			"devboxjson.packages.contains": assertDevboxJSONPackagesContains,
+			"devboxlock.packages.contains": assertDevboxLockPackagesContains,
+			"env.path.len":                 assertPathLength,
 			"json.superset":                assertJSONSuperset,
 			"path.order":                   assertPathOrder,
 			"source.path":                  sourcePath,