diff --git a/include/vcpkg/base/system.h b/include/vcpkg/base/system.h index 79df458d46..00bfe96564 100644 --- a/include/vcpkg/base/system.h +++ b/include/vcpkg/base/system.h @@ -15,6 +15,8 @@ namespace vcpkg { Optional get_environment_variable(ZStringView varname); + // identical to get_environment_variable but returns nullopt if the variable is set to an empty string + Optional get_environment_variable_nonempty(ZStringView varname); void set_environment_variable(ZStringView varname, Optional value) noexcept; std::vector get_environment_variables(); diff --git a/src/vcpkg-test/system.cpp b/src/vcpkg-test/system.cpp index b71b75e2af..865ae69190 100644 --- a/src/vcpkg-test/system.cpp +++ b/src/vcpkg-test/system.cpp @@ -31,7 +31,7 @@ namespace }; } -TEST_CASE ("[to_cpu_architecture]", "system") +TEST_CASE ("to_cpu_architecture", "[system]") { struct test_case { @@ -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}; + 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; diff --git a/src/vcpkg-test/util.cpp b/src/vcpkg-test/util.cpp index a419f4ab2f..176ab662e1 100644 --- a/src/vcpkg-test/util.cpp +++ b/src/vcpkg-test/util.cpp @@ -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 diff --git a/src/vcpkg/base/files.cpp b/src/vcpkg/base/files.cpp index 23f22cc8c6..e7ce295606 100644 --- a/src/vcpkg/base/files.cpp +++ b/src/vcpkg/base/files.cpp @@ -334,8 +334,8 @@ namespace std::vector 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{std::make_move_iterator(path_base_strings.begin()), std::make_move_iterator(path_base_strings.end())}; } @@ -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); diff --git a/src/vcpkg/base/system.cpp b/src/vcpkg/base/system.cpp index 77938630aa..2aeb14d27b 100644 --- a/src/vcpkg/base/system.cpp +++ b/src/vcpkg/base/system.cpp @@ -343,16 +343,43 @@ namespace vcpkg { #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(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); + } - 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(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; @@ -360,6 +387,20 @@ namespace vcpkg #endif } + Optional 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 value) noexcept { #if defined(_WIN32) @@ -435,13 +476,14 @@ namespace vcpkg #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( @@ -464,18 +506,20 @@ namespace vcpkg const ExpectedL& get_appdata_local() { static ExpectedL s_home = []() -> ExpectedL { - 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()) { @@ -487,7 +531,7 @@ namespace vcpkg return p; } - auto p = Path(*maybe_home.get()); + auto p = Path(std::move(*pappdata)); if (!p.is_absolute()) { return msg::format(msgEnvVarMustBeAbsolutePath, @@ -507,10 +551,11 @@ namespace vcpkg static ExpectedL 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, @@ -557,14 +602,15 @@ namespace vcpkg { static ExpectedL s_home = []() -> ExpectedL { 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&& { home /= ".cache"; - return home; + return std::move(home); }); }(); return s_home; @@ -605,7 +651,8 @@ namespace vcpkg static const ExpectedL result = get_appdata_local().map([](const Path& appdata_local) { return appdata_local / "vcpkg"; }); #else - static const ExpectedL result = Path(get_environment_variable("HOME").value_or("/var")) / ".vcpkg"; + static const ExpectedL result = + Path(get_environment_variable_nonempty("HOME").value_or("/var")) / ".vcpkg"; #endif return result; } @@ -714,27 +761,23 @@ namespace vcpkg static const Optional& get_program_files() { - static const auto PROGRAMFILES = []() -> Optional { - 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& get_program_files_32_bit() { static const auto PROGRAMFILES_x86 = []() -> Optional { - 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; @@ -743,11 +786,13 @@ namespace vcpkg const Optional& get_program_files_platform_bitness() { static const auto ProgramW6432 = []() -> Optional { - 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; @@ -756,13 +801,14 @@ namespace vcpkg 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&) { @@ -800,24 +846,22 @@ namespace vcpkg Optional 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; diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index 1e188599e3..6ea497dd37 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -1673,11 +1673,11 @@ namespace ExpectedL default_cache_path_impl() { - auto maybe_cachepath = get_environment_variable(EnvironmentVariableVcpkgDefaultBinaryCache); - if (auto p_str = maybe_cachepath.get()) + auto maybe_cachepath = get_environment_variable_nonempty(EnvironmentVariableVcpkgDefaultBinaryCache); + if (const auto pcachepath = maybe_cachepath.get()) { get_global_metrics_collector().track_define(DefineMetric::VcpkgDefaultBinaryCache); - Path path = std::move(*p_str); + Path path = std::move(*pcachepath); path.make_preferred(); if (!real_filesystem.is_directory(path)) { @@ -2423,20 +2423,22 @@ namespace vcpkg return {*p}; } - auto gh_repo = get_environment_variable(EnvironmentVariableGitHubRepository).value_or(""); - if (gh_repo.empty()) + auto maybe_gh_repo = get_environment_variable(EnvironmentVariableGitHubRepository); + const auto pgh_repo = maybe_gh_repo.get(); + if (!pgh_repo || pgh_repo->empty()) { return {}; } - auto gh_server = get_environment_variable(EnvironmentVariableGitHubServerUrl).value_or(""); - if (gh_server.empty()) + auto maybe_gh_server = get_environment_variable(EnvironmentVariableGitHubServerUrl); + const auto pgh_server = maybe_gh_server.get(); + if (!pgh_server || pgh_server->empty()) { return {}; } get_global_metrics_collector().track_define(DefineMetric::GitHubRepository); - return {Strings::concat(gh_server, '/', gh_repo, ".git"), + return {Strings::concat(*pgh_server, '/', *pgh_repo, ".git"), get_environment_variable(EnvironmentVariableGitHubRef).value_or(""), get_environment_variable(EnvironmentVariableGitHubSha).value_or("")}; } diff --git a/src/vcpkg/commands.build.cpp b/src/vcpkg/commands.build.cpp index ff094dcc9b..f3670d3cc3 100644 --- a/src/vcpkg/commands.build.cpp +++ b/src/vcpkg/commands.build.cpp @@ -490,7 +490,7 @@ namespace vcpkg for (auto&& env_var : pre_build_info.passthrough_env_vars) { auto maybe_env_val = get_environment_variable(env_var); - if (auto env_val = maybe_env_val.get()) + if (const auto env_val = maybe_env_val.get()) { env[env_var] = std::move(*env_val); } @@ -504,7 +504,10 @@ namespace vcpkg for (const auto& var : s_extra_vars) { auto val = get_environment_variable(var); - if (auto p_val = val.get()) env.emplace(var, *p_val); + if (const auto p_val = val.get()) + { + env.emplace(var, *p_val); + } } /* @@ -519,8 +522,8 @@ namespace vcpkg // 2021-05-09 Fix: Detect If there's already HTTP(S)_PROXY presented in the environment variables. // If so, we no longer overwrite them. - bool proxy_from_env = (get_environment_variable(EnvironmentVariableHttpProxy).has_value() || - get_environment_variable(EnvironmentVariableHttpsProxy).has_value()); + bool proxy_from_env = (get_environment_variable_nonempty(EnvironmentVariableHttpProxy).has_value() || + get_environment_variable_nonempty(EnvironmentVariableHttpsProxy).has_value()); if (proxy_from_env) { @@ -1338,10 +1341,10 @@ namespace vcpkg { for (const auto& env_var : pre_build_info.passthrough_env_vars_tracked) { - if (auto e = get_environment_variable(env_var)) + auto maybe_e = get_environment_variable(env_var); + if (const auto e = maybe_e.get()) { - abi_tag_entries.emplace_back( - "ENV:" + env_var, Hash::get_string_hash(e.value_or_exit(VCPKG_LINE_INFO), Hash::Algorithm::Sha256)); + abi_tag_entries.emplace_back("ENV:" + env_var, Hash::get_string_hash(*e, Hash::Algorithm::Sha256)); } } diff --git a/src/vcpkg/commands.edit.cpp b/src/vcpkg/commands.edit.cpp index 784d3f5799..1fcdce8f5a 100644 --- a/src/vcpkg/commands.edit.cpp +++ b/src/vcpkg/commands.edit.cpp @@ -179,9 +179,10 @@ namespace vcpkg // Scope to prevent use of moved-from variable { auto maybe_editor_path = get_environment_variable(EnvironmentVariableEditor); - if (std::string* editor_path = maybe_editor_path.get()) + const auto peditor_path = maybe_editor_path.get(); + if (peditor_path && !peditor_path->empty()) { - candidate_paths.emplace_back(std::move(*editor_path)); + candidate_paths.emplace_back(std::move(*peditor_path)); } } @@ -203,10 +204,11 @@ namespace vcpkg candidate_paths.push_back(*pf / VS_CODE); } - auto app_data = get_environment_variable(EnvironmentVariableAppData); - if (auto* ad = app_data.get()) + auto maybe_app_data = get_environment_variable(EnvironmentVariableAppData); + const auto papp_data = maybe_app_data.get(); + if (papp_data && !papp_data->empty()) { - Path default_base = std::move(*ad); + Path default_base = std::move(*papp_data); default_base.replace_filename("Local\\Programs"); candidate_paths.push_back(default_base / VS_CODE_INSIDERS); candidate_paths.push_back(std::move(default_base) / VS_CODE); diff --git a/src/vcpkg/commands.integrate.cpp b/src/vcpkg/commands.integrate.cpp index 8c2272e157..4ba196de7b 100644 --- a/src/vcpkg/commands.integrate.cpp +++ b/src/vcpkg/commands.integrate.cpp @@ -451,7 +451,7 @@ namespace vcpkg Checks::msg_exit_with_error( VCPKG_LINE_INFO, msgIntegrateNonWindowsOnly, msg::command_line = "vcpkg integrate bash"); #else // ^^^ _WIN32 // !_WIN32 vvv - const auto home_path = get_environment_variable("HOME").value_or_exit(VCPKG_LINE_INFO); + const auto home_path = get_environment_variable_nonempty("HOME").value_or_exit(VCPKG_LINE_INFO); #if defined(__APPLE__) const auto bashrc_path = Path{home_path} / ".bash_profile"; #else @@ -488,7 +488,7 @@ namespace vcpkg Checks::msg_exit_with_error( VCPKG_LINE_INFO, msgIntegrateNonWindowsOnly, msg::command_line = "vcpkg integrate zsh"); #else // ^^^ _WIN32 // !_WIN32 vvv - const auto home_path = get_environment_variable("HOME").value_or_exit(VCPKG_LINE_INFO); + const auto home_path = get_environment_variable_nonempty("HOME").value_or_exit(VCPKG_LINE_INFO); const auto zshrc_path = Path{home_path} / ".zshrc"; auto& fs = paths.get_filesystem(); @@ -532,14 +532,15 @@ namespace vcpkg VCPKG_LINE_INFO, msgIntegrateNonWindowsOnly, msg::command_line = "vcpkg integrate x-fish"); #else // ^^^ _WIN32 // !_WIN32 vvv Path fish_completions_path; - const auto config_path = get_environment_variable("XDG_CONFIG_HOME"); - if (config_path.has_value()) + const auto maybe_config_path = get_environment_variable("XDG_CONFIG_HOME"); + const auto pconfig_path = maybe_config_path.get(); + if (pconfig_path && !pconfig_path->empty()) { - fish_completions_path = config_path.value_or_exit(VCPKG_LINE_INFO); + fish_completions_path = std::move(*pconfig_path); } else { - Path home_path = get_environment_variable("HOME").value_or_exit(VCPKG_LINE_INFO); + Path home_path = get_environment_variable_nonempty("HOME").value_or_exit(VCPKG_LINE_INFO); fish_completions_path = std::move(home_path) / ".config"; } diff --git a/src/vcpkg/vcpkgcmdarguments.cpp b/src/vcpkg/vcpkgcmdarguments.cpp index eac02c0546..cdcf4c41db 100644 --- a/src/vcpkg/vcpkgcmdarguments.cpp +++ b/src/vcpkg/vcpkgcmdarguments.cpp @@ -648,9 +648,9 @@ namespace vcpkg s_reentrancy_guard = true; auto maybe_vcpkg_recursive_data = get_environment_variable(EnvironmentVariableXVcpkgRecursiveData); - if (auto vcpkg_recursive_data = maybe_vcpkg_recursive_data.get()) + if (const auto pvcpkg_recursive_data = maybe_vcpkg_recursive_data.get()) { - auto rec_doc = Json::parse(*vcpkg_recursive_data, EnvironmentVariableXVcpkgRecursiveData) + auto rec_doc = Json::parse(*pvcpkg_recursive_data, EnvironmentVariableXVcpkgRecursiveData) .value_or_exit(VCPKG_LINE_INFO) .value; const auto& obj = rec_doc.object(VCPKG_LINE_INFO); diff --git a/src/vcpkg/visualstudio.cpp b/src/vcpkg/visualstudio.cpp index 7dc3235363..3924e58808 100644 --- a/src/vcpkg/visualstudio.cpp +++ b/src/vcpkg/visualstudio.cpp @@ -144,11 +144,12 @@ namespace vcpkg::VisualStudio const auto maybe_append_comntools = [&](ZStringView env_var, ZStringView version, bool check_cl = true) { auto maybe_comntools = get_environment_variable(env_var); - if (const auto path_as_string = maybe_comntools.get()) + const auto pcomntools = maybe_comntools.get(); + if (pcomntools && !pcomntools->empty()) { // We want lexically_normal(), but it is not available // Correct root path might be 2 or 3 levels up, depending on if the path has trailing backslash. - Path common7_tools = *path_as_string; + Path common7_tools = std::move(*pcomntools); if (common7_tools.filename().empty()) { common7_tools = common7_tools.parent_path();