diff --git a/internal/devconfig/config.go b/internal/devconfig/config.go index c7e38d02965..cf0cc53a0f8 100644 --- a/internal/devconfig/config.go +++ b/internal/devconfig/config.go @@ -9,6 +9,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "syscall" "time" @@ -334,17 +335,39 @@ func (c *Config) Packages( } } - // Keep only the last occurrence of each package (by name). + // Keep only the last occurrence of each package. Plain packages are + // deduped by name so that a later "pkg@v2" replaces an earlier + // "pkg@v1". Flake references, however, can install several distinct + // outputs from the same flake via a "#fragment" (for example + // "git+https://host/repo.git#toolA" and "...#toolB"). Once parsed those + // share a name (and for URLs containing "user@host" the name is even + // truncated to the part before the "@"), so keying on name alone + // collapses them into a single package. Key such references on their + // full reference so each output is preserved. See jetify-com/devbox#2662. mutable.Reverse(packages) packages = lo.UniqBy( packages, - func(p configfile.Package) string { return p.Name }, + func(p configfile.Package) string { + if ref := p.VersionedName(); isFlakeReference(ref) { + return ref + } + return p.Name + }, ) mutable.Reverse(packages) return packages } +// isFlakeReference reports whether ref looks like a flake installable rather +// than a plain "name@version" package. Flake references contain a URL scheme +// separator ("://") or an output fragment ("#"); plain package versions never +// do. Such references may point at distinct outputs of the same flake, so they +// must not be collapsed by package name. +func isFlakeReference(ref string) bool { + return strings.Contains(ref, "://") || strings.Contains(ref, "#") +} + func (c *Config) NixPkgsCommitHash() string { return c.Root.NixPkgsCommitHash() } diff --git a/internal/devconfig/config_test.go b/internal/devconfig/config_test.go index 9f747796156..0dbba1db326 100644 --- a/internal/devconfig/config_test.go +++ b/internal/devconfig/config_test.go @@ -560,6 +560,70 @@ func TestLoadRecursiveMultipleBuiltinPluginIncludes(t *testing.T) { } } +// TestPackagesPreservesMultipleFlakeOutputs is a regression test for +// jetify-com/devbox#2662: adding two outputs from the same remote flake used to +// drop the first one. Both outputs are parsed with the same package Name (the +// "#fragment" that distinguishes them, and part of the "user@host" URL, is +// parsed into the version), so deduping by Name alone collapsed them. +func TestPackagesPreservesMultipleFlakeOutputs(t *testing.T) { + cfg, err := loadBytes([]byte(`{ + "packages": [ + "kubectl@latest", + "git+http://git@example.com/group/utils.git#kubeupdate", + "git+http://git@example.com/group/utils.git#getcrds" + ] + }`)) + if err != nil { + t.Fatalf("loadBytes error: %v", err) + } + + pkgs := cfg.Packages(false /*includeRemovedTriggerPackages*/) + counts := map[string]int{} + for _, pkg := range pkgs { + counts[pkg.VersionedName()]++ + } + + want := []string{ + "kubectl@latest", + "git+http://git@example.com/group/utils.git#kubeupdate", + "git+http://git@example.com/group/utils.git#getcrds", + } + for _, name := range want { + switch counts[name] { + case 0: + t.Errorf("Packages() dropped %q; got %v", name, pkgs) + case 1: // expected + default: + t.Errorf("Packages() returned %q %d times, want exactly once", name, counts[name]) + } + } + // Assert on the slice length (not the deduped map) so an accidental + // regression that returns duplicates is also caught. + if len(pkgs) != len(want) { + t.Errorf("Packages() returned %d packages, want %d: %v", len(pkgs), len(want), pkgs) + } +} + +// TestPackagesDedupesByNameForPlainPackages verifies that the flake-aware +// dedup key preserves the original "last version wins" behavior for plain +// (non-flake) packages. +func TestPackagesDedupesByNameForPlainPackages(t *testing.T) { + cfg, err := loadBytes([]byte(`{ + "packages": ["python@3.10", "python@3.11"] + }`)) + if err != nil { + t.Fatalf("loadBytes error: %v", err) + } + + pkgs := cfg.Packages(false /*includeRemovedTriggerPackages*/) + if len(pkgs) != 1 { + t.Fatalf("Packages() returned %d packages, want 1: %v", len(pkgs), pkgs) + } + if got := pkgs[0].VersionedName(); got != "python@3.11" { + t.Errorf("Packages() kept %q, want the last occurrence \"python@3.11\"", got) + } +} + // testLockProject satisfies the unexported lock.devboxProject interface for tests. type testLockProject struct { dir string