Skip to content

[perf] skip forcing printDevEnv when add/rm/update outside shellenv #1540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -1172,10 +1172,10 @@ func (d *Devbox) parseEnvAndExcludeSpecialCases(currentEnv []string) (map[string
if ignoreCurrentEnvVar[key] {
continue
}
// handling special cases to for pure shell
// Passing HOME for pure shell to leak through otherwise devbox binary won't work
// We also include PATH to find the nix installation. It is cleaned for pure mode below
// TERM leaking through is to enable colored text in the pure shell
// handling special cases for pure shell
// - HOME required for devbox binary to work
// - PATH to find the nix installation. It is cleaned for pure mode below.
// - TERM to enable colored text in the pure shell
if !d.pure || key == "HOME" || key == "PATH" || key == "TERM" {
env[key] = val
}
Expand Down
4 changes: 2 additions & 2 deletions internal/impl/envvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ func markEnvsAsSetByDevbox(envs ...map[string]string) {
}
}

// IsEnvEnabled checks if the devbox environment is enabled. We use the ogPathKey
// as a proxy for this. This allows us to differentiate between global and
// IsEnvEnabled checks if the devbox environment is enabled.
// This allows us to differentiate between global and
// individual project shells.
func (d *Devbox) IsEnvEnabled() bool {
fakeEnv := map[string]string{}
Expand Down
26 changes: 18 additions & 8 deletions internal/impl/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,22 @@ type installMode string
const (
install installMode = "install"
uninstall installMode = "uninstall"
ensure installMode = "ensure"
// update is both install new package version and uninstall old package version
update installMode = "update"
ensure installMode = "ensure"
)

// ensurePackagesAreInstalled ensures that the nix profile has the packages specified
// in the config (devbox.json). The `mode` is used for user messaging to explain
// what operations are happening, because this function may take time to execute.
// TODO we should rename this to ensureDevboxEnvironmentIsUpToDate since it does
// much more than ensuring packages are installed.
// ensurePackagesAreInstalled ensures:
// 1. Packages are installed, in nix-profile or runx.
// Extraneous packages are removed (references purged, not uninstalled).
// 2. Files for devbox shellenv are generated
// 3. Env-vars for shellenv are computed
// 4. Lockfile is synced
//
// The `mode` is used for:
// 1. Skipping certain operations that may not apply.
// 2. User messaging to explain what operations are happening, because this function may take time to execute.
// TODO: Rename method since it does more than just ensure packages are installed.
func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMode) error {
defer trace.StartRegion(ctx, "ensurePackages").End()
defer debug.FunctionTimer().End()
Expand Down Expand Up @@ -248,8 +256,10 @@ func (d *Devbox) ensurePackagesAreInstalled(ctx context.Context, mode installMod
return err
}

// Force print-dev-env cache to be recomputed.
nixEnv, err := d.computeNixEnv(ctx, false /*use cache*/)
// Use the printDevEnvCache if we are adding or removing or updating any package,
// AND we are not in the shellenv-enabled environment of the current devbox-project.
usePrintDevEnvCache := mode != ensure && !d.IsEnvEnabled()
nixEnv, err := d.computeNixEnv(ctx, usePrintDevEnvCache)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/impl/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
"go.jetpack.io/devbox/internal/shellgen"
)

// update overwrites golden files with the new test results.
var update = flag.Bool("update", false, "update the golden files with the test results")
// updateFlag overwrites golden files with the new test results.
var updateFlag = flag.Bool("update", false, "update the golden files with the test results")

func TestWriteDevboxShellrc(t *testing.T) {
testdirs, err := filepath.Glob("testdata/shellrc/*")
Expand Down Expand Up @@ -83,7 +83,7 @@ func testWriteDevboxShellrc(t *testing.T, testdirs []string) {
// Overwrite the golden file if the -update flag was
// set, and then proceed normally. The test should
// always pass in this case.
if *update {
if *updateFlag {
err = os.WriteFile(test.goldShellrcPath, gotShellrc, 0o666)
if err != nil {
t.Error("Error updating golden files:", err)
Expand Down
2 changes: 1 addition & 1 deletion internal/impl/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
}
}

if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
if err := d.ensurePackagesAreInstalled(ctx, update); err != nil {
return err
}

Expand Down