Skip to content

Fix empty vs. unset environment variables on Windows. #1997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BillyONeal wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix empty vs. unset environment variables on Windows. #1997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7da24cd
Fix empty vs. nonexistent environment variables on Windows.
BillyONeal Apr 15, 2026
7c06aff
Add one more test case.
BillyONeal Apr 15, 2026
4dc2856
Refactor Rename broke me for !WIN32
BillyONeal Apr 15, 2026
472be61
Apply suggestions from code review
BillyONeal Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/vcpkg/base/system.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
namespace vcpkg
{
Optional<std::string> get_environment_variable(ZStringView varname);
// identical to get_environment_variable but returns nullopt if the variable is set to an empty string
Optional<std::string> get_environment_variable_nonempty(ZStringView varname);
void set_environment_variable(ZStringView varname, Optional<ZStringView> value) noexcept;

std::vector<std::string> get_environment_variables();
Expand Down
39 changes: 38 additions & 1 deletion src/vcpkg-test/system.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace
};
}

TEST_CASE ("[to_cpu_architecture]", "system")
TEST_CASE ("to_cpu_architecture", "[system]")
{
struct test_case
{
Expand Down Expand Up @@ -113,6 +113,43 @@ TEST_CASE ("guess_visual_studio_prompt", "[system]")
CHECK(guess_visual_studio_prompt_target_architecture().value_or_exit(VCPKG_LINE_INFO) == CPUArchitecture::X64);
}

static constexpr StringLiteral test_variable_name = "VCPKG_TEST_SET_GET_ENV_VARIABLE";
static void check_environment_variable_roundtrip(const std::string& expected)
{
set_environment_variable(test_variable_name, expected.c_str());
auto maybe_actual = get_environment_variable(test_variable_name);
if (auto actual = maybe_actual.get())
{
CHECK(*actual == expected);
}
else
{
FAIL("no set variable");
}
}

TEST_CASE ("environment_variable_roundtrip", "[system]")
{
CHECK(!get_environment_variable(test_variable_name).has_value());
environment_variable_resetter reset_varname{test_variable_name};
Comment thread
BillyONeal marked this conversation as resolved.
CHECK(!get_environment_variable(test_variable_name).has_value());
check_environment_variable_roundtrip("a value that is not nullopt");
CHECK(get_environment_variable(test_variable_name).has_value());
check_environment_variable_roundtrip("");
CHECK(get_environment_variable(test_variable_name).has_value());
set_environment_variable(test_variable_name, nullopt);
CHECK(!get_environment_variable(test_variable_name).has_value());

check_environment_variable_roundtrip("x");

const auto wstring_default_capacity = std::wstring{}.capacity();
REQUIRE(wstring_default_capacity > 0);

check_environment_variable_roundtrip(std::string(wstring_default_capacity - 1, 'a'));
check_environment_variable_roundtrip(std::string(wstring_default_capacity, 'b'));
check_environment_variable_roundtrip(std::string(wstring_default_capacity + 1, 'c'));
}

TEST_CASE ("cmdlinebuilder", "[system]")
{
Command cmd;
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg-test/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ namespace vcpkg::Test
static Path internal_base_temporary_directory()
{
#if defined(_WIN32)
return Path(vcpkg::get_environment_variable("TEMP").value_or_exit(VCPKG_LINE_INFO)) / "vcpkg-test";
return Path(vcpkg::get_environment_variable_nonempty("TEMP").value_or_exit(VCPKG_LINE_INFO)) / "vcpkg-test";
#else
return "/tmp/vcpkg-test";
#endif
Expand Down
6 changes: 3 additions & 3 deletions src/vcpkg/base/files.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ namespace

std::vector<Path> calculate_path_bases()
{
auto path_base_strings =
Strings::split_paths(get_environment_variable(EnvironmentVariablePath).value_or_exit(VCPKG_LINE_INFO));
auto path_base_strings = Strings::split_paths(
get_environment_variable_nonempty(EnvironmentVariablePath).value_or_exit(VCPKG_LINE_INFO));
return std::vector<Path>{std::make_move_iterator(path_base_strings.begin()),
std::make_move_iterator(path_base_strings.end())};
}
Expand Down Expand Up @@ -3653,7 +3653,7 @@ namespace vcpkg
Path temp_folder_path = Path(Strings::to_utf8(temp_folder, length_without_null)) / "vcpkg";
#else // ^^^ _WIN32 // !_WIN32 vvv
const Path temp_folder_path =
Path(get_environment_variable("TMPDIR").value_or(std::string("/tmp"))) / "vcpkg";
Path(get_environment_variable_nonempty("TMPDIR").value_or(std::string("/tmp"))) / "vcpkg";
#endif // ^^^ !_WIN32

this->create_directories(temp_folder_path, ec);
Expand Down
150 changes: 97 additions & 53 deletions src/vcpkg/base/system.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,23 +343,64 @@
{
#if defined(_WIN32)
const auto w_varname = Strings::to_utf16(varname);
const auto sz = GetEnvironmentVariableW(w_varname.c_str(), nullptr, 0);
if (sz == 0) return nullopt;
// Try to read into the small string optimization buffer first to avoid a second API call
std::wstring ret;
const auto initial_capacity = ret.capacity();
Checks::check_exit(VCPKG_LINE_INFO, MAXDWORD > initial_capacity);
// +1 for null terminator
const DWORD initial_buffer_size = static_cast<DWORD>(initial_capacity + 1);
ret.resize(ret.capacity());
SetLastError(ERROR_SUCCESS);
const auto sz = GetEnvironmentVariableW(w_varname.c_str(), ret.data(), initial_buffer_size);
if (sz == 0)
{
const auto last_error = GetLastError();
if (last_error == ERROR_ENVVAR_NOT_FOUND)
{
return nullopt;
}
else if (last_error != ERROR_SUCCESS)
{
Checks::unreachable(VCPKG_LINE_INFO);
Comment thread
BillyONeal marked this conversation as resolved.
}

std::wstring ret(sz, L'\0');
// last_error == ERROR_SUCCESS means the variable is present but empty, so it will be returned in the "Fits
// in SSO buffer" block below
}

Checks::check_exit(VCPKG_LINE_INFO, MAXDWORD >= ret.size());
const auto sz2 = GetEnvironmentVariableW(w_varname.c_str(), ret.data(), static_cast<DWORD>(ret.size()));
if (sz < initial_buffer_size)
{
// Fits in SSO buffer
ret.resize(sz);
return Strings::to_utf8(ret);
}

// sz is the required size including the null terminator
ret.resize(sz - 1);
const auto sz2 = GetEnvironmentVariableW(w_varname.c_str(), ret.data(), sz);
Checks::check_exit(VCPKG_LINE_INFO, sz2 + 1 == sz);
ret.pop_back();
return Strings::to_utf8(ret.c_str());
return Strings::to_utf8(ret);
#else
auto v = getenv(varname.c_str());
if (!v) return nullopt;
return std::string(v);
#endif
}

Optional<std::string> get_environment_variable_nonempty(ZStringView varname)
{
auto maybe_result = get_environment_variable(varname);
if (auto presult = maybe_result.get())
{
if (presult->empty())
{
maybe_result.clear();
}
}

return maybe_result; // NRVO
}

void set_environment_variable(ZStringView varname, Optional<ZStringView> value) noexcept
{
#if defined(_WIN32)
Expand Down Expand Up @@ -435,13 +476,14 @@
#endif // ^^^ !_WIN32

auto maybe_home = get_environment_variable(HOMEVAR);
if (!maybe_home.has_value() || maybe_home.get()->empty())
const auto phome = maybe_home.get();
if (!phome || phome->empty())
{
return msg::format(msgUnableToReadEnvironmentVariable,
msg::env_var = format_environment_variable(HOMEVAR));
}

Path p = std::move(*maybe_home.get());
Path p = std::move(*phome);
if (!p.is_absolute())
{
return msg::format(
Expand All @@ -464,18 +506,20 @@
const ExpectedL<Path>& get_appdata_local()
{
static ExpectedL<Path> s_home = []() -> ExpectedL<Path> {
auto maybe_home = get_environment_variable(EnvironmentVariableLocalAppData);
if (!maybe_home.has_value() || maybe_home.get()->empty())
auto maybe_appdata = get_environment_variable(EnvironmentVariableLocalAppData);
auto pappdata = maybe_appdata.get();
if (!pappdata || pappdata->empty())
{
// Consult %APPDATA% as a workaround for Service accounts
// Microsoft/vcpkg#12285
maybe_home = get_environment_variable(EnvironmentVariableAppData);
if (!maybe_home.has_value() || maybe_home.get()->empty())
maybe_appdata = get_environment_variable(EnvironmentVariableAppData);
pappdata = maybe_appdata.get();
if (!pappdata || pappdata->empty())
{
return msg::format(msgUnableToReadAppDatas);
}

auto p = Path(Path(*maybe_home.get()).parent_path());
auto p = Path(Path(*pappdata).parent_path());
p /= "Local";
if (!p.is_absolute())
{
Expand All @@ -487,7 +531,7 @@
return p;
}

auto p = Path(*maybe_home.get());
auto p = Path(std::move(*pappdata));
if (!p.is_absolute())
{
return msg::format(msgEnvVarMustBeAbsolutePath,
Expand All @@ -507,10 +551,11 @@

static ExpectedL<Path> get_windows_forced_environment_variable(StringLiteral environment_variable)
{
auto env = get_environment_variable(environment_variable);
if (const auto p = env.get())
auto maybe_env = get_environment_variable(environment_variable);
const auto penv = maybe_env.get();
if (penv && !penv->empty())
{
return Path(std::move(*p));
return Path(std::move(*penv));
}

return msg::format(msgWindowsEnvMustAlwaysBePresent,
Expand Down Expand Up @@ -557,14 +602,15 @@
{
static ExpectedL<Path> s_home = []() -> ExpectedL<Path> {
auto maybe_home = get_environment_variable("XDG_CACHE_HOME");
if (auto p = maybe_home.get())
const auto phome = maybe_home.get();
if (phome && !phome->empty())
{
return Path(std::move(*p));
return Path(std::move(*phome));
}

return get_home_dir().map([](Path home) {
return get_home_dir().map([](Path&& home) -> Path&& {

Check failure on line 611 in src/vcpkg/base/system.cpp

View workflow job for this annotation

GitHub Actions / builds / build (macos-26, macos-ci) 

no matching member function for call to 'map'

Check failure on line 611 in src/vcpkg/base/system.cpp

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-24.04, linux-ci) 

no matching function for call to ‘vcpkg::ExpectedT<vcpkg::Path, vcpkg::LocalizedString>::map(vcpkg::get_xdg_cache_home()::<lambda()>::<lambda(vcpkg::Path&&)>) const’

Check failure on line 611 in src/vcpkg/base/system.cpp

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-24.04-arm, linux-arm64-ci) 

no matching function for call to ‘vcpkg::ExpectedT<vcpkg::Path, vcpkg::LocalizedString>::map(vcpkg::get_xdg_cache_home()::<lambda()>::<lambda(vcpkg::Path&&)>) const’
home /= ".cache";
return home;
return std::move(home);
});
Comment thread
BillyONeal marked this conversation as resolved.
}();
return s_home;
Expand Down Expand Up @@ -605,7 +651,8 @@
static const ExpectedL<Path> result =
get_appdata_local().map([](const Path& appdata_local) { return appdata_local / "vcpkg"; });
#else
static const ExpectedL<Path> result = Path(get_environment_variable("HOME").value_or("/var")) / ".vcpkg";
static const ExpectedL<Path> result =
Path(get_environment_variable_nonempty("HOME").value_or("/var")) / ".vcpkg";
#endif
return result;
}
Expand Down Expand Up @@ -714,27 +761,23 @@

static const Optional<Path>& get_program_files()
{
static const auto PROGRAMFILES = []() -> Optional<Path> {
auto value = get_environment_variable(EnvironmentVariableProgramFiles);
if (auto v = value.get())
{
return *v;
}

return nullopt;
}();

static const auto PROGRAMFILES =
get_environment_variable_nonempty(EnvironmentVariableProgramFiles).map([](std::string&& pf) {
return Path(std::move(pf));
});
return PROGRAMFILES;
}

const Optional<Path>& get_program_files_32_bit()
{
static const auto PROGRAMFILES_x86 = []() -> Optional<Path> {
auto value = get_environment_variable(EnvironmentVariableProgramFilesX86);
if (auto v = value.get())
auto maybe_value = get_environment_variable_nonempty(EnvironmentVariableProgramFilesX86);
const auto pvalue = maybe_value.get();
if (pvalue)
{
return *v;
return Path(std::move(*pvalue));
}

return get_program_files();
}();
return PROGRAMFILES_x86;
Expand All @@ -743,11 +786,13 @@
const Optional<Path>& get_program_files_platform_bitness()
{
static const auto ProgramW6432 = []() -> Optional<Path> {
auto value = get_environment_variable(EnvironmentVariableProgramW6432);
if (auto v = value.get())
auto maybe_value = get_environment_variable(EnvironmentVariableProgramW6432);
const auto pvalue = maybe_value.get();
if (pvalue && !pvalue->empty())
{
return *v;
return std::move(*pvalue);
}

return get_program_files();
}();
return ProgramW6432;
Expand All @@ -756,13 +801,14 @@
unsigned int get_concurrency()
{
static unsigned int concurrency = [] {
auto user_defined_concurrency = get_environment_variable(EnvironmentVariableVcpkgMaxConcurrency);
if (user_defined_concurrency)
auto maybe_user_defined_concurrency = get_environment_variable(EnvironmentVariableVcpkgMaxConcurrency);
const auto puser_defined_concurrency = maybe_user_defined_concurrency.get();
if (puser_defined_concurrency)
{
int res = -1;
try
{
res = std::stoi(user_defined_concurrency.value_or_exit(VCPKG_LINE_INFO));
res = std::stoi(*puser_defined_concurrency);
}
catch (std::exception&)
{
Expand Down Expand Up @@ -800,24 +846,22 @@
Optional<CPUArchitecture> guess_visual_studio_prompt_target_architecture()
{
// Check for the "vsdevcmd" infrastructure used by Visual Studio 2017 and later
const auto vscmd_arg_tgt_arch_env = get_environment_variable(EnvironmentVariableVscmdArgTgtArch);
if (vscmd_arg_tgt_arch_env)
auto maybe_vscmd_arg_tgt_arch_env = get_environment_variable_nonempty(EnvironmentVariableVscmdArgTgtArch);
if (const auto pvscmd_arg_tgt_arch_env = maybe_vscmd_arg_tgt_arch_env.get())
{
return to_cpu_architecture(vscmd_arg_tgt_arch_env.value_or_exit(VCPKG_LINE_INFO));
return to_cpu_architecture(*pvscmd_arg_tgt_arch_env);
}

// Check for the "vcvarsall" infrastructure used by Visual Studio 2015
if (get_environment_variable(EnvironmentVariableVCInstallDir))
if (get_environment_variable_nonempty(EnvironmentVariableVCInstallDir))
{
const auto Platform = get_environment_variable(EnvironmentVariablePlatform);
if (Platform)
auto maybe_platform = get_environment_variable_nonempty(EnvironmentVariablePlatform);
if (const auto pplatform = maybe_platform.get())
{
return to_cpu_architecture(Platform.value_or_exit(VCPKG_LINE_INFO));
}
else
{
return CPUArchitecture::X86;
return to_cpu_architecture(*pplatform);
}

return CPUArchitecture::X86;
}

return nullopt;
Expand Down
Loading
Loading