[perf] skip forcing printDevEnv when add/rm/update outside shellenv

Author: savil

internal/impl/devbox.go

@@ -168,7 +168,7 @@ 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 {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, ensure); err != nil {
 		return err
 	}
 	fmt.Fprintln(d.stderr, "Starting a devbox shell...")
@@ -210,7 +210,7 @@ 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 {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, ensure); err != nil {
 		return err
 	}
 
@@ -260,7 +260,7 @@ func (d *Devbox) Install(ctx context.Context) error {
 	ctx, task := trace.NewTask(ctx, "devboxInstall")
 	defer task.End()
 
-	return d.ensurePackagesAreInstalled(ctx, ensure)
+	return d.ensureDevboxEnvIsUpToDate(ctx, ensure)
 }
 
 func (d *Devbox) ListScripts() []string {
@@ -278,7 +278,7 @@ func (d *Devbox) NixEnv(ctx context.Context, includeHooks bool) (string, error)
 	ctx, task := trace.NewTask(ctx, "devboxNixEnv")
 	defer task.End()
 
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, ensure); err != nil {
 		return "", err
 	}
 
@@ -302,7 +302,7 @@ func (d *Devbox) EnvVars(ctx context.Context) ([]string, error) {
 	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 {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, ensure); err != nil {
 		return nil, err
 	}
 
@@ -466,7 +466,7 @@ func (d *Devbox) GenerateEnvrcFile(ctx context.Context, force bool, envFlags dev
 	}
 
 	// generate all shell files to ensure we can refer to them in the .envrc script
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, ensure); err != nil {
 		return err
 	}
 
@@ -1156,10 +1156,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

@@ -108,7 +108,7 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string,
 		}
 	}
 
-	// Resolving here ensures we allow insecure before running ensurePackagesAreInstalled
+	// Resolving here ensures we allow insecure before running ensureDevboxEnvIsUpToDate
 	// which will call print-dev-env. Resolving does not save the lockfile, we
 	// save at the end when everything has succeeded.
 	if d.allowInsecureAdds {
@@ -126,7 +126,7 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string,
 		}
 	}
 
-	if err := d.ensurePackagesAreInstalled(ctx, install); err != nil {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, install); err != nil {
 		return usererr.WithUserMessage(err, "There was an error installing nix packages")
 	}
 
@@ -190,28 +190,35 @@ func (d *Devbox) Remove(ctx context.Context, pkgs ...string) error {
 	}
 
 	// this will clean up the now-extra package from nix profile and the lockfile
-	if err := d.ensurePackagesAreInstalled(ctx, uninstall); err != nil {
+	if err := d.ensureDevboxEnvIsUpToDate(ctx, uninstall); err != nil {
 		return err
 	}
 
 	return d.saveCfg()
 }
 
-// installMode is an enum for helping with ensurePackagesAreInstalled implementation
+// installMode is an enum for helping with ensureDevboxEnvIsUpToDate implementation
 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.
-func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMode) error {
+// ensureDevboxEnvIsUpToDate 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.
+func (d *Devbox) ensureDevboxEnvIsUpToDate(ctx context.Context, mode installMode) error {
 	defer trace.StartRegion(ctx, "ensurePackages").End()
 	defer debug.FunctionTimer().End()
 
@@ -248,9 +255,15 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 		return err
 	}
 
-	fmt.Fprintf(d.stderr, "Recomputing the devbox environment.\n")
+	// We can use the print-dev-env cache 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()
+	if !usePrintDevEnvCache {
+		fmt.Fprintf(d.stderr, "Recomputing the devbox environment.\n")
+	}
+
 	// Force print-dev-env cache to be recomputed.
-	nixEnv, err := d.computeNixEnv(ctx, false /*use cache*/)
+	nixEnv, err := d.computeNixEnv(ctx, usePrintDevEnvCache)
 	if err != nil {
 		return 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.ensureDevboxEnvIsUpToDate(ctx, update); err != nil {
 		return err
 	}