From 9c19676d5bda70eb226d9003ecbc8f2fb8dbe9b7 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Mon, 30 Sep 2019 16:05:18 -0400 Subject: [PATCH] common: Port from spruce to filesystem (#12117) --- common/BUILD.bazel | 6 +-- common/drake_path.cc | 7 ++- common/proto/BUILD.bazel | 2 +- common/proto/rpc_pipe_temp_directory.cc | 11 ++--- common/temp_directory.cc | 23 +++++----- common/test/find_resource_test.cc | 9 ++-- common/test/temp_directory_test.cc | 60 ++++++++++++++++--------- 7 files changed, 68 insertions(+), 50 deletions(-) diff --git a/common/BUILD.bazel b/common/BUILD.bazel index 56d80009db11..52f7f8524d6d 100644 --- a/common/BUILD.bazel +++ b/common/BUILD.bazel @@ -362,8 +362,8 @@ drake_cc_library( visibility = ["//tools/install/libdrake:__pkg__"], deps = [ ":essential", + ":filesystem", ":find_resource", - "@spruce", ], ) @@ -443,7 +443,7 @@ drake_cc_library( hdrs = ["temp_directory.h"], deps = [ ":essential", - "@spruce", + ":filesystem", ], ) @@ -1218,8 +1218,8 @@ drake_cc_googletest( drake_cc_googletest( name = "temp_directory_test", deps = [ + ":filesystem", ":temp_directory", - "@spruce", ], ) diff --git a/common/drake_path.cc b/common/drake_path.cc index 50adf8216d06..538e0dd07a70 100644 --- a/common/drake_path.cc +++ b/common/drake_path.cc @@ -1,7 +1,6 @@ #include "drake/common/drake_path.h" -#include - +#include "drake/common/filesystem.h" #include "drake/common/find_resource.h" // N.B. This code is unit tested in test/find_resource_test.cc. @@ -16,11 +15,11 @@ optional MaybeGetDrakePath() { if (find_result.get_error_message()) { return nullopt; } - spruce::path sentinel_path = find_result.get_absolute_path_or_throw(); + filesystem::path sentinel_path(find_result.get_absolute_path_or_throw()); // Take the dirname of sentinel_path, so that we have a folder where looking // up "drake/foo/bar.urdf" makes sense. - return sentinel_path.root(); + return sentinel_path.parent_path().string(); } } // namespace drake diff --git a/common/proto/BUILD.bazel b/common/proto/BUILD.bazel index b40a69e99bed..74aa17edb4be 100644 --- a/common/proto/BUILD.bazel +++ b/common/proto/BUILD.bazel @@ -117,7 +117,7 @@ drake_cc_library( visibility = ["//visibility:private"], deps = [ "//common:essential", - "@spruce", + "//common:filesystem", ], ) diff --git a/common/proto/rpc_pipe_temp_directory.cc b/common/proto/rpc_pipe_temp_directory.cc index a152476aaec9..64d58e20b26d 100644 --- a/common/proto/rpc_pipe_temp_directory.cc +++ b/common/proto/rpc_pipe_temp_directory.cc @@ -2,9 +2,8 @@ #include -#include - #include "drake/common/drake_throw.h" +#include "drake/common/filesystem.h" namespace drake { namespace common { @@ -13,11 +12,9 @@ std::string GetRpcPipeTempDirectory() { const char* path_str = nullptr; (path_str = std::getenv("TEST_TMPDIR")) || (path_str = "/tmp"); - spruce::path path(path_str); - DRAKE_THROW_UNLESS(path.isDir()); - - // Spruce normalizes the path and strips any trailing /. - return path.getStr(); + const filesystem::path path(path_str); + DRAKE_THROW_UNLESS(filesystem::is_directory(path)); + return path.string(); } } // namespace common diff --git a/common/temp_directory.cc b/common/temp_directory.cc index 2729b2ea4af2..c85f457763bf 100644 --- a/common/temp_directory.cc +++ b/common/temp_directory.cc @@ -4,14 +4,13 @@ #include -#include - #include "drake/common/drake_throw.h" +#include "drake/common/filesystem.h" namespace drake { std::string temp_directory() { - spruce::path path; + filesystem::path path; const char* test_tmpdir = std::getenv("TEST_TMPDIR"); @@ -19,22 +18,26 @@ std::string temp_directory() { const char* tmpdir = nullptr; (tmpdir = std::getenv("TMPDIR")) || (tmpdir = "/tmp"); - spruce::path path_template(tmpdir); + filesystem::path path_template(tmpdir); path_template.append("robotlocomotion_drake_XXXXXX"); - std::string path_template_str = path_template.getStr(); + std::string path_template_str = path_template.string(); const char* dtemp = ::mkdtemp(&path_template_str[0]); DRAKE_THROW_UNLESS(dtemp != nullptr); - path.setStr(dtemp); + path = dtemp; } else { - path.setStr(test_tmpdir); + path = test_tmpdir; } - DRAKE_THROW_UNLESS(path.isDir()); + DRAKE_THROW_UNLESS(filesystem::is_directory(path)); - // Spruce normalizes the path and strips any trailing /. - return path.getStr(); + // Strip any trailing /. + std::string result = path.string(); + if (result.back() == '/') { + result.pop_back(); + } + return result; } } // namespace drake diff --git a/common/test/find_resource_test.cc b/common/test/find_resource_test.cc index 95cb86da595d..f741c135ccc2 100644 --- a/common/test/find_resource_test.cc +++ b/common/test/find_resource_test.cc @@ -10,11 +10,11 @@ #include #include -#include #include "drake/common/drake_assert.h" #include "drake/common/drake_path.h" #include "drake/common/drake_throw.h" +#include "drake/common/filesystem.h" using std::string; @@ -113,9 +113,10 @@ GTEST_TEST(GetDrakePathTest, PathIncludesDrake) { // Tests that the path returned includes the root of drake. const auto& result = MaybeGetDrakePath(); ASSERT_TRUE(result); - const spruce::path expected(*result + - "/common/test/find_resource_test_data.txt"); - EXPECT_TRUE(expected.exists()); + const filesystem::path expected = + filesystem::path(*result) / + filesystem::path("common/test/find_resource_test_data.txt"); + EXPECT_TRUE(filesystem::exists(expected)); } } // namespace diff --git a/common/test/temp_directory_test.cc b/common/test/temp_directory_test.cc index 4bf368a9dfae..9e1a2260fa8e 100644 --- a/common/test/temp_directory_test.cc +++ b/common/test/temp_directory_test.cc @@ -3,8 +3,11 @@ #include #include +#include #include -#include + +#include "drake/common/drake_assert.h" +#include "drake/common/filesystem.h" namespace drake { namespace { @@ -19,33 +22,48 @@ GTEST_TEST(TempDirectoryTest, TestTmpdirSet) { } GTEST_TEST(TempDirectoryTest, TestTmpdirUnset) { + // Temporarily set TMP to TEST_TMPDIR and unset TEST_TMPDIR, and then see + // what temp_directory() returns. + const char* tmpdir = std::getenv("TMPDIR"); const char* test_tmpdir = std::getenv("TEST_TMPDIR"); ASSERT_STRNE(nullptr, test_tmpdir); + DRAKE_DEMAND(::setenv("TMPDIR", test_tmpdir, 1) == 0); + DRAKE_DEMAND(::unsetenv("TEST_TMPDIR") == 0); + const std::string temp_directory_result = temp_directory(); - const char* tmpdir = std::getenv("TMPDIR"); - const int setenv_tmpdir_result_pre = ::setenv("TMPDIR", test_tmpdir, 1); - ASSERT_EQ(0, setenv_tmpdir_result_pre); - - const int unset_test_tmpdir_result = ::unsetenv("TEST_TMPDIR"); - ASSERT_EQ(0, unset_test_tmpdir_result); + // Revert the environment changes. + DRAKE_DEMAND(::setenv("TEST_TMPDIR", test_tmpdir, 1) == 0); + if (tmpdir == nullptr) { + DRAKE_DEMAND(::unsetenv("TMPDIR") == 0); + } else { + DRAKE_DEMAND(::setenv("TMPDIR", tmpdir, 1) == 0); + } - spruce::path temp_directory_prefix(test_tmpdir); - temp_directory_prefix.append("robotlocomotion_drake_"); + // Check the expected result. + filesystem::path expected_prefix(test_tmpdir); + expected_prefix.append("robotlocomotion_drake_"); + EXPECT_THAT(temp_directory_result, + ::testing::StartsWith(expected_prefix.string())); +} - const std::string temp_directory_with_test_tmpdir_unset = temp_directory(); - EXPECT_EQ(0, temp_directory_with_test_tmpdir_unset.find( - temp_directory_prefix.getStr())); +GTEST_TEST(TempDirectoryTest, TestTmpdirTrailingSlash) { + // Temporarily append a '/' to test_tmpdir, and then see what + // temp_directory() returns. + const char* test_tmpdir = std::getenv("TEST_TMPDIR"); + ASSERT_STRNE(nullptr, test_tmpdir); + std::string new_value(test_tmpdir); + new_value.push_back('/'); + DRAKE_DEMAND(::setenv("TEST_TMPDIR", new_value.c_str(), 1) == 0); + const std::string temp_directory_result = temp_directory(); - const int setenv_test_tmpdir_result = ::setenv("TEST_TMPDIR", test_tmpdir, 1); - ASSERT_EQ(0, setenv_test_tmpdir_result); + // Revert the environment change. + DRAKE_DEMAND(::setenv("TEST_TMPDIR", test_tmpdir, 1) == 0); - if (tmpdir == nullptr) { - const int unset_tmpdir_result = ::unsetenv("TMPDIR"); - ASSERT_EQ(0, unset_tmpdir_result); - } else { - const int setenv_tmpdir_result_post = ::setenv("TMPDIR", tmpdir, 1); - ASSERT_EQ(0, setenv_tmpdir_result_post); - } + // Check the expected result. + filesystem::path expected_prefix(test_tmpdir); + expected_prefix.append("robotlocomotion_drake_"); + EXPECT_THAT(temp_directory_result, + testing::Not(testing::EndsWith("/"))); } } // namespace