[Bug fix] Ensure bin-wrappers use latest devbox binary to prevent false update notifications (#1324)

## Summary

**Motivation**

In #1260, the bin-wrappers were changed to invoke the devbox-binary,
instead of the devbox-launcher. However, when devbox has been updated,
these bin-wrappers have the older devbox-binary's path hardcoded. So,
they call the older devbox-binary leading to two issues:
1. runs older functionality of the older devbox binary
2. re-prints the notice about "New devbox available. Run .... to
update", despite the user already having updated their devbox.

**Implementation**

1. Create a symlink that always points to the "current" devbox binary. 
- I use "current" instead of "latest" to be consistent with the
Launcher's usage of these terms.
2. In the bin-wrappers, we prepend the directory containing this
devbox-symlink to PATH. The bin-wrappers revert back to directly
invoking devbox, as in `devbox shellenv`.
- This is done for robustness: if the symlink is missing, then the
bin-wrappers do still invoke `devbox` via the launcher. This is
problematic for users who have installed `coreutils` via devbox, but for
most users this fallback would work.
3. We ensure the bin-wrappers are created in
`ensurePackagesAreInstalled` that runs in `devbox.PrintEnv` during
`devbox shellenv`.
- This is needed because we need existing users that have the
bin-wrappers with the hardcoded DevboxExecutablePath to refresh their
bin-wrappers. When they open a new terminal, `devbox global shellenv`
would run, refreshing these bin-wrappers.
4. Finally, we create the Devbox symlink during `devbox version update`,
so that it points to the _new_ Devbox binary.
     

**Drawbacks**
Switching between devbox versions (for testing and development) may lead
to subtle unexpected behavior: The devbox-symlink will point to the
version that created the bin-wrappers, rather than respecting the devbox
of the PATH. We need to ensure we run `devbox install` or similar to
re-create the bin-wrappers.

## How was it tested?

1. Sanity check to do `devbox shell` in
`examples/development/go/hello-world`.
2. Sanity check to do `devbox run build` using the devbox binary from
this PR.
3. Deleted devbox-symlink, and ran `devbox version -v` to ensure that it
was re-created.

Author: savil

internal/boxcli/version.go

@@ -9,14 +9,16 @@ 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
+	verbose             bool
+	updateDevboxSymlink bool
 }
 
 func versionCmd() *cobra.Command {
@@ -33,6 +35,14 @@ 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
 }
@@ -52,16 +62,24 @@ func selfUpdateCmd() *cobra.Command {
 
 func versionCmdFunc(cmd *cobra.Command, _ []string, flags versionFlags) error {
 	w := cmd.OutOrStdout()
-	v := getVersionInfo()
+	info := getVersionInfo()
 	if flags.verbose {
-		fmt.Fprintf(w, "Version:     %v\n", v.Version)
-		fmt.Fprintf(w, "Platform:    %v\n", v.Platform)
-		fmt.Fprintf(w, "Commit:      %v\n", v.Commit)
-		fmt.Fprintf(w, "Commit Time: %v\n", v.CommitDate)
-		fmt.Fprintf(w, "Go Version:  %v\n", v.GoVersion)
-		fmt.Fprintf(w, "Launcher:    %v\n", v.LauncherVersion)
+		fmt.Fprintf(w, "Version:     %v\n", info.Version)
+		fmt.Fprintf(w, "Platform:    %v\n", info.Platform)
+		fmt.Fprintf(w, "Commit:      %v\n", info.Commit)
+		fmt.Fprintf(w, "Commit Time: %v\n", info.CommitDate)
+		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.CreateDevboxSymlink(); err != nil {
+			return err
+		}
 	} else {
-		fmt.Fprintf(w, "%v\n", v.Version)
+		fmt.Fprintf(w, "%v\n", info.Version)
 	}
 	return nil
 }

internal/cloud/openssh/config.go

@@ -15,6 +15,7 @@ import (
 	"text/template"
 
 	"github.com/pkg/errors"
+	"go.jetpack.io/devbox/internal/fileutil"
 )
 
 // These must match what's in sshConfigTmpl. We should eventually make the hosts
@@ -211,17 +212,8 @@ func containsDevboxInclude(r io.Reader) bool {
 	return false
 }
 
-// move to a file utility
 func EnsureDirExists(path string, perm fs.FileMode, chmod bool) error {
-	if err := os.Mkdir(path, perm); err != nil && !errors.Is(err, fs.ErrExist) {
-		return errors.WithStack(err)
-	}
-	if chmod {
-		if err := os.Chmod(path, perm); err != nil {
-			return errors.WithStack(err)
-		}
-	}
-	return nil
+	return fileutil.EnsureDirExists(path, perm, chmod)
 }
 
 // returns path to ~/.config/devbox/ssh

internal/fileutil/fileutil.go

@@ -4,8 +4,11 @@
 package fileutil
 
 import (
+	"io/fs"
 	"os"
 	"strings"
+
+	"github.com/pkg/errors"
 )
 
 // TODO: publish as it's own shared package that other binaries can use.
@@ -55,3 +58,15 @@ func FileContains(path string, substring string) (bool, error) {
 	}
 	return strings.Contains(string(data), substring), nil
 }
+
+func EnsureDirExists(path string, perm fs.FileMode, chmod bool) error {
+	if err := os.MkdirAll(path, perm); err != nil && !errors.Is(err, fs.ErrExist) {
+		return errors.WithStack(err)
+	}
+	if chmod {
+		if err := os.Chmod(path, perm); err != nil {
+			return errors.WithStack(err)
+		}
+	}
+	return nil
+}

