[update] Show warning for legacy packages and fix with update (#1107)

mikeland73 · web-flow · commit 85f832ee0c9a · 2023-06-07T12:47:33.000-07:00
## Summary Add warning if devbox.json contains legacy packages. Fix them with update command. Update will turn legacy packages to `<pkg>@latest` and resolve them to current hash. This guarantees version will not change. If they run update again, it will update to newest version. ## How was it tested? <img width="840" alt="image" src="https://github.com/jetpack-io/devbox/assets/544948/fe612548-356a-4ed5-810a-78a3942913d1"> ```bash # modified devbox.json to have `curl` package. devbox update ``` Inspected devbox.json and devbox.lock and verified it was now `curl@latest`
diff --git a/devbox.go b/devbox.go
@@ -34,7 +34,6 @@ type Devbox interface {
 	ListScripts() []string
 	PrintEnv(ctx context.Context, includeHooks bool) (string, error)
 	PrintGlobalList() error
-	PrintEnvrcContent(w io.Writer) error
 	Pull(ctx context.Context, overwrite bool, path string) error
 	Push(url string) error
 	// Remove removes Nix packages from the config so that it no longer exists in
@@ -58,7 +57,11 @@ type Devbox interface {
 
 // Open opens a devbox by reading the config file in dir.
 func Open(dir string, writer io.Writer) (Devbox, error) {
-	return impl.Open(dir, writer)
+	return impl.Open(dir, writer, true)
+}
+
+func OpenWithoutWarnings(dir string, writer io.Writer) (Devbox, error) {
+	return impl.Open(dir, writer, false)
 }
 
 // InitConfig creates a default devbox config file if one doesn't already exist.
@@ -69,3 +72,7 @@ func InitConfig(dir string, writer io.Writer) (bool, error) {
 func GlobalDataPath() (string, error) {
 	return impl.GlobalDataPath()
 }
+
+func PrintEnvrcContent(w io.Writer) error {
+	return impl.PrintEnvrcContent(w)
+}
diff --git a/internal/boxcli/featureflag/autolatest.go b/internal/boxcli/featureflag/autolatest.go
diff --git a/internal/boxcli/generate.go b/internal/boxcli/generate.go
@@ -92,7 +92,7 @@ func direnvCmd() *cobra.Command {
 			"Requires direnv to be installed.",
 		Args: cobra.MaximumNArgs(0),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runGenerateCmd(cmd, flags)
+			return runGenerateDirenvCmd(cmd, flags)
 		},
 	}
 	command.Flags().BoolVarP(
@@ -140,11 +140,19 @@ func runGenerateCmd(cmd *cobra.Command, flags *generateCmdFlags) error {
 		return box.GenerateDevcontainer(flags.force)
 	case "dockerfile":
 		return box.GenerateDockerfile(flags.force)
-	case "direnv":
-		if flags.printEnvrcContent {
-			return box.PrintEnvrcContent(cmd.OutOrStdout())
-		}
-		return box.GenerateEnvrcFile(flags.force)
 	}
 	return nil
 }
+
+func runGenerateDirenvCmd(cmd *cobra.Command, flags *generateCmdFlags) error {
+	if flags.printEnvrcContent {
+		return devbox.PrintEnvrcContent(cmd.OutOrStdout())
+	}
+
+	box, err := devbox.Open(flags.config.path, cmd.ErrOrStderr())
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
+	return box.GenerateEnvrcFile(flags.force)
+}
diff --git a/internal/boxcli/midcobra/telemetry.go b/internal/boxcli/midcobra/telemetry.go
@@ -222,7 +222,7 @@ func getPackagesAndCommitHash(c *cobra.Command) ([]string, string) {
 		path = configFlag.Value.String()
 	}
 
-	box, err := devbox.Open(path, os.Stdout)
+	box, err := devbox.OpenWithoutWarnings(path, os.Stdout)
 	if err != nil {
 		return []string{}, ""
 	}
diff --git a/internal/boxcli/run.go b/internal/boxcli/run.go
@@ -44,7 +44,7 @@ func runCmd() *cobra.Command {
 }
 
 func listScripts(cmd *cobra.Command, flags runCmdFlags) []string {
-	box, err := devbox.Open(flags.config.path, cmd.ErrOrStderr())
+	box, err := devbox.OpenWithoutWarnings(flags.config.path, cmd.ErrOrStderr())
 	if err != nil {
 		debug.Log("failed to open devbox: %v", err)
 		return nil
diff --git a/internal/boxcli/shellenv.go b/internal/boxcli/shellenv.go
@@ -13,6 +13,7 @@ import (
 type shellEnvCmdFlags struct {
 	config               configFlags
 	runInitHook          bool
+	install              bool
 	useCachedPrintDevEnv bool
 }
 
@@ -36,6 +37,8 @@ func shellEnvCmd() *cobra.Command {
 
 	command.Flags().BoolVar(
 		&flags.runInitHook, "init-hook", false, "runs init hook after exporting shell environment")
+	command.Flags().BoolVar(
+		&flags.install, "install", false, "install packages before exporting shell environment")
 
 	// This is no longer used. Remove after 0.4.8 is released.
 	command.Flags().BoolVar(
@@ -56,6 +59,12 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
 		return "", err
 	}
 
+	if flags.install {
+		if err := box.Install(cmd.Context()); err != nil {
+			return "", err
+		}
+	}
+
 	envStr, err := box.PrintEnv(cmd.Context(), flags.runInitHook)
 	if err != nil {
 		return "", err
diff --git a/internal/boxcli/update.go b/internal/boxcli/update.go
@@ -22,7 +22,8 @@ func updateCmd() *cobra.Command {
 		Short: "Update packages in your devbox",
 		Long: "Update one, many, or all packages in your devbox. " +
 			"If no packages are specified, all packages will be updated. " +
-			"Only update versioned packages (e.g. `python@3.11`), not packages that are pinned to a nix channel (e.g. `python3`)",
+			"Legacy non-versioned packages will be converted to @latest versioned " +
+			"packages resolved to their current version.",
 		PreRunE: ensureNixInstalled,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return updateCmdFunc(cmd, args, flags)
diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go
@@ -70,7 +70,9 @@ type Devbox struct {
 	writer io.Writer
 }
 
-func Open(path string, writer io.Writer) (*Devbox, error) {
+var legacyPackagesWarningHasBeenShown = false
+
+func Open(path string, writer io.Writer, showWarnings bool) (*Devbox, error) {
 	projectDir, err := findProjectDir(path)
 	if err != nil {
 		return nil, err
@@ -98,6 +100,18 @@ func Open(path string, writer io.Writer) (*Devbox, error) {
 		plugin.WithLockfile(lock),
 	)
 	box.lockfile = lock
+
+	if showWarnings &&
+		!legacyPackagesWarningHasBeenShown &&
+		box.HasDeprecatedPackages() {
+		legacyPackagesWarningHasBeenShown = true
+		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",
+		)
+	}
+
 	return box, nil
 }
 
@@ -395,7 +409,7 @@ func (d *Devbox) GenerateDockerfile(force bool) error {
 	return errors.WithStack(generate.CreateDockerfile(tmplFS, d.projectDir, false /* isDevcontainer */))
 }
 
-func (d *Devbox) PrintEnvrcContent(w io.Writer) error {
+func PrintEnvrcContent(w io.Writer) error {
 	tmplName := "envrcContent.tmpl"
 	t := template.Must(template.ParseFS(tmplFS, "tmpl/"+tmplName))
 	// write content into file
@@ -977,6 +991,15 @@ func (d *Devbox) packagesAsInputs() []*nix.Input {
 	return nix.InputsFromStrings(d.Packages(), d.lockfile)
 }
 
+func (d *Devbox) HasDeprecatedPackages() bool {
+	for _, pkg := range d.packagesAsInputs() {
+		if pkg.IsLegacy() {
+			return true
+		}
+	}
+	return false
+}
+
 func (d *Devbox) findPackageByName(name string) (string, error) {
 	results := map[string]bool{}
 	for _, pkg := range d.cfg.Packages {
diff --git a/internal/impl/devbox_test.go b/internal/impl/devbox_test.go
@@ -41,7 +41,7 @@ func testShellPlan(t *testing.T, testPath string) {
 		t.Setenv(envir.XDGDataHome, "/tmp/devbox")
 		assert := assert.New(t)
 
-		_, err := Open(baseDir, os.Stdout)
+		_, err := Open(baseDir, os.Stdout, true)
 		assert.NoErrorf(err, "%s should be a valid devbox project", baseDir)
 	})
 }
@@ -65,7 +65,7 @@ func TestComputeNixEnv(t *testing.T) {
 	path := t.TempDir()
 	_, err := devconfig.Init(path, os.Stdout)
 	require.NoError(t, err, "InitConfig should not fail")
-	d, err := Open(path, os.Stdout)
+	d, err := Open(path, os.Stdout, true)
 	require.NoError(t, err, "Open should not fail")
 	d.nix = &testNix{}
 	ctx := context.Background()
@@ -78,7 +78,7 @@ func TestComputeNixPathIsIdempotent(t *testing.T) {
 	dir := t.TempDir()
 	_, err := devconfig.Init(dir, os.Stdout)
 	require.NoError(t, err, "InitConfig should not fail")
-	devbox, err := Open(dir, os.Stdout)
+	devbox, err := Open(dir, os.Stdout, true)
 	require.NoError(t, err, "Open should not fail")
 	devbox.nix = &testNix{"/tmp/my/path"}
 	ctx := context.Background()
@@ -104,7 +104,7 @@ func TestComputeNixPathWhenRemoving(t *testing.T) {
 	dir := t.TempDir()
 	_, err := devconfig.Init(dir, os.Stdout)
 	require.NoError(t, err, "InitConfig should not fail")
-	devbox, err := Open(dir, os.Stdout)
+	devbox, err := Open(dir, os.Stdout, true)
 	require.NoError(t, err, "Open should not fail")
 	devbox.nix = &testNix{"/tmp/my/path"}
 	ctx := context.Background()
diff --git a/internal/impl/tmpl/envrcContent.tmpl b/internal/impl/tmpl/envrcContent.tmpl
@@ -1,6 +1,5 @@
 use_devbox() {
     watch_file devbox.json
-    eval "$(devbox shellenv --init-hook)"
-    devbox install
+    eval "$(devbox shellenv --init-hook --install)"
 }
 use devbox
diff --git a/internal/impl/update.go b/internal/impl/update.go
@@ -8,37 +8,48 @@ import (
 	"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"
 	"go.jetpack.io/devbox/internal/wrapnix"
 )
 
 func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
-	var pkgsToUpdate []string
-	for _, pkg := range pkgs {
-		found, err := d.findPackageByName(pkg)
-		if err != nil {
-			return err
-		}
-		pkgsToUpdate = append(pkgsToUpdate, found)
+	inputs, err := d.inputsToUpdate(pkgs...)
+	if err != nil {
+		return err
 	}
-	if len(pkgsToUpdate) == 0 {
-		pkgsToUpdate = d.Packages()
+
+	for _, pkg := range inputs {
+		if pkg.IsLegacy() {
+			fmt.Fprintf(d.writer, "Updating %s -> %s\n", pkg.Raw, pkg.LegacyToVersioned())
+			if err := d.Remove(ctx, pkg.Raw); err != nil {
+				return err
+			}
+			if err := d.lockfile.ResolveToCurrentNixpkgCommitHash(
+				pkg.LegacyToVersioned(),
+			); err != nil {
+				return err
+			}
+			if err := d.Add(ctx, pkg.LegacyToVersioned()); err != nil {
+				return err
+			}
+		}
 	}
 
-	for _, pkg := range pkgsToUpdate {
-		if !lock.IsVersionedPackage(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
 		}
-		existing := d.lockfile.Packages[pkg]
-		newEntry, err := searcher.Client().Resolve(pkg)
+		existing := d.lockfile.Packages[pkg.Raw]
+		newEntry, err := searcher.Client().Resolve(pkg.Raw)
 		if err != nil {
 			return err
 		}
 		if existing != nil && existing.Version != newEntry.Version {
 			fmt.Fprintf(d.writer, "Updating %s %s -> %s\n", pkg, existing.Version, newEntry.Version)
-			if err := d.removePackagesFromProfile(ctx, []string{pkg}); err != nil {
+			if err := d.removePackagesFromProfile(ctx, []string{pkg.Raw}); err != nil {
 				// Warn but continue. TODO(landau): ensurePackagesAreInstalled should
 				// 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)
@@ -49,7 +60,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
 			fmt.Fprintf(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] = newEntry
+		d.lockfile.Packages[pkg.Raw] = newEntry
 	}
 
 	// TODO(landau): Improve output
@@ -59,3 +70,19 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
 
 	return wrapnix.CreateWrappers(ctx, d)
 }
+
+func (d *Devbox) inputsToUpdate(pkgs ...string) ([]*nix.Input, error) {
+	var pkgsToUpdate []string
+	for _, pkg := range pkgs {
+		found, err := d.findPackageByName(pkg)
+		if err != nil {
+			return nil, err
+		}
+		pkgsToUpdate = append(pkgsToUpdate, found)
+	}
+	if len(pkgsToUpdate) == 0 {
+		pkgsToUpdate = d.Packages()
+	}
+
+	return nix.InputsFromStrings(pkgsToUpdate, d.lockfile), nil
+}
diff --git a/internal/lock/lockfile.go b/internal/lock/lockfile.go
@@ -94,6 +94,17 @@ func (l *File) ForceResolve(pkg string) (*Package, error) {
 	return l.Resolve(pkg)
 }
 
+func (l *File) ResolveToCurrentNixpkgCommitHash(pkg string) error {
+	name, version, found := strings.Cut(pkg, "@")
+	if found && version != "latest" {
+		return errors.New(
+			"only allowed version is @latest. Otherwise we can't guarantee the " +
+				"version will resolve")
+	}
+	l.Packages[pkg] = &Package{Resolved: l.LegacyNixpkgsPath(name)}
+	return nil
+}
+
 func (l *File) Save() error {
 	// Never write lockfile if versioned packages is not enabled
 	if !featureflag.LockFile.Enabled() {
diff --git a/internal/nix/input.go b/internal/nix/input.go

Original file line number	Diff line number	Diff line change
`@@ -222,7 +222,7 @@ func getPackagesAndCommitHash(c *cobra.Command) ([]string, string) {`
`222`	`222`	`path = configFlag.Value.String()`
`223`	`223`	`}`
`224`	`224`
`225`		`- box, err := devbox.Open(path, os.Stdout)`
	`225`	`+ box, err := devbox.OpenWithoutWarnings(path, os.Stdout)`
`226`	`226`	`if err != nil {`
`227`	`227`	`return []string{}, ""`
`228`	`228`	`}`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ func runCmd() *cobra.Command {`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`func listScripts(cmd *cobra.Command, flags runCmdFlags) []string {`
`47`		`- box, err := devbox.Open(flags.config.path, cmd.ErrOrStderr())`
	`47`	`+ box, err := devbox.OpenWithoutWarnings(flags.config.path, cmd.ErrOrStderr())`
`48`	`48`	`if err != nil {`
`49`	`49`	`debug.Log("failed to open devbox: %v", err)`
`50`	`50`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,5 @@`
`1`	`1`	`use_devbox() {`
`2`	`2`	`watch_file devbox.json`
`3`		`- eval "$(devbox shellenv --init-hook)"`
`4`		`- devbox install`
	`3`	`+ eval "$(devbox shellenv --init-hook --install)"`
`5`	`4`	`}`
`6`	`5`	`use devbox`