Skip to content

Commit 2b0decf

Browse files
authored
[devbox] run: skip re-computing Devbox State if in devbox shell (#2144)
## Summary This PR implements a change to the semantics of `devbox run`. BEFORE: `devbox run` would always ensure that the Devbox State and Environment is up-to-date. AFTER: `devbox run` will only do so when _outside_ a Devbox Environment (i.e. `devbox shell` or equivalent like direnv-enabled shell). The motivation is two fold: 1. Speed. Users often want to just run a quick script, this slows them down. 2. Users often have init-hooks which are more appropriate for one-time setup, rather than when running a utility script inside a `devbox shell` that has already been set up appropriately. This was shared in the Discord channel for feedback: https://discord.com/channels/903306922852245526/1009933892385517619/1248724103750483978 Notably, the focus there was on skipping the init-hooks, but in this PR, we are going one-step beyond that to ensure the Devbox State itself is not re-computed. The reason is that rather than _partially_ updating the Devbox State and Environment (partially, because init-hooks are part of setting up the environment) we'd rather eschew the step entirely. This feels more straightforward for users to reason about. ## How was it tested? Did a basic sanity test in two scenarios: 1. Running the script in a `devbox shell` or direnv-enabled environment does NOT recompute the Devbox state. 2. Repeated the sanity test when outside of the Devbox environment, and confirmed it does recompute the Devbox state.
1 parent b9cf6c4 commit 2b0decf

File tree

5 files changed

+27
-50
lines changed

5 files changed

+27
-50
lines changed

internal/devbox/devbox.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,21 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
243243
}
244244

