diff --git a/Source/kwsys/CMakeLists.txt b/Source/kwsys/CMakeLists.txt index 69f7ac0e22..08494b9bf5 100644 --- a/Source/kwsys/CMakeLists.txt +++ b/Source/kwsys/CMakeLists.txt @@ -209,7 +209,6 @@ endif() # Include helper macros. include(${CMAKE_CURRENT_SOURCE_DIR}/kwsysPlatformTests.cmake) -include(CheckTypeSize) # Do full dependency headers. include_regular_expression("^.*$") diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx index 3a2dc5a980..9d4e40b657 100644 --- a/Source/kwsys/SystemTools.cxx +++ b/Source/kwsys/SystemTools.cxx @@ -675,15 +675,21 @@ char const* SystemTools::GetEnv(std::string const& key) bool SystemTools::GetEnv(char const* key, std::string& result) { #if defined(_WIN32) - auto wide_key = Encoding::ToWide(key); - auto result_size = GetEnvironmentVariableW(wide_key.data(), nullptr, 0); - if (result_size <= 0) { + std::wstring const wKey = Encoding::ToWide(key); + std::vector heapBuf; + wchar_t stackBuf[256]; + DWORD bufSz = static_cast(sizeof(stackBuf) / sizeof(stackBuf[0])); + wchar_t* buf = stackBuf; + DWORD r; + while ((r = GetEnvironmentVariableW(wKey.c_str(), buf, bufSz)) >= bufSz) { + heapBuf.resize(r); + bufSz = r; + buf = &heapBuf[0]; + } + if (r == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND) { return false; } - std::wstring wide_result; - wide_result.resize(result_size - 1); - GetEnvironmentVariableW(wide_key.data(), &wide_result[0], result_size); - result = Encoding::ToNarrow(wide_result); + result = Encoding::ToNarrow(buf); return true; #else char const* v = getenv(key); @@ -703,12 +709,12 @@ bool SystemTools::GetEnv(std::string const& key, std::string& result) bool SystemTools::HasEnv(char const* key) { #if defined(_WIN32) - std::wstring const wkey = Encoding::ToWide(key); - wchar_t const* v = _wgetenv(wkey.c_str()); + std::wstring const wKey = Encoding::ToWide(key); + DWORD r = GetEnvironmentVariableW(wKey.c_str(), nullptr, 0); + return !(r == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND); #else - char const* v = getenv(key); + return getenv(key) != nullptr; #endif - return v; } bool SystemTools::HasEnv(std::string const& key) @@ -732,8 +738,8 @@ static int kwsysUnPutEnv(std::string const& env) } #elif defined(__CYGWIN__) || defined(__GLIBC__) -/* putenv("A") removes A from the environment. It must not put the - memory in the environment because it does not have any "=" syntax. */ +// putenv("A") removes A from the environment with the GNU runtime. +// It cannot put the memory in the environment since there is no "=" syntax. static int kwsysUnPutEnv(std::string const& env) { @@ -750,12 +756,7 @@ static int kwsysUnPutEnv(std::string const& env) } #elif defined(_WIN32) -/* putenv("A=") places "A=" in the environment, which is as close to - removal as we can get with the putenv API. We have to leak the - most recent value placed in the environment for each variable name - on program exit in case exit routines access it. */ - -static kwsysEnvSet kwsysUnPutEnvSet; +// putenv("A=") removes A from the environment with the MSVC runtime. static int kwsysUnPutEnv(std::string const& env) { @@ -763,13 +764,7 @@ static int kwsysUnPutEnv(std::string const& env) size_t const pos = wEnv.find('='); size_t const len = pos == std::string::npos ? wEnv.size() : pos; wEnv.resize(len + 1, L'='); - wchar_t* newEnv = _wcsdup(wEnv.c_str()); - if (!newEnv) { - return -1; - } - kwsysEnvSet::Free oldEnv(kwsysUnPutEnvSet.Release(newEnv)); - kwsysUnPutEnvSet.insert(newEnv); - return _wputenv(newEnv); + return _wputenv(wEnv.c_str()); } #else @@ -846,7 +841,7 @@ public: bool Put(char const* env) { # if defined(_WIN32) - std::wstring const wEnv = Encoding::ToWide(env); + std::wstring wEnv = Encoding::ToWide(env); wchar_t* newEnv = _wcsdup(wEnv.c_str()); # else char* newEnv = strdup(env); @@ -854,7 +849,21 @@ public: Free oldEnv(this->Release(newEnv)); this->insert(newEnv); # if defined(_WIN32) - return _wputenv(newEnv) == 0; + // `_wputenv` updates both the C runtime library's `_wenviron` array + // and the process's environment block. + if (_wputenv(newEnv) != 0) { + return false; + } + // There seems to be no way to add an empty variable to `_wenviron` + // through the C runtime library: `_wputenv("A=")` removes "A". + // Add it directly to the process's environment block. + // This is used by child processes, and by our `GetEnv`. + std::string::size_type const eqPos = wEnv.find(L'='); + if (eqPos != std::string::npos && (eqPos + 1) == wEnv.size()) { + wEnv.resize(eqPos); + return SetEnvironmentVariableW(wEnv.c_str(), L""); + } + return true; # else return putenv(newEnv) == 0; # endif diff --git a/Source/kwsys/testSystemTools.cxx b/Source/kwsys/testSystemTools.cxx index 08e51e1f7f..dff9dd6224 100644 --- a/Source/kwsys/testSystemTools.cxx +++ b/Source/kwsys/testSystemTools.cxx @@ -696,6 +696,12 @@ static bool CheckPutEnv(std::string const& env, char const* name, << value << "\"!" << std::endl; return false; } + bool bv = kwsys::SystemTools::HasEnv(name); + if (!bv) { + std::cerr << "HasEnv(\"" << name << "\") returned \"" << bv + << "\", not \"true\"!" << std::endl; + return false; + } return true; } @@ -711,6 +717,12 @@ static bool CheckUnPutEnv(char const* env, char const* name) << "\", not (null)!" << std::endl; return false; } + bool bv = kwsys::SystemTools::HasEnv(name); + if (bv) { + std::cerr << "HasEnv(\"" << name << "\") returned \"" << bv + << "\", not \"false\"!" << std::endl; + return false; + } return true; } @@ -721,6 +733,7 @@ static bool CheckEnvironmentOperations() res &= CheckPutEnv("B=C", "B", "C"); res &= CheckPutEnv("C=D", "C", "D"); res &= CheckPutEnv("D=E", "D", "E"); + res &= CheckPutEnv("F=", "F", ""); res &= CheckUnPutEnv("A", "A"); res &= CheckUnPutEnv("B=", "B"); res &= CheckUnPutEnv("C=D", "C"); diff --git a/Tests/RunCMake/CommandLine/E_env-empty-stdout.txt b/Tests/RunCMake/CommandLine/E_env-empty-stdout.txt new file mode 100644 index 0000000000..cb78849579 --- /dev/null +++ b/Tests/RunCMake/CommandLine/E_env-empty-stdout.txt @@ -0,0 +1 @@ +^-- TEST_ENV is correctly set in environment: ''$ diff --git a/Tests/RunCMake/CommandLine/E_env-empty.cmake b/Tests/RunCMake/CommandLine/E_env-empty.cmake new file mode 100644 index 0000000000..17ade984d2 --- /dev/null +++ b/Tests/RunCMake/CommandLine/E_env-empty.cmake @@ -0,0 +1,5 @@ +if(DEFINED ENV{TEST_ENV}) + message(STATUS "TEST_ENV is correctly set in environment: '$ENV{TEST_ENV}'") +else() + message(FATAL_ERROR "TEST_ENV is incorrectly not set in environment") +endif() diff --git a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake index 10065715b2..c198881693 100644 --- a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake +++ b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake @@ -820,6 +820,7 @@ run_cmake_command(E_env-no-command0 ${CMAKE_COMMAND} -E env) run_cmake_command(E_env-no-command1 ${CMAKE_COMMAND} -E env TEST_ENV=1) run_cmake_command(E_env-bad-arg1 ${CMAKE_COMMAND} -E env -bad-arg1) run_cmake_command(E_env-set ${CMAKE_COMMAND} -E env TEST_ENV=1 ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/E_env-set.cmake) +run_cmake_command(E_env-empty ${CMAKE_COMMAND} -E env TEST_ENV= ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/E_env-empty.cmake) run_cmake_command(E_env-unset ${CMAKE_COMMAND} -E env TEST_ENV=1 ${CMAKE_COMMAND} -E env --unset=TEST_ENV ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/E_env-unset.cmake) run_cmake_command(E_env-stdin ${CMAKE_COMMAND} -DPRINT_STDIN_EXE=${PRINT_STDIN_EXE} -P ${RunCMake_SOURCE_DIR}/E_env-stdin.cmake)