common: Add add_text_logging_gflags, deprecate text_logging_gflags (R…

…obotLocomotion#12287) It is difficult to support Drake-installed libraries that depend on gflags, due to how gflags is designed for static linking and lacks CMake-supporting infrastructure. Thus, we are deprecating (and will later remove) Drake's support for installing anything that touches gflags.
mcx · Oct 31, 2019 · b7f7cdb · b7f7cdb
1 parent 2ebcca6
commit b7f7cdb
Show file tree

Hide file tree

Showing 52 changed files with 109 additions and 90 deletions.
diff --git a/attic/manipulation/scene_generation/BUILD.bazel b/attic/manipulation/scene_generation/BUILD.bazel
@@ -48,7 +48,7 @@ drake_cc_binary(
         ":random_clutter_generator",
         ":simulate_plant_to_rest",
         "//common",
-        "//common:text_logging_gflags",
+        "//common:add_text_logging_gflags",
         "@gflags",
     ],
 )
@@ -69,7 +69,6 @@ drake_cc_googletest(
         ":random_clutter_generator",
         ":simulate_plant_to_rest",
         "//common",
-        "//common:text_logging_gflags",
         "//common/test_utilities:eigen_matrix_compare",
         "@gflags",
     ],

diff --git a/attic/manipulation/scene_generation/random_clutter_demo.cc b/attic/manipulation/scene_generation/random_clutter_demo.cc
@@ -7,7 +7,6 @@
 #include <gflags/gflags.h>
 
 #include "drake/common/eigen_types.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/lcm/drake_lcm.h"
 #include "drake/manipulation/scene_generation/random_clutter_generator.h"
 #include "drake/manipulation/scene_generation/simulate_plant_to_rest.h"
