diff --git a/internal/devbox/devbox.go b/internal/devbox/devbox.go index 8ac001233d3..90f68df2dc7 100644 --- a/internal/devbox/devbox.go +++ b/internal/devbox/devbox.go @@ -379,6 +379,15 @@ func (d *Devbox) EnvExports(ctx context.Context, opts devopt.EnvExportsOpts) (st return "", err } + // Only export the variables that Devbox actually adds or changes relative to + // the current environment. The output of this function is meant to be eval'd + // in the caller's shell (e.g. `eval "$(devbox shellenv)"`), so re-exporting + // variables that are already set to the same value is unnecessary and can + // fail in shells that mark some inherited variables as read-only (e.g. + // PROFILEREAD on openSUSE). See + // https://github.com/jetify-com/devbox/issues/2826. + envs = onlyModifiedEnvs(envs, envir.PairsToMap(os.Environ())) + // Use the appropriate export format based on shell type var envStr string if opts.ShellFormat == devopt.ShellFormatNushell { diff --git a/internal/devbox/envvars.go b/internal/devbox/envvars.go index ec8231e19ab..1643511209a 100644 --- a/internal/devbox/envvars.go +++ b/internal/devbox/envvars.go @@ -14,6 +14,27 @@ import ( const devboxSetPrefix = "__DEVBOX_SET_" +// onlyModifiedEnvs returns the subset of envs whose values differ from those in +// baseEnv (or that are absent from baseEnv entirely). +// +// It is used to avoid emitting `export` statements for variables that Devbox did +// not actually change. Re-exporting a variable with the value it already has is +// not only redundant, it can break shells that mark some inherited variables as +// read-only. For example, openSUSE marks PROFILEREAD read-only, so +// `eval "$(devbox global shellenv)"` failed with +// "read-only variable: PROFILEREAD". See +// https://github.com/jetify-com/devbox/issues/2826. +func onlyModifiedEnvs(envs, baseEnv map[string]string) map[string]string { + modified := make(map[string]string, len(envs)) + for k, v := range envs { + if base, ok := baseEnv[k]; ok && base == v { + continue + } + modified[k] = v + } + return modified +} + // exportify formats vars as a line-separated string of shell export statements. // Each line is of the form `export key="value";` with any special characters in // value escaped. This means that the shell will always interpret values as diff --git a/internal/devbox/envvars_test.go b/internal/devbox/envvars_test.go new file mode 100644 index 00000000000..1ec55cc3c6e --- /dev/null +++ b/internal/devbox/envvars_test.go @@ -0,0 +1,88 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package devbox + +import ( + "maps" + "testing" +) + +func TestOnlyModifiedEnvs(t *testing.T) { + tests := []struct { + name string + envs map[string]string + baseEnv map[string]string + want map[string]string + }{ + { + name: "empty", + envs: map[string]string{}, + baseEnv: map[string]string{}, + want: map[string]string{}, + }, + { + name: "drops unchanged variables", + envs: map[string]string{ + "HOSTNAME": "myhost", + "LANG": "en_US.UTF-8", + "PROFILEREAD": "true", + }, + baseEnv: map[string]string{ + "HOSTNAME": "myhost", + "LANG": "en_US.UTF-8", + "PROFILEREAD": "true", + }, + want: map[string]string{}, + }, + { + name: "keeps changed and new variables", + envs: map[string]string{ + "PATH": "/nix/store/bin:/usr/bin", // changed + "HOSTNAME": "myhost", // unchanged + "DEVBOX_PROJECT_ROOT": "/home/user/project", // new + }, + baseEnv: map[string]string{ + "PATH": "/usr/bin", + "HOSTNAME": "myhost", + }, + want: map[string]string{ + "PATH": "/nix/store/bin:/usr/bin", + "DEVBOX_PROJECT_ROOT": "/home/user/project", + }, + }, + { + name: "keeps variable that became empty", + envs: map[string]string{ + "FOO": "", + }, + baseEnv: map[string]string{ + "FOO": "bar", + }, + want: map[string]string{ + "FOO": "", + }, + }, + { + name: "empty base env keeps everything", + envs: map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + baseEnv: map[string]string{}, + want: map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := onlyModifiedEnvs(test.envs, test.baseEnv) + if !maps.Equal(got, test.want) { + t.Errorf("onlyModifiedEnvs() = %v, want %v", got, test.want) + } + }) + } +}