[global] global shellenv should not recompute env (#1534)

## Summary

This causes the default behavior for `devbox global shellenv` to no
longer recompute environment. It adds a new flag:

`devbox global shellenv -r` which causes the recompute if needed (i.e.
not cached)

Recomputing the environment entails two steps:

* Ensuring everything is installed
* Running print-dev-env

It also removed a bunch of `ensurePackagesAreInstalled` calls that are
now handled by `nixEnv()`. Note that in all cases, we were calling
`ensurePackagesAreInstalled(ensure)` followed by `nixEnv()`

## How was it tested?

* Modified devbox.json manually
* Opened new tab
* Saw message:

```bash
Info: Your devbox environment may be out of date. Please run

eval "$(devbox global shellenv -r)"
```

* Ran `eval "$(devbox global shellenv -r)"`
* Opened another tab and did not see message.

---------

Co-authored-by: Savil Srivastava <[email protected]>

Author: mikeland73

devbox.go

@@ -23,7 +23,7 @@ type Devbox interface {
 	Install(ctx context.Context) error
 	IsEnvEnabled() bool
 	ListScripts() []string
-	NixEnv(ctx context.Context, includeHooks bool) (string, error)
+	NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, error)
 	PackageNames() []string
 	ProjectDir() string
 	Pull(ctx context.Context, opts devopt.PullboxOpts) error

internal/boxcli/global.go

@@ -14,7 +14,12 @@ import (
 	"go.jetpack.io/devbox/internal/ux"
 )
 
+type globalShellEnvCmdFlags struct {
+	recompute bool
+}
+
 func globalCmd() *cobra.Command {
+	globalShellEnvCmdFlags := globalShellEnvCmdFlags{}
 	globalCmd := &cobra.Command{}
 	persistentPreRunE := setGlobalConfigForDelegatedCommands(globalCmd)
 	*globalCmd = cobra.Command{
@@ -28,6 +33,12 @@ func globalCmd() *cobra.Command {
 		PersistentPostRunE: ensureGlobalEnvEnabled,
 	}
 
+	shellEnv := shellEnvCmd(&globalShellEnvCmdFlags.recompute)
+	shellEnv.Flags().BoolVar(
+		&globalShellEnvCmdFlags.recompute, "recompute", false,
+		"Recompute environment if needed",
+	)
+
 	addCommandAndHideConfigFlag(globalCmd, addCmd())
 	addCommandAndHideConfigFlag(globalCmd, installCmd())
 	addCommandAndHideConfigFlag(globalCmd, pathCmd())
@@ -36,7 +47,7 @@ func globalCmd() *cobra.Command {
 	addCommandAndHideConfigFlag(globalCmd, removeCmd())
 	addCommandAndHideConfigFlag(globalCmd, runCmd())
 	addCommandAndHideConfigFlag(globalCmd, servicesCmd(persistentPreRunE))
-	addCommandAndHideConfigFlag(globalCmd, shellEnvCmd())
+	addCommandAndHideConfigFlag(globalCmd, shellEnv)
 	addCommandAndHideConfigFlag(globalCmd, updateCmd())
 
 	// Create list for non-global? Mike: I want it :)

internal/boxcli/root.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/samber/lo"
 	"github.com/spf13/cobra"
 
 	"go.jetpack.io/devbox/internal/boxcli/featureflag"
@@ -72,7 +73,8 @@ func RootCmd() *cobra.Command {
 	command.AddCommand(servicesCmd())
 	command.AddCommand(setupCmd())
 	command.AddCommand(shellCmd())
-	command.AddCommand(shellEnvCmd())
+	// False to avoid recomputing the env in global shellenv:
+	command.AddCommand(shellEnvCmd(lo.ToPtr(false)))
 	command.AddCommand(updateCmd())
 	command.AddCommand(versionCmd())
 	// Preview commands

internal/boxcli/shell.go

@@ -66,7 +66,7 @@ func runShellCmd(cmd *cobra.Command, flags shellCmdFlags) error {
 	if flags.printEnv {
 		// false for includeHooks is because init hooks is not compatible with .envrc files generated
 		// by versions older than 0.4.6
-		script, err := box.NixEnv(cmd.Context(), false /*includeHooks*/)
+		script, err := box.NixEnv(cmd.Context(), devopt.NixEnvOpts{})
 		if err != nil {
 			return err
 		}

internal/boxcli/shellenv.go

@@ -22,15 +22,15 @@ type shellEnvCmdFlags struct {
 	preservePathStack bool
 }
 
-func shellEnvCmd() *cobra.Command {
+func shellEnvCmd(recomputeEnvIfNeeded *bool) *cobra.Command {
 	flags := shellEnvCmdFlags{}
 	command := &cobra.Command{
 		Use:     "shellenv",
 		Short:   "Print shell commands that add Devbox packages to your PATH",
 		Args:    cobra.ExactArgs(0),
 		PreRunE: ensureNixInstalled,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			s, err := shellEnvFunc(cmd, flags)
+			s, err := shellEnvFunc(cmd, flags, *recomputeEnvIfNeeded)
 			if err != nil {
 				return err
 			}
@@ -63,7 +63,11 @@ func shellEnvCmd() *cobra.Command {
 	return command
 }
 
-func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
+func shellEnvFunc(
+	cmd *cobra.Command,
+	flags shellEnvCmdFlags,
+	recomputeEnvIfNeeded bool,
+) (string, error) {
 	env, err := flags.Env(flags.config.path)
 	if err != nil {
 		return "", err
@@ -85,7 +89,10 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
 		}
 	}
 
-	envStr, err := box.NixEnv(cmd.Context(), flags.runInitHook)
+	envStr, err := box.NixEnv(cmd.Context(), devopt.NixEnvOpts{
+		DontRecomputeEnvironment: !recomputeEnvIfNeeded,
+		RunHooks:                 flags.runInitHook,
+	})
 	if err != nil {
 		return "", err
 	}

internal/impl/devbox.go

@@ -168,21 +168,22 @@ func (d *Devbox) Shell(ctx context.Context) error {
 	ctx, task := trace.NewTask(ctx, "devboxShell")
 	defer task.End()
 
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
-		return err
-	}
-	fmt.Fprintln(d.stderr, "Starting a devbox shell...")
-
 	profileDir, err := d.profilePath()
 	if err != nil {
 		return err
 	}
 
-	envs, err := d.nixEnv(ctx)
+	envs, err := d.ensurePackagesAreInstalledAndComputeEnv(ctx)
 	if err != nil {
 		return err
 	}
+
+	fmt.Fprintln(d.stderr, "Starting a devbox shell...")
+
 	// Used to determine whether we're inside a shell (e.g. to prevent shell inception)
+	// TODO: This is likely obsolete but we need to decide what happens when
+	// the user does shell-ception. One option is to leave the current shell and
+	// join a new one (that way they are not in nested shells.)
 	envs[envir.DevboxShellEnabled] = "1"
 
 	if err = createDevboxSymlink(d); err != nil {
@@ -210,15 +211,11 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 	ctx, task := trace.NewTask(ctx, "devboxRun")
 	defer task.End()
 
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
-		return err
-	}
-
 	if err := shellgen.WriteScriptsToFiles(d); err != nil {
 		return err
 	}
 
-	env, err := d.nixEnv(ctx)
+	env, err := d.ensurePackagesAreInstalledAndComputeEnv(ctx)
 	if err != nil {
 		return err
 	}
@@ -274,22 +271,39 @@ func (d *Devbox) ListScripts() []string {
 	return keys
 }
 
-func (d *Devbox) NixEnv(ctx context.Context, includeHooks bool) (string, error) {
+func (d *Devbox) NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, error) {
 	ctx, task := trace.NewTask(ctx, "devboxNixEnv")
 	defer task.End()
 
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
-		return "", err
+	var envs map[string]string
+	var err error
+
+	if opts.DontRecomputeEnvironment {
+		upToDate, _ := d.lockfile.IsUpToDateAndInstalled()
+		if !upToDate {
+			cmd := `eval "$(devbox global shellenv --recompute)"`
+			if strings.HasSuffix(os.Getenv("SHELL"), "fish") {
+				cmd = `devbox global shellenv --recompute | source`
+			}
+			ux.Finfo(
+				d.stderr,
+				"Your devbox environment may be out of date. Please run \n\n%s\n\n",
+				cmd,
+			)
+		}
+
+		envs, err = d.computeNixEnv(ctx, true /*usePrintDevEnvCache*/)
+	} else {
+		envs, err = d.ensurePackagesAreInstalledAndComputeEnv(ctx)
 	}
 
-	envs, err := d.nixEnv(ctx)
 	if err != nil {
 		return "", err
 	}
 
 	envStr := exportify(envs)
 
-	if includeHooks {
+	if opts.RunHooks {
 		hooksStr := ". " + shellgen.ScriptPath(d.ProjectDir(), shellgen.HooksFilename)
 		envStr = fmt.Sprintf("%s\n%s;\n", envStr, hooksStr)
 	}
@@ -301,12 +315,7 @@ func (d *Devbox) EnvVars(ctx context.Context) ([]string, error) {
 	ctx, task := trace.NewTask(ctx, "devboxEnvVars")
 	defer task.End()
 	// this only returns env variables for the shell environment excluding hooks
-	// and excluding "export " prefix in "export key=value" format
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
-		return nil, err
-	}
-
-	envs, err := d.nixEnv(ctx)
+	envs, err := d.ensurePackagesAreInstalledAndComputeEnv(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -903,25 +912,23 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	return env, d.addHashToEnv(env)
 }
 
-// nixEnv is a wrapper around computeNixEnv that returns a cached result if
-// it has previously computed and the local lock file is up to date.
-// Note that this is in-memory cache of the final environment, and not the same
-// as the nix print-dev-env cache which is stored in a file.
-func (d *Devbox) nixEnv(ctx context.Context) (map[string]string, error) {
+// ensurePackagesAreInstalledAndComputeEnv does what it says :P
+func (d *Devbox) ensurePackagesAreInstalledAndComputeEnv(
+	ctx context.Context,
+) (map[string]string, error) {
 	defer debug.FunctionTimer().End()
 
-	usePrintDevEnvCache := false
-
-	// If lockfile is up-to-date, we can use the print-dev-env cache.
-	upToDate, err := d.lockfile.IsUpToDateAndInstalled()
-	if err != nil {
+	// When ensurePackagesAreInstalled is called with ensure=true, it always
+	// returns early if the lockfile is up to date. So we don't need to check here
+	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
 		return nil, err
 	}
-	if upToDate {
-		usePrintDevEnvCache = true
-	}
 
-	return d.computeNixEnv(ctx, usePrintDevEnvCache)
+	// Since ensurePackagesAreInstalled calls computeNixEnv when not up do date,
+	// it's ok to use usePrintDevEnvCache=true here always. This does end up
+	// doing some non-nix work twice if lockfile is not up to date.
+	// TODO: Improve this to avoid extra work.
+	return d.computeNixEnv(ctx, true)
 }
 
 func (d *Devbox) nixPrintDevEnvCachePath() string {

internal/impl/devopt/devboxopts.go

@@ -43,3 +43,8 @@ type UpdateOpts struct {
 	Pkgs                  []string
 	IgnoreMissingPackages bool
 }
+
+type NixEnvOpts struct {
+	DontRecomputeEnvironment bool
+	RunHooks                 bool
+}