Skip to content

Commit

Permalink
[shellenv] Remove nix profile from path, add shellenv command (#667)
Browse files Browse the repository at this point in the history
## Summary

This removes nix profile from path when using unified environment. It
adds a new shellenv command and prints instructions when package is
added inside devbox shell.

@gcurtis I chose `eval $(devbox shellenv)` over `. <(devbox shellenv)`
to be consistent with the global instructions and also because it's easy
to miss the period when we print it. Not sure if there's a difference.
Also is this fish compatible or do we need different instructions for
fish?

Lastly, you'll notice `hash -r` is not there anymore. In my experiments
running `eval $(devbox shellenv)` causes the hash table to clear. I
believe this happens because `PATH` has changed, but I'm not 100% sure.

## How was it tested?

<img width="762" alt="image"
src="https://user-images.githubusercontent.com/544948/220529645-42a6578f-e733-4943-9478-bb415a904a40.png">


```bash
➜ devbox shell
➜ devbox add php82 php82Extensions.memcached
➜ which php
php not found
➜ eval $(devbox shellenv)
➜ which php
/nix/store/5yylkc71wmlzii2vl40021vgbfwqmajb-php-with-extensions-8.2.1/bin/php
➜ php -m | grep memcached
memcached
```
  • Loading branch information
mikeland73 authored Feb 22, 2023
1 parent 0fdee09 commit 0eda044
Show file tree
Hide file tree
Showing 13 changed files with 114 additions and 27 deletions.
2 changes: 1 addition & 1 deletion devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type Devbox interface {
GenerateEnvrc(force bool, source string) error
Info(pkg string, markdown bool) error
ListScripts() []string
PrintEnv() (string, error)
PrintEnv(setFullPath bool) (string, error)
PrintGlobalList() error
PullGlobal(path string) error
// Remove removes Nix packages from the config so that it no longer exists in
Expand Down
3 changes: 2 additions & 1 deletion devbox.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"build-linux": "GOOS=linux go build -o dist/devbox-linux cmd/devbox/main.go",
"build-linux-amd64": "GOOS=linux GOARCH=amd64 go build -o dist/devbox-linux-amd64 cmd/devbox/main.go",
"code": "code .",
"lint": "golangci-lint run"
"lint": "golangci-lint run",
"test": "go test -race -cover ./..."
}
},
"nixpkgs": {
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func globalPullCmd() *cobra.Command {
func globalShellenvCmd() *cobra.Command {
return &cobra.Command{
Use: "shellenv",
Short: "Print shell commands that add Devbox packages to your PATH",
Short: "Print shell commands that add global Devbox packages to your PATH",
Run: func(*cobra.Command, []string) {
fmt.Print(impl.GenerateShellEnv())
},
Expand Down
1 change: 1 addition & 0 deletions internal/boxcli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func RootCmd() *cobra.Command {
command.AddCommand(ServicesCmd())
command.AddCommand(SetupCmd())
command.AddCommand(ShellCmd())
command.AddCommand(shellEnvCmd())
command.AddCommand(VersionCmd())
command.AddCommand(genDocsCmd())

Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func runShellCmd(cmd *cobra.Command, args []string, flags shellCmdFlags) error {
}

if flags.PrintEnv {
script, err := box.PrintEnv()
script, err := box.PrintEnv(false)
if err != nil {
return err
}
Expand Down
45 changes: 45 additions & 0 deletions internal/boxcli/shellenv.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2022 Jetpack Technologies Inc and contributors. All rights reserved.
// Use of this source code is governed by the license in the LICENSE file.

package boxcli

import (
"fmt"

"github.com/spf13/cobra"
"go.jetpack.io/devbox"
)

type shellEnvCmdFlags struct {
config configFlags
}

func shellEnvCmd() *cobra.Command {
flags := shellEnvCmdFlags{}
command := &cobra.Command{
Use: "shellenv",
Short: "Print shell commands that add Devbox packages to your PATH",
Args: cobra.ExactArgs(0),
PreRunE: ensureNixInstalled,
RunE: func(cmd *cobra.Command, args []string) error {
s, err := shellEnvFunc(cmd, flags)
if err != nil {
return err
}
fmt.Fprintln(cmd.OutOrStdout(), s)
return nil
},
}

flags.config.register(command)
return command
}

func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
box, err := devbox.Open(flags.config.path, cmd.ErrOrStderr())
if err != nil {
return "", err
}

return box.PrintEnv(true)
}
13 changes: 4 additions & 9 deletions internal/impl/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ func (d *Devbox) Exec(cmds ...string) error {
}
}

func (d *Devbox) PrintEnv() (string, error) {
func (d *Devbox) PrintEnv(setFullPath bool) (string, error) {
script := ""
if featureflag.UnifiedEnv.Disabled() {
envs, err := plugin.Env(d.packages(), d.projectDir)
Expand All @@ -455,7 +455,7 @@ func (d *Devbox) PrintEnv() (string, error) {
}
return script, nil
}
envs, err := d.computeNixEnv(false)
envs, err := d.computeNixEnv(setFullPath)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -760,8 +760,7 @@ func (d *Devbox) printPackageUpdateMessage(
// 1. Start with the PATH as defined by nix (through nix print-dev-env).
// 2. Clean the host PATH of any nix paths.
// 3. Append the cleaned host PATH (tradeoff between reproducibility and ease of use).
// 4. Prepend the devbox-managed nix profile path (which is needed to support devbox add inside shell--can we do without it?).
// 5. Prepend the paths of any plugins (tbd whether it's actually needed).
// 4. Prepend the paths of any plugins (tbd whether it's actually needed).
func (d *Devbox) computeNixEnv(setFullPath bool) (map[string]string, error) {

vaf, err := nix.PrintDevEnv(d.nixShellFilePath(), d.nixFlakesFilePath())
Expand Down Expand Up @@ -826,18 +825,14 @@ func (d *Devbox) computeNixEnv(setFullPath bool) (map[string]string, error) {

// PATH handling.
pluginVirtenvPath := d.pluginVirtenvPath() // TODO: consider removing this; not being used?
nixProfilePath, err := d.profileBinPath()
if err != nil {
return nil, err
}
nixPath := env["PATH"]
hostPath := nix.CleanEnvPath(os.Getenv("PATH"), os.Getenv("NIX_PROFILES"))

// NOTE: for devbox shell, we need to defer the PATH setting, because a user's init file may prepend
// stuff to PATH, which will then take precedence over the devbox-set PATH. Instead, we do the path
// prepending in shellrc.tmpl. I chose to use the `setFullPath` variable instead of something like
// `isShell` to discourage the addition of more logic that makes shell/run differ more.
pathPrepend := fmt.Sprintf("%s:%s:%s", pluginVirtenvPath, nixProfilePath, nixPath)
pathPrepend := fmt.Sprintf("%s:%s", pluginVirtenvPath, nixPath)
if setFullPath {
env["PATH"] = fmt.Sprintf("%s:%s", pathPrepend, hostPath)
} else {
Expand Down
9 changes: 9 additions & 0 deletions internal/plugin/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/fatih/color"
"github.com/pkg/errors"
"github.com/samber/lo"
"go.jetpack.io/devbox/internal/boxcli/featureflag"
)

func PrintReadme(
Expand Down Expand Up @@ -134,6 +135,14 @@ func printInfoInstructions(pkg string, w io.Writer) error {
}

func PrintEnvUpdateMessage(pkgs []string, projectDir string, w io.Writer) error {
if featureflag.UnifiedEnv.Enabled() {
color.New(color.FgYellow).Fprint(
w,
"\nTo update your shell and ensure your new packages are usable, "+
"please run:\n\neval $(devbox shellenv)\n\n\n",
)
return nil
}
commands := []string{"hash -r"}
for _, pkg := range pkgs {
if path := getEnvFilePathIfExist(pkg, projectDir); path != "" {
Expand Down
9 changes: 3 additions & 6 deletions testscripts/run/path.test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@ stdout '/some/clean/path'
# Path contains plugin virtual env path
stdout '.devbox/virtenv/bin'

# Path contains devbox-managed nix profile path
stdout '.devbox/nix/profile/default/bin'

# Path contains path to installed nix packages
stdout '-which-'

# Verify PATH is set in correct order: virtual env path, nix profile, nix packages, host path.
path.order '.devbox/virtenv/bin' '.devbox/nix/profile/default/bin' '-which-' 'some/clean/path'
# Verify PATH is set in correct order: virtual env path nix packages, host path.
path.order '.devbox/virtenv/bin' '-which-' 'some/clean/path'

# TODO: verify that bashrc file prepends do not prepend before nix paths.
# TODO: verify that bashrc file prepends do not prepend before nix paths.
12 changes: 12 additions & 0 deletions testscripts/shell/shellenv.test.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
exec devbox init

# test adding and running hello
exec devbox add hello
! exec hello
! stdout .

# source shellenv and test again
exec devbox shellenv
source.path
exec hello
stdout 'Hello, world!'
7 changes: 0 additions & 7 deletions testscripts/testrunner/assert.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ import (
"github.com/rogpeppe/go-internal/testscript"
)

// Custom assertions we can use inside a testscript.
var assertionMap = map[string]func(ts *testscript.TestScript, neg bool, args []string){
"env.path.len": assertPathLength,
"json.superset": assertJSONSuperset,
"path.order": assertPathOrder,
}

// Usage: env.path.len <number>
// Checks that the PATH environment variable has the expected number of entries.
func assertPathLength(script *testscript.TestScript, neg bool, args []string) {
Expand Down
29 changes: 29 additions & 0 deletions testscripts/testrunner/source.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package testrunner

import (
"strings"

"github.com/rogpeppe/go-internal/testscript"
)

// Sources whatever path is exported in stdout. Ignored everything else
// Usage:
// exec devbox shellenv
// source.path
func sourcePath(script *testscript.TestScript, neg bool, args []string) {
if len(args) != 0 {
script.Fatalf("usage: source.path")
}
if neg {
script.Fatalf("source.path does not support negation")
}
sourcedScript := script.ReadFile("stdout")
for _, line := range strings.Split(sourcedScript, "\n") {
if strings.HasPrefix(line, "export PATH=") {
path := strings.TrimPrefix(line, "export PATH=")
path = strings.Trim(path, "\"")
script.Setenv("PATH", path)
break
}
}
}
7 changes: 6 additions & 1 deletion testscripts/testrunner/testrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ func getTestscriptParams(dir string) testscript.Params {
RequireExplicitExec: true,
TestWork: false, // Set to true if you're trying to debug a test.
Setup: setupTestEnv,
Cmds: assertionMap,
Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
"env.path.len": assertPathLength,
"json.superset": assertJSONSuperset,
"path.order": assertPathOrder,
"source.path": sourcePath,
},
}
}

0 comments on commit 0eda044

Please sign in to comment.