Skip to content

Commit 540b82c

Browse files
authored
Fix false duplicate detection for multiple declared built-in pluginss (#2799)
## Summary Fixes #2790, which prevents users from declaring multiple built-in plugins: When devbox computed a unique ID (hash) for a plugin like plugin:nodejs or plugin:python, it tried to use the fully resolved package name. But for built-in plugins added via include, the package isn’t resolved yet — so that name is an empty string. Two different plugins both hashing the empty string = same hash = devbox thinks they’re duplicates and throws a “circular or duplicate include” error. The fix: if the resolved name is empty, use the raw string ("nodejs", "python") instead. Different raw strings, different hashes, no false collision. This change should not affect normal package resolution, since Devbox already enforces that each entry of the packages array is unique. ## How was it tested? Added an additional test (`TestLoadRecursiveMultipleBuiltinPluginIncludes`) which checks that we can load 2 explicitly declared built-in plugins without issue.
1 parent bbe7a31 commit 540b82c

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

internal/devconfig/config_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"github.com/pkg/errors"
1212
"github.com/tailscale/hujson"
1313
"go.jetify.com/devbox/internal/devconfig/configfile"
14+
"go.jetify.com/devbox/internal/lock"
15+
"go.jetify.com/devbox/nix/flake"
1416
)
1517

1618
func TestOpen(t *testing.T) {
@@ -427,3 +429,43 @@ func TestOSExpandIfPossible(t *testing.T) {
427429
})
428430
}
429431
}
432+
433+
// TestLoadRecursiveMultipleBuiltinPluginIncludes is a regression test for
434+
// DEV-16: two explicit built-in plugin includes must not trigger a false
435+
// "circular or duplicate include detected" error.
436+
func TestLoadRecursiveMultipleBuiltinPluginIncludes(t *testing.T) {
437+
dir := t.TempDir()
438+
439+
const jsonContent = `{
440+
"packages": [],
441+
"include": ["plugin:nodejs", "plugin:python"]
442+
}`
443+
path := filepath.Join(dir, "devbox.json")
444+
if err := os.WriteFile(path, []byte(jsonContent), 0o644); err != nil {
445+
t.Fatalf("os.WriteFile error: %v", err)
446+
}
447+
448+
cfg, err := Open(path)
449+
if err != nil {
450+
t.Fatalf("Open error: %v", err)
451+
}
452+
453+
lockfile, err := lock.GetFile(&testLockProject{dir: dir})
454+
if err != nil {
455+
t.Fatalf("lock.GetFile error: %v", err)
456+
}
457+
458+
if err := cfg.LoadRecursive(lockfile); err != nil {
459+
t.Errorf("LoadRecursive returned unexpected error: %v", err)
460+
}
461+
}
462+
463+
// testLockProject satisfies the unexported lock.devboxProject interface for tests.
464+
type testLockProject struct {
465+
dir string
466+
}
467+
468+
func (p *testLockProject) ConfigHash() (string, error) { return "", nil }
469+
func (p *testLockProject) Stdenv() flake.Ref { return flake.Ref{} }
470+
func (p *testLockProject) AllPackageNamesIncludingRemovedTriggerPackages() []string { return nil }
471+
func (p *testLockProject) ProjectDir() string { return p.dir }

internal/devpkg/package.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ func (p *Package) Hash() string {
466466
}
467467

468468
if sum == "" {
469-
sum = cachehash.Bytes([]byte(p.installable.String()))
469+
sum = cachehash.Bytes([]byte(cmp.Or(p.installable.String(), p.Raw)))
470470
}
471471
return sum[:min(len(sum), 6)]
472472
}

0 commit comments

Comments
 (0)