[bin wrappers] add --omit-wrappers-from-path to shellenv to prevent a binary from invoking a wrapped-binary. (#1151)

## Summary

**Motivation**
In elixir projects, `iex -S mix <script>` has the `iex` interpreter
invoking the
`mix` build tool. However, the `mix` build tool has been wrapped by
devbox in a bash script.
And this leads to a failure as `iex` is unable to interpret a bash
script.

**Fix**
Binaries should be able to directly invoke other installed binaries.

We implemented the binary-wrappers so that the shell-environment is
correctly
configured prior to the installed binary being invoked. In this case,
the `iex`
binary wrapper will run thereby correctly configuring the environment.
Any subsequent
binary invoked in the same sub-shell will inherit this up-to-date
environment. So,
the other binaries can be directly invoked (instead of the
binary-wrapper scripts).


**Implementation**
In this PR, we change the `PATH` that is set within a binary-wrapper.

Specifically, the `PATH` has the form:
```
<project-dir>/.devbox/virtenv/.wrappers/bin : <project-dir>/.devbox/nix/profile/default/bin
```
Within the binary-wrapper, we now omit the `.wrappers/bin` directory
from PATH
by using a new hidden flag `--omit-wrappers-from-path` on `devbox
shellenv`.


**perf analysis**

I expect the perf impact to be minimal.

1. Speed up: Binaries invoking other binaries will be a bit faster since
they no longer first execute the binary-wrapper.

2. Slowdown: we remove one layer of caching. Previously, we'd check the
shellenv-hash in the binary wrapper, and skip invoking `devbox shellenv`
if the hash was up-to-date. Now, on every invocation of a binary-wrapper
we'll call `devbox shellenv`. This should still be fast due to the
internal caching we do within `devbox shellenv`.



Fixes #1142

## How was it tested?

BEFORE: doing `devbox run -- iex -S mix` would result in the error in
the linked issue.

AFTER: 
```
> devbox run -- iex -S mix
* creating .nix-mix/archives/hex-2.0.6
* creating .nix-mix/elixir/1-14/rebar3
Resolving Hex dependencies...
Resolution completed in 0.015s
Unchanged:
  cowboy 2.9.0
  cowlib 2.11.0
  ranch 1.8.0
All dependencies are up to date
Erlang/OTP 25 [erts-13.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1]

Hello World!
Interactive Elixir (1.14.4) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Goodbye World

15:14:23.204 [notice] Application elixir_hello exited: shutdown

BREAK: (a)bort (A)bort with dump (c)ontinue (p)roc info (i)nfo
       (l)oaded (v)ersion (k)ill (D)b-tables (d)istribution
```

Also - did test plan of #1093
to verify `devbox update` continues working. A sanity check since we
dropped ShellEnvHash from the wrappers and I wanted to ensure this works
as expected.

Author: savil

devbox.go

@@ -33,7 +33,7 @@ type Devbox interface {
 	Install(ctx context.Context) error
 	IsEnvEnabled() bool
 	ListScripts() []string
-	PrintEnv(ctx context.Context, includeHooks bool) (string, error)
+	PrintEnv(opts *devopt.PrintEnv) (string, error)
 	PrintGlobalList() error
 	Pull(ctx context.Context, overwrite bool, path string) error
 	Push(url string) error

internal/boxcli/shell.go

@@ -59,7 +59,12 @@ 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.PrintEnv(cmd.Context(), false /*includeHooks*/)
+		opts := &devopt.PrintEnv{
+			Ctx:                  cmd.Context(),
+			IncludeHooks:         false,
+			OmitWrappersFromPath: false,
+		}
+		script, err := box.PrintEnv(opts)
 		if err != nil {
 			return err
 		}

internal/boxcli/shellenv.go

@@ -13,10 +13,11 @@ import (
 
 type shellEnvCmdFlags struct {
 	config               configFlags
-	runInitHook          bool
 	install              bool
-	useCachedPrintDevEnv bool
+	omitWrappersFromPath bool
 	pure                 bool
+	runInitHook          bool
+	useCachedPrintDevEnv bool
 }
 
 func shellEnvCmd() *cobra.Command {
@@ -45,6 +46,12 @@ func shellEnvCmd() *cobra.Command {
 	command.Flags().BoolVar(
 		&flags.pure, "pure", false, "If this flag is specified, devbox creates an isolated environment inheriting almost no variables from the current environment. A few variables, in particular HOME, USER and DISPLAY, are retained.")
 
+	// This flag is to be used by our generated bin-wrappers shell script.
+	command.Flags().BoolVar(
+		&flags.omitWrappersFromPath, "omit-wrappers-from-path", false, "If this flag is specified, "+
+			"the PATH from shellenv will not include the binary wrappers")
+	command.Flag("omit-wrappers-from-path").Hidden = true
+
 	// This is no longer used. Remove after 0.4.8 is released.
 	command.Flags().BoolVar(
 		&flags.useCachedPrintDevEnv,
@@ -74,7 +81,12 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
 		}
 	}
 
-	envStr, err := box.PrintEnv(cmd.Context(), flags.runInitHook)
+	opts := &devopt.PrintEnv{
+		Ctx:                  cmd.Context(),
+		IncludeHooks:         flags.runInitHook,
+		OmitWrappersFromPath: flags.omitWrappersFromPath,
+	}
+	envStr, err := box.PrintEnv(opts)
 	if err != nil {
 		return "", err
 	}

internal/impl/devbox.go

@@ -293,7 +293,12 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 // creates all wrappers, but does not run init hooks. It is used to power
 // devbox install cli command.
 func (d *Devbox) Install(ctx context.Context) error {
-	if _, err := d.PrintEnv(ctx, false /* run init hooks */); err != nil {
+	opts := &devopt.PrintEnv{
+		Ctx:                  ctx,
+		IncludeHooks:         false,
+		OmitWrappersFromPath: false,
+	}
+	if _, err := d.PrintEnv(opts); err != nil {
 		return err
 	}
 	return wrapnix.CreateWrappers(ctx, d)
@@ -309,8 +314,8 @@ func (d *Devbox) ListScripts() []string {
 	return keys
 }
 
-func (d *Devbox) PrintEnv(ctx context.Context, includeHooks bool) (string, error) {
-	ctx, task := trace.NewTask(ctx, "devboxPrintEnv")
+func (d *Devbox) PrintEnv(opts *devopt.PrintEnv) (string, error) {
+	ctx, task := trace.NewTask(opts.Ctx, "devboxPrintEnv")
 	defer task.End()
 
 	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
@@ -322,9 +327,19 @@ func (d *Devbox) PrintEnv(ctx context.Context, includeHooks bool) (string, error
 		return "", err
 	}
 
+	if opts.OmitWrappersFromPath {
+		path := []string{}
+		for _, p := range strings.Split(envs["PATH"], string(filepath.ListSeparator)) {
+			if !strings.Contains(p, plugin.WrapperPath) {
+				path = append(path, p)
+			}
+		}
+		envs["PATH"] = strings.Join(path, string(filepath.ListSeparator))
+	}
+
 	envStr := exportify(envs)
 
-	if includeHooks {
+	if opts.IncludeHooks {
 		hooksStr := ". " + d.scriptPath(hooksFilename)
 		envStr = fmt.Sprintf("%s\n%s;\n", envStr, hooksStr)
 	}

internal/impl/devopt/devboxopts.go

@@ -1,10 +1,19 @@
 package devopt
 
-import "io"
+import (
+	"context"
+	"io"
+)
 
 type Opts struct {
 	Dir            string
 	Pure           bool
 	IgnoreWarnings bool
 	Writer         io.Writer
 }
+
+type PrintEnv struct {
+	Ctx                  context.Context
+	IncludeHooks         bool
+	OmitWrappersFromPath bool
+}

internal/wrapnix/wrapper.go

@@ -34,11 +34,6 @@ var wrapperTemplate = template.Must(template.New("wrapper").Parse(wrapper))
 
 // CreateWrappers creates wrappers for all the executables in nix paths
 func CreateWrappers(ctx context.Context, devbox devboxer) error {
-	shellEnvHash, err := devbox.ShellEnvHash(ctx)
-	if err != nil {
-		return err
-	}
-
 	services, err := devbox.Services()
 	if err != nil {
 		return err
@@ -54,22 +49,20 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 	bashPath := cmdutil.GetPathOrDefault("bash", "/bin/bash")
 	for _, service := range services {
 		if err = createWrapper(&createWrapperArgs{
-			devboxer:     devbox,
-			BashPath:     bashPath,
-			Command:      service.Start,
-			Env:          service.Env,
-			ShellEnvHash: shellEnvHash,
-			destPath:     filepath.Join(destPath, service.StartName()),
+			devboxer: devbox,
+			BashPath: bashPath,
+			Command:  service.Start,
+			Env:      service.Env,
+			destPath: filepath.Join(destPath, service.StartName()),
 		}); err != nil {
 			return err
 		}
 		if err = createWrapper(&createWrapperArgs{
-			devboxer:     devbox,
-			BashPath:     bashPath,
-			Command:      service.Stop,
-			Env:          service.Env,
-			ShellEnvHash: shellEnvHash,
-			destPath:     filepath.Join(destPath, service.StopName()),
+			devboxer: devbox,
+			BashPath: bashPath,
+			Command:  service.Stop,
+			Env:      service.Env,
+			destPath: filepath.Join(destPath, service.StopName()),
 		}); err != nil {
 			return err
 		}
@@ -82,11 +75,10 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 
 	for _, bin := range bins {
 		if err = createWrapper(&createWrapperArgs{
-			devboxer:     devbox,
-			BashPath:     bashPath,
-			Command:      bin,
-			ShellEnvHash: shellEnvHash,
-			destPath:     filepath.Join(destPath, filepath.Base(bin)),
+			devboxer: devbox,
+			BashPath: bashPath,
+			Command:  bin,
+			destPath: filepath.Join(destPath, filepath.Base(bin)),
 		}); err != nil {
 			return errors.WithStack(err)
 		}
@@ -100,10 +92,9 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 
 type createWrapperArgs struct {
 	devboxer
-	BashPath     string
-	Command      string
-	Env          map[string]string
-	ShellEnvHash string
+	BashPath string
+	Command  string
+	Env      map[string]string
 
 	destPath string
 }

internal/wrapnix/wrapper.sh.tmpl

@@ -14,19 +14,23 @@ fi
 {{- end }}
 
 {{/*
-We use ShellEnvHashKey to prevent doing shellenv if the correct environment is
-already set. (perf optimization)
 
 We use the guard to prevent infinite loop if something in shellenv causes 
 another wrapped binary to be called. The guard is specific to this project so shellenv
 could still cause another project's shellenv to be called.
+Note, this guard can likely be removed since we use --omit-wrappers-from-path=true below,
+but leaving in to minimize change.
 
 DO_NOT_TRACK=1 can be removed once we optimize segment to queue events.
+
+--omit-wrappers-from-path so that we do not invoke other bin-wrappers from
+this bin-wrapper. Instead, we directly invoke the binary from the nix store, which
+should be in PATH.
 */ -}}
 
-if [[ "${{ .ShellEnvHashKey }}" != "{{ .ShellEnvHash }}" ]] && [[ -z "${{ .ShellEnvHashKey }}_GUARD" ]]; then
+if [[ -z "${{ .ShellEnvHashKey }}_GUARD" ]]; then
 export {{ .ShellEnvHashKey }}_GUARD=true
-eval "$(DO_NOT_TRACK=1 devbox shellenv -c {{ .ProjectDir }})"
+eval "$(DO_NOT_TRACK=1 devbox shellenv --omit-wrappers-from-path=true -c {{ .ProjectDir }})"
 fi
 
 exec {{ .Command }} "$@"