[shell] Remove bin wrappers 🎉 (#1584)

## Summary

This removes bin wrappers. It also does the following:

* Creates `refresh` and `refresh-global` aliases.
* If environment is out of date, it prompts user to run the alias.
* If alias is missing, it shows the ugly raw command.
* If user has direnv active, we don't show message (because we refresh
automatically)
* We add aliases in two ways: 1) we add as part of `shellrc` for shell.
2) we add as part of shellenv for global, run, etc

Caveats:

* calling `refresh` causes environment to refresh including environment
variables (plugin and devbox.json)
* If user does not call refresh, it is possible some packages that
require setup hooks will not work, but the binary is available and in
PATH.

## How was it tested?

On zsh, fish shells:

* I installed `hello` globally, verified with `which hello`
* Installed `hello` in shell, got refresh message. Did `which` again and
verified still global
* Did `refresh` and `which` again and verified correct.

On global zsh:

* Added `hello` and got `refresh-global` message.
* Ran refresh and verified env was up to date.

On direnv:

* Installed `hello` and verified environment was up to date.

Author: mikeland73

devbox.go

@@ -65,12 +65,3 @@ func GlobalDataPath() (string, error) {
 func PrintEnvrcContent(w io.Writer, envFlags devopt.EnvFlags) error {
 	return impl.PrintEnvrcContent(w, envFlags)
 }
-
-// ExportifySystemPathWithoutWrappers reads $PATH, removes `virtenv/.wrappers/bin` paths,
-// and returns a string of the form `export PATH=....`
-//
-// This small utility function could have been inlined in the boxcli caller, but
-// needed the impl.exportify functionality. It does not depend on core-devbox.
-func ExportifySystemPathWithoutWrappers() string {
-	return impl.ExportifySystemPathWithoutWrappers()
-}

devbox.json

@@ -6,7 +6,7 @@
   ],
   "env": {
     "GOENV": "off",
-    "PATH": "$PATH:$PWD/dist"
+    "PATH":  "$PATH:$PWD/dist"
   },
   "shell": {
     "init_hook": [
@@ -20,9 +20,9 @@
         "GOOS=linux GOARCH=amd64 go build -o dist/devbox-linux-amd64 ./cmd/devbox",
         "GOOS=linux GOARCH=arm64 go build -o dist/devbox-linux-arm64 ./cmd/devbox"
       ],
-      "code": "code .",
-      "lint": "golangci-lint run --timeout 5m && scripts/gofumpt.sh",
-      "test": "go test -race -cover ./...",
+      "code":            "code .",
+      "lint":            "golangci-lint run --timeout 5m && scripts/gofumpt.sh",
+      "test":            "go test -race -cover ./...",
       "update-examples": "devbox run build && go run testscripts/testrunner/updater/main.go"
     }
   }

internal/boxcli/global.go

@@ -34,8 +34,8 @@ func globalCmd() *cobra.Command {
 	}
 
 	shellEnv := shellEnvCmd(&globalShellEnvCmdFlags.recompute)
