feat: Add recompute flag to shellenv command #1963
Conversation
There was a problem hiding this comment.
Thanks! I generally like the approach. That said, looking at the implementation I think a nicer fix would be to push the flag inside shellEnvCmd function's implementation.
Right now, we have this awkward parameter in func shellEnvCmd(recomputeEnvIfNeeded *bool) because we wanted it selectively exposed for just global and not regular devbox. If we're seeking to expose the --recompute flag in both global and regular devbox, then no need to expose this parameter. The flag can be handled internally inside shellEnvCmd.
Would you wanna make that change? If you are strapped for time, then happy to approve this PR, land it and I can make a follow up fix for it.
|
would you mind also running |
|
|
Thank you @savil for the suggestion. I'll push an update soon (and run the linter and formatter 😄 ) |
dax
force-pushed
the
add-recompute-flag-to-shellenv-command
branch
from
3ed1f03 to
50cfc7d
Compare
|
@savil The PR is now ready 🙏 |
There was a problem hiding this comment.
I think adding this flags makes sense, but the current implementation will cause a change in behavior in global shellenv (because recompute was previously defaulted to false, and now it is defaulted to true).
I think you may be able to simply override the flag in the global case (but we need some testing to confirm, cobra internals don't always behave intuitively).
|
Thanks @mikeland73 for spotting the regression 🙏 |
…shellenv's recompute flag with default=false (#2013) ## Summary Credit to @dax and #1963 for pointing out the need for this, and pioneering this solution. This PR merely nudges the code to be a bit neater. ## How was it tested? ``` % git diff diff --git a/internal/boxcli/shellenv.go b/internal/boxcli/shellenv.go index 991c2bec..8076e125 100644 --- a/internal/boxcli/shellenv.go +++ b/internal/boxcli/shellenv.go @@ -78,6 +78,9 @@ func shellEnvFunc( cmd *cobra.Command, flags shellEnvCmdFlags, ) (string, error) { + fmt.Printf("shellenv --recompute flag is %v\n", flags.recompute) + return "", nil + env, err := flags.Env(flags.config.path) if err != nil { return "", err ``` compiled via `devbox run build`, and then: ``` % devbox shellenv shellenv --recompute flag is true hash -r % devbox global shellenv shellenv --recompute flag is false hash -r ```
## Summary This PR continues the work from #2013 to also add the `--recompute` flag option for `devbox run` and `devbox shell`. For some users on bad networks, this can save them annoyance and time for when they _know_ their devbox environment is up-to-date. Fixes #2315 ## How was it tested? This PR affects 3 commands: `run`, `shell` and `shellenv`. 1. For `run`: Added `"hello": "latest",` to devbox.json of this project. ``` devbox run --recompute=false -- echo "hello world" Warning: Your devbox environment may be out of date. Run with --recompute=true to update it. hello world ``` then ``` devbox run -- echo "hello world" Info: Ensuring packages are installed. ✓ Computed the Devbox environment. hello world ``` 2. For `shell`. Ran similar commands as above. 3. For `shellenv`. Followed test plan of #1963. Changed the `.envrc` to be: ``` .envrc @@ -1,7 +1,13 @@ # Automatically sets up your devbox environment whenever you cd into this # directory via our direnv integration: -eval "$(devbox generate direnv --print-envrc)" +#eval "$(devbox generate direnv --print-envrc)" + # output of `devbox generate direnv --print-envrc` to modify it +use_devbox() { + watch_file devbox.json devbox.lock + # eval "$(devbox shellenv --init-hook --install --no-refresh-alias)" + eval "$(devbox shellenv --init-hook --no-refresh-alias --recompute=false)" +} +use devbox # check out https://www.jetify.com/devbox/docs/ide_configuration/direnv/ # for more details ``` Then modified devbox.json and saw the warning get printed. --------- Co-authored-by: savil <>
Summary
(related issue)
When used with direnv and when regularly switching between branches, Devbox slows down the workflow because it tries to keep the packages in sync with the devbox.lock state with a Ensuring packages are installed.
This PR add the
--recomputeflag to theshellenvcommand that allow to set it to--recompute=falseand then disable the automatic installation of package in casedevbox.jsonis out of sync.How was it tested?
Setting the
.envrc(ie. the direnv config file) as:--installhas been removed and--recompute=falsecompared with whatdevbox generate direnv --print-envrcgenerate by default.Then updating
devbox.jsondoes not trigger package installation or removal but the context (ie. the env vars) are correctly updated.