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

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,11 @@ func versionCmd() *cobra.Command {
 	command.Flags().BoolVarP(&flags.verbose, "verbose", "v", false, // value
 		"displays additional version information",
 	)
+	command.Flags().BoolVarP(&flags.updateDevboxSymlink, "update-devbox-symlink", "u", false, // value
+		"update the devbox symlink to point to the current version",
+	)
+	_ = command.Flags().MarkHidden("update-devbox-symlink")
+
 	command.AddCommand(selfUpdateCmd())
 	return command
 }
@@ -60,6 +67,14 @@ func versionCmdFunc(cmd *cobra.Command, _ []string, flags versionFlags) error {
 		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)
+
+		// 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)
 	}

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 {
@@ -51,7 +53,7 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
 	}
 	// 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 +64,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 +74,50 @@ 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 the 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. This means
+//     that bin-wrappers may invoke older devbox binaries, which have different
+//     functionality. Annoyingly, they will also print "New devbox available" messages,
+//     even though the user has just updated devbox.
+//     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", "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)
+	}
+
+	if err := os.Symlink(devboxBinaryPath, currentDevboxSymlinkPath); err != nil {
+		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 }} "$@"