-	shellEnv.Flags().BoolVar(
-		&globalShellEnvCmdFlags.recompute, "recompute", false,
+	shellEnv.Flags().BoolVarP(
+		&globalShellEnvCmdFlags.recompute, "recompute", "r", false,
 		"Recompute environment if needed",
 	)
 

internal/boxcli/root.go

@@ -74,8 +74,8 @@ func RootCmd() *cobra.Command {
 	command.AddCommand(servicesCmd())
 	command.AddCommand(setupCmd())
 	command.AddCommand(shellCmd())
-	// False to avoid recomputing the env in global shellenv:
-	command.AddCommand(shellEnvCmd(lo.ToPtr(false)))
+	// True to always recompute environment if needed.
+	command.AddCommand(shellEnvCmd(lo.ToPtr(true)))
 	command.AddCommand(updateCmd())
 	command.AddCommand(versionCmd())
 	// Preview commands

internal/boxcli/shellenv.go

@@ -58,8 +58,6 @@ func shellEnvCmd(recomputeEnvIfNeeded *bool) *cobra.Command {
 	flags.config.register(command)
 	flags.envFlag.register(command)
 
-	command.AddCommand(shellEnvOnlyPathWithoutWrappersCmd())
-
 	return command
 }
 
@@ -99,25 +97,3 @@ func shellEnvFunc(
 
 	return envStr, nil
 }
-
-func shellEnvOnlyPathWithoutWrappersCmd() *cobra.Command {
-	// Deprecated: will be removed after devbox 0.7.0
-	// Don't add deprecated field to avoid printing anything to stdout
-	command := &cobra.Command{
-		Use:     "only-path-without-wrappers",
-		Hidden:  true,
-		Short:   "[internal] Print shell command that exports the system $PATH without the bin-wrappers paths.",
-		Args:    cobra.ExactArgs(0),
-		PreRunE: ensureNixInstalled,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			s := shellEnvOnlyPathWithoutWrappersFunc()
-			fmt.Fprintln(cmd.OutOrStdout(), s)
-			return nil
-		},
-	}
-	return command
-}
-
-func shellEnvOnlyPathWithoutWrappersFunc() string {
-	return devbox.ExportifySystemPathWithoutWrappers()
-}

internal/boxcli/version.go

@@ -9,16 +9,14 @@ import (
 	"runtime"
 
 	"github.com/spf13/cobra"
-	"go.jetpack.io/devbox/internal/wrapnix"
 
 	"go.jetpack.io/devbox/internal/build"
 	"go.jetpack.io/devbox/internal/envir"
 	"go.jetpack.io/devbox/internal/vercheck"
 )
 
 type versionFlags struct {
-	verbose             bool
-	updateDevboxSymlink bool
+	verbose bool
 }
 
 func versionCmd() *cobra.Command {
@@ -35,13 +33,6 @@ func versionCmd() *cobra.Command {
 	command.Flags().BoolVarP(&flags.verbose, "verbose", "v", false, // value
 		"displays additional version information",
 	)
-	// Make this flag hidden because:
-	// This functionality doesn't strictly belong in this command, but we add it here
-	// since `devbox version update` calls `devbox version -v` to trigger an update.
-	command.Flags().BoolVarP(&flags.updateDevboxSymlink, "update-devbox-symlink", "u", false, // value
-		"update the devbox symlink to point to the current binary",
-	)
-	_ = command.Flags().MarkHidden("update-devbox-symlink")
 
 	command.AddCommand(selfUpdateCmd())
 	return command
@@ -71,13 +62,6 @@ func versionCmdFunc(cmd *cobra.Command, _ []string, flags versionFlags) error {
 		fmt.Fprintf(w, "Go Version:  %v\n", info.GoVersion)
 		fmt.Fprintf(w, "Launcher:    %v\n", info.LauncherVersion)
 
-		// TODO: in a subsequent PR, we should do this when flags.updateDevboxSymlink is true.
-		// Not doing for now, since users who have Devbox binary prior to this edit
-		// (before Devbox v0.5.9) will not invoke this flag in `devbox version update`.
-		// But we still want this to run for them.
-		if err := wrapnix.CreateDevboxSymlinkIfPossible(); err != nil {
-			return err
-		}
 	} else {
 		fmt.Fprintf(w, "%v\n", info.Version)
 	}

internal/impl/devbox.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"io/fs"
 	"maps"
 	"os"
 	"os/exec"
@@ -30,7 +29,6 @@ import (
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/shellgen"
 	"go.jetpack.io/devbox/internal/telemetry"
-	"go.jetpack.io/devbox/internal/wrapnix"
 
 	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/cmdutil"
@@ -215,18 +213,6 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 		return err
 	}
 
-	// By default we always remove bin wrappers when using `run`. This env var is
-	// for testing. Once we completely remove bin wrappers we can remove this.
-	// It helps simulate shell using "run".
-	if includeBinWrappers, _ := strconv.ParseBool(
-		os.Getenv("DEVBOX_INCLUDE_BIN_WRAPPERS_IN_PATH"),
-	); !includeBinWrappers {
-		env["PATH"] = envpath.RemoveFromPath(
-			env["PATH"],
-			wrapnix.WrapperBinPath(d.projectDir),
-		)
-	}
-
 	// Used to determine whether we're inside a shell (e.g. to prevent shell inception)
 	// This is temporary because StartServices() needs it but should be replaced with
 	// better alternative since devbox run and devbox shell are not the same.
@@ -258,9 +244,8 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 	return nix.RunScript(d.projectDir, strings.Join(cmdWithArgs, " "), env)
 }
 
-// Install ensures that all the packages in the config are installed and
-// creates all wrappers, but does not run init hooks. It is used to power
-// devbox install cli command.
+// Install ensures that all the packages in the config are installed
+// but does not run init hooks. It is used to power devbox install cli command.
 func (d *Devbox) Install(ctx context.Context) error {
 	ctx, task := trace.NewTask(ctx, "devboxInstall")
 	defer task.End()
@@ -290,7 +275,7 @@ func (d *Devbox) NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, er
 		upToDate, _ := d.lockfile.IsUpToDateAndInstalled()
 		if !upToDate {
 			cmd := `eval "$(devbox global shellenv --recompute)"`
-			if strings.HasSuffix(os.Getenv("SHELL"), "fish") {
+			if isFishShell() {
 				cmd = `devbox global shellenv --recompute | source`
 			}
 			ux.Finfo(
@@ -316,6 +301,8 @@ func (d *Devbox) NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, er
 		envStr = fmt.Sprintf("%s\n%s;\n", envStr, hooksStr)
 	}
 
+	envStr += "\n" + d.refreshAlias()
+
 	return envStr, nil
 }
 
@@ -863,7 +850,6 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	debug.Log("nix environment PATH is: %s", env)
 
 	// Add any vars defined in plugins.
-	// TODO: Now that we have bin wrappers, this may can eventually be removed.
 	// We still need to be able to add env variables to non-service binaries
 	// (e.g. ruby). This would involve understanding what binaries are associated
 	// to a given plugin.
@@ -875,7 +861,6 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	addEnvIfNotPreviouslySetByDevbox(env, pluginEnv)
 
 	env["PATH"] = envpath.JoinPathLists(
-		filepath.Join(d.projectDir, plugin.WrapperBinPath),
 		nix.ProfileBinPath(d.projectDir),
 		env["PATH"],
 	)
@@ -888,7 +873,7 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	// Add helpful env vars for a Devbox project
 	env["DEVBOX_PROJECT_ROOT"] = d.projectDir
 	env["DEVBOX_CONFIG_DIR"] = d.projectDir + "/devbox.d"
-	env["DEVBOX_PACKAGES_DIR"] = d.projectDir + "/.devbox/virtenv/.wrappers"
+	env["DEVBOX_PACKAGES_DIR"] = d.projectDir + "/" + nix.ProfilePath
 
 	// Include env variables in devbox.json
 	configEnv, err := d.configEnvs(ctx, env)
@@ -1143,31 +1128,6 @@ 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
-// 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(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) {
-			continue
-		}
-		files, err := os.ReadDir(binPath)
-		if err != nil {
-			return nil, errors.WithStack(err)
-		}
-		for _, file := range files {
-			if _, alreadySet := bins[file.Name()]; !alreadySet {
-				bins[file.Name()] = filepath.Join(binPath, file.Name())
-			}
-		}
-	}
-	return lo.Values(bins), nil
-}
-
 func (d *Devbox) projectDirHash() string {
 	hash, _ := cuecfg.Hash(d.projectDir)
 	return hash
@@ -1217,24 +1177,6 @@ func (d *Devbox) parseEnvAndExcludeSpecialCases(currentEnv []string) (map[string
 	return env, nil
 }
 
-// ExportifySystemPathWithoutWrappers is a small utility to filter WrapperBin paths from PATH
-func ExportifySystemPathWithoutWrappers() string {
-	path := []string{}
-	for _, p := range strings.Split(os.Getenv("PATH"), string(filepath.ListSeparator)) {
-		// Intentionally do not include projectDir with plugin.WrapperBinPath so that
-		// we filter out bin-wrappers for devbox-global and devbox-project.
-		if !strings.Contains(p, plugin.WrapperBinPath) {
-			path = append(path, p)
-		}
-	}
-
-	envs := map[string]string{
-		"PATH": strings.Join(path, string(filepath.ListSeparator)),
-	}
-
-	return exportify(envs)
-}
-
 func (d *Devbox) PluginManager() *plugin.Manager {
 	return d.pluginManager
 }

internal/impl/packages.go

@@ -25,7 +25,6 @@ import (
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/plugin"
 	"go.jetpack.io/devbox/internal/ux"
-	"go.jetpack.io/devbox/internal/wrapnix"
 )
 
 // packages.go has functions for adding, removing and getting info about nix
@@ -225,11 +224,12 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 
 	// if mode is install or uninstall, then we need to update the nix-profile
 	// and lockfile, so we must continue below.
-	if mode == ensure {
-		// if mode is ensure, then we only continue if needed.
-		if upToDate, err := d.lockfile.IsUpToDateAndInstalled(); err != nil || upToDate {
-			return err
-		}
+	upToDate, err := d.lockfile.IsUpToDateAndInstalled()
+	if err != nil {
+		return err
+	}
+	// if mode is ensure and we are up to date, then we can skip the rest
+	if mode == ensure && upToDate {
 		fmt.Fprintln(d.stderr, "Ensuring packages are installed.")
 	}
 
@@ -256,38 +256,27 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 		return err
 	}
 
-	// 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
-	}
-
 	// Ensure we clean out packages that are no longer needed.
 	d.lockfile.Tidy()
 
-	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
-	}
-
 	// Update lockfile with new packages that are not to be installed
 	for _, pkg := range d.configPackages() {
 		if err := pkg.EnsureUninstallableIsInLockfile(); err != nil {
 			return err
 		}
 	}
 
+	// If we're in a devbox shell (global or project), then the environment might
+	// be out of date after the user installs something. If have direnv active
+	// it should reload automatically so we don't need to refresh.
+	if d.IsEnvEnabled() && !upToDate && !d.IsDirenvActive() {
+		ux.Fwarning(
+			d.stderr,
+			"Your shell environment may be out of date. Run `%s` to update it.\n",
+			d.refreshAliasOrCommand(),
+		)
+	}
+
 	return d.lockfile.Save()
 }
 

internal/impl/pure_shell.go

@@ -42,7 +42,7 @@ func findNixInPATH(env map[string]string) ([]string, error) {
 	return nixBinsInPath, nil
 }
 
-// Creates a symlink for devbox in .devbox/virtenv/.wrappers/bin
+// Creates a symlink for devbox in .devbox/bin
 // so that devbox can be available inside a pure shell
 func createDevboxSymlink(d *Devbox) error {
 	// Get absolute path for where devbox is called