[Outputs] Add store-paths of package outputs to devbox.lock (#1814)

## Summary

This PR updates the `devbox.lock` file to incorporate the expanded
response from `/v2/resolve` api. This `/v2/response` incorporates
store-paths for each output of the nix package.

To do so, we update `lock.SystemInfo` to expect `[]Output` instead of
the previous `StorePath string`. This `SystemInfo` struct needs to be
backwards-compatible to read existing `devbox.lock` files that have the
`StorePath` string. So, the read-path in `lock.GetFile` has some
fallback code to handle this scenario, where the `StorePath` is added as
`Output{ Name: "out", path: <path>, Default: true}`.

The main challenge with writing this PR was that we don't want to update
`devbox.lock` _unless_ there's been an explicit user action of the form
`devbox update` or `devbox add` or `devbox remove`. Unfortunately, the
`lock.File` gets internally updated by various code paths using its
`file.Resolve()` function, and so it was (previously) hard for the
lockfile
to track when an add/remove/update action happened. 

To work around this, I have more explicitly invoked `lockfile.Add` and
`lockfile.Remove` in the "add package" and "remove package" code paths.
I think this is safe to do, but let me know if you feel a better path
exists. This lets us track whether to write the lockfile with the legacy
format of `lock.SystemInfo` (i.e. Store Path) or the modern format (i.e.
[]Output).

## How was it tested?

1. `devbox update` in devbox-repo doesn't have changes so the lock file
wasn't changed.
2. `rm -rf .devbox && devbox shell`. The lockfile wasn't changed.
3. `devbox add prometheus` added the package in the modern []Output
format of SystemInfo, and also updated the other packages to use the
modern SystemInfo format.

Author: savil

internal/devbox/update.go

@@ -149,7 +149,7 @@ func (d *Devbox) mergeResolvedPackageToLockfile(
 				// overwrite an existing StorePath, but to ensure correctness we should ensure that all StorePaths
 				// come from the same package version.
 				existingSysInfo, exists := existing.Systems[sysName]
-				if !exists || existingSysInfo.StorePath != newSysInfo.StorePath {
+				if !exists || !existingSysInfo.Equals(newSysInfo) {
 					updated = true
 				}
 			}

internal/devbox/update_test.go

@@ -39,7 +39,13 @@ func TestUpdateNewCurrentSysInfoIsAdded(t *testing.T) {
 		Resolved: "resolved-flake-reference",
 		Systems: map[string]*lock.SystemInfo{
 			sys: {
-				StorePath: "store_path1",
+				Outputs: []lock.Output{
+					{
+						Name:    "out",
+						Default: true,
+						Path:    "store_path1",
+					},
+				},
 			},
 		},
 	}
@@ -57,7 +63,7 @@ func TestUpdateNewCurrentSysInfoIsAdded(t *testing.T) {
 
 	require.Contains(t, lockfile.Packages, raw)
 	require.Contains(t, lockfile.Packages[raw].Systems, sys)
-	require.Equal(t, "store_path1", lockfile.Packages[raw].Systems[sys].StorePath)
+	require.Equal(t, "store_path1", lockfile.Packages[raw].Systems[sys].DefaultStorePath())
 }
 
 func TestUpdateNewSysInfoIsAdded(t *testing.T) {
@@ -72,10 +78,22 @@ func TestUpdateNewSysInfoIsAdded(t *testing.T) {
 		Resolved: "resolved-flake-reference",
 		Systems: map[string]*lock.SystemInfo{
 			sys1: {
-				StorePath: "store_path1",
+				Outputs: []lock.Output{
+					{
+						Name:    "out",
+						Default: true,
+						Path:    "store_path1",
+					},
+				},
 			},
 			sys2: {
-				StorePath: "store_path2",
+				Outputs: []lock.Output{
+					{
+						Name:    "out",
+						Default: true,
+						Path:    "store_path2",
+					},
+				},
 			},
 		},
 	}
@@ -85,7 +103,13 @@ func TestUpdateNewSysInfoIsAdded(t *testing.T) {
 				Resolved: "resolved-flake-reference",
 				Systems: map[string]*lock.SystemInfo{
 					sys1: {
-						StorePath: "store_path1",
+						Outputs: []lock.Output{
+							{
+								Name:    "out",
+								Default: true,
+								Path:    "store_path1",
+							},
+						},
 					},
 					// Missing sys2
 				},
@@ -99,7 +123,7 @@ func TestUpdateNewSysInfoIsAdded(t *testing.T) {
 	require.Contains(t, lockfile.Packages, raw)
 	require.Contains(t, lockfile.Packages[raw].Systems, sys1)
 	require.Contains(t, lockfile.Packages[raw].Systems, sys2)
-	require.Equal(t, "store_path2", lockfile.Packages[raw].Systems[sys2].StorePath)
+	require.Equal(t, "store_path2", lockfile.Packages[raw].Systems[sys2].DefaultStorePath())
 }
 
 func TestUpdateOtherSysInfoIsReplaced(t *testing.T) {
@@ -114,10 +138,22 @@ func TestUpdateOtherSysInfoIsReplaced(t *testing.T) {
 		Resolved: "resolved-flake-reference",
 		Systems: map[string]*lock.SystemInfo{
 			sys1: {
-				StorePath: "store_path1",
+				Outputs: []lock.Output{
+					{
+						Name:    "out",
+						Default: true,
+						Path:    "store_path1",
+					},
+				},
 			},
 			sys2: {
-				StorePath: "store_path2",
+				Outputs: []lock.Output{
+					{
+						Name:    "out",
+						Default: true,
+						Path:    "store_path2",
+					},
+				},
 			},
 		},
 	}
@@ -127,10 +163,22 @@ func TestUpdateOtherSysInfoIsReplaced(t *testing.T) {
 				Resolved: "resolved-flake-reference",
 				Systems: map[string]*lock.SystemInfo{
 					sys1: {
-						StorePath: "store_path1",
+						Outputs: []lock.Output{
+							{
+								Name:    "out",
+								Default: true,
+								Path:    "store_path1",
+							},
+						},
 					},
 					sys2: {
-						StorePath: "mismatching_store_path",
+						Outputs: []lock.Output{
+							{
+								Name:    "out",
+								Default: true,
+								Path:    "mismatching_store_path",
+							},
+						},
 					},
 				},
 			},
@@ -143,8 +191,8 @@ func TestUpdateOtherSysInfoIsReplaced(t *testing.T) {
 	require.Contains(t, lockfile.Packages, raw)
 	require.Contains(t, lockfile.Packages[raw].Systems, sys1)
 	require.Contains(t, lockfile.Packages[raw].Systems, sys2)
-	require.Equal(t, "store_path1", lockfile.Packages[raw].Systems[sys1].StorePath)
-	require.Equal(t, "store_path2", lockfile.Packages[raw].Systems[sys2].StorePath)
+	require.Equal(t, "store_path1", lockfile.Packages[raw].Systems[sys1].DefaultStorePath())
+	require.Equal(t, "store_path2", lockfile.Packages[raw].Systems[sys2].DefaultStorePath())
 }
 
 func currentSystem(_t *testing.T) string {

internal/devpkg/narinfo_cache.go

@@ -112,7 +112,7 @@ func (p *Package) fetchNarInfoStatus() (bool, error) {
 		)
 	}
 
-	pathParts := nix.NewStorePathParts(sysInfo.StorePath)
+	pathParts := nix.NewStorePathParts(sysInfo.DefaultStorePath())
 	reqURL := BinaryCache + "/" + pathParts.Hash + ".narinfo"
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()

internal/devpkg/package.go

@@ -207,7 +207,7 @@ func resolve(pkg *Package) error {
 		return err
 	}
 	if inCache, err := pkg.IsInBinaryCache(); err == nil && inCache {
-		pkg.storePath = resolved.Systems[nix.System()].StorePath
+		pkg.storePath = resolved.Systems[nix.System()].DefaultStorePath()
 	}
 	parsed, err := flake.ParseInstallable(resolved.Resolved)
 	if err != nil {
@@ -576,7 +576,7 @@ func (p *Package) InputAddressedPath() (string, error) {
 	}
 
 	sysInfo := entry.Systems[nix.System()]
-	return sysInfo.StorePath, nil
+	return sysInfo.DefaultStorePath(), nil
 }
 
 func (p *Package) HasAllowInsecure() bool {
@@ -600,6 +600,7 @@ func (p *Package) StoreName() (string, error) {
 }
 
 func (p *Package) EnsureUninstallableIsInLockfile() error {
+	// TODO savil: Should !p.isInstallable() be the opposite i.e. p.IsInstallable()?
 	// TODO savil: Do we need the IsDevboxPackage check here?
 	if !p.IsInstallable() || !p.IsDevboxPackage {
 		return nil

internal/lock/lockfile.go

@@ -46,6 +46,10 @@ func GetFile(project devboxProject) (*File, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// If the lockfile has legacy StorePath fields, we need to convert them to the new format
+	ensurePackagesHaveOutputs(lockFile.Packages)
+
 	return lockFile, nil
 }
 
@@ -92,12 +96,27 @@ func (f *File) Resolve(pkg string) (*Package, error) {
 	return f.Packages[pkg], nil
 }
 
-func (f *File) ForceResolve(pkg string) (*Package, error) {
-	delete(f.Packages, pkg)
-	return f.Resolve(pkg)
-}
-
 func (f *File) Save() error {
+	isDirty, err := f.isDirty()
+	if err != nil {
+		return err
+	}
+	// In SystemInfo, preserve legacy StorePath field and clear out modern Outputs before writing
+	// Reason: We want to update `devbox.lock` file only upon a user action
+	// such as `devbox update` or `devbox add` or `devbox remove`.
+	for pkgName, pkg := range f.Packages {
+		for sys, sysInfo := range pkg.Systems {
+			if !isDirty && sysInfo.StorePath != "" {
+				f.Packages[pkgName].Systems[sys].Outputs = nil
+			} else {
+				f.Packages[pkgName].Systems[sys].StorePath = ""
+			}
+		}
+	}
+	// We set back the Outputs, if needed, after writing the file, so that future
+	// users of the `lock.File` struct will have the correct data.
+	defer ensurePackagesHaveOutputs(f.Packages)
+
 	return cuecfg.WriteFile(lockFilePath(f.devboxProject.ProjectDir()), f)
 }
 

internal/lock/package.go

@@ -3,6 +3,11 @@
 
 package lock
 
+import (
+	"fmt"
+	"slices"
+)
+
 const (
 	nixpkgSource       string = "nixpkg"
 	devboxSearchSource string = "devbox-search"
@@ -22,13 +27,30 @@ type Package struct {
 }
 
 type SystemInfo struct {
-	// StorePath is the input-addressed path for the nix package in /nix/store
-	// It is the cache key in the Binary Cache Store (cache.nixos.org)
-	// It is of the form /nix/store/<hash>-<name>-<version>
-	// <name> may be different from the canonicalName so we store the full store path.
+	Outputs []Output `json:"outputs,omitempty"`
+
+	// Legacy Format
 	StorePath string `json:"store_path,omitempty"`
 }
 
+// Output refers to a nix package output. This struct is derived from searcher.Output
+type Output struct {
+	// Name is the output's name. Nix appends the name to
+	// the output's store path unless it's the default name
+	// of "out". Output names can be anything, but
+	// conventionally they follow the various "make install"
+	// directories such as "bin", "lib", "src", "man", etc.
+	Name string `json:"name,omitempty"`
+
+	// Path is the absolute store path (with the /nix/store/
+	// prefix) of the output.
+	Path string `json:"path,omitempty"`
+
+	// Default indicates if Nix installs this output by
+	// default.
+	Default bool `json:"default,omitempty"`
+}
+
 func (p *Package) GetSource() string {
 	if p == nil {
 		return ""
@@ -43,9 +65,49 @@ func (p *Package) IsAllowInsecure() bool {
 	return p.AllowInsecure
 }
 
+// Useful for debugging when we print the struct
+func (i *SystemInfo) String() string {
+	return fmt.Sprintf("%+v", *i)
+}
+
+func (i *SystemInfo) DefaultStorePath() string {
+	if i == nil || len(i.Outputs) == 0 {
+		return ""
+	}
+
+	for _, output := range i.Outputs {
+		if output.Default {
+			return output.Path
+		}
+	}
+
+	return i.Outputs[0].Path
+}
+
 func (i *SystemInfo) Equals(other *SystemInfo) bool {
 	if i == nil || other == nil {
 		return i == other
 	}
-	return *i == *other
+
+	return slices.Equal(i.Outputs, other.Outputs)
+}
+
+// ensurePackagesHaveOutputs is used for backwards-compatibility with the old
+// lockfile format where each SystemInfo had a StorePath but no Outputs.
+func ensurePackagesHaveOutputs(packages map[string]*Package) {
+	for _, pkg := range packages {
+		for sys, sysInfo := range pkg.Systems {
+			// If we have a StorePath and no Outputs, we need to convert to the new format.
+			// Note: for a non-empty StorePath, Outputs should be empty, but being cautious.
+			if sysInfo.StorePath != "" && len(sysInfo.Outputs) == 0 {
+				pkg.Systems[sys].Outputs = []Output{
+					{
+						Default: true,
+						Name:    "out",
+						Path:    sysInfo.StorePath,
+					},
+				}
+			}
+		}
+	}
 }

internal/lock/resolve.go

@@ -102,8 +102,16 @@ func resolveV2(ctx context.Context, name, version string) (*Package, error) {
 	}
 	for sys, info := range resolved.Systems {
 		if len(info.Outputs) != 0 {
+			outputs := make([]Output, len(info.Outputs))
+			for i, out := range info.Outputs {
+				outputs[i] = Output{
+					Name:    out.Name,
+					Path:    out.Path,
+					Default: out.Default,
+				}
+			}
 			pkg.Systems[sys] = &SystemInfo{
-				StorePath: info.Outputs[0].Path,
+				Outputs: outputs,
 			}
 		}
 	}
@@ -166,6 +174,13 @@ func buildLockSystemInfos(pkg *searcher.PackageVersion) (map[string]*SystemInfo,
 	sysInfos := map[string]*SystemInfo{}
 	for sysName, storePath := range sysStorePaths {
 		sysInfos[sysName] = &SystemInfo{
+			Outputs: []Output{
+				{
+					Default: true,
+					Name:    "out",
+					Path:    storePath,
+				},
+			},
 			StorePath: storePath,
 		}
 	}