[perf] skip forcing printDevEnv when add/rm/update outside shellenv (#1540)

## Summary

In this PR, we skip forcing `computeNixEnv` if a user is
adding/removing/updating packages, AND if we are not in a
shellenv-enabled environment from that devbox project.

Also:
- ~rename `ensurePackagesAreInstalled` to `ensureDevboxEnvIsUpToDate`~
- some minor comment cleanups

## How was it tested?

did `devbox add vim` when:
1. in devbox project's directory, but direnv disabled => did not print
"Recomputing Devbox environment."
2. in devbox project's directory with direnv enabled => did print
"Recomputing Devbox environment".
3. in devbox shell => did print again

Repeated the same exercise with `devbox global`:
1. with `devbox global shellenv | source` run in shellrc => did print
"Recomputing Devbox environment".
2. after commenting out that line in shellrc => did not print
NOTE: there's no special devbox-global specific code change here.

Author: savil

internal/impl/devbox.go

@@ -1172,10 +1172,10 @@ func (d *Devbox) parseEnvAndExcludeSpecialCases(currentEnv []string) (map[string
 		if ignoreCurrentEnvVar[key] {
 			continue
 		}
-		// handling special cases to for pure shell
-		// Passing HOME for pure shell to leak through otherwise devbox binary won't work
-		// We also include PATH to find the nix installation. It is cleaned for pure mode below
-		// TERM leaking through is to enable colored text in the pure shell
+		// handling special cases for pure shell
+		// - HOME required for devbox binary to work
+		// - PATH to find the nix installation. It is cleaned for pure mode below.
+		// - TERM to enable colored text in the pure shell
 		if !d.pure || key == "HOME" || key == "PATH" || key == "TERM" {
 			env[key] = val
 		}

internal/impl/envvars.go

@@ -69,8 +69,8 @@ func markEnvsAsSetByDevbox(envs ...map[string]string) {
 	}
 }
 
-// IsEnvEnabled checks if the devbox environment is enabled. We use the ogPathKey
-// as a proxy for this. This allows us to differentiate between global and
+// IsEnvEnabled checks if the devbox environment is enabled.
+// This allows us to differentiate between global and
 // individual project shells.
 func (d *Devbox) IsEnvEnabled() bool {
 	fakeEnv := map[string]string{}

internal/impl/packages.go

@@ -203,14 +203,22 @@ type installMode string
 const (
 	install   installMode = "install"
 	uninstall installMode = "uninstall"
-	ensure    installMode = "ensure"
+	// update is both install new package version and uninstall old package version
+	update installMode = "update"
+	ensure installMode = "ensure"
 )
 
-// ensurePackagesAreInstalled ensures that the nix profile has the packages specified
-// in the config (devbox.json). The `mode` is used for user messaging to explain
-// what operations are happening, because this function may take time to execute.
-// TODO we should rename this to ensureDevboxEnvironmentIsUpToDate since it does
-// much more than ensuring packages are installed.
+// ensurePackagesAreInstalled ensures:
+//  1. Packages are installed, in nix-profile or runx.
+//     Extraneous packages are removed (references purged, not uninstalled).
+//  2. Files for devbox shellenv are generated
+//  3. Env-vars for shellenv are computed
+//  4. Lockfile is synced
+//
+// The `mode` is used for:
+// 1. Skipping certain operations that may not apply.
+// 2. User messaging to explain what operations are happening, because this function may take time to execute.
+// TODO: Rename method since it does more than just ensure packages are installed.
 func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMode) error {
 	defer trace.StartRegion(ctx, "ensurePackages").End()
 	defer debug.FunctionTimer().End()
@@ -248,8 +256,10 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 		return err
 	}
 
-	// Force print-dev-env cache to be recomputed.
-	nixEnv, err := d.computeNixEnv(ctx, false /*use cache*/)
+	// Use the printDevEnvCache if we are adding or removing or updating any package,
+	// AND we are not in the shellenv-enabled environment of the current devbox-project.
+	usePrintDevEnvCache := mode != ensure && !d.IsEnvEnabled()
+	nixEnv, err := d.computeNixEnv(ctx, usePrintDevEnvCache)
 	if err != nil {
 		return err
 	}

internal/impl/shell_test.go

@@ -17,8 +17,8 @@ import (
 	"go.jetpack.io/devbox/internal/shellgen"
 )
 
-// update overwrites golden files with the new test results.
-var update = flag.Bool("update", false, "update the golden files with the test results")
+// updateFlag overwrites golden files with the new test results.
+var updateFlag = flag.Bool("update", false, "update the golden files with the test results")
 
 func TestWriteDevboxShellrc(t *testing.T) {
 	testdirs, err := filepath.Glob("testdata/shellrc/*")
@@ -83,7 +83,7 @@ func testWriteDevboxShellrc(t *testing.T, testdirs []string) {
 			// Overwrite the golden file if the -update flag was
 			// set, and then proceed normally. The test should
 			// always pass in this case.
-			if *update {
+			if *updateFlag {
 				err = os.WriteFile(test.goldShellrcPath, gotShellrc, 0o666)
 				if err != nil {
 					t.Error("Error updating golden files:", err)

internal/impl/update.go

@@ -62,7 +62,7 @@ func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
 		}
 	}
 
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
+	if err := d.ensurePackagesAreInstalled(ctx, update); err != nil {
 		return err
 	}