[bin-wrappers] Reduce/improve when we create bin wrappers

internal/impl/devbox.go

@@ -43,7 +43,6 @@ import (
 	"go.jetpack.io/devbox/internal/redact"
 	"go.jetpack.io/devbox/internal/services"
 	"go.jetpack.io/devbox/internal/ux"
-	"go.jetpack.io/devbox/internal/wrapnix"
 )
 
 const (
@@ -185,10 +184,6 @@ func (d *Devbox) Shell(ctx context.Context) error {
 	// Used to determine whether we're inside a shell (e.g. to prevent shell inception)
 	envs[envir.DevboxShellEnabled] = "1"
 
-	if err := wrapnix.CreateWrappers(ctx, d); err != nil {
-		return err
-	}
-
 	if err = createDevboxSymlink(d); err != nil {
 		return err
 	}
@@ -231,10 +226,6 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 	// better alternative since devbox run and devbox shell are not the same.
 	env["DEVBOX_SHELL_ENABLED"] = "1"
 
-	if err = wrapnix.CreateWrappers(ctx, d); err != nil {
-		return err
-	}
-
 	// wrap the arg in double-quotes, and escape any double-quotes inside it
 	for idx, arg := range cmdArgs {
 		cmdArgs[idx] = strconv.Quote(arg)
@@ -268,10 +259,7 @@ func (d *Devbox) Install(ctx context.Context) error {
 	ctx, task := trace.NewTask(ctx, "devboxInstall")
 	defer task.End()
 
-	if _, err := d.NixEnv(ctx, false /*includeHooks*/); err != nil {
-		return err
-	}
-	return wrapnix.CreateWrappers(ctx, d)
+	return d.ensurePackagesAreInstalled(ctx, ensure)
 }
 
 func (d *Devbox) ListScripts() []string {
@@ -324,16 +312,7 @@ func (d *Devbox) EnvVars(ctx context.Context) ([]string, error) {
 	return envir.MapToPairs(envs), nil
 }
 
-func (d *Devbox) ShellEnvHash(ctx context.Context) (string, error) {
-	envs, err := d.nixEnv(ctx)
-	if err != nil {
-		return "", err
-	}
-
-	return envs[d.ShellEnvHashKey()], nil
-}
-
-func (d *Devbox) ShellEnvHashKey() string {
+func (d *Devbox) shellEnvHashKey() string {
 	// Don't make this a const so we don't use it by itself accidentally
 	return "__DEVBOX_SHELLENV_HASH_" + d.projectDirHash()
 }
@@ -916,16 +895,12 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	return env, d.addHashToEnv(env)
 }
 
-var nixEnvCache map[string]string
-
-// nixEnv is a wrapper around computeNixEnv that caches the result.
+// 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) {
 	defer debug.FunctionTimer().End()
-	if nixEnvCache != nil {
-		return nixEnvCache, nil
-	}
 
 	usePrintDevEnvCache := false
 
@@ -1120,20 +1095,16 @@ func (d *Devbox) setCommonHelperEnvVars(env map[string]string) {
 	env["LIBRARY_PATH"] = envpath.JoinPathLists(profileLibDir, env["LIBRARY_PATH"])
 }
 
-// NixBins returns the paths to all the nix binaries that are installed by
+// nixBins returns the paths to all the nix binaries that are installed by
 // the flake. If there are conflicts, it returns the first one it finds of a
 // give name. This matches how nix flakes behaves if there are conflicts in
 // buildInputs
-func (d *Devbox) NixBins(ctx context.Context) ([]string, error) {
-	env, err := d.nixEnv(ctx)
-	if err != nil {
-		return nil, err
-	}
+func (d *Devbox) nixBins(env map[string]string) ([]string, error) {
 	dirs := strings.Split(env["buildInputs"], " ")
 	bins := map[string]string{}
 	for _, dir := range dirs {
 		binPath := filepath.Join(dir, "bin")
-		if _, err = os.Stat(binPath); errors.Is(err, fs.ErrNotExist) {
+		if _, err := os.Stat(binPath); errors.Is(err, fs.ErrNotExist) {
 			continue
 		}
 		files, err := os.ReadDir(binPath)
@@ -1157,7 +1128,7 @@ func (d *Devbox) projectDirHash() string {
 func (d *Devbox) addHashToEnv(env map[string]string) error {
 	hash, err := cuecfg.Hash(env)
 	if err == nil {
-		env[d.ShellEnvHashKey()] = hash
+		env[d.shellEnvHashKey()] = hash
 	}
 	return err
 }

internal/impl/packages.go

@@ -134,11 +134,7 @@ func (d *Devbox) Add(ctx context.Context, platforms, excludePlatforms []string,
 		}
 	}
 
-	if err := d.lockfile.Save(); err != nil {
-		return err
-	}
-
-	return wrapnix.CreateWrappers(ctx, d)
+	return d.lockfile.Save()
 }
 
 // Remove removes the `pkgs` from the config (i.e. devbox.json) and nix profile
@@ -183,11 +179,7 @@ func (d *Devbox) Remove(ctx context.Context, pkgs ...string) error {
 		return err
 	}
 
-	if err := d.saveCfg(); err != nil {
-		return err
-	}
-
-	return wrapnix.CreateWrappers(ctx, d)
+	return d.saveCfg()
 }
 
 // installMode is an enum for helping with ensurePackagesAreInstalled implementation
@@ -233,17 +225,27 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 		return err
 	}
 
-	// Force print-dev-env cache to be recomputed.
-	// We may be able to remove this after improving cache hits.
 	fmt.Fprintf(d.stderr, "Recomputing the devbox environment.\n")
-	if _, err := d.computeNixEnv(ctx, false /*use cache*/); err != nil {
+	// Force print-dev-env cache to be recomputed.
+	nixEnv, err := d.computeNixEnv(ctx, false /*use cache*/)
+	if err != nil {
 		return err
 	}
 
 	// Ensure we clean out packages that are no longer needed.
 	d.lockfile.Tidy()
 
-	if err := wrapnix.CreateWrappers(ctx, d); err != nil {
+	nixBins, err := d.nixBins(nixEnv)
+	if err != nil {
+		return err
+	}
+
+	if err := wrapnix.CreateWrappers(ctx, wrapnix.CreateWrappersArgs{
+		NixBins:         nixBins,
+		ProjectDir:      d.projectDir,
+		ShellEnvHash:    nixEnv[d.shellEnvHashKey()],
+		ShellEnvHashKey: d.shellEnvHashKey(),
+	}); err != nil {
 		return err
 	}
 

internal/impl/update.go

@@ -17,7 +17,6 @@ import (
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/shellgen"
 	"go.jetpack.io/devbox/internal/ux"
-	"go.jetpack.io/devbox/internal/wrapnix"
 )
 
 func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
@@ -67,10 +66,6 @@ func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
 		return err
 	}
 
-	if err := wrapnix.CreateWrappers(ctx, d); err != nil {
-		return err
-	}
-
 	return nix.FlakeUpdate(shellgen.FlakePath(d))
 }
 

internal/wrapnix/wrapper.go

@@ -21,11 +21,11 @@ import (
 	"go.jetpack.io/devbox/internal/xdg"
 )
 
-type devboxer interface {
-	NixBins(ctx context.Context) ([]string, error)
-	ShellEnvHash(ctx context.Context) (string, error)
-	ShellEnvHashKey() string
-	ProjectDir() string
+type CreateWrappersArgs struct {
+	NixBins         []string
+	ShellEnvHash    string
+	ShellEnvHashKey string
+	ProjectDir      string
 }
 
 //go:embed wrapper.sh.tmpl
@@ -37,44 +37,35 @@ var wrapperTemplate = template.Must(template.New("wrapper").Parse(wrapper))
 var devboxSymlinkDir = xdg.CacheSubpath(filepath.Join("devbox", "bin", "current"))
 
 // CreateWrappers creates wrappers for all the executables in nix paths
-func CreateWrappers(ctx context.Context, devbox devboxer) error {
+func CreateWrappers(ctx context.Context, args CreateWrappersArgs) error {
 	defer debug.FunctionTimer().End()
-	shellEnvHash, err := devbox.ShellEnvHash(ctx)
-	if err != nil {
-		return err
-	}
 
 	// Remove all old wrappers
-	_ = os.RemoveAll(filepath.Join(devbox.ProjectDir(), plugin.WrapperPath))
+	_ = os.RemoveAll(filepath.Join(args.ProjectDir, plugin.WrapperPath))
 
 	// Recreate the bin wrapper directory
-	destPath := filepath.Join(wrapperBinPath(devbox))
+	destPath := filepath.Join(wrapperBinPath(args.ProjectDir))
 	_ = os.MkdirAll(destPath, 0o755)
 
 	bashPath := cmdutil.GetPathOrDefault("bash", "/bin/bash")
 
-	bins, err := devbox.NixBins(ctx)
-	if err != nil {
-		return err
-	}
 	if err := CreateDevboxSymlinkIfPossible(); err != nil {
 		return err
 	}
 
-	for _, bin := range bins {
-		if err = createWrapper(&createWrapperArgs{
-			devboxer:         devbox,
-			BashPath:         bashPath,
-			Command:          bin,
-			ShellEnvHash:     shellEnvHash,
-			DevboxSymlinkDir: devboxSymlinkDir,
-			destPath:         filepath.Join(destPath, filepath.Base(bin)),
+	for _, bin := range args.NixBins {
+		if err := createWrapper(&createWrapperArgs{
+			CreateWrappersArgs: args,
+			BashPath:           bashPath,
+			Command:            bin,
+			DevboxSymlinkDir:   devboxSymlinkDir,
+			destPath:           filepath.Join(destPath, filepath.Base(bin)),
 		}); err != nil {
 			return errors.WithStack(err)
 		}
 	}
 
-	return createSymlinksForSupportDirs(devbox.ProjectDir())
+	return createSymlinksForSupportDirs(args.ProjectDir)
 }
 
 // CreateDevboxSymlinkIfPossible creates a symlink to the devbox binary.
@@ -134,12 +125,11 @@ func CreateDevboxSymlinkIfPossible() error {
 }
 
 type createWrapperArgs struct {
-	devboxer
+	CreateWrappersArgs
 	BashPath         string
 	Command          string
-	ShellEnvHash     string
-	DevboxSymlinkDir string
 	destPath         string
+	DevboxSymlinkDir string
 }
 
 func createWrapper(args *createWrapperArgs) error {
@@ -200,6 +190,6 @@ func createSymlinksForSupportDirs(projectDir string) error {
 	return nil
 }
 
-func wrapperBinPath(devbox devboxer) string {
-	return filepath.Join(devbox.ProjectDir(), plugin.WrapperBinPath)
+func wrapperBinPath(projectDir string) string {
+	return filepath.Join(projectDir, plugin.WrapperBinPath)
 }