Skip to content

Commit a1d3832

Browse files
authored
Improve handling when no SHELL env-var is defined (#700)
## Summary Previously, if `SHELL` env-var was not set, we'd return an error from `nix.DetectShell` and initialize an empty struct `nix.Shell{}`. Then, in `Shell.Run` we'd see that `Shell.binPath` is empty and call `nix-shell`. This has the following problems: 1. Initializing an empty `nix.Shell{}` means that we drop all the other `ShellOptions` in the constructor. 2. In `Shell.Run`, we'd use `nix-shell` but fail if the flakes feature was not enabled. This `nix-shell` also loses all the print-dev-env work that we have done for Unified-Env. To solve and/or mitigate these issues, I propose the following changes: - Rename `nix.Shell` to `nix.DevboxShell` to make it clear that its not a raw nix shell. TODO for future to move this out of `nix` package. - Rename `nix.DetectShell` to `nix.NewDevboxShell` to make clear that it is a constructor function. Decompose its functionality into helper functions `shellPath` and `initShellBinaryFields`. - If `SHELL` is undefined, ~we loop over a list of common shells to see if they are accessible in the `PATH`.~ we use the bash from nix. - Failing to find ~any recognizable shell in `PATH`,~ bash from nix, we fail the program with an error. - Remove the fallback to `nix-shell` in `Shell.Run`. This means we _always_ use the Unified-Env codepath. - Update `shell_test` for Flakes feature, since it now finds a shell from PATH and uses that (previously, would error). ## How was it tested? in `examples/testdata/go/go-1.19`: ``` ❯ DEVBOX_DEBUG=0 SHELL= DEVBOX_FEATURE_FLAKES=0 devbox shell Ensuring packages are installed. Starting a devbox shell... (devbox) Savil-Srivastavas-MacBook-Pro:go-1.19 savil$ echo $SHELL /nix/store/1bsjl5incfnszv7scdh4d02sh45vw2w1-bash-5.1-p16/bin/bash (devbox) Savil-Srivastavas-MacBook-Pro:go-1.19 savil$ exit exit ❯ DEVBOX_DEBUG=0 SHELL= DEVBOX_FEATURE_FLAKES=1 devbox shell Ensuring packages are installed. Installing package: go_1_19. [1/1] go_1_19 [1/1] go_1_19: Success Starting a devbox shell... (devbox) Savil-Srivastavas-MacBook-Pro:go-1.19 savil$ echo $SHELL /nix/store/1bsjl5incfnszv7scdh4d02sh45vw2w1-bash-5.1-p16/bin/bash (devbox) Savil-Srivastavas-MacBook-Pro:go-1.19 savil$ ``` Also: `go test ./...` with flakes feature enabled and disabled.
1 parent e12d3b6 commit a1d3832

File tree

4 files changed

+132
-86
lines changed

4 files changed

+132
-86
lines changed

internal/boxcli/shell_test.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ import (
1616
"time"
1717

1818
"github.com/stretchr/testify/assert"
19-
"go.jetpack.io/devbox/internal/boxcli/featureflag"
20-
"go.jetpack.io/devbox/internal/nix"
2119
"go.jetpack.io/devbox/internal/testframework"
2220
)
2321

@@ -314,13 +312,6 @@ func TestShell(t *testing.T) {
314312
err := td.SetDevboxJSON(devboxJSON)
315313
assert.NoError(t, err)
316314
output, err := td.RunCommand(ShellCmd())
317-
if featureflag.Flakes.Enabled() {
318-
assert.Error(t, err)
319-
if !errors.Is(err, nix.ErrNoDefaultShellUnsupportedInFlakesMode) {
320-
assert.Fail(t, "Expected error %s but received %s", nix.ErrNoDefaultShellUnsupportedInFlakesMode, err)
321-
}
322-
} else {
323-
assert.NoError(t, err)
324-
assert.Contains(t, output, "Starting a devbox shell...")
325-
}
315+
assert.NoError(t, err)
316+
assert.Contains(t, output, "Starting a devbox shell...")
326317
}

internal/impl/devbox.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,9 @@ func (d *Devbox) Shell() error {
271271
nix.WithShellStartTime(shellStartTime),
272272
}
273273

274-
shell, err := nix.DetectShell(opts...)
274+
shell, err := nix.NewDevboxShell(d.cfg.Nixpkgs.Commit, opts...)
275275
if err != nil {
276-
// Fall back to using a plain Nix shell.
277-
shell = &nix.Shell{}
276+
return err
278277
}
279278

280279
shell.UserInitHook = d.cfg.Shell.InitHook.String()
@@ -358,11 +357,11 @@ func (d *Devbox) RunScriptInNewNixShell(scriptName string) error {
358357
nix.WithPKGConfigDir(d.pluginVirtenvPath()),
359358
}
360359

361-
shell, err := nix.DetectShell(opts...)
360+
shell, err := nix.NewDevboxShell(d.cfg.Nixpkgs.Commit, opts...)
362361

363362
if err != nil {
364363
fmt.Fprint(d.writer, err)
365-
shell = &nix.Shell{}
364+
return err
366365
}
367366

368367
shell.UserInitHook = d.cfg.Shell.InitHook.String()
@@ -381,7 +380,8 @@ func (d *Devbox) RunScriptInShell(scriptName string) error {
381380
return usererr.New("unable to find a script with name %s", scriptName)
382381
}
383382

384-
shell, err := nix.DetectShell(
383+
shell, err := nix.NewDevboxShell(
384+
d.cfg.Nixpkgs.Commit,
385385
nix.WithProfile(profileDir),
386386
nix.WithHistoryFile(filepath.Join(d.projectDir, shellHistoryFile)),
387387
nix.WithUserScript(scriptName, script.String()),
@@ -390,7 +390,7 @@ func (d *Devbox) RunScriptInShell(scriptName string) error {
390390

391391
if err != nil {
392392
fmt.Fprint(d.writer, err)
393-
shell = &nix.Shell{}
393+
return err
394394
}
395395

396396
return shell.RunInShell()

0 commit comments

Comments
 (0)