[bugs]: add other sysinfos, but respect existing ca paths

internal/boxcli/featureflag/feature.go

@@ -6,6 +6,7 @@ package featureflag
 import (
 	"os"
 	"strconv"
+	"testing"
 
 	"go.jetpack.io/devbox/internal/debug"
 	"go.jetpack.io/devbox/internal/envir"
@@ -54,6 +55,10 @@ func (f *feature) Enabled() bool {
 	return f.enabled
 }
 
+func (f *feature) EnableForTest(t *testing.T) {
+	t.Setenv(envir.DevboxFeaturePrefix+f.name, "1")
+}
+
 // All returns a map of all known features flags and whether they're enabled.
 func All() map[string]bool {
 	m := make(map[string]bool, len(features))

internal/impl/update.go

@@ -128,19 +128,34 @@ func (d *Devbox) mergeResolvedPackageToLockfile(
 			return err
 		}
 
-		// If the resolved has a system info for the user's system, then add/overwrite it. We don't overwrite
-		// other system infos because we don't want to clobber system-dependent CAStorePaths.
-		if newSysInfo, ok := resolved.Systems[userSystem]; ok {
-			if !newSysInfo.Equals(existing.Systems[userSystem]) {
-				// We only guard this so that the ux messaging is accurate. We could overwrite every time.
-				if lockfile.Packages[pkg.Raw].Systems == nil {
-					lockfile.Packages[pkg.Raw].Systems = map[string]*lock.SystemInfo{}
+		if lockfile.Packages[pkg.Raw].Systems == nil {
+			lockfile.Packages[pkg.Raw].Systems = map[string]*lock.SystemInfo{}
+		}
+
+		updated := false
+		for sysName, newSysInfo := range resolved.Systems {
+			if sysName == userSystem {
+				// The resolved pkg has a system info for the user's system, so add/overwrite it.
+				if !newSysInfo.Equals(existing.Systems[userSystem]) {
+					// We only guard this so that the ux messaging is accurate. We could overwrite every time.
+					lockfile.Packages[pkg.Raw].Systems[userSystem] = newSysInfo
+					updated = true
+				}
+			} else {
+				// Add other system infos if they don't exist, or if we have a different StorePath. This may
+				// overwrite an existing CAPath, 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 {
+					lockfile.Packages[pkg.Raw].Systems[sysName] = newSysInfo
+					updated = true
 				}
-				lockfile.Packages[pkg.Raw].Systems[userSystem] = newSysInfo
-				ux.Finfo(d.writer, "Updated system information for %s\n", pkg)
-				return nil
 			}
 		}
+		if updated {
+			ux.Finfo(d.writer, "Updated system information for %s\n", pkg)
+			return nil
+		}
 	}
 
 	ux.Finfo(d.writer, "Already up-to-date %s %s\n", pkg, existing.Version)

internal/impl/update_test.go

@@ -5,8 +5,10 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"go.jetpack.io/devbox/internal/boxcli/featureflag"
 	"go.jetpack.io/devbox/internal/devpkg"
 	"go.jetpack.io/devbox/internal/lock"
+	"go.jetpack.io/devbox/internal/nix"
 )
 
 func TestUpdateNewPackageIsAdded(t *testing.T) {
@@ -28,3 +30,193 @@ func TestUpdateNewPackageIsAdded(t *testing.T) {
 
 	require.Contains(t, lockfile.Packages, raw)
 }
+
+func TestUpdateNewCurrentSysInfoIsAdded(t *testing.T) {
+	featureflag.RemoveNixpkgs.EnableForTest(t)
+	devbox := devboxForTesting(t)
+
+	raw := "hello@1.2.3"
+	sys := currentSystem(t)
+	devPkg := &devpkg.Package{
+		Raw: raw,
+	}
+	resolved := &lock.Package{
+		Resolved: "resolved-flake-reference",
+		Systems: map[string]*lock.SystemInfo{
+			sys: {
+				StorePath:   "store_path1",
+				CAStorePath: "ca_path1",
+			},
+		},
+	}
+	lockfile := &lock.File{
+		Packages: map[string]*lock.Package{
+			raw: {
+				Resolved: "resolved-flake-reference",
+				// No system infos.
+			},
+		},
+	}
+
+	err := devbox.mergeResolvedPackageToLockfile(context.Background(), devPkg, resolved, lockfile)
+	require.NoError(t, err, "update failed")
+
+	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, "ca_path1", lockfile.Packages[raw].Systems[sys].CAStorePath)
+}
+
+func TestUpdateNewSysInfoIsAdded(t *testing.T) {
+	featureflag.RemoveNixpkgs.EnableForTest(t)
+	devbox := devboxForTesting(t)
+
+	raw := "hello@1.2.3"
+	sys1 := currentSystem(t)
+	sys2 := "system2"
+	devPkg := &devpkg.Package{
+		Raw: raw,
+	}
+	resolved := &lock.Package{
+		Resolved: "resolved-flake-reference",
+		Systems: map[string]*lock.SystemInfo{
+			sys1: {
+				StorePath:   "store_path1",
+				CAStorePath: "ca_path1",
+			},
+			sys2: {
+				StorePath: "store_path2",
+			},
+		},
+	}
+	lockfile := &lock.File{
+		Packages: map[string]*lock.Package{
+			raw: {
+				Resolved: "resolved-flake-reference",
+				Systems: map[string]*lock.SystemInfo{
+					sys1: {
+						StorePath:   "store_path1",
+						CAStorePath: "ca_path1",
+					},
+					// Missing sys2
+				},
+			},
+		},
+	}
+
+	err := devbox.mergeResolvedPackageToLockfile(context.Background(), devPkg, resolved, lockfile)
+	require.NoError(t, err, "update failed")
+
+	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)
+}
+
+func TestUpdateOtherSysInfoIsReplaced(t *testing.T) {
+	featureflag.RemoveNixpkgs.EnableForTest(t)
+	devbox := devboxForTesting(t)
+
+	raw := "hello@1.2.3"
+	sys1 := currentSystem(t)
+	sys2 := "system2"
+	devPkg := &devpkg.Package{
+		Raw: raw,
+	}
+	resolved := &lock.Package{
+		Resolved: "resolved-flake-reference",
+		Systems: map[string]*lock.SystemInfo{
+			sys1: {
+				StorePath:   "store_path1",
+				CAStorePath: "ca_path1",
+			},
+			sys2: {
+				StorePath: "store_path2",
+			},
+		},
+	}
+	lockfile := &lock.File{
+		Packages: map[string]*lock.Package{
+			raw: {
+				Resolved: "resolved-flake-reference",
+				Systems: map[string]*lock.SystemInfo{
+					sys1: {
+						StorePath:   "store_path1",
+						CAStorePath: "ca_path1",
+					},
+					sys2: {
+						StorePath:   "mismatching_store_path",
+						CAStorePath: "ca_path2",
+					},
+				},
+			},
+		},
+	}
+
+	err := devbox.mergeResolvedPackageToLockfile(context.Background(), devPkg, resolved, lockfile)
+	require.NoError(t, err, "update failed")
+
+	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.Empty(t, lockfile.Packages[raw].Systems[sys2].CAStorePath)
+}
+
+func TestUpdateCAPathIsNotReplaced(t *testing.T) {
+	featureflag.RemoveNixpkgs.EnableForTest(t)
+	devbox := devboxForTesting(t)
+
+	raw := "hello@1.2.3"
+	sys1 := currentSystem(t)
+	sys2 := "system2"
+	devPkg := &devpkg.Package{
+		Raw: raw,
+	}
+	resolved := &lock.Package{
+		Resolved: "resolved-flake-reference",
+		Systems: map[string]*lock.SystemInfo{
+			sys1: {
+				StorePath:   "store_path1",
+				CAStorePath: "ca_path1",
+			},
+			sys2: {
+				StorePath: "store_path2",
+				// No CAPath here because this is not the current system.
+			},
+		},
+	}
+	lockfile := &lock.File{
+		Packages: map[string]*lock.Package{
+			raw: {
+				Resolved: "resolved-flake-reference",
+				Systems: map[string]*lock.SystemInfo{
+					sys1: {
+						StorePath:   "store_path1",
+						CAStorePath: "ca_path1",
+					},
+					sys2: {
+						StorePath:   "store_path2",
+						CAStorePath: "ca_path2", // we already have CAPath for this system; it should not be replaced
+					},
+				},
+			},
+		},
+	}
+
+	err := devbox.mergeResolvedPackageToLockfile(context.Background(), devPkg, resolved, lockfile)
+	require.NoError(t, err, "update failed")
+
+	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, "ca_path2", lockfile.Packages[raw].Systems[sys2].CAStorePath)
+}
+
+func currentSystem(t *testing.T) string {
+	sys, err := nix.System() // NOTE: we could mock this too, if it helps.
+	require.NoError(t, err)
+	return sys
+}