From db338e8751a2e463d5c7bdb6a2cd848800b17dc8 Mon Sep 17 00:00:00 2001 From: Jamie Snape Date: Tue, 1 Oct 2019 09:56:07 -0400 Subject: [PATCH] Revert "common: Port from spruce to filesystem (#12117)" This reverts commit 9c19676d5bda70eb226d9003ecbc8f2fb8dbe9b7. --- 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, 50 insertions(+), 68 deletions(-) diff --git a/common/BUILD.bazel b/common/BUILD.bazel index 52f7f8524d6d..56d80009db11 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", - ":filesystem", + "@spruce", ], ) @@ -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 538e0dd07a70..50adf8216d06 100644 --- a/common/drake_path.cc +++ b/common/drake_path.cc @@ -1,6 +1,7 @@ #include "drake/common/drake_path.h" -#include "drake/common/filesystem.h" +#include + #include "drake/common/find_resource.h" // N.B. This code is unit tested in test/find_resource_test.cc. @@ -15,11 +16,11 @@ optional MaybeGetDrakePath() { if (find_result.get_error_message()) { return nullopt; } - filesystem::path sentinel_path(find_result.get_absolute_path_or_throw()); + spruce::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.parent_path().string(); + return sentinel_path.root(); } } // namespace drake diff --git a/common/proto/BUILD.bazel b/common/proto/BUILD.bazel index 74aa17edb4be..b40a69e99bed 100644 --- a/common/proto/BUILD.bazel +++ b/common/proto/BUILD.bazel @@ -117,7 +117,7 @@ drake_cc_library( visibility = ["//visibility:private"], deps = [ "//common:essential", - "//common:filesystem", + "@spruce", ], ) diff --git a/common/proto/rpc_pipe_temp_directory.cc b/common/proto/rpc_pipe_temp_directory.cc index 64d58e20b26d..a152476aaec9 100644 --- a/common/proto/rpc_pipe_temp_directory.cc +++ b/common/proto/rpc_pipe_temp_directory.cc @@ -2,8 +2,9 @@ #include +#include + #include "drake/common/drake_throw.h" -#include "drake/common/filesystem.h" namespace drake { namespace common { @@ -12,9 +13,11 @@ std::string GetRpcPipeTempDirectory() { const char* path_str = nullptr; (path_str = std::getenv("TEST_TMPDIR")) || (path_str = "/tmp"); - const filesystem::path path(path_str); - DRAKE_THROW_UNLESS(filesystem::is_directory(path)); - return path.string(); + spruce::path path(path_str); + DRAKE_THROW_UNLESS(path.isDir()); + + // Spruce normalizes the path and strips any trailing /. + return path.getStr(); } } // namespace common diff --git a/common/temp_directory.cc b/common/temp_directory.cc index c85f457763bf..2729b2ea4af2 100644 --- a/common/temp_directory.cc +++ b/common/temp_directory.cc @@ -4,13 +4,14 @@ #include +#include + #include "drake/common/drake_throw.h" -#include "drake/common/filesystem.h" namespace drake { std::string temp_directory() { - filesystem::path path; + spruce::path path; const char* test_tmpdir = std::getenv("TEST_TMPDIR"); @@ -18,26 +19,22 @@ std::string temp_directory() { const char* tmpdir = nullptr; (tmpdir = std::getenv("TMPDIR")) || (tmpdir = "/tmp"); - filesystem::path path_template(tmpdir); + spruce::path path_template(tmpdir); path_template.append("robotlocomotion_drake_XXXXXX"); - std::string path_template_str = path_template.string(); + std::string path_template_str = path_template.getStr(); const char* dtemp = ::mkdtemp(&path_template_str[0]); DRAKE_THROW_UNLESS(dtemp != nullptr); - path = dtemp; + path.setStr(dtemp); } else { - path = test_tmpdir; + path.setStr(test_tmpdir); } - DRAKE_THROW_UNLESS(filesystem::is_directory(path)); + DRAKE_THROW_UNLESS(path.isDir()); - // Strip any trailing /. - std::string result = path.string(); - if (result.back() == '/') { - result.pop_back(); - } - return result; + // Spruce normalizes the path and strips any trailing /. + return path.getStr(); } } // namespace drake diff --git a/common/test/find_resource_test.cc b/common/test/find_resource_test.cc index f741c135ccc2..95cb86da595d 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,10 +113,9 @@ GTEST_TEST(GetDrakePathTest, PathIncludesDrake) { // Tests that the path returned includes the root of drake. const auto& result = MaybeGetDrakePath(); ASSERT_TRUE(result); - const filesystem::path expected = - filesystem::path(*result) / - filesystem::path("common/test/find_resource_test_data.txt"); - EXPECT_TRUE(filesystem::exists(expected)); + const spruce::path expected(*result + + "/common/test/find_resource_test_data.txt"); + EXPECT_TRUE(expected.exists()); } } // namespace diff --git a/common/test/temp_directory_test.cc b/common/test/temp_directory_test.cc index 9e1a2260fa8e..4bf368a9dfae 100644 --- a/common/test/temp_directory_test.cc +++ b/common/test/temp_directory_test.cc @@ -3,11 +3,8 @@ #include #include -#include #include - -#include "drake/common/drake_assert.h" -#include "drake/common/filesystem.h" +#include namespace drake { namespace { @@ -22,48 +19,33 @@ 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(); - // 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); - } + const char* tmpdir = std::getenv("TMPDIR"); + const int setenv_tmpdir_result_pre = ::setenv("TMPDIR", test_tmpdir, 1); + ASSERT_EQ(0, setenv_tmpdir_result_pre); - // 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 int unset_test_tmpdir_result = ::unsetenv("TEST_TMPDIR"); + ASSERT_EQ(0, unset_test_tmpdir_result); -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(); + spruce::path temp_directory_prefix(test_tmpdir); + temp_directory_prefix.append("robotlocomotion_drake_"); + + 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())); - // Revert the environment change. - DRAKE_DEMAND(::setenv("TEST_TMPDIR", test_tmpdir, 1) == 0); + const int setenv_test_tmpdir_result = ::setenv("TEST_TMPDIR", test_tmpdir, 1); + ASSERT_EQ(0, setenv_test_tmpdir_result); - // Check the expected result. - filesystem::path expected_prefix(test_tmpdir); - expected_prefix.append("robotlocomotion_drake_"); - EXPECT_THAT(temp_directory_result, - testing::Not(testing::EndsWith("/"))); + 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); + } } } // namespace