requested changes

Author: savil

devbox.go

@@ -23,7 +23,7 @@ type Devbox interface {
 	Install(ctx context.Context) error
 	IsEnvEnabled() bool
 	ListScripts() []string
-	DevboxEnvExports(ctx context.Context, opts devopt.DevboxEnvExports) (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.DevboxEnvExports(cmd.Context(), devopt.DevboxEnvExports{})
+		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.DevboxEnvExports(cmd.Context(), devopt.DevboxEnvExports{
+	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.computeCurrentDevboxEnv(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.computeCurrentDevboxEnv(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.ensureProjectStateIsCurrent(ctx, ensure)
+	return d.ensureStateIsUpToDate(ctx, ensure)
 }
 
 func (d *Devbox) ListScripts() []string {
@@ -274,11 +274,11 @@ func (d *Devbox) ListScripts() []string {
 	return keys
 }
 
-// DevboxEnvExports returns a string of the env-vars that would need to be applied
+// 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) DevboxEnvExports(ctx context.Context, opts devopt.DevboxEnvExports) (string, error) {
-	ctx, task := trace.NewTask(ctx, "devboxNixEnv")
+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
@@ -298,9 +298,9 @@ func (d *Devbox) DevboxEnvExports(ctx context.Context, opts devopt.DevboxEnvExpo
 			)
 		}
 
-		envs, err = d.computeDevboxEnv(ctx, true /*usePrintDevEnvCache*/)
+		envs, err = d.computeEnv(ctx, true /*usePrintDevEnvCache*/)
 	} else {
-		envs, err = d.computeCurrentDevboxEnv(ctx)
+		envs, err = d.ensureStateIsUpToDateAndComputeEnv(ctx)
 	}
 
 	if err != nil {
@@ -325,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.computeCurrentDevboxEnv(ctx)
+	envs, err := d.ensureStateIsUpToDateAndComputeEnv(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -485,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.ensureProjectStateIsCurrent(ctx, ensure); err != nil {
+	if err := d.ensureStateIsUpToDate(ctx, ensure); err != nil {
 		return err
 	}
 
@@ -757,7 +757,7 @@ func (d *Devbox) StartProcessManager(
 	)
 }
 
-// computeDevboxEnv 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.
@@ -781,10 +781,10 @@ func (d *Devbox) StartProcessManager(
 // programs.
 //
 // Note that the shellrc.tmpl template (which sources this environment) does
-// some additional processing. The computeDevboxEnv environment won't necessarily
+// some additional processing. The computeEnv environment won't necessarily
 // represent the final "devbox run" or "devbox shell" environments.
-func (d *Devbox) computeDevboxEnv(ctx context.Context, usePrintDevEnvCache bool) (map[string]string, error) {
-	defer trace.StartRegion(ctx, "computeDevboxEnv").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()
@@ -898,6 +898,12 @@ func (d *Devbox) computeDevboxEnv(ctx context.Context, usePrintDevEnvCache bool)
 
 	markEnvsAsSetByDevbox(pluginEnv, configEnv)
 
+	// 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)
 
@@ -960,26 +966,27 @@ func (d *Devbox) computeDevboxEnv(ctx context.Context, usePrintDevEnvCache bool)
 	return env, d.addHashToEnv(env)
 }
 
-// computeCurrentDevboxEnv will return a map of the env-vars for the Devbox  Environment
+// 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.
-func (d *Devbox) computeCurrentDevboxEnv(
+// TODO: find a better name for this function.
+func (d *Devbox) ensureStateIsUpToDateAndComputeEnv(
 	ctx context.Context,
 ) (map[string]string, error) {
 	defer debug.FunctionTimer().End()
 
-	// When ensureProjectStateIsCurrent 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.ensureProjectStateIsCurrent(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 computeDevboxEnv 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.computeDevboxEnv(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 TestComputeDevboxEnv(t *testing.T) {
+func TestComputeEnv(t *testing.T) {
 	d := devboxForTesting(t)
 	d.nix = &testNix{}
 	ctx := context.Background()
-	env, err := d.computeDevboxEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeDevboxEnv should not fail")
-	assert.NotNil(t, env, "computeDevboxEnv 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 TestComputeDevboxPathIsIdempotent(t *testing.T) {
 	devbox := devboxForTesting(t)
 	devbox.nix = &testNix{"/tmp/my/path"}
 	ctx := context.Background()
-	env, err := devbox.computeDevboxEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeDevboxEnv 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,8 +90,8 @@ func TestComputeDevboxPathIsIdempotent(t *testing.T) {
 	t.Setenv(envpath.PathStackEnv, env[envpath.PathStackEnv])
 	t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
 
-	env, err = devbox.computeDevboxEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeDevboxEnv 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")
@@ -101,8 +101,8 @@ func TestComputeDevboxPathWhenRemoving(t *testing.T) {
 	devbox := devboxForTesting(t)
 	devbox.nix = &testNix{"/tmp/my/path"}
 	ctx := context.Background()
-	env, err := devbox.computeDevboxEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeDevboxEnv 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 TestComputeDevboxPathWhenRemoving(t *testing.T) {
 	t.Setenv(envpath.Key(devbox.projectDirHash()), env[envpath.Key(devbox.projectDirHash())])
 
 	devbox.nix.(*testNix).path = ""
-	env, err = devbox.computeDevboxEnv(ctx, false /*use cache*/)
-	require.NoError(t, err, "computeDevboxEnv 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 DevboxEnvExports struct {
+type EnvExportsOpts struct {
 	DontRecomputeEnvironment bool
 	NoRefreshAlias           bool
 	RunHooks                 bool

internal/impl/devopt/devboxopts.go

@@ -98,12 +98,12 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames []string, opts devopt.AddOpt
 		addedPackageNames = append(addedPackageNames, packageNameForConfig)
 	}
 
-	// Options must be set before ensureProjectStateIsCurrent. See comment in function
+	// Options must be set before ensureStateIsUpToDate. See comment in function
 	if err := d.setPackageOptions(addedPackageNames, opts); err != nil {
 		return err
 	}
 
-	if err := d.ensureProjectStateIsCurrent(ctx, install); err != nil {
+	if err := d.ensureStateIsUpToDate(ctx, install); err != nil {
 		return usererr.WithUserMessage(err, "There was an error installing nix packages")
 	}
 
@@ -135,7 +135,7 @@ func (d *Devbox) setPackageOptions(pkgs []string, opts devopt.AddOpts) error {
 		}
 	}
 
-	// Resolving here ensures we allow insecure before running ensureProjectStateIsCurrent
+	// Resolving here ensures we allow insecure before running ensureStateIsUpToDate
 	// which will call print-dev-env. Resolving does not save the lockfile, we
 	// save at the end when everything has succeeded.
 	if opts.AllowInsecure {
@@ -218,14 +218,14 @@ func (d *Devbox) Remove(ctx context.Context, pkgs ...string) error {
 	}
 
 	// this will clean up the now-extra package from nix profile and the lockfile
-	if err := d.ensureProjectStateIsCurrent(ctx, uninstall); err != nil {
+	if err := d.ensureStateIsUpToDate(ctx, uninstall); err != nil {
 		return err
 	}
 
 	return d.saveCfg()
 }
 
-// installMode is an enum for helping with ensureProjectStateIsCurrent implementation
+// installMode is an enum for helping with ensureStateIsUpToDate implementation
 type installMode string
 
 const (
@@ -236,7 +236,7 @@ const (
 	ensure installMode = "ensure"
 )
 
-// ensureProjectStateIsCurrent ensures the Devbox project state is up to date.
+// ensureStateIsUpToDate ensures the Devbox project state is up to date.
 // Namely:
 //  1. Packages are installed, in nix-profile or runx.
 //     Extraneous packages are removed (references purged, not uninstalled).
@@ -248,8 +248,8 @@ const (
 // 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.
-func (d *Devbox) ensureProjectStateIsCurrent(ctx context.Context, mode installMode) error {
-	defer trace.StartRegion(ctx, "ensureProjectStateIsCurrent").End()
+func (d *Devbox) ensureStateIsUpToDate(ctx context.Context, mode installMode) error {
+	defer trace.StartRegion(ctx, "devboxEnsureStateIsUpToDate").End()
 	defer debug.FunctionTimer().End()
 
 	// if mode is install or uninstall, then we need to update the nix-profile
@@ -293,7 +293,7 @@ func (d *Devbox) ensureProjectStateIsCurrent(ctx context.Context, mode installMo
 	// 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()
-	if _, err := d.computeDevboxEnv(ctx, usePrintDevEnvCache); err != nil {
+	if _, err := d.computeEnv(ctx, usePrintDevEnvCache); err != nil {
 		return err
 	}
 

internal/impl/packages.go

@@ -65,7 +65,7 @@ func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
 		}
 	}
 
-	if err := d.ensureProjectStateIsCurrent(ctx, update); err != nil {
+	if err := d.ensureStateIsUpToDate(ctx, update); err != nil {
 		return err
 	}