internal/impl/packages.go

@@ -215,6 +215,10 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
 	// Ensure we clean out packages that are no longer needed.
 	d.lockfile.Tidy()
 
+	if err := wrapnix.CreateWrappers(ctx, d); err != nil {
+		return err
+	}
+
 	return d.lockfile.Save()
 }
 

internal/vercheck/vercheck.go

@@ -175,7 +175,7 @@ func triggerUpdate(stdErr io.Writer) (*updatedVersions, error) {
 	}
 
 	// TODO savil. Add a --json flag to devbox version and parse the output as JSON
-	cmd := exec.Command(exePath, "version", "-v")
+	cmd := exec.Command(exePath, "version", "-v", "--update-devbox-symlink")
 
 	buf := new(bytes.Buffer)
 	cmd.Stdout = io.MultiWriter(stdErr, buf)

internal/wrapnix/wrapper.go

@@ -14,8 +14,10 @@ import (
 
 	"github.com/pkg/errors"
 	"go.jetpack.io/devbox/internal/cmdutil"
+	"go.jetpack.io/devbox/internal/fileutil"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/plugin"
+	"go.jetpack.io/devbox/internal/xdg"
 )
 
 type devboxer interface {
@@ -49,9 +51,7 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 	if err != nil {
 		return err
 	}
-	// get absolute path of devbox binary that the launcher script invokes
-	// to avoid causing an infinite loop when coreutils gets installed
-	executablePath, err := os.Executable()
+	devboxSymlinkPath, err := CreateDevboxSymlink()
 	if err != nil {
 		return err
 	}
@@ -62,7 +62,7 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 			BashPath:         bashPath,
 			Command:          bin,
 			ShellEnvHash:     shellEnvHash,
-			DevboxBinaryPath: executablePath,
+			DevboxSymlinkDir: filepath.Dir(devboxSymlinkPath),
 			destPath:         filepath.Join(destPath, filepath.Base(bin)),
 		}); err != nil {
 			return errors.WithStack(err)
@@ -72,12 +72,53 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 	return createSymlinksForSupportDirs(devbox.ProjectDir())
 }
 
+// CreateDevboxSymlink creates a symlink to the devbox binary
+//
+// Needed because:
+//
+//  1. The bin-wrappers cannot invoke devbox via the Launcher. The Launcher script
+//     invokes some coreutils commands that may themselves be installed by devbox
+//     and so be bin-wrappers. This causes an infinite loop.
+//
+//     So, the bin-wrappers need to directly invoke the devbox binary.
+//
+//  2. The devbox binary's path will change when devbox is updated. Hence
+//     using absolute paths to the devbox binaries in the bin-wrappers
+//     will result in bin-wrappers invoking older devbox binaries.
+//
+//     So, the bin-wrappers need to use a symlink to the latest devbox binary. This
+//     symlink is updated when devbox is updated.
+func CreateDevboxSymlink() (string, error) {
+	curDir := xdg.CacheSubpath(filepath.Join("devbox", "bin", "current"))
+	if err := fileutil.EnsureDirExists(curDir, 0755, false /*chmod*/); err != nil {
+		return "", err
+	}
+	currentDevboxSymlinkPath := filepath.Join(curDir, "devbox")
+
+	devboxBinaryPath, err := os.Executable()
+	if err != nil {
+		return "", errors.WithStack(err)
+	}
+
+	// We will always re-create this symlink to ensure correctness.
+	if err := os.Remove(currentDevboxSymlinkPath); err != nil && !errors.Is(err, os.ErrNotExist) {
+		return "", errors.WithStack(err)
+	}
+
+	// Don't return error if error is os.ErrExist to protect against race conditions.
+	if err := os.Symlink(devboxBinaryPath, currentDevboxSymlinkPath); err != nil && !errors.Is(err, os.ErrExist) {
+		return "", errors.WithStack(err)
+	}
+
+	return currentDevboxSymlinkPath, nil
+}
+
 type createWrapperArgs struct {
 	devboxer
 	BashPath         string
 	Command          string
 	ShellEnvHash     string
-	DevboxBinaryPath string
+	DevboxSymlinkDir string
 	destPath         string
 }
 

internal/wrapnix/wrapper.sh.tmpl

@@ -1,5 +1,7 @@
 #!{{ .BashPath }}
 
+export PATH="{{ .DevboxSymlinkDir }}:$PATH"
+
 {{/*
 # If env variable has never been set by devbox we set it, but also
 # default to env value set by user. This means plugin env variables behave a bit
@@ -19,7 +21,7 @@ DO_NOT_TRACK=1 can be removed once we optimize segment to queue events.
 
 if [[ "${{ .ShellEnvHashKey }}" != "{{ .ShellEnvHash }}" ]] && [[ -z "${{ .ShellEnvHashKey }}_GUARD" ]]; then
 export {{ .ShellEnvHashKey }}_GUARD=true
-eval "$(DO_NOT_TRACK=1 {{ .DevboxBinaryPath }} shellenv -c {{ .ProjectDir }})"
+eval "$(DO_NOT_TRACK=1 devbox shellenv -c {{ .ProjectDir }})"
 fi
 
 {{/*
@@ -29,6 +31,6 @@ should be in PATH.
 
 DO_NOT_TRACK=1 can be removed once we optimize segment to queue events.
 */ -}}
-eval "$(DO_NOT_TRACK=1 {{ .DevboxBinaryPath }} shellenv only-path-without-wrappers)"
+eval "$(DO_NOT_TRACK=1 devbox shellenv only-path-without-wrappers)"
 
 exec {{ .Command }} "$@"