Skip to content

[search] Update search endpoints #1249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions devbox.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
"version": "1.6.23"
},
"[email protected]": {
"last_modified": "2023-05-01T16:53:22Z",
"resolved": "github:NixOS/nixpkgs/8670e496ffd093b60e74e7fa53526aa5920d09eb#go",
"version": "1.20.3"
"last_modified": "2023-05-25T03:54:59Z",
"resolved": "github:NixOS/nixpkgs/8d4d822bc0efa9de6eddc79cb0d82897a9baa750#go",
"version": "1.20.4"
},
"[email protected]": {
"last_modified": "2023-05-01T16:53:22Z",
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func addCmd() *cobra.Command {
}
err := addCmdFunc(cmd, args, flags)
if errors.Is(err, nix.ErrPackageNotFound) {
return usererr.New("%s\n\n%s", err, toSearchForPackages)
return usererr.WithUserMessage(err, toSearchForPackages)
}
return err
},
Expand Down
97 changes: 95 additions & 2 deletions internal/boxcli/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,115 @@
package boxcli

import (
"fmt"
"io"
"math"
"strings"

"github.com/spf13/cobra"

"go.jetpack.io/devbox/internal/boxcli/usererr"
"go.jetpack.io/devbox/internal/searcher"
"go.jetpack.io/devbox/internal/ux"
)

type searchCmdFlags struct {
showAll bool
}

func searchCmd() *cobra.Command {
flags := &searchCmdFlags{}
command := &cobra.Command{
Use: "search <pkg>",
Short: "Search for nix packages",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
ux.Fwarning(cmd.ErrOrStderr(), "Search is experimental and may not work as expected.\n\n")
return searcher.SearchAndPrint(cmd.OutOrStdout(), args[0])
query := args[0]
name, version, isVersioned := searcher.ParseVersionedPackage(query)
if !isVersioned {
results, err := searcher.Client().Search(query)
if err != nil {
return err
}
return printSearchResults(
cmd.OutOrStdout(), query, results, flags.showAll)
}
packageVersion, err := searcher.Client().Resolve(name, version)
if err != nil {
// This is not ideal. Search service should return valid response we
// can parse
return usererr.WithUserMessage(err, "No results found for %q\n", query)
}
fmt.Fprintf(
cmd.OutOrStdout(),
"%s resolves to: %s@%s\n",
query,
packageVersion.Name,
packageVersion.Version,
)
return nil
},
}

command.Flags().BoolVar(
&flags.showAll, "show-all", false,
"show all available templates",
)

return command
}

func printSearchResults(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the Test Plan, it would be good to copy-paste examples of the output from this function. It's hard to imagine in my head...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of get it...but for future PRs, consider this a request.

w io.Writer,
query string,
results *searcher.SearchResults,
showAll bool,
) error {
if len(results.Packages) == 0 {
fmt.Fprintf(w, "No results found for %q\n", query)
return nil
}
fmt.Fprintf(
w,
"Found %d+ results for %q:\n\n",
results.NumResults,
query,
)

resultsAreTrimmed := false
pkgs := results.Packages
if !showAll && len(pkgs) > 10 {
resultsAreTrimmed = true
pkgs = results.Packages[:int(math.Min(10, float64(len(results.Packages))))]
}

for _, pkg := range pkgs {
nonEmptyVersions := []string{}
for i, v := range pkg.Versions {
if !showAll && i >= 10 {
resultsAreTrimmed = true
break
}
if v.Version != "" {
nonEmptyVersions = append(nonEmptyVersions, v.Version)
}
}

versionString := ""
if len(nonEmptyVersions) > 0 {
versionString = fmt.Sprintf(" (%s)", strings.Join(nonEmptyVersions, ", "))
}
fmt.Fprintf(w, "* %s %s\n", pkg.Name, versionString)
}

if resultsAreTrimmed {
fmt.Println()
ux.Fwarning(
w,
"Showing top 10 results and truncated versions. Use --show-all to "+
"show all.\n\n",
)
}

return nil
}
3 changes: 2 additions & 1 deletion internal/devpkg/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ func (p *Package) PathInBinaryStore() (string, error) {
if isInStore, err := p.IsInBinaryStore(); err != nil {
return "", err
} else if !isInStore {
return "", errors.Errorf("Package %q cannot be fetched from binary cache store", p.Raw)
return "",
errors.Errorf("Package %q cannot be fetched from binary cache store", p.Raw)
}

entry, err := p.lockfile.Resolve(p.Raw)
Expand Down
3 changes: 1 addition & 2 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"go.jetpack.io/devbox/internal/nix"
"go.jetpack.io/devbox/internal/plugin"
"go.jetpack.io/devbox/internal/redact"
"go.jetpack.io/devbox/internal/searcher"
"go.jetpack.io/devbox/internal/services"
"go.jetpack.io/devbox/internal/ux"
"go.jetpack.io/devbox/internal/wrapnix"
Expand Down Expand Up @@ -90,7 +89,7 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
pure: opts.Pure,
}

