Empty environment variables passed to vs code are unset

[aidantwoods]

Does this issue occur when all extensions are disabled?: Yes

• VS Code Version: 1.75.1 (Universal)
• OS Version: macOS 13.2

When VS Code is launched with a set but empty env var, it will drop it and treat it as unset. This breaks workflows which check for the presence of env vars (which may be empty in some cases) when running VS Code integrations, e.g. running tests, running the debugger.

Steps to Reproduce:

1. Launch vscode using command with FOO="" BAR="test" code .
2. In a terminal within vscode run: [[ -v FOO ]] && echo "foo set", and then [[ -v BAR ]] && echo "bar set"
3. Observe that FOO is missing, BAR is set

Compare with running in a terminal manually, with an empty FOO:

$ export FOO=""
$ [[ -v FOO ]] && echo "foo set"
foo set

Of the available historical downloads, I found that vscode 1.73.1 still replicates the above, but does seem to correctly pass the empty env vars down to other processes (e.g. the debugger).

[roblourens]

I'm seeing the variable make it to the debugger, eg here in the debug console, it's in process.env

But I'm not seeing it in the terminal. @Tyriar @meganrogge does one of the terminal processes clear out variables with empty values?

[meganrogge]

yep, looks like that would happen here

vscode/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts

Lines 72 to 82 in 96aa2a7

	function mergeNonNullKeys(env: IProcessEnvironment, other: ITerminalEnvironment | undefined) {
	if (!other) {
	return;
	}
	for (const key of Object.keys(other)) {
	const value = other[key];
	if (value) {
	env[key] = value;
	}
	}
	}

[aidantwoods]

To give some more context on how I was experiencing the issue primarily: it was when running tests or the debugger (with the Go extension installed). Is it possible a similar (or maybe the same) behaviour to is affecting how env vars get passed down to extensions (as well as the terminal process)?

[roblourens]

Maybe if the debuggee is launched from a terminal? Or something particular to that debug extension. It worked for me when debugging node.

[aidantwoods]

I did think it could be one of those at first, but I can reliably repro the issue in latest VS Code, whilst 1.73.1 works fine with the same setup (although still reproduces the integrated terminal bug).

When I get a moment, I'll put together a minimal repro for the extension issue just so it is testable.

[fedorareis]

I just tested out the insider build with this fix and it looks like the terminal itself now has the environment variables set correctly. But other areas, like the test runners, don't. For example I tried running a golang test using the play button in the gutter and the tests still failed with an error about an environment variable not being set. As @aidantwoods mentioned, this worked fine in 1.73.1.

[Tyriar]

@fedorareis thanks, maybe we have a similar check for terminals created via extensions. Will have a look

[Tyriar]

This is working fine for me. To verify:

• Test to ensure a variable in terminal.integrated.env.windows/linux with a value of "" comes through to the actual terminal environment
• Do the same when the environment change is applied via the context.environmentVariableCollection.replace extension API

[fedorareis]

@Tyriar I'm still seeing the issue in the current insider build. I've created a minimal reproduction of the issue.

1. Create a new go project
2. Create a new go file ex.go with the following content

package main

import (
	"errors"
	"os"
)

func getVars(env string) (string, error) {
	val, set := os.LookupEnv(env)
	if !set {
		return "", errors.New("can't find variable:" + env)
	}
	return val, nil
}

3. Create another new go file ex_test.go with the following content

package main

import "testing"

func TestGetVars(t *testing.T) {
	env := "BAR"
	_, err := getVars(env)
	if err != nil {
		t.Fatalf("%s not set", env)
	}

	env = "FOO"
	_, err = getVars(env)
	if err != nil {
		t.Fatalf("%s not set", env)
	}

}

4. open the project using FOO="" BAR="test" code-insiders
5. navigate to ex_test.go and run the test
6. the test will fail on line 15 with the error "FOO not set"

As @aidantwoods mentioned above this works properly in version 1.73.1 and the test passes

[fedorareis]

@Tyriar should I file a new issue for the scenario I described above or will this issue be reopened to look into it?

[Tyriar]

@fedorareis I think it's tracked in #182560