diff --git a/internal/boxcli/featureflag/feature.go b/internal/boxcli/featureflag/feature.go index 80dcd9dd09e..d70f9835fe2 100644 --- a/internal/boxcli/featureflag/feature.go +++ b/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)) diff --git a/internal/impl/update.go b/internal/impl/update.go index e8439632975..5ec95db0f27 100644 --- a/internal/impl/update.go +++ b/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) diff --git a/internal/impl/update_test.go b/internal/impl/update_test.go index 24736628530..af8142645f7 100644 --- a/internal/impl/update_test.go +++ b/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 +}