[update] UX fixes to update path (#1129)

## Summary

Fixes/refactors:

* If legacy package is global, show correct update message
* If a legacy package is updated, don't try to update it again (which
will show warning because it is not versioned)
* Made `ShowWarnings` -> `IgnoreWarnings` because we ignore in the vast
majority of cases. Fixed a few cases that were accidentally ignored.

## How was it tested?

* Modified global config to have "curl" pacakge.
* Saw `devbox global update` and ran it.
* Inspected config.

Repeated process for non-global as well.

Author: mikeland73

devbox.go

@@ -58,12 +58,7 @@ type Devbox interface {
 
 // Open opens a devbox by reading the config file in dir.
 func Open(opts *devopt.Opts) (Devbox, error) {
-	return impl.Open(&devopt.Opts{
-		Dir:          opts.Dir,
-		Pure:         opts.Pure,
-		ShowWarnings: opts.ShowWarnings,
-		Writer:       opts.Writer,
-	})
+	return impl.Open(opts)
 }
 
 // InitConfig creates a default devbox config file if one doesn't already exist.

internal/boxcli/midcobra/telemetry.go

@@ -224,9 +224,9 @@ func getPackagesAndCommitHash(c *cobra.Command) ([]string, string) {
 	}
 
 	box, err := devbox.Open(&devopt.Opts{
-		Dir:          path,
-		Writer:       os.Stdout,
-		ShowWarnings: false,
+		Dir:            path,
+		Writer:         os.Stdout,
+		IgnoreWarnings: true,
 	})
 	if err != nil {
 		return []string{}, ""

internal/boxcli/run.go

@@ -49,10 +49,10 @@ func runCmd() *cobra.Command {
 
 func listScripts(cmd *cobra.Command, flags runCmdFlags) []string {
 	box, err := devbox.Open(&devopt.Opts{
-		Dir:          flags.config.path,
-		Writer:       cmd.ErrOrStderr(),
-		Pure:         flags.pure,
-		ShowWarnings: false,
+		Dir:            flags.config.path,
+		Writer:         cmd.ErrOrStderr(),
+		Pure:           flags.pure,
+		IgnoreWarnings: true,
 	})
 	if err != nil {
 		debug.Log("failed to open devbox: %v", err)

internal/impl/devbox.go

@@ -104,14 +104,19 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
 	)
 	box.lockfile = lock
 
-	if opts.ShowWarnings &&
+	if !opts.IgnoreWarnings &&
 		!legacyPackagesWarningHasBeenShown &&
 		box.HasDeprecatedPackages() {
 		legacyPackagesWarningHasBeenShown = true
+		globalPath, err := GlobalDataPath()
+		if err != nil {
+			return nil, err
+		}
 		ux.Fwarning(
 			os.Stderr, // Always stderr. box.writer should probably always be err.
 			"Your devbox.json contains packages in legacy format. "+
-				"Please run `devbox update` to update your devbox.json.\n",
+				"Please run `devbox %supdate` to update your devbox.json.\n",
+			lo.Ternary(box.projectDir == globalPath, "global ", ""),
 		)
 	}
 

internal/impl/devbox_test.go

@@ -43,10 +43,9 @@ func testShellPlan(t *testing.T, testPath string) {
 		assert := assert.New(t)
 
 		_, err := Open(&devopt.Opts{
-			Dir:          baseDir,
-			Writer:       os.Stdout,
-			ShowWarnings: true,
-			Pure:         false,
+			Dir:    baseDir,
+			Writer: os.Stdout,
+			Pure:   false,
 		})
 		assert.NoErrorf(err, "%s should be a valid devbox project", baseDir)
 	})
@@ -72,10 +71,9 @@ func TestComputeNixEnv(t *testing.T) {
 	_, err := devconfig.Init(path, os.Stdout)
 	require.NoError(t, err, "InitConfig should not fail")
 	d, err := Open(&devopt.Opts{
-		Dir:          path,
-		Writer:       os.Stdout,
-		ShowWarnings: true,
-		Pure:         false,
+		Dir:    path,
+		Writer: os.Stdout,
+		Pure:   false,
 	})
 	require.NoError(t, err, "Open should not fail")
 	d.nix = &testNix{}
@@ -90,10 +88,9 @@ func TestComputeNixPathIsIdempotent(t *testing.T) {
 	_, err := devconfig.Init(dir, os.Stdout)
 	require.NoError(t, err, "InitConfig should not fail")
 	devbox, err := Open(&devopt.Opts{
-		Dir:          dir,
-		Writer:       os.Stdout,
-		ShowWarnings: true,
-		Pure:         false,
+		Dir:    dir,
+		Writer: os.Stdout,
+		Pure:   false,
 	})
 	require.NoError(t, err, "Open should not fail")
 	devbox.nix = &testNix{"/tmp/my/path"}
@@ -121,10 +118,9 @@ func TestComputeNixPathWhenRemoving(t *testing.T) {
 	_, err := devconfig.Init(dir, os.Stdout)
 	require.NoError(t, err, "InitConfig should not fail")
 	devbox, err := Open(&devopt.Opts{
-		Dir:          dir,
-		Writer:       os.Stdout,
-		ShowWarnings: true,
-		Pure:         false,
+		Dir:    dir,
+		Writer: os.Stdout,
+		Pure:   false,
 	})
 	require.NoError(t, err, "Open should not fail")
 	devbox.nix = &testNix{"/tmp/my/path"}

internal/impl/devopt/devboxopts.go

@@ -3,8 +3,8 @@ package devopt
 import "io"
 
 type Opts struct {
-	Dir          string
-	Pure         bool
-	ShowWarnings bool
-	Writer       io.Writer
+	Dir            string
+	Pure           bool
+	IgnoreWarnings bool
+	Writer         io.Writer
 }

internal/impl/update.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"fmt"
 
-	"go.jetpack.io/devbox/internal/lock"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/ux"
@@ -20,6 +19,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
 		return err
 	}
 
+	pendingPackagesToUpdate := []*nix.Input{}
 	for _, pkg := range inputs {
 		if pkg.IsLegacy() {
 			fmt.Fprintf(d.writer, "Updating %s -> %s\n", pkg.Raw, pkg.LegacyToVersioned())
@@ -34,14 +34,12 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
 			if err := d.Add(ctx, pkg.LegacyToVersioned()); err != nil {
 				return err
 			}
+		} else {
+			pendingPackagesToUpdate = append(pendingPackagesToUpdate, pkg)
 		}
 	}
 
-	for _, pkg := range inputs {
-		if !lock.IsVersionedPackage(pkg.Raw) {
-			fmt.Fprintf(d.writer, "Skipping %s because it is not a versioned package\n", pkg)
-			continue
-		}
+	for _, pkg := range pendingPackagesToUpdate {
 		existing := d.lockfile.Packages[pkg.Raw]
 		newEntry, err := searcher.Client().Resolve(pkg.Raw)
 		if err != nil {