[devbox] run: skip re-computing Devbox State if in devbox shell

[savil]

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.

[mikeland73]

I don't think this will actually skip init hooks.

init hooks are baked into the scripts that are written to (.devbox/gen/scripts).

If you want to brainstorm a few approaches happy to chat

btw, currently init hooks are skipped if there is a script called inside an init hook. That's the if statement you will find in the generated sh file.

[savil]

init hooks are baked into the scripts that are written to (.devbox/gen/scripts).

oh yeah, good point!

[savil]

@mikeland73 PTAL. Made the update as discussed IRL.

Notably, I chose to keep the init-hook-wrapper.tmpl unchanged. Reason: we need it to run for devbox shell scenario that calls . /path/to/projectDir/.devbox/gen/scripts/.hooks.sh (recall, init-hook-wrapper.tmpl generates .hooks.sh).

[gcurtis]

For some reason I still see the init hooks running twice. Maybe there's something weird with my setup, but this is what I see:

$ devbox shell Info: Ensuring packages are installed. ✓ Computed the Devbox environment. Starting a devbox shell... this is my init hook (devbox) $ devbox run test this is my init hook this is my script 

Config:

{ "$schema": "https://raw.githubusercontent.com/jetify-com/devbox/0.0.0-dev/.schema/devbox.schema.json", "packages": [], "shell": { "init_hook": "echo this is my init hook", "scripts": { "test": "echo this is my script" } } }

Build Info:

$ command -v devbox /Users/gcurtis/bin/devbox $ go version -m ~/bin/devbox | rg -o vcs.+ vcs=git vcs.revision=6f165e38b605247f62f8e68d147d6f8a9b7e7d42 vcs.time=2024-06-13T23:02:04Z vcs.modified=false 

[gcurtis]

If we don't recompute the state, does that mean changes to devbox.json won't take effect from inside the shell? For example, what would be printed if I did:

$ echo '{"shell":{"scripts":{"test":"echo aaa"}}}' $ devbox shell (devbox) $ devbox run test aaa (devbox) $ echo '{"shell":{"scripts":{"test":"echo bbb"}}}' (devbox) $ devbox run test ??? # would I get "aaa" or "bbb"? 

[savil]

For some reason I still see the init hooks running twice. Maybe there's something weird with my setup, but this is what I see:

Can you double check which devbox binary you are running?

Here's the output I see with your devbox.json:

% devbox shell Info: Ensuring packages are installed. ✓ Computed the Devbox environment. Starting a devbox shell... this is my init hook (devbox) savil@Savil-Srivastavas-MacBook-Pro no-recompute-if-run-in-shell % devbox run test this is my script (devbox) savil@Savil-Srivastavas-MacBook-Pro no-recompute-if-run-in-shell % 

[savil]

If we don't recompute the state, does that mean changes to devbox.json won't take effect from inside the shell? For example, what would be printed if I did:

$ echo '{"shell":{"scripts":{"test":"echo aaa"}}}' $ devbox shell (devbox) $ devbox run test aaa (devbox) $ echo '{"shell":{"scripts":{"test":"echo bbb"}}}' (devbox) $ devbox run test ??? # would I get "aaa" or "bbb"? 

in this case, the output will be bbb because we still re-generate the script files via shellgen.WriteScriptsToFiles(d) on line 241. This lets one iterate on scripts.

However, changes to the shell environment will not show up until the user executes the refresh alias (when inside the devbox shell)

[mikeland73]

Notably, I chose to keep the init-hook-wrapper.tmpl unchanged. Reason: we need it to run for devbox shell scenario that calls . /path/to/projectDir/.devbox/gen/scripts/.hooks.sh (recall, init-hook-wrapper.tmpl generates .hooks.sh).

Can you elaborate on this?

Why does init-hook-wrapper.tmpl need to set export {{ .InitHookHash }}=true ?

My understanding would be:

If an init hook has a devbox run in it, by the time it runs d.IsEnvEnabled() returns true (since init hooks run after env is already set). This means that any devbox run inside init hooks would not run the hook again (thanks to the code in this PR).

A benefit is that removing the wrapper would simplify code. Basically:

No need to generate the wrapper, so hooks can be sourced directly.

We could also rename the env var to something more accurate, e.g. SkipInitHooks

[savil]

Notably, I chose to keep the init-hook-wrapper.tmpl unchanged. Reason: we need it to run for devbox shell scenario that calls . /path/to/projectDir/.devbox/gen/scripts/.hooks.sh (recall, init-hook-wrapper.tmpl generates .hooks.sh).

Can you elaborate on this?

Why does init-hook-wrapper.tmpl need to set export {{ .InitHookHash }}=true ?

My understanding would be:

If an init hook has a devbox run in it, by the time it runs d.IsEnvEnabled() returns true (since init hooks run after env is already set). This means that any devbox run inside init hooks would not run the hook again (thanks to the code in this PR).

A benefit is that removing the wrapper would simplify code. Basically:

No need to generate the wrapper, so hooks can be sourced directly.

We could also rename the env var to something more accurate, e.g. SkipInitHooks

🤔 I think I mis-diagnosed this...or mis-tested something yesterday. You are correct that I can comment out the env-var lines in init-hook-wrapper and it works as expected. Updating...

[savil]

@mikeland73 PTAL

[mikeland73]

👏 👏 👏