From 760c07cad4f5c113ab52774b1718fdcc8b5c1cab Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Fri, 2 Jun 2023 16:24:32 -0700 Subject: [PATCH 1/5] [shellenv] filter out /nix/store paths from PATH --- internal/impl/devbox.go | 5 +++++ internal/impl/shell.go | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go index 73c06dcbbad..02fdc540a36 100644 --- a/internal/impl/devbox.go +++ b/internal/impl/devbox.go @@ -841,6 +841,11 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m nixEnvPath := env["PATH"] debug.Log("PATH after plugins and config is: %s", nixEnvPath) + nixEnvPath = filterPathList(nixEnvPath, func(path string) bool { + return !strings.HasPrefix(path, "/nix/store") + }) + debug.Log("PATH after removing /nix/store paths is: %s", nixEnvPath) + env["PATH"] = JoinPathLists(nixEnvPath, originalPath) debug.Log("computed environment PATH is: %s", env["PATH"]) diff --git a/internal/impl/shell.go b/internal/impl/shell.go index 95c29276fb1..f0430758a14 100644 --- a/internal/impl/shell.go +++ b/internal/impl/shell.go @@ -399,3 +399,13 @@ func JoinPathLists(pathLists ...string) string { } return strings.Join(cleaned, string(filepath.ListSeparator)) } + +func filterPathList(pathList string, keep func(string) bool) string { + filtered := []string{} + for _, path := range filepath.SplitList(pathList) { + if keep(path) { + filtered = append(filtered, path) + } + } + return strings.Join(filtered, string(filepath.ListSeparator)) +} From 0c1b36a3eae9da71feb46160075aad0f725cdf79 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Tue, 6 Jun 2023 16:59:25 -0700 Subject: [PATCH 2/5] add comment --- internal/impl/devbox.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go index 02fdc540a36..10d8761c3c2 100644 --- a/internal/impl/devbox.go +++ b/internal/impl/devbox.go @@ -841,6 +841,10 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m nixEnvPath := env["PATH"] debug.Log("PATH after plugins and config is: %s", nixEnvPath) + // We filter out nix store paths so that if a user removes a package from their devbox + // it no longer is available in their environment. This is needed because nix may keep + // packages around if they are used somewhere else or if the user hasn't triggered + // garbage collection. nixEnvPath = filterPathList(nixEnvPath, func(path string) bool { return !strings.HasPrefix(path, "/nix/store") }) From 7da66df1626e0f37e108e400e3696dc181f6b63f Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Wed, 7 Jun 2023 17:56:51 -0700 Subject: [PATCH 3/5] update testscript unit test --- testscripts/run/path.test.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/testscripts/run/path.test.txt b/testscripts/run/path.test.txt index 3fd19fb15ce..fd2076b6fb3 100644 --- a/testscripts/run/path.test.txt +++ b/testscripts/run/path.test.txt @@ -12,10 +12,11 @@ exec devbox run echo '$PATH' ! stdout '/some//dirty/../clean/path' stdout '/some/clean/path' -# Path contains path to installed nix packages -stdout '-which-' +# Path contains path to installed nix packages in the wrappers and nix profile +stdout '.devbox/virtenv/.wrappers/bin' +stdout '.devbox/nix/profile/default/bin' # Verify PATH is set in correct order: virtual env path nix packages, host path. -path.order '-which-' 'some/clean/path' +path.order '.devbox/virtenv/.wrappers/bin' '/some/clean/path' # TODO: verify that bashrc file prepends do not prepend before nix paths. From 5b58a330d6353f9a8d23643470b602ee0e224f90 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Fri, 9 Jun 2023 11:13:26 -0700 Subject: [PATCH 4/5] filter only buildInputs --- internal/impl/devbox.go | 19 +++++++++++++------ internal/impl/devbox_test.go | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go index 10d8761c3c2..6333bdb2564 100644 --- a/internal/impl/devbox.go +++ b/internal/impl/devbox.go @@ -841,14 +841,21 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m nixEnvPath := env["PATH"] debug.Log("PATH after plugins and config is: %s", nixEnvPath) - // We filter out nix store paths so that if a user removes a package from their devbox - // it no longer is available in their environment. This is needed because nix may keep - // packages around if they are used somewhere else or if the user hasn't triggered - // garbage collection. + // 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"], " ") nixEnvPath = filterPathList(nixEnvPath, func(path string) bool { - return !strings.HasPrefix(path, "/nix/store") + for _, input := range buildInputs { + // input is of the form: /nix/store/-- + // path is of the form: /nix/store/--/bin + if strings.HasPrefix(path, input) { + return false + } + } + return true }) - debug.Log("PATH after removing /nix/store paths is: %s", nixEnvPath) + debug.Log("PATH after filtering with buildInputs (%v) is: %s", buildInputs, nixEnvPath) env["PATH"] = JoinPathLists(nixEnvPath, originalPath) debug.Log("computed environment PATH is: %s", env["PATH"]) diff --git a/internal/impl/devbox_test.go b/internal/impl/devbox_test.go index 139f68a4abd..604cbe4934e 100644 --- a/internal/impl/devbox_test.go +++ b/internal/impl/devbox_test.go @@ -126,5 +126,5 @@ func TestComputeNixPathWhenRemoving(t *testing.T) { path2 := env["PATH"] assert.NotContains(t, path2, "/tmp/my/path", "path should not contain /tmp/my/path") - assert.NotEqual(t, path, path2, "path should be the same") + assert.NotEqual(t, path, path2, "path should not be the same") } From 03443a6ce0c25f59f98bd252d35032a0d8a5cb94 Mon Sep 17 00:00:00 2001 From: Savil Srivastava <676452+savil@users.noreply.github.com> Date: Fri, 9 Jun 2023 14:25:55 -0700 Subject: [PATCH 5/5] debugging --- internal/impl/devbox.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/impl/devbox.go b/internal/impl/devbox.go index 6333bdb2564..b82b5290e0e 100644 --- a/internal/impl/devbox.go +++ b/internal/impl/devbox.go @@ -849,7 +849,8 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m for _, input := range buildInputs { // input is of the form: /nix/store/-- // path is of the form: /nix/store/--/bin - if strings.HasPrefix(path, input) { + if strings.TrimSpace(input) != "" && strings.HasPrefix(path, input) { + debug.Log("returning false for path %s and input %s\n", path, input) return false } }