Skip to content

Commit 0728b41

Browse files
authored
[bin wrappers] fixes for only-path-without-wrappers call (#1163)
## Summary follow up to #1160 correctness: - consolidate some code into calling `wrapnix.GetWrapperBinPath` - the earlier code had a bug where I forgot to call `-c <project dir>`, but that is moot now (see below change) perf: - call DO_NOT_TRACK=1 to minimize segment perf impact - simplify OnlyPathWithoutWrappers to directly call `os.Getenv("PATH")`, remove WrapperBins, and exportify it. This function is used in a very specific context, so we can _know_ we can skip the nix.ComputeEnv calculation. - Doing `time devbox shellenv only-path-without-wrappers` is now consistently ~ 18-25 milliseconds. I have a slight preference for this over writing PATH to a file and sourcing it, since we may run into correctness issues with that file getting outdated, and it'll introduce perf regression on every shellenv call to write the file. ## How was it tested? - [x] testscripts pass - [x] `iex -S mix` works visual inspection: ``` 1 #!/nix/store/w849dr5qcbrrnxv69sy7kdnifa9jpjyf-bash-5.2-p15/bin/bash 2 3 4 5 if [[ "$__DEVBOX_SHELLENV_HASH_ac6a60d54a9d1ba6618cce8d40bfdfb50059d814edfe5f49bca3168c5c34e970" != "b1a4903de70aa4d610ecb00ffb3dd755913b4505651e6bebbf41571a406d415c" ]] & & [[ -z "$__DEVBOX_SHELLENV_HASH_ac6a60d54a9d1ba6618cce8d40bfdfb50059d814edfe5f49bca3168c5c34e970_GUARD" ]]; then 6 export __DEVBOX_SHELLENV_HASH_ac6a60d54a9d1ba6618cce8d40bfdfb50059d814edfe5f49bca3168c5c34e970_GUARD=true 7 eval "$(DO_NOT_TRACK=1 devbox shellenv -c /Users/savil/code/jetpack/devbox/examples/development/elixir/elixir_hello)" 8 fi 9 10 eval "$(DO_NOT_TRACK=1 devbox shellenv only-path-without-wrappers -c /Users/savil/code/jetpack/devbox/examples/development/elixir/elixir_hello)" 11 12 exec /nix/store/47whvyx9x48ishndcjw71xs7l0a6v6sd-elixir-1.14.4/bin/iex "$@" ```
1 parent a19be01 commit 0728b41

File tree

7 files changed

+56
-75
lines changed

7 files changed

+56
-75
lines changed

devbox.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type Devbox interface {
3333
Install(ctx context.Context) error
3434
IsEnvEnabled() bool
3535
ListScripts() []string
36-
PrintEnv(opts *devopt.PrintEnv) (string, error)
36+
PrintEnv(ctx context.Context, includeHooks bool) (string, error)
3737
PrintEnvVars(ctx context.Context) ([]string, error)
3838
PrintGlobalList() error
3939
Pull(ctx context.Context, overwrite bool, path string) error
@@ -74,3 +74,12 @@ func GlobalDataPath() (string, error) {
7474
func PrintEnvrcContent(w io.Writer) error {
7575
return impl.PrintEnvrcContent(w)
7676
}
77+
78+
// ExportifySystemPathWithoutWrappers reads $PATH, removes `virtenv/.wrappers/bin` paths,
79+
// and returns a string of the form `export PATH=....`
80+
//
81+
// This small utility function could have been inlined in the boxcli caller, but
82+
// needed the impl.exportify functionality. It does not depend on core-devbox.
83+
func ExportifySystemPathWithoutWrappers() string {
84+
return impl.ExportifySystemPathWithoutWrappers()
85+
}

internal/boxcli/shell.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func runShellCmd(cmd *cobra.Command, flags shellCmdFlags) error {
5959
if flags.printEnv {
6060
// false for includeHooks is because init hooks is not compatible with .envrc files generated
6161
// by versions older than 0.4.6
62-
script, err := box.PrintEnv(&devopt.PrintEnv{Ctx: cmd.Context()})
62+
script, err := box.PrintEnv(cmd.Context(), false /*includeHooks*/)
6363
if err != nil {
6464
return err
6565
}

internal/boxcli/shellenv.go

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -77,63 +77,30 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
7777
}
7878
}
7979

80-
opts := &devopt.PrintEnv{
81-
Ctx: cmd.Context(),
82-
IncludeHooks: flags.runInitHook,
83-
}
84-
envStr, err := box.PrintEnv(opts)
80+
envStr, err := box.PrintEnv(cmd.Context(), flags.runInitHook)
8581
if err != nil {
8682
return "", err
8783
}
8884

8985
return envStr, nil
9086
}
9187

92-
type shellEnvOnlyPathWithoutWrappersCmdFlags struct {
93-
config configFlags
94-
}
95-
9688
func shellEnvOnlyPathWithoutWrappersCmd() *cobra.Command {
97-
flags := shellEnvOnlyPathWithoutWrappersCmdFlags{}
9889
command := &cobra.Command{
9990
Use: "only-path-without-wrappers",
10091
Hidden: true,
101-
Short: "Print shell commands that export PATH without the bin-wrappers",
92+
Short: "[internal] Print shell command that exports the system $PATH without the bin-wrappers paths.",
10293
Args: cobra.ExactArgs(0),
10394
PreRunE: ensureNixInstalled,
10495
RunE: func(cmd *cobra.Command, args []string) error {
105-
s, err := shellEnvOnlyPathWithoutWrappersFunc(cmd, &flags)
106-
if err != nil {
107-
return err
108-
}
96+
s := shellEnvOnlyPathWithoutWrappersFunc()
10997
fmt.Fprintln(cmd.OutOrStdout(), s)
110-
fmt.Fprintln(cmd.OutOrStdout(), "hash -r")
11198
return nil
11299
},
113100
}
114-
flags.config.register(command)
115101
return command
116102
}
117103

118-
func shellEnvOnlyPathWithoutWrappersFunc(cmd *cobra.Command, flags *shellEnvOnlyPathWithoutWrappersCmdFlags) (string, error) {
119-
120-
box, err := devbox.Open(&devopt.Opts{
121-
Dir: flags.config.path,
122-
Writer: cmd.ErrOrStderr(),
123-
Pure: false,
124-
})
125-
if err != nil {
126-
return "", err
127-
}
128-
129-
opts := &devopt.PrintEnv{
130-
Ctx: cmd.Context(),
131-
OnlyPathWithoutWrappers: true,
132-
}
133-
envStr, err := box.PrintEnv(opts)
134-
if err != nil {
135-
return "", err
136-
}
137-
138-
return envStr, nil
104+
func shellEnvOnlyPathWithoutWrappersFunc() string {
105+
return devbox.ExportifySystemPathWithoutWrappers()
139106
}

internal/impl/devbox.go

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
293293
// creates all wrappers, but does not run init hooks. It is used to power
294294
// devbox install cli command.
295295
func (d *Devbox) Install(ctx context.Context) error {
296-
if _, err := d.PrintEnv(&devopt.PrintEnv{Ctx: ctx}); err != nil {
296+
if _, err := d.PrintEnv(ctx, false /*includeHooks*/); err != nil {
297297
return err
298298
}
299299
return wrapnix.CreateWrappers(ctx, d)
@@ -309,8 +309,8 @@ func (d *Devbox) ListScripts() []string {
309309
return keys
310310
}
311311

312-
func (d *Devbox) PrintEnv(opts *devopt.PrintEnv) (string, error) {
313-
ctx, task := trace.NewTask(opts.Ctx, "devboxPrintEnv")
312+
func (d *Devbox) PrintEnv(ctx context.Context, includeHooks bool) (string, error) {
313+
ctx, task := trace.NewTask(ctx, "devboxPrintEnv")
314314
defer task.End()
315315

316316
if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
@@ -322,23 +322,9 @@ func (d *Devbox) PrintEnv(opts *devopt.PrintEnv) (string, error) {
322322
return "", err
323323
}
324324

325-
if opts.OnlyPathWithoutWrappers {
326-
path := []string{}
327-
for _, p := range strings.Split(envs["PATH"], string(filepath.ListSeparator)) {
328-
if !strings.Contains(p, plugin.WrapperPath) {
329-
path = append(path, p)
330-
}
331-
}
332-
333-
// reset envs to be PATH only!
334-
envs = map[string]string{
335-
"PATH": strings.Join(path, string(filepath.ListSeparator)),
336-
}
337-
}
338-
339325
envStr := exportify(envs)
340326

341-
if opts.IncludeHooks {
327+
if includeHooks {
342328
hooksStr := ". " + d.scriptPath(hooksFilename)
343329
envStr = fmt.Sprintf("%s\n%s;\n", envStr, hooksStr)
344330
}
@@ -1242,3 +1228,22 @@ func (d *Devbox) convertEnvToMap(currentEnv []string) (map[string]string, error)
12421228
}
12431229
return env, nil
12441230
}
1231+
1232+
// ExportifySystemPathWithoutWrappers is a small utility to filter WrapperBin paths from PATH
1233+
func ExportifySystemPathWithoutWrappers() string {
1234+
1235+
path := []string{}
1236+
for _, p := range strings.Split(os.Getenv("PATH"), string(filepath.ListSeparator)) {
1237+
// Intentionally do not include projectDir with plugin.WrapperBinPath so that
1238+
// we filter out bin-wrappers for devbox-global and devbox-project.
1239+
if !strings.Contains(p, plugin.WrapperBinPath) {
1240+
path = append(path, p)
1241+
}
1242+
}
1243+
1244+
envs := map[string]string{
1245+
"PATH": strings.Join(path, string(filepath.ListSeparator)),
1246+
}
1247+
1248+
return exportify(envs)
1249+
}

internal/impl/devopt/devboxopts.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package devopt
22

33
import (
4-
"context"
54
"io"
65
)
76

@@ -11,9 +10,3 @@ type Opts struct {
1110
IgnoreWarnings bool
1211
Writer io.Writer
1312
}
14-
15-
type PrintEnv struct {
16-
Ctx context.Context
17-
IncludeHooks bool
18-
OnlyPathWithoutWrappers bool
19-
}

internal/wrapnix/wrapper.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"text/template"
1414

1515
"github.com/pkg/errors"
16-
1716
"go.jetpack.io/devbox/internal/cmdutil"
1817
"go.jetpack.io/devbox/internal/nix"
1918
"go.jetpack.io/devbox/internal/plugin"
@@ -48,7 +47,7 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
4847
_ = os.RemoveAll(filepath.Join(devbox.ProjectDir(), plugin.WrapperPath))
4948

5049
// Recreate the bin wrapper directory
51-
destPath := filepath.Join(devbox.ProjectDir(), plugin.WrapperBinPath)
50+
destPath := filepath.Join(wrapperBinPath(devbox))
5251
_ = os.MkdirAll(destPath, 0755)
5352

5453
bashPath := cmdutil.GetPathOrDefault("bash", "/bin/bash")
@@ -91,7 +90,7 @@ func CreateWrappers(ctx context.Context, devbox devboxer) error {
9190
return errors.WithStack(err)
9291
}
9392
}
94-
if err = createDevboxSymlink(devbox.ProjectDir()); err != nil {
93+
if err = createDevboxSymlink(devbox); err != nil {
9594
return err
9695
}
9796

@@ -168,17 +167,21 @@ func createSymlinksForSupportDirs(projectDir string) error {
168167

169168
// Creates a symlink for devbox in .devbox/virtenv/.wrappers/bin
170169
// so that devbox can be available inside a pure shell
171-
func createDevboxSymlink(projectDir string) error {
170+
func createDevboxSymlink(devbox devboxer) error {
172171

173172
// Get absolute path for where devbox is called
174173
devboxPath, err := filepath.Abs(os.Args[0])
175174
if err != nil {
176175
return errors.Wrap(err, "failed to create devbox symlink. Devbox command won't be available inside the shell")
177176
}
178177
// Create a symlink between devbox in .wrappers/bin
179-
err = os.Symlink(devboxPath, filepath.Join(projectDir, plugin.WrapperBinPath, "devbox"))
178+
err = os.Symlink(devboxPath, filepath.Join(wrapperBinPath(devbox), "devbox"))
180179
if err != nil && !errors.Is(err, fs.ErrExist) {
181180
return errors.Wrap(err, "failed to create devbox symlink. Devbox command won't be available inside the shell")
182181
}
183182
return nil
184183
}
184+
185+
func wrapperBinPath(devbox devboxer) string {
186+
return filepath.Join(devbox.ProjectDir(), plugin.WrapperBinPath)
187+
}

internal/wrapnix/wrapper.sh.tmpl

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#!{{ .BashPath }}
22

3-
{{/*
3+
{{/*
44
# If env variable has never been set by devbox we set it, but also
55
# default to env value set by user. This means plugin env variables behave a bit
66
# differently than devbox.json env variables which are always set once.
@@ -29,9 +29,13 @@ export {{ .ShellEnvHashKey }}_GUARD=true
2929
eval "$(DO_NOT_TRACK=1 devbox shellenv -c {{ .ProjectDir }})"
3030
fi
3131

32-
# So that we do not invoke other bin-wrappers from
33-
# this bin-wrapper. Instead, we directly invoke the binary from the nix store, which
34-
# should be in PATH.
35-
eval "$(devbox shellenv only-path-without-wrappers)"
32+
{{/*
33+
We call only-path-without-wrappers so that we do not invoke other bin-wrappers from
34+
this bin-wrapper. Instead, we directly invoke the binary from the nix store, which
35+
should be in PATH.
36+
37+
DO_NOT_TRACK=1 can be removed once we optimize segment to queue events.
38+
*/ -}}
39+
eval "$(DO_NOT_TRACK=1 devbox shellenv only-path-without-wrappers)"
3640

3741
exec {{ .Command }} "$@"

0 commit comments

Comments
 (0)