Skip to content

Commit

Permalink
common: Remove deprecated methods (2019-08) (RobotLocomotion#11841)
Browse files Browse the repository at this point in the history
  • Loading branch information
jwnimmer-tri authored Aug 1, 2019
1 parent e128284 commit fe16af6
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 229 deletions.
20 changes: 0 additions & 20 deletions bindings/pydrake/common/module_py.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,26 +70,6 @@ PYBIND11_MODULE(_module_py, m) {
PyErr_SetString(PyExc_SystemExit, e.what());
}
});
// Convenient wrapper to add a resource search path.
m.def("AddResourceSearchPath",
[](const std::string& x) {
WarnDeprecated("See API docs for deprecation notice.");
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
AddResourceSearchPath(x);
#pragma GCC diagnostic pop
},
py::arg("search_path"), doc.AddResourceSearchPath.doc_deprecated);
// Convenient wrapper to get the list of resource search paths.
m.def("GetResourceSearchPaths",
[]() {
WarnDeprecated("See API docs for deprecation notice.");
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
return GetResourceSearchPaths();
#pragma GCC diagnostic pop
},
doc.GetResourceSearchPaths.doc_deprecated);
// Convenient wrapper for querying FindResource(resource_path).
m.def("FindResourceOrThrow", &FindResourceOrThrow,
"Attempts to locate a Drake resource named by the given path string. "
Expand Down
2 changes: 1 addition & 1 deletion bindings/pydrake/test/all_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def test_symbols_subset(self):
"SimpleCar",
# common
# - __init__
"AddResourceSearchPath",
"RandomDistribution",
# - compatibility
"maybe_patch_numpy_formatters",
# - containers
Expand Down
1 change: 0 additions & 1 deletion common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ drake_cc_library(
srcs = [
"find_loaded_library.cc",
"find_resource.cc",
"find_resource_deprecated.cc",
],
hdrs = [
"find_loaded_library.h",
Expand Down
39 changes: 0 additions & 39 deletions common/find_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,27 +110,11 @@ bool StartsWith(const string& str, const string& prefix) {
return str.compare(0, prefix.size(), prefix) == 0;
}

bool EndsWith(const string& str, const string& suffix) {
if (suffix.size() > str.size()) { return false; }
return str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0;
}

// Returns true iff the path is relative (not absolute nor empty).
bool IsRelativePath(const string& path) {
return !path.empty() && (path[0] != '/');
}

// Add a commented-out macro, so that the deprecation grep will find it:
// DRAKE_DEPRECATED("2019-08-01", "See below")
void WarnDeprecatedDirectory(const string& resource_path) {
static const logging::Warn log_once(
"Using drake::FindResource to locate a directory (e.g., '{}') "
"is deprecated, and will become an error after 2019-08-01. "
"Always request a file within the directory instead, e.g., find "
"'drake/manipulation/models/iiwa_description/package.xml', not "
"'drake/manipulation/models/iiwa_description'.", resource_path);
}

// Taking `root` to be Drake's resource root, confirm that the sentinel file
// exists and return the found resource_path (or an error if either the
// sentinel or resource_path was missing).
Expand All @@ -154,11 +138,6 @@ Result CheckAndMakeResult(

// Check for the resource_path.
const string abspath = root + '/' + resource_path;
if (internal::IsDir(abspath)) {
// As a compatibility shim, allow directory resources for now.
WarnDeprecatedDirectory(resource_path);
return Result::make_success(resource_path, abspath);
}
if (!internal::IsFile(abspath)) {
return Result::make_error(resource_path, fmt::format(
"Could not find Drake resource_path '{}' because {} specified a "
Expand Down Expand Up @@ -295,24 +274,6 @@ Result FindResource(const string& resource_path) {
return Result::make_success(
resource_path, rlocation_or_error.abspath);
}
// As a compatibility shim, allow for directory resources for now.
{
const std::string sentinel_relpath(kSentinelRelpath);
auto sentinel_rlocation_or_error =
internal::FindRunfile(sentinel_relpath);
DRAKE_THROW_UNLESS(sentinel_rlocation_or_error.error.empty());
const std::string sentinel_abspath =
sentinel_rlocation_or_error.abspath;
DRAKE_THROW_UNLESS(EndsWith(sentinel_abspath, sentinel_relpath));
const std::string resource_abspath =
sentinel_abspath.substr(
0, sentinel_abspath.size() - sentinel_relpath.size()) +
resource_path;
if (internal::IsDir(resource_abspath)) {
WarnDeprecatedDirectory(resource_path);
return Result::make_success(resource_path, resource_abspath);
}
}
// As a compatibility shim, for resource paths that have been moved into the
// attic, we opportunistically try a fallback search path for them. This
// heuristic is only helpful for source trees -- any install data files from
Expand Down
15 changes: 0 additions & 15 deletions common/find_resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include "drake/common/drake_assert.h"
#include "drake/common/drake_copyable.h"
#include "drake/common/drake_deprecated.h"
#include "drake/common/drake_optional.h"

namespace drake {
Expand Down Expand Up @@ -73,20 +72,6 @@ class FindResourceResult {
optional<std::string> error_message_;
};

/// (Deprecated.) Sets the kDrakeResourceRootEnvironmentVariableName
/// environment variable to @p root_directory.
/// @throws std::runtime_error if the environment variable is already set.
/// @throws std::runtime_error if the given path is not absolute.
DRAKE_DEPRECATED("2019-08-01",
"Call setenv(kDrakeResourceRootEnvironmentVariableName) instead.")
void AddResourceSearchPath(const std::string& root_directory);

/// Returns a single-element vector containing the last root_directory passed
/// to AddResourceSearchPath() if any; otherwise, returns an empty vector.
DRAKE_DEPRECATED("2019-08-01",
"Call getenv(kDrakeResourceRootEnvironmentVariableName) instead.")
std::vector<std::string> GetResourceSearchPaths();

/// Attempts to locate a Drake resource named by the given @p resource_path.
/// The @p resource_path refers to the relative path within the Drake source
/// repository, prepended with `drake/`. For example, to find the source
Expand Down
43 changes: 0 additions & 43 deletions common/find_resource_deprecated.cc

This file was deleted.

27 changes: 3 additions & 24 deletions common/resource_tool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@ DEFINE_bool(
"Print the name of the environment variable that provides the "
"first place where this tool attempts to look. This flag cannot be used "
"in combination with the other flags.");
DEFINE_string(
add_resource_search_path, "",
"Adds path in which the resources live. This directory will be"
"searched after the environment variable but before the directory in which"
" `.drake-resource-sentinel` is, if such a directory is found. This flag "
"can be used in combination with `print_resource_path`");

namespace drake {
namespace {
Expand All @@ -31,17 +25,11 @@ int main(int argc, char* argv[]) {
gflags::ParseCommandLineFlags(&argc, &argv, true);
logging::HandleSpdlogGflags();

// Allowed flag combinations (num_commands value):
// FLAGS_print_resource_path (1)
// FLAGS_print_resource_path + FLAGS_add_resource_search_path (1)
// FLAGS_print_resource_root_environment_variable_name (1)
// The user must supply exactly one of --print_resource_path or
// --print_resource_root_environment_variable_name.
const int num_commands =
(FLAGS_print_resource_path.empty() ? 0 : 1) +
(FLAGS_print_resource_root_environment_variable_name ? 1 : 0) +
(!FLAGS_add_resource_search_path.empty() &&
FLAGS_print_resource_root_environment_variable_name
? 1
: 0);
(FLAGS_print_resource_root_environment_variable_name ? 1 : 0);
if (num_commands != 1) {
gflags::ShowUsageWithFlags(argv[0]);
return 1;
Expand All @@ -52,15 +40,6 @@ int main(int argc, char* argv[]) {
return 0;
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
if (!FLAGS_add_resource_search_path.empty()) {
std::cerr << "resource_tool: WARNING: "
<< "--add_resource_search_path is deprecated.\n";
AddResourceSearchPath(FLAGS_add_resource_search_path);
}
#pragma GCC diagnostic pop

const FindResourceResult& result = FindResource(FLAGS_print_resource_path);
if (result.get_absolute_path()) {
std::cout << *result.get_absolute_path()
Expand Down
63 changes: 3 additions & 60 deletions common/test/find_resource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,40 +95,12 @@ GTEST_TEST(FindResourceTest, FoundDeclaredData) {
EXPECT_EQ(FindResourceOrThrow(relpath), absolute_path);
}

GTEST_TEST(FindResourceTest, DeprecatedFoundDirectory) {
GTEST_TEST(FindResourceTest, FoundDirectory) {
// Looking up a directory (not file) should fail.
const string relpath = "drake/common";
const auto& result = FindResource(relpath);

// We get a path back.
ASSERT_FALSE(result.get_error_message());
EXPECT_EQ(result.get_resource_path(), relpath);
string absolute_path;
EXPECT_NO_THROW(absolute_path = result.get_absolute_path_or_throw());

// The path is the correct answer.
ASSERT_FALSE(absolute_path.empty());
EXPECT_EQ(absolute_path[0], '/');
std::ifstream input(
absolute_path + "/test/find_resource_test_data.txt",
std::ios::binary);
ASSERT_TRUE(input);
std::stringstream buffer;
buffer << input.rdbuf();
EXPECT_EQ(
buffer.str(),
"Test data for drake/common/test/find_resource_test.cc.\n");
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
// Check that adding a relative resource path fails on purpose.
GTEST_TEST(FindResourceTest, RelativeResourcePathShouldFail) {
// Test `AddResourceSearchPath()` with a relative path. It is expected to
// fail.
const std::string test_directory = "find_resource_test_scratch";
EXPECT_THROW(AddResourceSearchPath(test_directory), std::runtime_error);
ASSERT_TRUE(result.get_error_message());
}
#pragma GCC diagnostic pop

GTEST_TEST(GetDrakePathTest, BasicTest) {
// Just test that we find a path, without any exceptions.
Expand All @@ -146,34 +118,5 @@ GTEST_TEST(GetDrakePathTest, PathIncludesDrake) {
EXPECT_TRUE(expected.exists());
}

// Create an empty file with the given filename.
void Touch(const std::string& filename) {
std::ofstream(filename.c_str(), std::ios::out).close();
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
// NOTE: This test modifies the result of calls to GetDrakePath() and variants.
// However, it does *not* clean up the modifications. As such, it must run
// *last*. Relying on execution order being alphabetical, we make sure it is
// in the last test suite and the last test case (with the ZZZ_ prefix).
GTEST_TEST(ZZZ_FindResourceTest, ZZZ_AlternativeDirectory) {
// Test `AddResourceSearchPath()` and `GetResourceSearchPaths()` by creating
// an empty file in a scratch directory with a sentinel file. Bazel tests are
// run in a scratch directory, so we don't need to remove anything manually.
const std::string test_directory = spruce::dir::getcwd().getStr() +
"/find_resource_test_scratch";
const std::string candidate_filename = "drake/candidate.ext";
spruce::dir::mkdir(test_directory);
spruce::dir::mkdir(test_directory + "/drake");
Touch(test_directory + "/drake/.drake-find_resource-sentinel");
Touch(test_directory + "/" + candidate_filename);
AddResourceSearchPath(test_directory);
EXPECT_TRUE(!GetResourceSearchPaths().empty());
EXPECT_EQ(GetResourceSearchPaths()[0], test_directory);
EXPECT_NO_THROW(drake::FindResourceOrThrow(candidate_filename));
}
#pragma GCC diagnostic pop

} // namespace
} // namespace drake
26 changes: 0 additions & 26 deletions common/test/resource_tool_installed_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,6 @@ def test_install_and_run(self):
with open(absolute_path, 'r') as data:
self.assertEqual(data.read(), resource_data)

# Use the installed resource_tool to find a directory.
# (This feature is deprecated and will eventually be removed.)
absolute_path = install_test_helper.check_output(
[resource_tool,
"--print_resource_path",
"drake/common/test",
],
env=tool_env,
).strip()
with open(absolute_path + '/tmp_resource', 'r') as data:
self.assertEqual(data.read(), resource_data)

# Use the installed resource_tool to find a resource, but with a bogus
# DRAKE_RESOURCE_ROOT that should be ignored.
tool_env[env_name] = os.path.join(tmp_dir, "share", "drake")
Expand All @@ -88,20 +76,6 @@ def test_install_and_run(self):
self.assertTrue(os.path.exists(absolute_path),
absolute_path + " does not exist")

# Use --add_resource_search_path instead of environment variable
# to find resources.
# (This feature is deprecated and will eventually be removed.)
absolute_path = install_test_helper.check_output(
[resource_tool,
"--print_resource_path",
"drake/common/test/tmp_resource",
"--add_resource_search_path",
os.path.join(tmp_dir, "share"),
],
).strip()
with open(absolute_path, 'r') as data:
self.assertEqual(data.read(), resource_data)


if __name__ == '__main__':
unittest.main()

0 comments on commit fe16af6

Please sign in to comment.