Skip to content

Commit

Permalink
common: Port from spruce to filesystem (#12117)
Browse files Browse the repository at this point in the history
  • Loading branch information
jwnimmer-tri authored Sep 30, 2019
1 parent 8ce9378 commit 9c19676
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 50 deletions.
6 changes: 3 additions & 3 deletions common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,8 @@ drake_cc_library(
visibility = ["//tools/install/libdrake:__pkg__"],
deps = [
":essential",
":filesystem",
":find_resource",
"@spruce",
],
)

Expand Down Expand Up @@ -443,7 +443,7 @@ drake_cc_library(
hdrs = ["temp_directory.h"],
deps = [
":essential",
"@spruce",
":filesystem",
],
)

Expand Down Expand Up @@ -1218,8 +1218,8 @@ drake_cc_googletest(
drake_cc_googletest(
name = "temp_directory_test",
deps = [
":filesystem",
":temp_directory",
"@spruce",
],
)

Expand Down
7 changes: 3 additions & 4 deletions common/drake_path.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include "drake/common/drake_path.h"

#include <spruce.hh>

#include "drake/common/filesystem.h"
#include "drake/common/find_resource.h"

// N.B. This code is unit tested in test/find_resource_test.cc.
Expand All @@ -16,11 +15,11 @@ optional<std::string> 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
2 changes: 1 addition & 1 deletion common/proto/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ drake_cc_library(
visibility = ["//visibility:private"],
deps = [
"//common:essential",
"@spruce",
"//common:filesystem",
],
)

Expand Down
11 changes: 4 additions & 7 deletions common/proto/rpc_pipe_temp_directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

#include <cstdlib>

#include <spruce.hh>

#include "drake/common/drake_throw.h"
#include "drake/common/filesystem.h"

namespace drake {
namespace common {
Expand All @@ -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
Expand Down
23 changes: 13 additions & 10 deletions common/temp_directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,40 @@

#include <cstdlib>

#include <spruce.hh>

#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");

if (test_tmpdir == nullptr) {
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
9 changes: 5 additions & 4 deletions common/test/find_resource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <spruce.hh>

#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;

Expand Down Expand Up @@ -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
Expand Down
60 changes: 39 additions & 21 deletions common/test/temp_directory_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
#include <cstdlib>
#include <string>

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <spruce.hh>

#include "drake/common/drake_assert.h"
#include "drake/common/filesystem.h"

namespace drake {
namespace {
Expand All @@ -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
Expand Down

0 comments on commit 9c19676

Please sign in to comment.