[Renaming] ComputeNixEnv -> ComputeEnv; ensurePackagesAreInstalled -> ensureStateIsCurrent (#1675)

## Summary

These names were getting rather stale. Hopefully, the new ones represent
them more accurately.

## How was it tested?

compiles and cicd tests should pass

Author: savil

devbox.go

@@ -23,7 +23,7 @@ type Devbox interface {
 	Install(ctx context.Context) error
 	IsEnvEnabled() bool
 	ListScripts() []string
-	NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, error)
+	EnvExports(ctx context.Context, opts devopt.EnvExportsOpts) (string, error)
 	PackageNames() []string
 	ProjectDir() string
 	Pull(ctx context.Context, opts devopt.PullboxOpts) error

examples/development/go/hello-world/trace.out

@@ -67,7 +67,7 @@ func runShellCmd(cmd *cobra.Command, flags shellCmdFlags) error {
 	if flags.printEnv {
 		// false for includeHooks is because init hooks is not compatible with .envrc files generated
 		// by versions older than 0.4.6
-		script, err := box.NixEnv(cmd.Context(), devopt.NixEnvOpts{})
+		script, err := box.EnvExports(cmd.Context(), devopt.EnvExportsOpts{})
 		if err != nil {
 			return err
 		}

internal/boxcli/shell.go

@@ -94,7 +94,7 @@ func shellEnvFunc(
 		}
 	}
 
-	envStr, err := box.NixEnv(cmd.Context(), devopt.NixEnvOpts{
+	envStr, err := box.EnvExports(cmd.Context(), devopt.EnvExportsOpts{
 		DontRecomputeEnvironment: !recomputeEnvIfNeeded,
 		NoRefreshAlias:           flags.noRefreshAlias,
 		RunHooks:                 flags.runInitHook,

internal/boxcli/shellenv.go

@@ -177,7 +177,7 @@ func (d *Devbox) Shell(ctx context.Context) error {
 	ctx, task := trace.NewTask(ctx, "devboxShell")
 	defer task.End()
 
-	envs, err := d.ensurePackagesAreInstalledAndComputeEnv(ctx)
+	envs, err := d.ensureStateIsUpToDateAndComputeEnv(ctx)
 	if err != nil {
 		return err
 	}
@@ -218,7 +218,7 @@ func (d *Devbox) RunScript(ctx context.Context, cmdName string, cmdArgs []string
 		return err
 	}
 
-	env, err := d.ensurePackagesAreInstalledAndComputeEnv(ctx)
+	env, err := d.ensureStateIsUpToDateAndComputeEnv(ctx)
 	if err != nil {
 		return err
 	}
@@ -260,7 +260,7 @@ func (d *Devbox) Install(ctx context.Context) error {
 	ctx, task := trace.NewTask(ctx, "devboxInstall")
 	defer task.End()
 
-	return d.ensurePackagesAreInstalled(ctx, ensure)
+	return d.ensureStateIsUpToDate(ctx, ensure)
 }
 
 func (d *Devbox) ListScripts() []string {
@@ -274,8 +274,11 @@ func (d *Devbox) ListScripts() []string {
 	return keys
 }
 
-func (d *Devbox) NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, error) {
-	ctx, task := trace.NewTask(ctx, "devboxNixEnv")
+// EnvExports returns a string of the env-vars that would need to be applied
+// to define a Devbox environment. The string is of the form `export KEY=VALUE` for each
+// env-var that needs to be applied.
+func (d *Devbox) EnvExports(ctx context.Context, opts devopt.EnvExportsOpts) (string, error) {
+	ctx, task := trace.NewTask(ctx, "devboxEnvExports")
 	defer task.End()
 
 	var envs map[string]string
@@ -295,9 +298,9 @@ func (d *Devbox) NixEnv(ctx context.Context, opts devopt.NixEnvOpts) (string, er
 			)
 		}
 
-		envs, err = d.computeNixEnv(ctx, true /*usePrintDevEnvCache*/)
+		envs, err = d.computeEnv(ctx, true /*usePrintDevEnvCache*/)
 	} else {
-		envs, err = d.ensurePackagesAreInstalledAndComputeEnv(ctx)
+		envs, err = d.ensureStateIsUpToDateAndComputeEnv(ctx)
 	}
 
 	if err != nil {
@@ -322,7 +325,7 @@ func (d *Devbox) EnvVars(ctx context.Context) ([]string, error) {
 	ctx, task := trace.NewTask(ctx, "devboxEnvVars")
 	defer task.End()
 	// this only returns env variables for the shell environment excluding hooks
-	envs, err := d.ensurePackagesAreInstalledAndComputeEnv(ctx)
+	envs, err := d.ensureStateIsUpToDateAndComputeEnv(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -482,7 +485,7 @@ func (d *Devbox) GenerateEnvrcFile(ctx context.Context, force bool, envFlags dev
 	}
 
 	// generate all shell files to ensure we can refer to them in the .envrc script
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil {
+	if err := d.ensureStateIsUpToDate(ctx, ensure); err != nil {
 		return err
 	}
 
@@ -754,7 +757,7 @@ func (d *Devbox) StartProcessManager(
 	)
 }
 
-// computeNixEnv computes the set of environment variables that define a Devbox
+// computeEnv computes the set of environment variables that define a Devbox
 // environment. The "devbox run" and "devbox shell" commands source these
 // variables into a shell before executing a command or showing an interactive
 // prompt.
@@ -778,11 +781,10 @@ func (d *Devbox) StartProcessManager(
 // programs.
 //
 // Note that the shellrc.tmpl template (which sources this environment) does
-// some additional processing. The computeNixEnv environment won't necessarily
+// some additional processing. The computeEnv environment won't necessarily
 // represent the final "devbox run" or "devbox shell" environments.
-// TODO: Rename to computeDevboxEnv?
-func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (map[string]string, error) {
-	defer trace.StartRegion(ctx, "computeNixEnv").End()
+func (d *Devbox) computeEnv(ctx context.Context, usePrintDevEnvCache bool) (map[string]string, error) {
+	defer trace.StartRegion(ctx, "devboxComputeEnv").End()
 
 	// Append variables from current env if --pure is not passed
 	currentEnv := os.Environ()
@@ -896,15 +898,21 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 
 	markEnvsAsSetByDevbox(pluginEnv, configEnv)
 
-	nixEnvPath := env["PATH"]
-	debug.Log("PATH after plugins and config is: %s", nixEnvPath)
+	// devboxEnvPath starts with the initial PATH from print-dev-env, and is
+	// transformed to be the "PATH of the Devbox environment"
+	// TODO: The prior statement is not fully true,
+	//  since env["PATH"] is written to above and so it is already no longer "PATH
+	//  from print-dev-env". Consider moving devboxEnvPath higher up in this function
+	//  where env["PATH"] is written to.
+	devboxEnvPath := env["PATH"]
+	debug.Log("PATH after plugins and config is: %s", devboxEnvPath)
 
 	// We filter out nix store paths of devbox-packages (represented here as buildInputs).
 	// Motivation: if a user removes a package from their devbox it should no longer
 	// be available in their environment.
 	buildInputs := strings.Split(env["buildInputs"], " ")
 	var glibcPatchPath []string
-	nixEnvPath = filterPathList(nixEnvPath, func(path string) bool {
+	devboxEnvPath = filterPathList(devboxEnvPath, func(path string) bool {
 		// TODO(gcurtis): this is a massive hack. Please get rid
 		// of this and install the package to the profile.
 		if strings.Contains(path, "patched-glibc") {
@@ -921,24 +929,24 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 		}
 		return true
 	})
-	debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath)
+	debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, devboxEnvPath)
 
 	// TODO(gcurtis): this is a massive hack. Please get rid
 	// of this and install the package to the profile.
 	if len(glibcPatchPath) != 0 {
 		patchedPath := strings.Join(glibcPatchPath, string(filepath.ListSeparator))
-		nixEnvPath = envpath.JoinPathLists(patchedPath, nixEnvPath)
-		debug.Log("PATH after glibc-patch hack is: %s", nixEnvPath)
+		devboxEnvPath = envpath.JoinPathLists(patchedPath, devboxEnvPath)
+		debug.Log("PATH after glibc-patch hack is: %s", devboxEnvPath)
 	}
 
 	runXPaths, err := d.RunXPaths(ctx)
 	if err != nil {
 		return nil, err
 	}
-	nixEnvPath = envpath.JoinPathLists(nixEnvPath, runXPaths)
+	devboxEnvPath = envpath.JoinPathLists(devboxEnvPath, runXPaths)
 
 	pathStack := envpath.Stack(env, originalEnv)
-	pathStack.Push(env, d.projectDirHash(), nixEnvPath, d.preservePathStack)
+	pathStack.Push(env, d.projectDirHash(), devboxEnvPath, d.preservePathStack)
 	env["PATH"] = pathStack.Path(env)
 	debug.Log("New path stack is: %s", pathStack)
 
@@ -958,25 +966,27 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
 	return env, d.addHashToEnv(env)
 }
 
-// ensurePackagesAreInstalledAndComputeEnv does what it says :P
-func (d *Devbox) ensurePackagesAreInstalledAndComputeEnv(
+// ensureStateIsUpToDateAndComputeEnv will return a map of the env-vars for the Devbox  Environment
+// while ensuring these reflect the current (up to date) state of the project.
+// TODO: find a better name for this function.
+func (d *Devbox) ensureStateIsUpToDateAndComputeEnv(
 	ctx context.Context,
 ) (map[string]string, error) {
 	defer debug.FunctionTimer().End()
 
-	// When ensurePackagesAreInstalled is called with ensure=true, it always
+	// When ensureStateIsUpToDate is called with ensure=true, it always
 	// returns early if the lockfile is up to date. So we don't need to check here
-	if err := d.ensurePackagesAreInstalled(ctx, ensure); err != nil && !strings.Contains(err.Error(), "no such host") {
+	if err := d.ensureStateIsUpToDate(ctx, ensure); err != nil && !strings.Contains(err.Error(), "no such host") {
 		return nil, err
 	} else if err != nil {
 		ux.Fwarning(d.stderr, "Error connecting to the internet. Will attempt to use cached environment.\n")
 	}
 
-	// Since ensurePackagesAreInstalled calls computeNixEnv when not up do date,
+	// Since ensureStateIsUpToDate calls computeEnv when not up do date,
 	// it's ok to use usePrintDevEnvCache=true here always. This does end up
 	// doing some non-nix work twice if lockfile is not up to date.
 	// TODO: Improve this to avoid extra work.
-	return d.computeNixEnv(ctx, true /*usePrintDevEnvCache*/)
+	return d.computeEnv(ctx, true /*usePrintDevEnvCache*/)
 }
 
 func (d *Devbox) nixPrintDevEnvCachePath() string {

internal/impl/devbox.go

@@ -67,21 +67,21 @@ func (n *testNix) PrintDevEnv(ctx context.Context, args *nix.PrintDevEnvArgs) (*
 	}, nil
 }
 
-func TestComputeNixEnv(t *testing.T) {
+func TestComputeEnv(t *testing.T) {
 	d := devboxForTesting(t)
 	d.nix = &testNix{}
 	ctx := context.Background()
-	env, err := d.computeNixEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeNixEnv should not fail")
-	assert.NotNil(t, env, "computeNixEnv should return a valid env")
+	env, err := d.computeEnv(ctx, false /*use cache*/)
+	require.NoError(t, err, "computeEnv should not fail")
+	assert.NotNil(t, env, "computeEnv should return a valid env")
 }
 
-func TestComputeNixPathIsIdempotent(t *testing.T) {
+func TestComputeDevboxPathIsIdempotent(t *testing.T) {
 	devbox := devboxForTesting(t)
 	devbox.nix = &testNix{"/tmp/my/path"}
 	ctx := context.Background()
-	env, err := devbox.computeNixEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeNixEnv should not fail")
+	env, err := devbox.computeEnv(ctx, false /*use cache*/)
+	require.NoError(t, err, "computeEnv should not fail")
 	path := env["PATH"]
 	assert.NotEmpty(t, path, "path should not be nil")
 
@@ -90,19 +90,19 @@ func TestComputeNixPathIsIdempotent(t *testing.T) {
 	t.Setenv(envpath.PathStackEnv, env[envpath.PathStackEnv])
 	t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
 
-	env, err = devbox.computeNixEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeNixEnv should not fail")
+	env, err = devbox.computeEnv(ctx, false /*use cache*/)
+	require.NoError(t, err, "computeEnv should not fail")
 	path2 := env["PATH"]
 
 	assert.Equal(t, path, path2, "path should be the same")
 }
 
-func TestComputeNixPathWhenRemoving(t *testing.T) {
+func TestComputeDevboxPathWhenRemoving(t *testing.T) {
 	devbox := devboxForTesting(t)
 	devbox.nix = &testNix{"/tmp/my/path"}
 	ctx := context.Background()
-	env, err := devbox.computeNixEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeNixEnv should not fail")
+	env, err := devbox.computeEnv(ctx, false /*use cache*/)
+	require.NoError(t, err, "computeEnv should not fail")
 	path := env["PATH"]
 	assert.NotEmpty(t, path, "path should not be nil")
 	assert.Contains(t, path, "/tmp/my/path", "path should contain /tmp/my/path")
@@ -113,8 +113,8 @@ func TestComputeNixPathWhenRemoving(t *testing.T) {
 	t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
 
 	devbox.nix.(*testNix).path = ""
-	env, err = devbox.computeNixEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeNixEnv should not fail")
+	env, err = devbox.computeEnv(ctx, false /*use cache*/)
+	require.NoError(t, err, "computeEnv should not fail")
 	path2 := env["PATH"]
 	assert.NotContains(t, path2, "/tmp/my/path", "path should not contain /tmp/my/path")
 

internal/impl/devbox_test.go

@@ -4,6 +4,10 @@ import (
 	"io"
 )
 
+// Naming Convention:
+// - suffix Opts for structs corresponding to a Devbox api function
+// - omit suffix Opts for other structs that are composed into an Opts struct
+
 type Opts struct {
 	Dir                      string
 	Env                      map[string]string
@@ -51,7 +55,7 @@ type UpdateOpts struct {
 	IgnoreMissingPackages bool
 }
 
-type NixEnvOpts struct {
+type EnvExportsOpts struct {
 	DontRecomputeEnvironment bool
 	NoRefreshAlias           bool
 	RunHooks                 bool

internal/impl/devopt/devboxopts.go

@@ -10,7 +10,7 @@ import (
 const (
 	// PathStackEnv stores the string representation of the stack, as a ":" separated list.
 	// Each element in the list is also the key to the env-var that stores the
-	// nixEnvPath for that devbox-project. Except for the last element, which is InitPathEnv.
+	// devboxEnvPath for that devbox-project. Except for the last element, which is InitPathEnv.
 	PathStackEnv = "DEVBOX_PATH_STACK"
 
 	// InitPathEnv stores the path prior to any devbox shellenv modifying the environment
@@ -19,9 +19,9 @@ const (
 
 // stack has the following design:
 // 1. The stack enables tracking which sub-paths in PATH come from which devbox-project
-// 2. It is an ordered-list of keys to env-vars that store nixEnvPath values of devbox-projects.
-// 3. The final PATH is reconstructed by concatenating the env-var values of each nixEnvPathKey.
-// 5. The stack is stored in its own env-var PathStackEnv, shared by all devbox-projects in this shell.
+// 2. It is an ordered-list of keys to env-vars that store devboxEnvPath values of devbox-projects.
+// 3. The final PATH is reconstructed by concatenating the env-var values of each of these keys.
+// 4. The stack is stored in its own env-var PathStackEnv, shared by all devbox-projects in this shell.
 type stack struct {
 	// keys holds the stack elements.
 	// Earlier (lower index number) keys get higher priority.
@@ -56,28 +56,27 @@ func (s *stack) Path(env map[string]string) string {
 }
 
 // Key is the element stored in the stack for a devbox-project. It represents
-// a pointer to the nixEnvPath value stored in its own env-var, also using this same
-// Key.
+// a pointer to the devboxEnvPath value stored in its own env-var, also using this same Key.
 func Key(projectHash string) string {
 	return "DEVBOX_NIX_ENV_PATH_" + projectHash
 }
 
-// Push adds the new nixEnvPath for the devbox-project identified by projectHash.
-// The nixEnvPath is pushed to the top of the stack (given highest priority),
+// Push adds the new PATH for the devbox-project identified by projectHash.
+// This PATH is pushed to the top of the stack (given highest priority),
 // unless preservePathStack is enabled.
 //
 // It also updates the env by modifying the PathStack env-var, and the env-var
-// for storing the nixEnvPath.
+// for storing this path.
 func (s *stack) Push(
 	env map[string]string,
 	projectHash string,
-	nixEnvPath string,
+	path string, // new PATH of the devbox-project of projectHash
 	preservePathStack bool,
 ) {
 	key := Key(projectHash)
 
-	// Add this nixEnvPath to env
-	env[key] = nixEnvPath
+	// Add this path to env
+	env[key] = path
 
 	// Common case: ensure this key is at the top of the stack
 	if !preservePathStack ||
@@ -90,8 +89,7 @@ func (s *stack) Push(
 	env[PathStackEnv] = s.String()
 }
 
-// Has tests if the stack has the specified key. Refer to the Key function for constructing
-// the appropriate key for any devbox-project.
+// Has tests if the stack has the key corresponding to projectHash
 func (s *stack) Has(projectHash string) bool {
 	return lo.Contains(s.keys, Key(projectHash))
 }