lock, err := lock.GetFile(box, searcher.Client())
lock, err := lock.GetFile(box)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames ...string) error {
return err
}
if !ok {
return errors.WithMessage(nix.ErrPackageNotFound, pkg.Raw)
return errors.Wrap(nix.ErrPackageNotFound, pkg.Raw)
}
}

Expand Down
10 changes: 5 additions & 5 deletions internal/impl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"

"go.jetpack.io/devbox/internal/devpkg"
"go.jetpack.io/devbox/internal/devpkg/devpkgutil"
"go.jetpack.io/devbox/internal/nix"
"go.jetpack.io/devbox/internal/nix/nixprofile"
"go.jetpack.io/devbox/internal/searcher"
Expand Down Expand Up @@ -44,7 +43,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
}

for _, pkg := range pendingPackagesToUpdate {
if _, _, isVersioned := devpkgutil.ParseVersionedPackage(pkg.Raw); !isVersioned {
if _, _, isVersioned := searcher.ParseVersionedPackage(pkg.Raw); !isVersioned {
if err = d.attemptToUpgradeFlake(pkg); err != nil {
return err
}
Expand Down Expand Up @@ -87,7 +86,7 @@ func (d *Devbox) updateDevboxPackage(
pkg *devpkg.Package,
) error {
existing := d.lockfile.Packages[pkg.Raw]
newEntry, err := searcher.Client().Resolve(pkg.Raw)
newEntry, err := d.lockfile.FetchResolvedPackage(pkg.Raw)
if err != nil {
return err
}
Expand All @@ -98,13 +97,14 @@ func (d *Devbox) updateDevboxPackage(
// sync the profile so we don't need to do this manually.
ux.Fwarning(d.writer, "Failed to remove %s from profile: %s\n", pkg, err)
}
d.lockfile.Packages[pkg.Raw] = newEntry
} else if existing == nil {
ux.Finfo(d.writer, "Resolved %s to %[1]s %[2]s\n", pkg, newEntry.Resolved)
d.lockfile.Packages[pkg.Raw] = newEntry
} else {
ux.Finfo(d.writer, "Already up-to-date %s %s\n", pkg, existing.Version)
}
// Set the new entry after we've removed the old package from the profile
d.lockfile.Packages[pkg.Raw] = newEntry

return nil
}

Expand Down
6 changes: 1 addition & 5 deletions internal/lock/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ type devboxProject interface {
ProjectDir() string
}

type resolver interface {
Resolve(pkg string) (*Package, error)
}

type Locker interface {
resolver
LegacyNixpkgsPath(string) string
ProjectDir() string
Resolve(string) (*Package, error)
}
12 changes: 5 additions & 7 deletions internal/lock/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"github.com/pkg/errors"
"github.com/samber/lo"
"go.jetpack.io/devbox/internal/boxcli/featureflag"
"go.jetpack.io/devbox/internal/devpkg/devpkgutil"
"go.jetpack.io/devbox/internal/nix"
"go.jetpack.io/devbox/internal/searcher"

"go.jetpack.io/devbox/internal/cuecfg"
)
Expand All @@ -23,7 +23,6 @@ const lockFileVersion = "1"
// Lightly inspired by package-lock.json
type File struct {
devboxProject
resolver

LockFileVersion string `json:"lockfile_version"`

Expand All @@ -49,10 +48,9 @@ type SystemInfo struct {
ToHash string `json:"to_hash,omitempty"`
}

func GetFile(project devboxProject, resolver resolver) (*File, error) {
func GetFile(project devboxProject) (*File, error) {
lockFile := &File{
devboxProject: project,
resolver: resolver,

LockFileVersion: lockFileVersion,
Packages: map[string]*Package{},
Expand Down Expand Up @@ -101,8 +99,8 @@ func (l *File) Resolve(pkg string) (*Package, error) {
if !hasEntry || entry.Resolved == "" || needsSysInfo {
locked := &Package{}
var err error
if _, _, versioned := devpkgutil.ParseVersionedPackage(pkg); versioned {
locked, err = l.resolver.Resolve(pkg)
if _, _, versioned := searcher.ParseVersionedPackage(pkg); versioned {
locked, err = l.FetchResolvedPackage(pkg)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -161,7 +159,7 @@ func (l *File) LegacyNixpkgsPath(pkg string) string {
// This probably belongs in input.go but can't add it there because it will
// create a circular dependency. We could move Input into own package.
func IsLegacyPackage(pkg string) bool {
_, _, versioned := devpkgutil.ParseVersionedPackage(pkg)
_, _, versioned := searcher.ParseVersionedPackage(pkg)
return !versioned &&
!strings.Contains(pkg, ":") &&
// We don't support absolute paths without "path:" prefix, but adding here
Expand Down
90 changes: 90 additions & 0 deletions internal/lock/resolve.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright 2023 Jetpack Technologies Inc and contributors. All rights reserved.
// Use of this source code is governed by the license in the LICENSE file.

package lock

import (
"fmt"
"time"

"github.com/pkg/errors"
"go.jetpack.io/devbox/internal/boxcli/featureflag"
"go.jetpack.io/devbox/internal/boxcli/usererr"
"go.jetpack.io/devbox/internal/nix"
"go.jetpack.io/devbox/internal/searcher"
"golang.org/x/exp/maps"
)

// FetchResolvedPackage fetches a resolution but does not write it to the lock
// struct. This allows testing new versions of packages without writing to the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how and why would one test new versions of packages?

// lock. This is useful to avoid changing nixpkgs commit hashes when version has
// not changed. This can happen when doing `devbox update` and search has
// a newer hash than the lock file but same version. In that case we don't want
// to update because it would be slow and wasteful.
func (l *File) FetchResolvedPackage(pkg string) (*Package, error) {
name, version, _ := searcher.ParseVersionedPackage(pkg)
if version == "" {
return nil, usererr.New("No version specified for %q.", name)
}

packageVersion, err := searcher.Client().Resolve(name, version)
if err != nil {
return nil, errors.Wrapf(nix.ErrPackageNotFound, "%s@%s", name, version)
}

sysInfos := map[string]*SystemInfo{}
if featureflag.RemoveNixpkgs.Enabled() {
sysInfos = buildLockSystemInfos(packageVersion)
}
packageInfo, err := selectForSystem(packageVersion)
if err != nil {
return nil, err
}

if len(packageInfo.AttrPaths) == 0 {
return nil, fmt.Errorf("no attr paths found for package %q", name)
}

return &Package{
LastModified: time.Unix(int64(packageInfo.LastUpdated), 0).UTC().
Format(time.RFC3339),
Resolved: fmt.Sprintf(
"github:NixOS/nixpkgs/%s#%s",
packageInfo.CommitHash,
packageInfo.AttrPaths[0],
),
Version: packageInfo.Version,
Systems: sysInfos,
}, nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, wow, this is much cleaner than the old code!

Also, a 100 bonus points for having the searcher functions return searcher structs, and transforming them into lock structs here. I was also thinking of doing that.


func selectForSystem(pkg *searcher.PackageVersion) (searcher.PackageInfo, error) {
currentSystem, err := nix.System()
if err != nil {
return searcher.PackageInfo{}, err
}
if pi, ok := pkg.Systems[currentSystem]; ok {
return pi, nil
}
if pi, ok := pkg.Systems["x86_64-linux"]; ok {
return pi, nil
}
if len(pkg.Systems) == 0 {
return searcher.PackageInfo{},
fmt.Errorf("no systems found for package %q", pkg.Name)
}
return maps.Values(pkg.Systems)[0], nil
}

func buildLockSystemInfos(pkg *searcher.PackageVersion) map[string]*SystemInfo {
sysInfos := map[string]*SystemInfo{}
for sysName, sysInfo := range pkg.Systems {
sysInfos[sysName] = &SystemInfo{
System: sysName,
FromHash: sysInfo.StoreHash,
StoreName: sysInfo.StoreName,
StoreVersion: sysInfo.StoreVersion,
}
}
return sysInfos
}
3 changes: 2 additions & 1 deletion internal/nix/nixprofile/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strings"

"github.com/fatih/color"
"github.com/pkg/errors"
"go.jetpack.io/devbox/internal/devpkg"
"go.jetpack.io/devbox/internal/nix"

Expand Down Expand Up @@ -85,7 +86,7 @@ func ProfileListIndex(args *ProfileListIndexArgs) (int, error) {
return item.index, nil
}
}
return -1, nix.ErrPackageNotFound
return -1, errors.Wrap(nix.ErrPackageNotFound, args.Input.String())
}

// NixProfileListItem is a go-struct of a line of printed output from `nix profile list`
Expand Down
Loading