[lockfile] add lockfile.Tidy and call from ensurePackagesAreInstalled (#1109)

## Summary

Problem:
if I change devbox.json from [email protected] to [email protected], and do devbox install,
I still see the [email protected] in the devbox.lock (in addition to the new
[email protected])


This PR ensures that when we update installed packages, we also update
the lockfile
to have the set of packages that match those of the devbox.json config.

implementation note: I needed to rename lockfile.Packages to a different
name
since devboxProject is embedded in the lockfile.File struct and we have
a name
conflict.

Fixes #1095

## How was it tested?

- [x] will add a testscript unit test that matches the steps below:


```
# initial devbox.lock file
(devbox) savil@Savil-Srivastavas-MacBook-Pro ~/c/j/devbox (savil/update-lockfile-v2)> cat devbox.lock
{
  "lockfile_version": "1",
  "packages": {
    "[email protected]": {
      "last_modified": "2023-03-31T22:52:29Z",
      "resolved": "github:NixOS/nixpkgs/242246ee1e58f54d2322227fc5eef53b4a616a31#actionlint",
      "version": "1.6.23"
    },
    "[email protected]": {
      "last_modified": "2023-05-01T16:53:22Z",
      "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#go",
      "version": "1.20.3"
    },
    "[email protected]": {
      "last_modified": "2023-05-01T16:53:22Z",
      "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#golangci-lint",
      "version": "1.52.2"
    }
  }
}

# edit the [email protected] to be [email protected]⏎
(devbox) savil@Savil-Srivastavas-MacBook-Pro ~/c/j/devbox (savil/update-lockfile-v2)> vim devbox.json
(devbox) savil@Savil-Srivastavas-MacBook-Pro ~/c/j/devbox (savil/update-lockfile-v2)> devbox install
Ensuring packages are installed.
Finished installing packages.


(devbox) savil@Savil-Srivastavas-MacBook-Pro ~/c/j/devbox (savil/update-lockfile-v2)> cat devbox.lock
{
  "lockfile_version": "1",
  "packages": {
    "[email protected]": {
      "last_modified": "2023-03-31T22:52:29Z",
      "resolved": "github:NixOS/nixpkgs/242246ee1e58f54d2322227fc5eef53b4a616a31#actionlint",
      "version": "1.6.23"
    },
    "[email protected]": {
      "last_modified": "2023-05-01T16:53:22Z",
      "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#go_1_19",
      "version": "1.19.8"
    },
    "[email protected]": {
      "last_modified": "2023-05-01T16:53:22Z",
      "resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#golangci-lint",
      "version": "1.52.2"
    }
  }
}⏎
# Notice that the older [email protected] is now gone
```

Author: savil

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,