@@ -162,6 +161,5 @@ int DoMain() {
 
 int main(int argc, char* argv[]) {
   gflags::ParseCommandLineFlags(&argc, &argv, true);
-  drake::logging::HandleSpdlogGflags();
   drake::manipulation::scene_generation::DoMain();
 }
diff --git a/attic/systems/sensors/BUILD.bazel b/attic/systems/sensors/BUILD.bazel
@@ -127,7 +127,7 @@ drake_cc_binary(
     ],
     deps = [
         ":accelerometer_example_diagram",
-        "//common:text_logging_gflags",
+        "//common:add_text_logging_gflags",
         "//lcm",
         "//systems/analysis:simulator",
         "@gflags",

diff --git a/attic/systems/sensors/test/accelerometer_test/accelerometer_example.cc b/attic/systems/sensors/test/accelerometer_test/accelerometer_example.cc
@@ -3,7 +3,6 @@
 #include <gflags/gflags.h>
 
 #include "drake/common/drake_assert.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/lcm/drake_lcm.h"
 #include "drake/systems/analysis/simulator.h"
 #include "drake/systems/sensors/test/accelerometer_test/accelerometer_example_diagram.h"
@@ -28,7 +27,6 @@ namespace {
 
 int exec(int argc, char* argv[]) {
   gflags::ParseCommandLineFlags(&argc, &argv, true);
-  logging::HandleSpdlogGflags();
 
   DRAKE_DEMAND(FLAGS_simulation_sec >= 0);
   ::drake::lcm::DrakeLcm real_lcm;

diff --git a/common/BUILD.bazel b/common/BUILD.bazel
@@ -446,16 +446,32 @@ drake_cc_library(
     ],
 )
 
+# This is a Drake-internal utility for use only as a direct dependency
+# of executable (drake_cc_binary) targets.  Thus, we exclude it from
+# the ":common" package library and from libdrake.
+drake_cc_library(
+    name = "add_text_logging_gflags",
+    srcs = ["add_text_logging_gflags.cc"],
+    tags = [
+        "exclude_from_libdrake",
+        "exclude_from_package",
+    ],
+    visibility = ["//:__subpackages__"],
+    deps = [
+        ":essential",
+        ":unused",
+        "@gflags",
+    ],
+    alwayslink = 1,
+)
+
+# TODO(jwnimmer-tri) When this header file is removed, be sure to also remove
+# the CMake install helpers related to gflags (in drake.cps, FindGFlags.cmake,
+# etc).
 drake_cc_library(
     name = "text_logging_gflags",
     hdrs = ["text_logging_gflags.h"],
-    # TODO(jwnimmer-tri) Ideally, gflags BUILD would do this for us.  Figure
-    # out what's going on.  Definitely don't let "-pthread" get copy-pasta'd
-    # throughout our code.
-    linkopts = select({
-        "//tools/cc_toolchain:linux": ["-pthread"],
-        "//conditions:default": [],
-    }),
+    deprecation = "DRAKE DEPRECATED: The drake/common/text_logging_gflags.h convenience header is being removed from Drake on or after 2020-02-01.  Downstream projects should maintain their own implementation of this feature.  See drake/common/add_text_logging_gflags.cc for an example.",  # noqa
     tags = [
         # Don't add this library into the ":common" package library.
         # Only programs with a main() function should ever use this header.
@@ -467,13 +483,19 @@ drake_cc_library(
         # this cc_library entirely, and use a separate rule below for the
         # installation.
         "exclude_from_libdrake",
+        # Avoid 'build //...' yelling at us.
+        "manual",
     ],
     deps = [
         ":essential",
         "@gflags",
     ],
 )
 
+# TODO(jwnimmer-tri) When this header file is removed, be sure to also remove
+# the CMake install helpers related to gflags (in drake.cps, FindGFlags.cmake,
+# etc).
+#
 # Compared to the above rule, this removes the deps attribute; we don't want to
 # link against gflags for libdrake.so, we just want to publish this header for
 # callers who have their own gflags to link against.
@@ -533,9 +555,9 @@ drake_cc_binary(
     name = "resource_tool",
     srcs = ["resource_tool.cc"],
     deps = [
+        ":add_text_logging_gflags",
         ":essential",
         ":find_resource",
-        ":text_logging_gflags",
     ],
 )
 

diff --git a/common/add_text_logging_gflags.cc b/common/add_text_logging_gflags.cc
@@ -0,0 +1,26 @@
+// This is a Drake-internal utility for use only as a direct dependency
+// of executable (drake_cc_binary) targets.
+//
+// To add --spdlog_level to the gflags command line of any `drake_cc_binary`,
+// add "//common:add_text_logging_gflags" to the `deps` attribute in its BUILD.
+
+#include <string>
+
+#include <gflags/gflags.h>
+
+#include "drake/common/text_logging.h"
+#include "drake/common/unused.h"
+
+// Declare the --spdlog_level gflags option.
+DEFINE_string(spdlog_level, drake::logging::kSetLogLevelUnchanged,
+              drake::logging::kSetLogLevelHelpMessage);
+
+// Validate --spdlog_level and update Drake's configuration to match the flag.
+namespace {
+bool ValidateSpdlogLevel(const char* name, const std::string& value) {
+  drake::unused(name);
+  drake::logging::set_log_level(value);
+  return true;
+}
+}  // namespace
+DEFINE_validator(spdlog_level, &ValidateSpdlogLevel);
diff --git a/common/resource_tool.cc b/common/resource_tool.cc
@@ -3,7 +3,6 @@
 #include <gflags/gflags.h>
 
 #include "drake/common/find_resource.h"
-#include "drake/common/text_logging_gflags.h"
 
 DEFINE_string(
     print_resource_path, "",
@@ -23,7 +22,6 @@ namespace {
 int main(int argc, char* argv[]) {
   gflags::SetUsageMessage("Find Drake-related resources");
   gflags::ParseCommandLineFlags(&argc, &argv, true);
-  logging::HandleSpdlogGflags();
 
   // The user must supply exactly one of --print_resource_path or
   // --print_resource_root_environment_variable_name.

diff --git a/common/test_utilities/BUILD.bazel b/common/test_utilities/BUILD.bazel
@@ -142,7 +142,7 @@ drake_cc_library(
         "exclude_from_package",
     ],
     deps = [
-        "//common:text_logging_gflags",
+        "//common:add_text_logging_gflags",
         "@gtest//:without_main",
     ],
 )

diff --git a/common/test_utilities/drake_cc_googletest_main.cc b/common/test_utilities/drake_cc_googletest_main.cc
@@ -1,10 +1,9 @@
 /// @file
 /// This is Drake's default main() function for gtest-based unit tests.
 
+#include <gflags/gflags.h>
 #include <gmock/gmock.h>
 
-#include "drake/common/text_logging_gflags.h"
-
 // TODO(SeanCurtis-TRI): Remove this when CLion bazel plug-in no longer executes
 // its debugger with the --gunit_color flag. Most recent versions known to be
 // an issue:
@@ -25,9 +24,6 @@ int main(int argc, char** argv) {
   google::SetUsageMessage(" ");  // Nerf a silly warning emitted by gflags.
   google::ParseCommandLineFlags(&argc, &argv, true);
 
-  // Adjust Drake's log setting per the gflags results.
-  drake::logging::HandleSpdlogGflags();
-
   // Actually run the tests.
   return RUN_ALL_TESTS();
 }
diff --git a/common/text_logging_gflags.h b/common/text_logging_gflags.h
@@ -4,8 +4,12 @@
 /// This file defines gflags settings to control spdlog levels.
 /// Only include this from translation units that declare a `main` function.
 
+// NOLINTNEXTLINE(whitespace/line_length)
+#warning DRAKE DEPRECATED: The drake/common/text_logging_gflags.h convenience header is being removed from Drake on or after 2020-02-01.  Downstream projects should maintain their own implementation of this feature.  See drake/common/add_text_logging_gflags.cc for an example.
+
 #include <gflags/gflags.h>
 
+#include "drake/common/drake_deprecated.h"
 #include "drake/common/text_logging.h"
 
 // Declare the --spdlog_level gflags option.
@@ -18,6 +22,7 @@ namespace logging {
 
 /// Check the gflags settings for validity.  If spdlog is enabled, update its
 /// configuration to match the flags.
+DRAKE_DEPRECATED("2020-02-01", "The drake/common/text_logging_gflags.h convenience header is being removed from Drake.  Downstream projects should maintain their own implementation of this feature.  See drake/common/add_text_logging_gflags.cc for an example.")  // NOLINT(whitespace/line_length)
 inline void HandleSpdlogGflags() {
   drake::logging::set_log_level(FLAGS_spdlog_level);
 }

diff --git a/examples/allegro_hand/BUILD.bazel b/examples/allegro_hand/BUILD.bazel
@@ -41,8 +41,8 @@ drake_cc_binary(
         "//manipulation/models/allegro_hand_description:models",
     ],
     deps = [
+        "//common:add_text_logging_gflags",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//geometry:geometry_visualization",
         "//lcm",
         "//multibody/parsing",

diff --git a/examples/allegro_hand/joint_control/BUILD.bazel b/examples/allegro_hand/joint_control/BUILD.bazel
@@ -18,8 +18,8 @@ drake_cc_binary(
         "//manipulation/models/allegro_hand_description:models",
     ],
     deps = [
+        "//common:add_text_logging_gflags",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//examples/allegro_hand:allegro_common",
         "//examples/allegro_hand:allegro_lcm",
         "//geometry:geometry_visualization",

diff --git a/examples/allegro_hand/joint_control/allegro_single_object_simulation.cc b/examples/allegro_hand/joint_control/allegro_single_object_simulation.cc
@@ -11,7 +11,6 @@
 
 #include "drake/common/drake_assert.h"
 #include "drake/common/find_resource.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/examples/allegro_hand/allegro_common.h"
 #include "drake/examples/allegro_hand/allegro_lcm.h"
 #include "drake/geometry/geometry_visualization.h"

diff --git a/examples/allegro_hand/run_allegro_constant_load_demo.cc b/examples/allegro_hand/run_allegro_constant_load_demo.cc
@@ -9,7 +9,6 @@
 
 #include "drake/common/drake_assert.h"
 #include "drake/common/find_resource.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/geometry/geometry_visualization.h"
 #include "drake/geometry/scene_graph.h"
 #include "drake/lcm/drake_lcm.h"

diff --git a/examples/atlas/BUILD.bazel b/examples/atlas/BUILD.bazel
@@ -23,8 +23,8 @@ drake_cc_binary(
         "--target_realtime_rate=0.0",
     ],
     deps = [
+        "//common:add_text_logging_gflags",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//geometry:geometry_visualization",
         "//multibody/parsing",
         "//multibody/plant:contact_results_to_lcm",

diff --git a/examples/atlas/atlas_run_dynamics.cc b/examples/atlas/atlas_run_dynamics.cc
@@ -3,7 +3,6 @@
 #include <gflags/gflags.h>
 
 #include "drake/common/find_resource.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/geometry/geometry_visualization.h"
 #include "drake/geometry/scene_graph.h"
 #include "drake/lcm/drake_lcm.h"
@@ -132,6 +131,5 @@ int main(int argc, char* argv[]) {
       "\njoint limits. "
       "\nLaunch drake-visualizer before running this example.");
   gflags::ParseCommandLineFlags(&argc, &argv, true);
-  drake::logging::HandleSpdlogGflags();
   return drake::examples::atlas::do_main();
 }
diff --git a/examples/double_pendulum/BUILD.bazel b/examples/double_pendulum/BUILD.bazel
@@ -56,9 +56,9 @@ drake_cc_binary(
         "//attic/multibody:rigid_body_tree",
         "//attic/multibody/rigid_body_plant",
         "//attic/multibody/rigid_body_plant:drake_visualizer",
+        "//common:add_text_logging_gflags",
         "//common:essential",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//lcm",
         "//systems/analysis",
         "//systems/framework",

diff --git a/examples/double_pendulum/double_pendulum_demo.cc b/examples/double_pendulum/double_pendulum_demo.cc
@@ -5,8 +5,9 @@
 #include <limits>
 #include <memory>
 
+#include <gflags/gflags.h>
+
 #include "drake/common/find_resource.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/examples/double_pendulum/sdf_helpers.h"
 #include "drake/multibody/rigid_body_plant/drake_visualizer.h"
 #include "drake/multibody/rigid_body_plant/rigid_body_plant.h"
@@ -36,7 +37,6 @@ int main(int argc, char* argv[]) {
   gflags::SetUsageMessage("Simple sdformat usage example, just"
                           "make sure drake-visualizer is running!");
   gflags::ParseCommandLineFlags(&argc, &argv, true);
-  logging::HandleSpdlogGflags();
   // Load and parse double pendulum SDF from file into a tree.
   const std::string sdf_path = FindResourceOrThrow(kDoublePendulumSdfPath);
   auto tree = std::make_unique<RigidBodyTree<double>>();

diff --git a/examples/kinova_jaco_arm/BUILD.bazel b/examples/kinova_jaco_arm/BUILD.bazel
@@ -34,8 +34,8 @@ drake_cc_binary(
     ],
     deps = [
         ":jaco_lcm",
+        "//common:add_text_logging_gflags",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//lcm",
         "//manipulation/planner:robot_plan_interpolator",
         "//systems/analysis:simulator",
@@ -60,8 +60,8 @@ drake_cc_binary(
     test_rule_args = ["--simulation_sec=0.1"],
     deps = [
         ":jaco_lcm",
+        "//common:add_text_logging_gflags",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//geometry:geometry_visualization",
         "//geometry:scene_graph",
         "//multibody/parsing",
@@ -80,8 +80,8 @@ drake_cc_binary(
     ],
     deps = [
         ":jaco_lcm",
+        "//common:add_text_logging_gflags",
         "//common:find_resource",
-        "//common:text_logging_gflags",
         "//lcmtypes:jaco",
         "//manipulation/planner:differential_inverse_kinematics",
         "//math:geometric_transform",

diff --git a/examples/kinova_jaco_arm/jaco_controller.cc b/examples/kinova_jaco_arm/jaco_controller.cc
@@ -10,7 +10,6 @@
 #include "drake/common/drake_assert.h"
 #include "drake/common/find_resource.h"
 #include "drake/common/text_logging.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/examples/kinova_jaco_arm/jaco_lcm.h"
 #include "drake/lcm/drake_lcm.h"
 #include "drake/lcmt_jaco_command.hpp"

diff --git a/examples/kinova_jaco_arm/move_jaco_ee.cc b/examples/kinova_jaco_arm/move_jaco_ee.cc
@@ -11,7 +11,6 @@
 
 #include "drake/common/find_resource.h"
 #include "drake/common/text_logging.h"
-#include "drake/common/text_logging_gflags.h"
 #include "drake/lcmt_jaco_command.hpp"
 #include "drake/lcmt_jaco_status.hpp"
 #include "drake/manipulation/planner/differential_inverse_kinematics.h"
@@ -182,7 +181,6 @@ class MoveDemoRunner {
 
 int main(int argc, char* argv[]) {
   gflags::ParseCommandLineFlags(&argc, &argv, true);
-  drake::logging::HandleSpdlogGflags();
   drake::examples::kinova_jaco_arm::MoveDemoRunner runner;
   runner.Run();
 }