Skip to content
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
23 changes: 21 additions & 2 deletions internal/boxcli/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package boxcli

import (
"fmt"
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -32,8 +33,26 @@ func listCmd() *cobra.Command {
if err != nil {
return errors.WithStack(err)
}
for _, p := range box.AllPackageNamesIncludingRemovedTriggerPackages() {
fmt.Fprintf(cmd.OutOrStdout(), "* %s\n", p)

for _, pkg := range box.AllPackagesIncludingRemovedTriggerPackages() {
resolvedVersion, err := pkg.ResolvedVersion()
if err != nil {
// Continue to print the package even if we can't resolve the version
// so that the user can see the error for this package, as well as get the
// results for the other packages
resolvedVersion = "<error resolving version>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe omit the version from the list and print it on stderr at the end?

}
msg := ""

// Print the resolved version, unless the user has specified a version already
if strings.HasSuffix(pkg.Versioned(), "latest") && resolvedVersion != "" {
// Runx packages have a "v" prefix (why?). Trim for consistency.
Copy link
Contributor

Choose a reason for hiding this comment

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

technically semver has (or can have?) a prefix v

resolvedVersion = strings.TrimPrefix(resolvedVersion, "v")
msg = fmt.Sprintf("* %s - %s\n", pkg.Versioned(), resolvedVersion)
} else {
msg = fmt.Sprintf("* %s\n", pkg.Versioned())
}
fmt.Fprint(cmd.OutOrStdout(), msg)
}
return nil
},
Expand Down
9 changes: 7 additions & 2 deletions internal/devbox/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -842,12 +842,17 @@ func (d *Devbox) flakeDir() string {
// flakes (lockfile vs devbox list)
func (d *Devbox) AllPackageNamesIncludingRemovedTriggerPackages() []string {
result := []string{}
for _, p := range d.cfg.Packages(true /*includeRemovedTriggerPackages*/) {
result = append(result, p.VersionedName())
for _, p := range d.AllPackagesIncludingRemovedTriggerPackages() {
result = append(result, p.Versioned())
}
return result
}

func (d *Devbox) AllPackagesIncludingRemovedTriggerPackages() []*devpkg.Package {
Copy link
Contributor

Choose a reason for hiding this comment

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

Change AllPackageNamesIncludingRemovedTriggerPackages to use this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

There's a small subtle difference that I think is okay in practice, but let me call it out.

  1. In AllPackageNamesIncludingRemovedTriggerPackages, the p.VersionedName() will append the Version if it exists
  2. In devpkg's Package.Versioned(), if the version is missing we'll append @latest

I think this doesn't matter in practice, because we always append @latest during devbox add <name>.

packages := d.cfg.Packages(true /*includeRemovedTriggerPackages*/)
return devpkg.PackagesFromConfig(packages, d.lockfile)
}

// AllPackages returns the packages that are defined in devbox.json and
// recursively added by plugins.
// NOTE: This will not return packages removed by their plugin with the
Expand Down
12 changes: 12 additions & 0 deletions internal/devpkg/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,3 +832,15 @@ func packageInstallErrorHandler(err error, pkg *Package, installableOrEmpty stri

return usererr.WithUserMessage(err, "error installing package %s", pkg.Raw)
}

func (p *Package) ResolvedVersion() (string, error) {
if err := p.resolve(); err != nil {
return "", err
}
Comment on lines +837 to +839
Copy link
Contributor

Choose a reason for hiding this comment

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

If package is not in lockfile this is just current resolved version, but that's probably ok.

lockPackage := p.lockfile.Get(p.Raw)
// Flake packages don't have any values in the lockfile
if lockPackage == nil {
return "", nil
}
return p.lockfile.Get(p.Raw).Version, nil
}
Loading