encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve ScopedEnvironmentVar to distinguish between null and empty 

Found while investigating Swiftshader support. Some Vulkan loaders
fail to create an instance if VK_ICD_FILENAMES is empty string
rather than entirely absent. It was set to empty string because
Dawn did not distinguish between nonexistent environment variables
and the empty string. This CL adds distinguishing between the two,
including tests for the behavior.

Bug: chromium:1266550
Change-Id: I1680a281f62e6b340009e01da65db9d485e2975e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/69520
Auto-Submit: Austin Eng <enga@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
This commit is contained in:
Austin Eng 2021-11-16 23:37:15 +00:00 committed by Dawn LUCI CQ
parent 970739e4e3
commit 09c308cfcf
5 changed files with 85 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
examples/SampleUtils.cpp
View File

@ -92,8 +92,9 @@ static utils::TerribleCommandBuffer* c2sBuf = nullptr;
static utils::TerribleCommandBuffer* s2cBuf = nullptr;
wgpu::Device CreateCppDawnDevice() {
if (GetEnvironmentVar("ANGLE_DEFAULT_PLATFORM").empty()) {
SetEnvironmentVar("ANGLE_DEFAULT_PLATFORM", "swiftshader");
ScopedEnvironmentVar angleDefaultPlatform;
if (GetEnvironmentVar("ANGLE_DEFAULT_PLATFORM").first.empty()) {
angleDefaultPlatform.Set("ANGLE_DEFAULT_PLATFORM", "swiftshader");
}
glfwSetErrorCallback(PrintGLFWError);

40
src/common/SystemUtils.cpp
View File

@ -15,6 +15,7 @@
#include "common/SystemUtils.h"
#include "common/Assert.h"
#include "common/Log.h"
#if defined(DAWN_PLATFORM_WINDOWS)
# include <Windows.h>
@ -35,21 +36,29 @@ const char* GetPathSeparator() {
return "\\";
}
std::string GetEnvironmentVar(const char* variableName) {
std::pair<std::string, bool> GetEnvironmentVar(const char* variableName) {
// First pass a size of 0 to get the size of variable value.
char* tempBuf = nullptr;
DWORD result = GetEnvironmentVariableA(variableName, tempBuf, 0);
if (result == 0) {
return "";
DWORD sizeWithNullTerminator = GetEnvironmentVariableA(variableName, nullptr, 0);
if (sizeWithNullTerminator == 0) {
DWORD err = GetLastError();
if (err != ERROR_ENVVAR_NOT_FOUND) {
dawn::WarningLog() << "GetEnvironmentVariableA failed with code " << err;
}
return std::make_pair(std::string(), false);
}
// Then get variable value with its actual size.
std::vector<char> buffer(result + 1);
if (GetEnvironmentVariableA(variableName, buffer.data(), static_cast<DWORD>(buffer.size())) ==
0) {
return "";
std::vector<char> buffer(sizeWithNullTerminator);
DWORD sizeStored =
GetEnvironmentVariableA(variableName, buffer.data(), static_cast<DWORD>(buffer.size()));
if (sizeStored + 1 != sizeWithNullTerminator) {
DWORD err = GetLastError();
if (err) {
dawn::WarningLog() << "GetEnvironmentVariableA failed with code " << err;
}
return std::make_pair(std::string(), false);
}
return std::string(buffer.data());
return std::make_pair(std::string(buffer.data(), sizeStored), true);
}
bool SetEnvironmentVar(const char* variableName, const char* value) {
@ -60,12 +69,16 @@ const char* GetPathSeparator() {
return "/";
}
std::string GetEnvironmentVar(const char* variableName) {
std::pair<std::string, bool> GetEnvironmentVar(const char* variableName) {
char* value = getenv(variableName);
return value == nullptr ? "" : std::string(value);
return value == nullptr ? std::make_pair(std::string(), false)
: std::make_pair(std::string(value), true);
}
bool SetEnvironmentVar(const char* variableName, const char* value) {
if (value == nullptr) {
return unsetenv(variableName) == 0;
}
return setenv(variableName, value, 1) == 0;
}
#else
@ -133,7 +146,8 @@ ScopedEnvironmentVar::ScopedEnvironmentVar(const char* variableName, const char*
ScopedEnvironmentVar::~ScopedEnvironmentVar() {
if (mIsSet) {
bool success = SetEnvironmentVar(mName.c_str(), mOriginalValue.c_str());
bool success = SetEnvironmentVar(
mName.c_str(), mOriginalValue.second ? mOriginalValue.first.c_str() : nullptr);
// If we set the environment variable in the constructor, we should never fail restoring it.
ASSERT(success);
}

6
src/common/SystemUtils.h
View File

@ -20,7 +20,9 @@
#include <string>
const char* GetPathSeparator();
std::string GetEnvironmentVar(const char* variableName);
// Returns a pair of the environment variable's value, and a boolean indicating whether the variable
// was present.
std::pair<std::string, bool> GetEnvironmentVar(const char* variableName);
bool SetEnvironmentVar(const char* variableName, const char* value);
std::string GetExecutableDirectory();
@ -42,7 +44,7 @@ class ScopedEnvironmentVar {
private:
std::string mName;
std::string mOriginalValue;
std::pair<std::string, bool> mOriginalValue;
bool mIsSet = false;
};

6
src/tests/DawnTest.cpp
View File

@ -436,9 +436,11 @@ std::unique_ptr<dawn_native::Instance> DawnTestEnvironment::CreateInstanceAndDis
#ifdef DAWN_ENABLE_BACKEND_OPENGLES
if (GetEnvironmentVar("ANGLE_DEFAULT_PLATFORM").empty()) {
SetEnvironmentVar("ANGLE_DEFAULT_PLATFORM", "swiftshader");
ScopedEnvironmentVar angleDefaultPlatform;
if (GetEnvironmentVar("ANGLE_DEFAULT_PLATFORM").first.empty()) {
angleDefaultPlatform.Set("ANGLE_DEFAULT_PLATFORM", "swiftshader");
}
if (!glfwInit()) {
return instance;
}

65
src/tests/unittests/SystemUtilsTests.cpp
View File

@ -12,32 +12,38 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#include <gmock/gmock-matchers.h>
#include <gtest/gtest.h>
#include "common/Assert.h"
#include "common/SystemUtils.h"
using ::testing::_;
using ::testing::Pair;
// Tests for GetEnvironmentVar
TEST(SystemUtilsTests, GetEnvironmentVar) {
// Test nonexistent environment variable
ASSERT_EQ(GetEnvironmentVar("NonexistentEnvironmentVar"), "");
EXPECT_THAT(GetEnvironmentVar("NonexistentEnvironmentVar"), Pair("", false));
}
// Tests for SetEnvironmentVar
TEST(SystemUtilsTests, SetEnvironmentVar) {
// Test new environment variable
ASSERT_TRUE(SetEnvironmentVar("EnvironmentVarForTest", "NewEnvironmentVarValue"));
ASSERT_EQ(GetEnvironmentVar("EnvironmentVarForTest"), "NewEnvironmentVarValue");
EXPECT_TRUE(SetEnvironmentVar("EnvironmentVarForTest", "NewEnvironmentVarValue"));
EXPECT_THAT(GetEnvironmentVar("EnvironmentVarForTest"), Pair("NewEnvironmentVarValue", true));
// Test override environment variable
ASSERT_TRUE(SetEnvironmentVar("EnvironmentVarForTest", "OverrideEnvironmentVarValue"));
ASSERT_EQ(GetEnvironmentVar("EnvironmentVarForTest"), "OverrideEnvironmentVarValue");
EXPECT_TRUE(SetEnvironmentVar("EnvironmentVarForTest", "OverrideEnvironmentVarValue"));
EXPECT_THAT(GetEnvironmentVar("EnvironmentVarForTest"),
Pair("OverrideEnvironmentVarValue", true));
}
// Tests for GetExecutableDirectory
TEST(SystemUtilsTests, GetExecutableDirectory) {
// Test returned value is non-empty string
ASSERT_NE(GetExecutableDirectory(), "");
EXPECT_NE(GetExecutableDirectory(), "");
// Test last charecter in path
ASSERT_EQ(GetExecutableDirectory().back(), *GetPathSeparator());
EXPECT_EQ(GetExecutableDirectory().back(), *GetPathSeparator());
}
// Tests for ScopedEnvironmentVar
@ -51,36 +57,59 @@ TEST(SystemUtilsTests, ScopedEnvironmentVar) {
{
ScopedEnvironmentVar var;
var.Set("ScopedEnvironmentVarForTest", "NewEnvironmentVarValue");
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "NewEnvironmentVarValue");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"),
Pair("NewEnvironmentVarValue", true));
}
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("original", true));
// Test that the environment variable can be set, and it is unset at the end of the scope.
{
ScopedEnvironmentVar var("ScopedEnvironmentVarForTest", "NewEnvironmentVarValue");
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "NewEnvironmentVarValue");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"),
Pair("NewEnvironmentVarValue", true));
}
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("original", true));
// Test nested scopes
{
ScopedEnvironmentVar outer("ScopedEnvironmentVarForTest", "outer");
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "outer");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("outer", true));
{
ScopedEnvironmentVar inner("ScopedEnvironmentVarForTest", "inner");
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "inner");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("inner", true));
}
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "outer");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("outer", true));
}
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("original", true));
// Test redundantly setting scoped variables
{
ScopedEnvironmentVar var1("ScopedEnvironmentVarForTest", "var1");
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "var1");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("var1", true));
ScopedEnvironmentVar var2("ScopedEnvironmentVarForTest", "var2");
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "var2");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("var2", true));
}
ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("original", true));
}
// Test that restoring a scoped environment variable to the empty string.
TEST(SystemUtilsTests, ScopedEnvironmentVarRestoresEmptyString) {
ScopedEnvironmentVar empty("ScopedEnvironmentVarForTest", "");
{
ScopedEnvironmentVar var1("ScopedEnvironmentVarForTest", "var1");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("var1", true));
}
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("", true));
}
// Test that restoring a scoped environment variable to not set (distuishable from empty string)
// works.
TEST(SystemUtilsTests, ScopedEnvironmentVarRestoresNotSet) {
ScopedEnvironmentVar null("ScopedEnvironmentVarForTest", nullptr);
{
ScopedEnvironmentVar var1("ScopedEnvironmentVarForTest", "var1");
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("var1", true));
}
EXPECT_THAT(GetEnvironmentVar("ScopedEnvironmentVarForTest"), Pair("", false));
}