Skip to content

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

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
Dec 13, 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
2 changes: 1 addition & 1 deletion devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Binary file removed examples/development/go/hello-world/trace.out
Binary file not shown.
2 changes: 1 addition & 1 deletion internal/boxcli/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/shellenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
66 changes: 38 additions & 28 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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") {
Expand All @@ -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)

Expand All @@ -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 {
Expand Down
28 changes: 14 additions & 14 deletions internal/impl/devbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")
Expand All @@ -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")

Expand Down
6 changes: 5 additions & 1 deletion internal/impl/devopt/devboxopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -51,7 +55,7 @@ type UpdateOpts struct {
IgnoreMissingPackages bool
}

type NixEnvOpts struct {
type EnvExportsOpts struct {
DontRecomputeEnvironment bool
NoRefreshAlias bool
RunHooks bool
Expand Down
26 changes: 12 additions & 14 deletions internal/impl/envpath/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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 ||
Expand All @@ -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))
}
Loading