Skip to content

Commit bec08b4

Browse files
mikeland73mikeland73
authored
[wrappers] Don't re-source shellenv if already sourced (#872)
## Summary This fixes issue described by https://github.com/amithgeorge/devbox-nodejs-repro-20230406 It's not a perfect fix to all wrapper issues. Specifically: `USER=foo node -e "console.log(process.env.USER);"` will print the shellenv user instead of foo. Will follow up with a fix for that. cc: @amithgeorge ## How was it tested? Followed steps in https://github.com/amithgeorge/devbox-nodejs-repro-20230406 Added example that breaks in current release, passes in this PR
1 parent d5050e8 commit bec08b4

9 files changed

Lines changed: 586 additions & 0 deletions

File tree

‎internal/wrapnix/wrapper.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package wrapnix
33
import (
44
"bytes"
55
"context"
6+
"crypto/md5"
67
_ "embed"
8+
"encoding/hex"
79
"os"
810
"path/filepath"
911
"text/template"
@@ -89,6 +91,11 @@ type createWrapperArgs struct {
8991
destPath string
9092
}
9193

94+
func (c *createWrapperArgs) ShellEnvHash() string {
95+
hash := md5.Sum([]byte(c.ShellEnv))
96+
return hex.EncodeToString(hash[:])
97+
}
98+
9299
func createWrapper(args *createWrapperArgs) error {
93100
buf := &bytes.Buffer{}
94101
if err := wrapperTemplate.Execute(buf, args); err != nil {

‎internal/wrapnix/wrapper.sh.tmpl‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,15 @@ if [[ -z "$__DEVBOX_SET_{{ $k }}" ]]; then
1313
fi
1414
{{- end }}
1515

16+
{{/*
17+
We use __DEVBOX_SHELLENV_HASH to avoid re-sourcing shellenv. Since wrappers
18+
call other wrappers and potentially modify the environment, we don't want the
19+
environment to get re-written.
20+
*/ -}}
21+
22+
if [[ "$__DEVBOX_SHELLENV_HASH" != "{{ .ShellEnvHash }}" ]]; then
1623
{{ .ShellEnv }}
24+
export __DEVBOX_SHELLENV_HASH="{{ .ShellEnvHash }}"
25+
fi
26+
1727
{{ .Command }} "$@"

‎testscripts/shellenv/node/README.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Inspired by https://github.com/amithgeorge/devbox-nodejs-repro-20230406
2+
3+
This example shows a wrapped binary calling setting an env variable (PATH) and
4+
calling another wrapped binary without the PATH getting overwritten
5+
6+
## Steps
7+
8+
- devbox run run_test
9+
- exit code should be 0

‎testscripts/shellenv/node/devbox.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"packages": [
3+
"nodejs-18_x"
4+
],
5+
"shell": {
6+
"init_hook": [
7+
"npm install"
8+
],
9+
"scripts": {
10+
"run_test": "npm run run_test"
11+
}
12+
},
13+
"nixpkgs": {
14+
"commit": "4a65e9f64e53fdca6eed31adba836717a11247d2"
15+
}
16+
}

‎testscripts/shellenv/node/less-out/style.css‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#header {
2+
width: 10px;
3+
height: 20px;
4+
}

‎testscripts/shellenv/node/less-src/style.less‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
@width: 10px;
2+
@height: @width + 10px;
3+
4+
#header {
5+
width: @width;
6+
height: @height;
7+
}

0 commit comments

Comments
 (0)