diff --git a/internal/devbox/packages.go b/internal/devbox/packages.go index 6c8d7319e75..685cddd2bef 100644 --- a/internal/devbox/packages.go +++ b/internal/devbox/packages.go @@ -63,7 +63,9 @@ func (d *Devbox) Outdated(ctx context.Context) (map[string]UpdateVersion, error) continue } existingLockPackage := lockfile.Packages[pkg.Raw] - if lockPackage.Version == existingLockPackage.Version { + // Packages that aren't locked yet will be locked to the latest version + // on the next install, so they aren't outdated. + if existingLockPackage == nil || lockPackage.Version == existingLockPackage.Version { continue } diff --git a/internal/devpkg/package.go b/internal/devpkg/package.go index 5844b1ab86f..7944c8dca00 100644 --- a/internal/devpkg/package.go +++ b/internal/devpkg/package.go @@ -25,6 +25,7 @@ import ( "go.jetify.com/devbox/internal/devpkg/pkgtype" "go.jetify.com/devbox/internal/lock" "go.jetify.com/devbox/internal/nix" + "go.jetify.com/devbox/internal/searcher" "go.jetify.com/devbox/internal/ux" "go.jetify.com/devbox/nix/flake" "go.jetify.com/devbox/plugins" @@ -159,7 +160,7 @@ func newPackage(raw string, isInstallable func() bool, locker lock.Locker) *Pack return resolve(pkg) }) pkg.setInstallable(parsed, locker.ProjectDir()) - pkg.outputs = outputs{selectedNames: strings.Split(parsed.Outputs, ",")} + pkg.outputs = outputs{selectedNames: lo.Compact(strings.Split(parsed.Outputs, ","))} pkg.Patch = pkgNeedsPatch(pkg.CanonicalName(), configfile.PatchAuto) return pkg } @@ -232,6 +233,11 @@ func (p *Package) FlakeInputName() string { } else if p.installable.Ref.Ref != "" { result += "-" + p.installable.Ref.Ref } + // Disambiguate flakes in different subdirectories or hosts of + // the same repo. + if p.installable.Ref.Dir != "" || p.installable.Ref.Host != "" { + result += "-" + cachehash.Bytes6([]byte(p.installable.Ref.String())) + } } default: result = p.installable.Ref.String() + "-" + p.Hash() @@ -501,8 +507,10 @@ func (p *Package) CanonicalName() string { if !p.IsDevboxPackage { return "" } - name, _, _ := strings.Cut(p.Raw, "@") - return name + if name, _, ok := searcher.ParseVersionedPackage(p.Raw); ok { + return name + } + return p.Raw } func (p *Package) Versioned() string { @@ -560,12 +568,13 @@ func (p *Package) version() string { if !p.IsDevboxPackage { return "" } - _, version, _ := strings.Cut(p.Raw, "@") + _, version, _ := searcher.ParseVersionedPackage(p.Raw) return version } func (p *Package) isVersioned() bool { - return p.IsDevboxPackage && strings.Contains(p.Raw, "@") + _, _, versioned := searcher.ParseVersionedPackage(p.Raw) + return p.IsDevboxPackage && versioned } func (p *Package) HashFromNixPkgsURL() string { diff --git a/internal/devpkg/package_test.go b/internal/devpkg/package_test.go index b35912a0f12..7d5f92877dc 100644 --- a/internal/devpkg/package_test.go +++ b/internal/devpkg/package_test.go @@ -6,10 +6,12 @@ package devpkg import ( "fmt" "path/filepath" + "slices" "strings" "testing" "github.com/samber/lo" + "go.jetify.com/devbox/internal/devbox/devopt" "go.jetify.com/devbox/internal/lock" "go.jetify.com/devbox/internal/nix" "go.jetify.com/devbox/nix/flake" @@ -97,6 +99,23 @@ func TestInput(t *testing.T) { } } +func TestFlakeInputNameUnique(t *testing.T) { + pkgs := []string{ + "github:owner/repo/abc123#x", + "github:owner/repo/abc123?dir=a#x", + "github:owner/repo/abc123?dir=b#y", + "github:owner/repo/abc123?host=git.example.com#x", + } + seen := map[string]string{} + for _, pkg := range pkgs { + name := testInputFromString(pkg, "/tmp/my-project").FlakeInputName() + if other, ok := seen[name]; ok { + t.Errorf("FlakeInputName() = %q for both %q and %q", name, other, pkg) + } + seen[name] = pkg + } +} + type testInput struct { *Package } @@ -194,6 +213,8 @@ func TestCanonicalName(t *testing.T) { {"go", "go"}, {"go@latest", "go"}, {"go@1.21", "go"}, + {"nodePackages.@vue/cli@5.0", "nodePackages.@vue/cli"}, + {"emacsPackages.@", "emacsPackages.@"}, {"runx:golangci/golangci-lint@latest", "runx:golangci/golangci-lint"}, {"runx:golangci/golangci-lint@v0.0.2", "runx:golangci/golangci-lint"}, {"runx:golangci/golangci-lint", "runx:golangci/golangci-lint"}, @@ -211,3 +232,48 @@ func TestCanonicalName(t *testing.T) { }) } } + +func TestVersioned(t *testing.T) { + tests := []struct { + pkgName string + version string + versioned string + }{ + {"go", "", "go@latest"}, + {"go@1.21", "1.21", "go@1.21"}, + {"nodePackages.@vue/cli@5.0", "5.0", "nodePackages.@vue/cli@5.0"}, + } + for _, tt := range tests { + t.Run(tt.pkgName, func(t *testing.T) { + pkg := PackageFromStringWithDefaults(tt.pkgName, &lockfile{}) + if got := pkg.version(); got != tt.version { + t.Errorf("version() = %q, want %q", got, tt.version) + } + if got := pkg.Versioned(); got != tt.versioned { + t.Errorf("Versioned() = %q, want %q", got, tt.versioned) + } + }) + } +} + +func TestFlakeOutputNames(t *testing.T) { + tests := []struct { + pkg string + opts []string + outputs []string + }{ + {"github:NixOS/nixpkgs/12345#curl", nil, []string{}}, + {"github:NixOS/nixpkgs/12345#curl", []string{"dev", "out"}, []string{"dev", "out"}}, + {"github:NixOS/nixpkgs/12345#curl^bin,dev", nil, []string{"bin", "dev"}}, + {"github:NixOS/nixpkgs/12345#curl^bin", []string{"dev"}, []string{"bin", "dev"}}, + } + for _, tt := range tests { + t.Run(tt.pkg, func(t *testing.T) { + pkg := PackageFromStringWithOptions(tt.pkg, &lockfile{}, devopt.AddOpts{Outputs: tt.opts}) + got := pkg.outputs.selectedNames + if !slices.Equal(got, tt.outputs) { + t.Errorf("got outputs %q, want %q", got, tt.outputs) + } + }) + } +} diff --git a/internal/lock/lockfile.go b/internal/lock/lockfile.go index c1f7cd34f01..7e0e2e08014 100644 --- a/internal/lock/lockfile.go +++ b/internal/lock/lockfile.go @@ -145,10 +145,18 @@ func (f *File) UpdateStdenv() error { if err := nix.ClearFlakeCache(f.devboxProject.Stdenv()); err != nil { return err } - if err := f.Remove(f.devboxProject.Stdenv().String()); err != nil { + // Remove the old entry in memory only so that a failed resolve doesn't + // leave the lockfile without a stdenv. + stdenv := f.devboxProject.Stdenv().String() + old, hasOld := f.Packages[stdenv] + delete(f.Packages, stdenv) + if err := f.Add(stdenv); err != nil { + if hasOld { + f.Packages[stdenv] = old + } return err } - return f.Add(f.devboxProject.Stdenv().String()) + return nil } // TODO: We should improve a few issues with this function: diff --git a/internal/lock/resolve.go b/internal/lock/resolve.go index 2183d7c6024..529b8541f82 100644 --- a/internal/lock/resolve.go +++ b/internal/lock/resolve.go @@ -42,10 +42,11 @@ func (f *File) FetchResolvedPackage(pkg string, refresh bool) (*Package, error) if err != nil { return nil, err } - return &Package{ - Resolved: installable.String(), - LastModified: time.Unix(installable.Ref.LastModified, 0).UTC().Format(time.RFC3339), - }, nil + locked := &Package{Resolved: installable.String()} + if installable.Ref.LastModified != 0 { + locked.LastModified = time.Unix(installable.Ref.LastModified, 0).UTC().Format(time.RFC3339) + } + return locked, nil } name, version, _ := searcher.ParseVersionedPackage(pkg) @@ -68,9 +69,12 @@ func (f *File) FetchResolvedPackage(pkg string, refresh bool) (*Package, error) } packageVersion, err := searcher.Client().Resolve(name, version) - if err != nil { + if errors.Is(err, searcher.ErrNotFound) { return nil, errors.Wrapf(nix.ErrPackageNotFound, "%s@%s", name, version) } + if err != nil { + return nil, err + } sysInfos, err := buildLockSystemInfos(packageVersion) if err != nil { @@ -220,9 +224,9 @@ func lockFlake(ctx context.Context, ref flake.Ref, refresh bool) (flake.Ref, err // file is a bit more lenient and only requires a revision so that we // don't need to download the nixpkgs source for cached packages. If the // search index is ever able to return the NAR hash then we can remove - // this check. Skip this shortcut when refresh is requested — the caller - // is asking us to re-query upstream for a newer rev. - if !refresh && ref.Type == flake.TypeGitHub && (ref.Rev != "") { + // this check. A pinned rev can't change, so this also applies when + // refresh is requested. + if ref.Type == flake.TypeGitHub && ref.Rev != "" { return ref, nil } diff --git a/internal/lock/resolve_test.go b/internal/lock/resolve_test.go new file mode 100644 index 00000000000..cb981ceae07 --- /dev/null +++ b/internal/lock/resolve_test.go @@ -0,0 +1,58 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package lock + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" + + "go.jetify.com/devbox/internal/envir" + "go.jetify.com/devbox/internal/nix" +) + +func TestFetchResolvedPackageSearchErrors(t *testing.T) { + tests := []struct { + status int + wantNotFound bool + }{ + {http.StatusNotFound, true}, + {http.StatusInternalServerError, false}, + } + for _, test := range tests { + t.Run(http.StatusText(test.status), func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(test.status) + })) + defer srv.Close() + t.Setenv(envir.DevboxSearchHost, srv.URL) + t.Setenv("DEVBOX_FEATURE_RESOLVE_V2", "0") + + _, err := (&File{}).FetchResolvedPackage("hello@1.0", false) + if err == nil { + t.Fatal("got nil error") + } + if got := errors.Is(err, nix.ErrPackageNotFound); got != test.wantNotFound { + t.Errorf("errors.Is(err, ErrPackageNotFound) = %v, want %v (err: %v)", got, test.wantNotFound, err) + } + }) + } +} + +func TestFetchResolvedPackagePinnedGitHubFlake(t *testing.T) { + const pkg = "github:owner/repo/1111111111111111111111111111111111111111#hello" + for _, refresh := range []bool{false, true} { + got, err := (&File{}).FetchResolvedPackage(pkg, refresh) + if err != nil { + t.Fatalf("refresh=%v: got error: %v", refresh, err) + } + if got.Resolved != pkg { + t.Errorf("refresh=%v: got Resolved %q, want %q", refresh, got.Resolved, pkg) + } + if got.LastModified != "" { + t.Errorf("refresh=%v: got LastModified %q, want empty", refresh, got.LastModified) + } + } +} diff --git a/internal/nix/config.go b/internal/nix/config.go index 99ea19eff10..86e2a82cc5f 100644 --- a/internal/nix/config.go +++ b/internal/nix/config.go @@ -56,6 +56,10 @@ func (c Config) IsUserTrusted(ctx context.Context, username string) (bool, error if len(trusted) == 0 { return false, nil } + // A "*" entry trusts all users. + if slices.Contains(trusted, "*") { + return true, nil + } current, err := user.Lookup(username) if err != nil { diff --git a/internal/nix/config_test.go b/internal/nix/config_test.go index da897f64c4d..a9d1a38a765 100644 --- a/internal/nix/config_test.go +++ b/internal/nix/config_test.go @@ -29,6 +29,16 @@ func TestConfigIsUserTrusted(t *testing.T) { t.Error("got trusted = false, want true") } }) + t.Run("Wildcard", func(t *testing.T) { + cfg := Config{TrustedUsers: ConfigField[[]string]{Value: []string{"*"}}} + trusted, err := cfg.IsUserTrusted(t.Context(), curUser.Username) + if err != nil { + t.Fatal(err) + } + if !trusted { + t.Error("got trusted = false, want true") + } + }) t.Run("UserGroupInList", func(t *testing.T) { g, err := user.LookupGroupId(curUser.Gid) if err != nil { diff --git a/internal/nix/nix.go b/internal/nix/nix.go index 3b99d2d4760..4ec8e6dbd0b 100644 --- a/internal/nix/nix.go +++ b/internal/nix/nix.go @@ -59,7 +59,9 @@ func (*NixInstance) PrintDevEnv(ctx context.Context, args *PrintDevEnvArgs) (*Pr data, err = os.ReadFile(args.PrintDevEnvCachePath) if err == nil { if err := json.Unmarshal(data, &out); err != nil { - return nil, errors.WithStack(err) + // The cache may be truncated or corrupt, so recompute it. + slog.Debug("ignoring invalid print-dev-env cache", "err", err) + data, out = nil, PrintDevEnvOut{} } } else if !errors.Is(err, fs.ErrNotExist) { return nil, errors.WithStack(err) diff --git a/internal/nix/store.go b/internal/nix/store.go index 3509d08d29a..72a28f85130 100644 --- a/internal/nix/store.go +++ b/internal/nix/store.go @@ -161,7 +161,7 @@ func DaemonVersion(ctx context.Context) (string, error) { } if len(out) == 0 { - return "", redact.Errorf("command %s: empty output", redact.Safe(cmd), err) + return "", redact.Errorf("command %s: empty output", redact.Safe(cmd)) } if canJSON { info := struct{ Version string }{} diff --git a/internal/nix/storepath.go b/internal/nix/storepath.go index 2fbe48b1f2c..86756c4a5ea 100644 --- a/internal/nix/storepath.go +++ b/internal/nix/storepath.go @@ -26,13 +26,20 @@ func NewStorePathParts(path string) StorePathParts { hash, name := path[:32], path[33:] dashIndex := 0 - for i, r := range name { - if dashIndex != 0 && !unicode.IsLetter(r) { - version, output, _ := strings.Cut(name[i:], "-") + for i, char := range name { + if dashIndex != 0 && !unicode.IsLetter(char) { + // Versions may contain dashes (2.39-52, 0-unstable-2024-01-01), + // so only treat the last component as the output if it starts + // with a letter. + version, output := name[i:], "" + if j := strings.LastIndex(version, "-"); j != -1 && j+1 < len(version) && + unicode.IsLetter(rune(version[j+1])) { + version, output = version[:j], version[j+1:] + } return StorePathParts{Hash: hash, Name: name[:dashIndex], Version: version, Output: output} } dashIndex = 0 - if r == '-' { + if char == '-' { dashIndex = i } } diff --git a/internal/nix/storepath_test.go b/internal/nix/storepath_test.go index 7122250d47b..4b06d3d22f9 100644 --- a/internal/nix/storepath_test.go +++ b/internal/nix/storepath_test.go @@ -45,6 +45,35 @@ func TestStorePathParts(t *testing.T) { Output: "bar", }, }, + // version with dashes and an output + { + storePath: "/nix/store/0z1zq1zq1zq1zq1zq1zq1zq1zq1zq1zq-glibc-2.39-52-bin", + expected: StorePathParts{ + Hash: "0z1zq1zq1zq1zq1zq1zq1zq1zq1zq1zq", + Name: "glibc", + Version: "2.39-52", + Output: "bin", + }, + }, + // version with dashes and no output + { + storePath: "/nix/store/0z1zq1zq1zq1zq1zq1zq1zq1zq1zq1zq-foo-0-unstable-2024-01-01", + expected: StorePathParts{ + Hash: "0z1zq1zq1zq1zq1zq1zq1zq1zq1zq1zq", + Name: "foo", + Version: "0-unstable-2024-01-01", + }, + }, + // unstable version with an output + { + storePath: "/nix/store/0z1zq1zq1zq1zq1zq1zq1zq1zq1zq1zq-foo-0-unstable-2024-01-01-dev", + expected: StorePathParts{ + Hash: "0z1zq1zq1zq1zq1zq1zq1zq1zq1zq1zq", + Name: "foo", + Version: "0-unstable-2024-01-01", + Output: "dev", + }, + }, } for _, testCase := range testCases {