Skip to content

Commit 0d0de12

Browse files
authored
[perf] Replace only-path-without-wrappers with sed (#1535)
## Summary This replaces our existing `only-path-without-wrappers` wrapper implementation to use `sed`. Context: We use `only-path-without-wrappers` to remove bin wrappers in the environment of our binaries. This is particularly useful when a binary calls another binary. See #1160 for more context. Unfortunately this adds a perf hit to every single binary call. In my informal experiments this cost was ~40ms. For example: Previously, `time go version` was taking ~60ms even if environment is fully up to date. Now, `time go version` takes ~20ms. @Lagoja I'm getting an error in the elixer example (since that was the motivation for the original fix) but it looks like nix elixer on Sonoma is broken: https://elixirforum.com/t/bus-error-after-upgrading-to-sonoma-beta/56354/33 (Our CICD runners will test on Ubuntu and older macOS, Monterrey I believe) ## How was it tested? * Tested time to call `go version` before and after * Inspected PATH of shell to ensure `.wrappers` was still there. * Copy pasted `sed` code from one of the bin wrappers
1 parent 06050da commit 0d0de12

File tree

3 files changed

+8
-3
lines changed

3 files changed

+8
-3
lines changed

internal/boxcli/shellenv.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
9494
}
9595

9696
func shellEnvOnlyPathWithoutWrappersCmd() *cobra.Command {
97+
// Deprecated: will be removed after devbox 0.7.0
98+
// Don't add deprecated field to avoid printing anything to stdout
9799
command := &cobra.Command{
98100
Use: "only-path-without-wrappers",
99101
Hidden: true,

internal/wrapnix/wrapper.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func CreateWrappers(ctx context.Context, args CreateWrappersArgs) error {
5555

5656
for _, bin := range args.NixBins {
5757
if err := createWrapper(&createWrapperArgs{
58+
WrapperBinPath: destPath,
5859
CreateWrappersArgs: args,
5960
BashPath: bashPath,
6061
Command: bin,
@@ -130,6 +131,7 @@ type createWrapperArgs struct {
130131
Command string
131132
destPath string
132133
DevboxSymlinkDir string
134+
WrapperBinPath string // This is the directory where all bin wrappers live
133135
}
134136

135137
func createWrapper(args *createWrapperArgs) error {

internal/wrapnix/wrapper.sh.tmpl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@ eval "$(DO_NOT_TRACK=1 devbox shellenv --preserve-path-stack -c {{ .ProjectDir }
2525
fi
2626

2727
{{/*
28-
We call only-path-without-wrappers so that we do not invoke other bin-wrappers from
28+
Remove wrapper bin path from PATH so that we don't call more bin-wrappers from
2929
this bin-wrapper. Instead, we directly invoke the binary from the nix store, which
3030
should be in PATH.
3131

32-
DO_NOT_TRACK=1 can be removed once we optimize segment to queue events.
32+
This is implemented in sed for efficiency. sed is POSIX so we assume it's available.
33+
3334
*/ -}}
34-
eval "$(DO_NOT_TRACK=1 devbox shellenv only-path-without-wrappers)"
35+
export PATH=$(echo $PATH | sed -e 's#:{{ .WrapperBinPath }}##' -e 's#{{ .WrapperBinPath }}:##' -e 's#{{ .WrapperBinPath }}##')
3536

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

0 commit comments

Comments
 (0)