245245
lock.SetIgnoreShellMismatch(true)
246-
env, err := d.ensureStateIsUpToDateAndComputeEnv(ctx)
247-
if err != nil {
248-
return err
246+
247+
var env map[string]string
248+
if d.IsEnvEnabled() {
249+
// Skip ensureStateIsUpToDate if we are already in a shell of this devbox-project
250+
env = envir.PairsToMap(os.Environ())
251+
252+
// We set this to ensure that init-hooks do NOT re-run. They would have
253+
// run when initializing the Devbox Environment in the current shell.
254+
env[d.SkipInitHookEnvName()] = "true"
255+
} else {
256+
var err error
257+
env, err = d.ensureStateIsUpToDateAndComputeEnv(ctx)
258+
if err != nil {
259+
return err
260+
}
249261
}
250262

251263
// Used to determine whether we're inside a shell (e.g. to prevent shell inception)

internal/devbox/envvars.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,7 @@ func (d *Devbox) IsEnvEnabled() bool {
7878
pathStack := envpath.Stack(fakeEnv, envir.PairsToMap(os.Environ()))
7979
return pathStack.Has(d.ProjectDirHash())
8080
}
81+
82+
func (d *Devbox) SkipInitHookEnvName() string {
83+
return "__DEVBOX_SKIP_INIT_HOOK_" + d.ProjectDirHash()
84+
}

internal/shellgen/scripts.go

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ package shellgen
22

33
import (
44
"bytes"
5+
_ "embed"
56
"fmt"
67
"log/slog"
78
"os"
89
"path/filepath"
910
"strings"
1011
"text/template"
1112

12-
_ "embed"
13-
1413
"github.com/pkg/errors"
1514
"go.jetpack.io/devbox/internal/boxcli/featureflag"
1615
"go.jetpack.io/devbox/internal/debug"
@@ -24,26 +23,17 @@ import (
2423
var scriptWrapperTmplString string
2524
var scriptWrapperTmpl = template.Must(template.New("script-wrapper").Parse(scriptWrapperTmplString))
2625

27-
//go:embed tmpl/init-hook-wrapper.tmpl
28-
var initHookWrapperString string
29-
var initHookWrapperTmpl = template.Must(template.New("init-hook-wrapper").Parse(initHookWrapperString))
30-
3126
const scriptsDir = ".devbox/gen/scripts"
3227

33-
// HooksFilename is the name of the file that contains a wrapper of the
34-
// project's init-hooks and plugin hooks
35-
const (
36-
HooksFilename = ".hooks"
37-
rawHooksFilename = ".raw-hooks"
38-
)
28+
const HooksFilename = ".hooks"
3929

4030
type devboxer interface {
4131
Config() *devconfig.Config
4232
Lockfile() *lock.File
4333
InstallablePackages() []*devpkg.Package
4434
PluginManager() *plugin.Manager
4535
ProjectDir() string
46-
ProjectDirHash() string
36+
SkipInitHookEnvName() string
4737
}
4838

4939
// WriteScriptsToFiles writes scripts defined in devbox.json into files inside .devbox/gen/scripts.
@@ -68,12 +58,6 @@ func WriteScriptsToFiles(devbox devboxer) error {
6858
if err != nil {
6959
return errors.WithStack(err)
7060
}
71-
written[rawHooksFilename] = struct{}{}
72-
73-
err = writeInitHookWrapperFile(devbox)
74-
if err != nil {
75-
return errors.WithStack(err)
76-
}
7761
written[HooksFilename] = struct{}{}
7862

7963
// Write scripts to files.
@@ -104,7 +88,7 @@ func WriteScriptsToFiles(devbox devboxer) error {
10488
}
10589

10690
func writeRawInitHookFile(devbox devboxer, body string) (err error) {
107-
script, err := createScriptFile(devbox, rawHooksFilename)
91+
script, err := createScriptFile(devbox, HooksFilename)
10892
if err != nil {
10993
return errors.WithStack(err)
11094
}
@@ -114,19 +98,6 @@ func writeRawInitHookFile(devbox devboxer, body string) (err error) {
11498
return errors.WithStack(err)
11599
}
116100

117-
func writeInitHookWrapperFile(devbox devboxer) (err error) {
118-
script, err := createScriptFile(devbox, HooksFilename)
119-
if err != nil {
120-
return errors.WithStack(err)
121-
}
122-
defer script.Close() // best effort: close file
123-
124-
return initHookWrapperTmpl.Execute(script, map[string]string{
125-
"InitHookHash": "__DEVBOX_INIT_HOOK_" + devbox.ProjectDirHash(),
126-
"RawHooksFile": ScriptPath(devbox.ProjectDir(), rawHooksFilename),
127-
})
128-
}
129-
130101
func WriteScriptFile(devbox devboxer, name, body string) (err error) {
131102
script, err := createScriptFile(devbox, name)
132103
if err != nil {
@@ -168,9 +139,9 @@ func ScriptPath(projectDir, scriptName string) string {
168139
func ScriptBody(d devboxer, body string) (string, error) {
169140
var buf bytes.Buffer
170141
err := scriptWrapperTmpl.Execute(&buf, map[string]string{
171-
"Body": body,
172-
"InitHookHash": "__DEVBOX_INIT_HOOK_" + d.ProjectDirHash(),
173-
"InitHookPath": ScriptPath(d.ProjectDir(), HooksFilename),
142+
"Body": body,
143+
"SkipInitHookHash": d.SkipInitHookEnvName(),
144+
"InitHookPath": ScriptPath(d.ProjectDir(), HooksFilename),
174145
})
175146
if err != nil {
176147
return "", errors.WithStack(err)

internal/shellgen/tmpl/init-hook-wrapper.tmpl

Lines changed: 0 additions & 9 deletions
This file was deleted.

internal/shellgen/tmpl/script-wrapper.tmpl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
scripts. (though users can run a fish script within their script)
88
*/ -}}
99

10-
if [ -z "${{ .InitHookHash }}" ]; then
11-
{{/* init hooks will export InitHookHash ensuring no recursive sourcing*/ -}}
10+
if [ -z "${{ .SkipInitHookHash }}" ]; then
1211
. {{ .InitHookPath }}
1312
fi
1413

0 commit comments

Comments
 (0)