From 59884cb9bf44af2a12ae2f268f686c7ec51c6543 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Tue, 24 Apr 2018 14:47:08 -0400 Subject: [PATCH 1/2] Re-apply "Add drake_cc_package_library and library_lint" This reapplies f942383ac50417a7842deb75657a05247b82eb28 which was reverted by cd3dc22c3ab7d75e79fd95799fcccac36e09729c. --- automotive/maliput/api/BUILD.bazel | 17 ++- .../maliput/api/test_utilities/BUILD.bazel | 10 +- automotive/maliput/dragway/BUILD.bazel | 13 +- automotive/maliput/monolane/BUILD.bazel | 7 +- automotive/maliput/multilane/BUILD.bazel | 7 +- .../multilane/test_utilities/BUILD.bazel | 9 +- automotive/maliput/rndf/BUILD.bazel | 7 +- .../maliput/rndf/test_utilities/BUILD.bazel | 9 +- automotive/maliput/simplerulebook/BUILD.bazel | 7 +- automotive/maliput/utility/BUILD.bazel | 15 +- common/BUILD.bazel | 22 +-- common/test_utilities/BUILD.bazel | 16 +-- examples/rod2d/BUILD.bazel | 3 +- lcm/BUILD.bazel | 6 +- multibody/benchmarks/acrobot/BUILD.bazel | 6 +- multibody/benchmarks/free_body/BUILD.bazel | 17 ++- multibody/collision/BUILD.bazel | 22 ++- multibody/constraint/BUILD.bazel | 17 ++- multibody/joints/BUILD.bazel | 17 ++- multibody/multibody_tree/BUILD.bazel | 23 +++- .../multibody_plant/BUILD.bazel | 13 +- multibody/parsers/BUILD.bazel | 15 +- multibody/rigid_body_plant/BUILD.bazel | 3 + multibody/shapes/BUILD.bazel | 17 ++- systems/analysis/BUILD.bazel | 16 ++- systems/analysis/test_utilities/BUILD.bazel | 9 +- systems/controllers/plan_eval/BUILD.bazel | 2 +- .../qp_inverse_dynamics/BUILD.bazel | 21 +-- systems/framework/BUILD.bazel | 13 +- systems/framework/test_utilities/BUILD.bazel | 11 +- systems/lcm/BUILD.bazel | 6 +- systems/plants/spring_mass_system/BUILD.bazel | 17 ++- systems/primitives/BUILD.bazel | 14 +- systems/sensors/BUILD.bazel | 15 +- tools/install/libdrake/build_components.bzl | 3 +- tools/lint/BUILD.bazel | 6 + tools/lint/library_lint.bzl | 129 ++++++++++++++++++ tools/lint/library_lint_reporter.py | 82 +++++++++++ tools/lint/lint.bzl | 7 +- tools/skylark/drake_cc.bzl | 36 +++++ util/BUILD.bazel | 23 +++- 41 files changed, 598 insertions(+), 110 deletions(-) create mode 100644 tools/lint/library_lint.bzl create mode 100644 tools/lint/library_lint_reporter.py diff --git a/automotive/maliput/api/BUILD.bazel b/automotive/maliput/api/BUILD.bazel index 905cdd4f657a..c3235984c6d4 100644 --- a/automotive/maliput/api/BUILD.bazel +++ b/automotive/maliput/api/BUILD.bazel @@ -1,12 +1,24 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "api", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "lane.cc", "lane_data.cc", @@ -25,6 +37,7 @@ drake_cc_library( "segment.h", "type_specific_identifier.h", ], + visibility = ["//visibility:private"], deps = [ "//common:default_scalars", "//common:essential", diff --git a/automotive/maliput/api/test_utilities/BUILD.bazel b/automotive/maliput/api/test_utilities/BUILD.bazel index 89c83304b3bd..98f4e6a6503d 100644 --- a/automotive/maliput/api/test_utilities/BUILD.bazel +++ b/automotive/maliput/api/test_utilities/BUILD.bazel @@ -1,16 +1,20 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -# This should encompass every cc_library in this package. -drake_cc_library( +drake_cc_package_library( name = "test_utilities", testonly = 1, deps = [ ":maliput_types_compare", + ":rules_test_utilities", ], ) diff --git a/automotive/maliput/dragway/BUILD.bazel b/automotive/maliput/dragway/BUILD.bazel index 9ef82c50d73a..a70ee7414e1a 100644 --- a/automotive/maliput/dragway/BUILD.bazel +++ b/automotive/maliput/dragway/BUILD.bazel @@ -1,17 +1,25 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "dragway", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "branch_point.cc", "junction.cc", @@ -26,6 +34,7 @@ drake_cc_library( "road_geometry.h", "segment.h", ], + visibility = ["//visibility:private"], deps = [ "//automotive/maliput/api", "//common:essential", diff --git a/automotive/maliput/monolane/BUILD.bazel b/automotive/maliput/monolane/BUILD.bazel index 6b6c452af771..9b003bd4d536 100644 --- a/automotive/maliput/monolane/BUILD.bazel +++ b/automotive/maliput/monolane/BUILD.bazel @@ -1,10 +1,11 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load( "@drake//tools/skylark:drake_py.bzl", @@ -14,10 +15,8 @@ load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "monolane", - srcs = [], - hdrs = [], deps = [ ":builder", ":lanes", diff --git a/automotive/maliput/multilane/BUILD.bazel b/automotive/maliput/multilane/BUILD.bazel index 742dd2ac5399..f36d2c1117ad 100644 --- a/automotive/maliput/multilane/BUILD.bazel +++ b/automotive/maliput/multilane/BUILD.bazel @@ -1,10 +1,11 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load( "@drake//tools/skylark:drake_py.bzl", @@ -14,10 +15,8 @@ load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "multilane", - srcs = [], - hdrs = [], deps = [ ":builder", ":lanes", diff --git a/automotive/maliput/multilane/test_utilities/BUILD.bazel b/automotive/maliput/multilane/test_utilities/BUILD.bazel index b7f990977520..588e3dcda036 100644 --- a/automotive/maliput/multilane/test_utilities/BUILD.bazel +++ b/automotive/maliput/multilane/test_utilities/BUILD.bazel @@ -1,13 +1,16 @@ # -*- python -*- # This file contains rules for Bazel; see drake/doc/bazel.rst. -load("//tools:drake.bzl", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -# This should encompass every cc_library in this package. -drake_cc_library( +drake_cc_package_library( name = "test_utilities", testonly = 1, deps = [ diff --git a/automotive/maliput/rndf/BUILD.bazel b/automotive/maliput/rndf/BUILD.bazel index d8ebb3b51e4b..031ae46b6bd1 100644 --- a/automotive/maliput/rndf/BUILD.bazel +++ b/automotive/maliput/rndf/BUILD.bazel @@ -1,19 +1,18 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "rndf", - srcs = [], - hdrs = [], deps = [ ":builder", ":lanes", diff --git a/automotive/maliput/rndf/test_utilities/BUILD.bazel b/automotive/maliput/rndf/test_utilities/BUILD.bazel index 22ed04f55106..18f0638a266f 100644 --- a/automotive/maliput/rndf/test_utilities/BUILD.bazel +++ b/automotive/maliput/rndf/test_utilities/BUILD.bazel @@ -1,12 +1,15 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -# This should encompass every cc_library in this package. -drake_cc_library( +drake_cc_package_library( name = "test_utilities", testonly = 1, deps = [ diff --git a/automotive/maliput/simplerulebook/BUILD.bazel b/automotive/maliput/simplerulebook/BUILD.bazel index b7094682ea01..a2a3585784c7 100644 --- a/automotive/maliput/simplerulebook/BUILD.bazel +++ b/automotive/maliput/simplerulebook/BUILD.bazel @@ -1,18 +1,17 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "simplerulebook", - srcs = [], - hdrs = [], deps = [ ":simple_rulebook", ":yaml_io", diff --git a/automotive/maliput/utility/BUILD.bazel b/automotive/maliput/utility/BUILD.bazel index 52e6d5aeaa7d..9120ea3bd3ff 100644 --- a/automotive/maliput/utility/BUILD.bazel +++ b/automotive/maliput/utility/BUILD.bazel @@ -1,10 +1,11 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", - "drake_cc_binary", + "drake_cc_package_library", ) load( "@drake//tools/skylark:drake_py.bzl", @@ -14,8 +15,15 @@ load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "utility", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "generate_obj.cc", "generate_urdf.cc", @@ -24,6 +32,7 @@ drake_cc_library( "generate_obj.h", "generate_urdf.h", ], + visibility = ["//visibility:private"], deps = [ "//automotive/maliput/api", "//math:geometric_transform", diff --git a/common/BUILD.bazel b/common/BUILD.bazel index ba391c592cf6..58826cca1cd2 100644 --- a/common/BUILD.bazel +++ b/common/BUILD.bazel @@ -1,10 +1,11 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load( "@drake//tools/skylark:drake_py.bzl", @@ -16,11 +17,8 @@ load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -# This should encompass every non-testonly cc_library in this package, -# except for items that should only ever be linked into main() programs. -drake_cc_library( +drake_cc_package_library( name = "common", - srcs = [], deps = [ ":autodiff", ":autodiffxd_make_coherent", @@ -52,9 +50,6 @@ drake_cc_library( ":type_safe_index", ":unused", ], - # Intentionally excluded items: - # - text_logging_gflags - # - text_logging_gflags_h ) # A library of things that EVERYONE should want and MUST EAT. @@ -379,6 +374,11 @@ drake_cc_library( "//tools/cc_toolchain:linux": ["-pthread"], "//conditions:default": [], }), + tags = [ + # Don't add this library into the ":common" package library. + # Only programs with a main() function should ever use this header. + "exclude_from_package", + ], deps = [ ":essential", "@gflags", @@ -391,6 +391,12 @@ drake_cc_library( drake_cc_library( name = "text_logging_gflags_h", hdrs = ["text_logging_gflags.h"], + tags = [ + # Don't add this library into the ":common" package library. + # This rule should only be used for install purposes. + "exclude_from_package", + ], + # This rule should only be used for install purposes. visibility = ["//tools/install/libdrake:__pkg__"], ) diff --git a/common/test_utilities/BUILD.bazel b/common/test_utilities/BUILD.bazel index 7b379ab016eb..8a5f2cf237d7 100644 --- a/common/test_utilities/BUILD.bazel +++ b/common/test_utilities/BUILD.bazel @@ -1,9 +1,10 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load( "@drake//tools/skylark:drake_py.bzl", @@ -19,11 +20,9 @@ exports_files([ # This should encompass every cc_library in this package, except for items that # should only ever be linked into main() programs. -drake_cc_library( +drake_cc_package_library( name = "test_utilities", testonly = 1, - srcs = [], - hdrs = [], deps = [ ":eigen_geometry_compare", ":eigen_matrix_compare", @@ -34,8 +33,6 @@ drake_cc_library( ":random_polynomial_matrix", ":symbolic_test_util", ], - # Intentionally excluded items: - # - drake_cc_googletest_main ) drake_cc_library( @@ -105,12 +102,15 @@ drake_cc_library( ], ) -# This is only intended to be used by the drake_cc_googletest() macro. -# Do not add it to ":test_utilities". drake_cc_library( name = "drake_cc_googletest_main", testonly = 1, srcs = ["drake_cc_googletest_main.cc"], + tags = [ + # This is only intended to be used by the drake_cc_googletest() macro. + # Don't add this library into the ":test_utilities" package library. + "exclude_from_package", + ], deps = [ "//common:text_logging_gflags", "@gtest//:without_main", diff --git a/examples/rod2d/BUILD.bazel b/examples/rod2d/BUILD.bazel index 43aff0ade06b..c49ac68be6b4 100644 --- a/examples/rod2d/BUILD.bazel +++ b/examples/rod2d/BUILD.bazel @@ -51,7 +51,6 @@ drake_cc_library( ":rod2d_state_vector", "//common:essential", "//multibody/constraint", - "//multibody/constraint:constraint_solver", "//solvers:mathematical_program", "//systems/framework:leaf_system", "//systems/rendering:pose_vector", @@ -66,7 +65,7 @@ drake_cc_googletest( ":rod2d", "//common:essential", "//common/test_utilities:eigen_matrix_compare", - "//multibody/constraint:constraint_solver", + "//multibody/constraint", "//systems/analysis", ], ) diff --git a/lcm/BUILD.bazel b/lcm/BUILD.bazel index efbe4a004c7e..0b0755b79fe7 100644 --- a/lcm/BUILD.bazel +++ b/lcm/BUILD.bazel @@ -131,4 +131,8 @@ drake_py_test( deps = ["@lcm//:lcm-python"], ) -add_lint_tests() +add_lint_tests( + # We need to more seriously refactor and deprecate some names here before + # we can start following package_library conventions. + enable_library_lint = False, +) diff --git a/multibody/benchmarks/acrobot/BUILD.bazel b/multibody/benchmarks/acrobot/BUILD.bazel index 54f836d3b737..efbdd2144aa0 100644 --- a/multibody/benchmarks/acrobot/BUILD.bazel +++ b/multibody/benchmarks/acrobot/BUILD.bazel @@ -40,4 +40,8 @@ drake_cc_library( ], ) -add_lint_tests() +add_lint_tests( + # We need to more seriously refactor and deprecate some names here before + # we can start following package_library conventions. + enable_library_lint = False, +) diff --git a/multibody/benchmarks/free_body/BUILD.bazel b/multibody/benchmarks/free_body/BUILD.bazel index b1b8f1a3bff0..2f3090c94859 100644 --- a/multibody/benchmarks/free_body/BUILD.bazel +++ b/multibody/benchmarks/free_body/BUILD.bazel @@ -1,6 +1,11 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) @@ -16,14 +21,22 @@ filegroup( ]), ) -drake_cc_library( +drake_cc_package_library( name = "free_body", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "free_body.cc", ], hdrs = [ "free_body.h", ], + visibility = ["//visibility:private"], deps = [ "//common", "//math:geometric_transform", diff --git a/multibody/collision/BUILD.bazel b/multibody/collision/BUILD.bazel index d5dad66cfb4b..dde89cc80819 100644 --- a/multibody/collision/BUILD.bazel +++ b/multibody/collision/BUILD.bazel @@ -1,10 +1,27 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) +drake_cc_package_library( + name = "collision", + deps = [ + ":bullet_collision", + ":collision_api", + ":drake_collision", + ":fcl_collision", + ":model", + ":unusable_collision", + ], +) + drake_cc_library( name = "model", srcs = [ @@ -80,11 +97,12 @@ drake_cc_library( # By default, supply a bullet-only model. As we gain more collision library # choices, we may want to be more subtle here. drake_cc_library( - name = "collision", + name = "drake_collision", srcs = [ "drake_collision.cc", ], defines = ["BULLET_COLLISION"], + visibility = ["//visibility:private"], deps = [ ":bullet_collision", ":collision_api", diff --git a/multibody/constraint/BUILD.bazel b/multibody/constraint/BUILD.bazel index 7c3c80e0dbb9..7d2df532a9dc 100644 --- a/multibody/constraint/BUILD.bazel +++ b/multibody/constraint/BUILD.bazel @@ -1,17 +1,26 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", - "drake_cc_binary", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "constraint", + deps = [ + ":constraint_problem_data", + ":constraint_solver", + ], +) + +drake_cc_library( + name = "constraint_problem_data", hdrs = [ "constraint_problem_data.h", ], @@ -27,7 +36,7 @@ drake_cc_library( "constraint_solver.h", ], deps = [ - ":constraint", + ":constraint_problem_data", "//solvers:mathematical_program", ], ) diff --git a/multibody/joints/BUILD.bazel b/multibody/joints/BUILD.bazel index ecf4c24c16cf..a8fd62f43dd6 100644 --- a/multibody/joints/BUILD.bazel +++ b/multibody/joints/BUILD.bazel @@ -1,12 +1,24 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "joints", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "drake_joint.cc", "fixed_joint.cc", @@ -31,6 +43,7 @@ drake_cc_library( "revolute_joint.h", "roll_pitch_yaw_floating_joint.h", ], + visibility = ["//visibility:private"], deps = [ "//common:essential", "//common:unused", diff --git a/multibody/multibody_tree/BUILD.bazel b/multibody/multibody_tree/BUILD.bazel index 29996298a421..42e94b083609 100755 --- a/multibody/multibody_tree/BUILD.bazel +++ b/multibody/multibody_tree/BUILD.bazel @@ -1,9 +1,10 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") @@ -11,6 +12,21 @@ package( default_visibility = ["//visibility:public"], ) +drake_cc_package_library( + name = "multibody_tree", + deps = [ + ":articulated_body_inertia", + ":multibody_tree_context", + ":multibody_tree_core", + ":multibody_tree_element", + ":multibody_tree_indexes", + ":multibody_tree_topology", + ":rotational_inertia", + ":spatial_inertia", + ":unit_inertia", + ], +) + drake_cc_library( name = "multibody_tree_indexes", srcs = [], @@ -69,7 +85,7 @@ drake_cc_library( ) drake_cc_library( - name = "multibody_tree", + name = "multibody_tree_core", srcs = [ "body.cc", "body_node_impl.cc", @@ -107,6 +123,9 @@ drake_cc_library( "space_xyz_mobilizer.h", "uniform_gravity_field_element.h", ], + # Hide this library outside of this package; users shoud depend on + # ":multibody_tree" broadly, not just ":multibody_tree_core". + visibility = ["//visibility:private"], deps = [ ":multibody_tree_context", ":multibody_tree_element", diff --git a/multibody/multibody_tree/multibody_plant/BUILD.bazel b/multibody/multibody_tree/multibody_plant/BUILD.bazel index 96ca9b53daac..4ccfd032793f 100644 --- a/multibody/multibody_tree/multibody_plant/BUILD.bazel +++ b/multibody/multibody_tree/multibody_plant/BUILD.bazel @@ -2,9 +2,10 @@ # This file contains rules for Bazel; see drake/doc/bazel.rst. load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") @@ -12,14 +13,22 @@ package( default_visibility = ["//visibility:public"], ) -drake_cc_library( +drake_cc_package_library( name = "multibody_plant", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "multibody_plant.cc", ], hdrs = [ "multibody_plant.h", ], + visibility = ["//visibility:private"], deps = [ ":coulomb_friction", "//common:default_scalars", diff --git a/multibody/parsers/BUILD.bazel b/multibody/parsers/BUILD.bazel index ed99db71cf12..823fc5736fb0 100644 --- a/multibody/parsers/BUILD.bazel +++ b/multibody/parsers/BUILD.bazel @@ -1,17 +1,25 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", - "drake_cc_binary", + "drake_cc_package_library", ) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "parsers", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "model_instance_id_table.cc", "package_map.cc", @@ -28,6 +36,7 @@ drake_cc_library( "urdf_parser.h", "xml_util.h", ], + visibility = ["//visibility:private"], deps = [ "//multibody:rigid_body_tree", "//multibody/rigid_body_plant:compliant_material", diff --git a/multibody/rigid_body_plant/BUILD.bazel b/multibody/rigid_body_plant/BUILD.bazel index dd2bc4960250..41dd3529d2b2 100644 --- a/multibody/rigid_body_plant/BUILD.bazel +++ b/multibody/rigid_body_plant/BUILD.bazel @@ -291,6 +291,9 @@ filegroup( ) add_lint_tests( + # We need to more seriously refactor and deprecate some names here before + # we can start following package_library conventions. + enable_library_lint = False, python_lint_extra_srcs = glob([ "visualization/*.py", ]), diff --git a/multibody/shapes/BUILD.bazel b/multibody/shapes/BUILD.bazel index bec4afc2291b..8974d0c54626 100644 --- a/multibody/shapes/BUILD.bazel +++ b/multibody/shapes/BUILD.bazel @@ -1,12 +1,24 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "shapes", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "element.cc", "geometry.cc", @@ -18,6 +30,7 @@ drake_cc_library( "geometry.h", "visual_element.h", ], + visibility = ["//visibility:private"], deps = [ "//common:essential", "//common:unused", diff --git a/systems/analysis/BUILD.bazel b/systems/analysis/BUILD.bazel index b8baca45ab2e..65d7c6786ac9 100644 --- a/systems/analysis/BUILD.bazel +++ b/systems/analysis/BUILD.bazel @@ -1,19 +1,27 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "analysis", - srcs = [], - hdrs = [], deps = [ + ":antiderivative_function", ":explicit_euler_integrator", ":implicit_euler_integrator", + ":initial_value_problem", + ":integrator_base", + ":lyapunov", ":runge_kutta2_integrator", ":runge_kutta3_integrator", + ":scalar_initial_value_problem", ":semi_explicit_euler_integrator", ":simulator", ], diff --git a/systems/analysis/test_utilities/BUILD.bazel b/systems/analysis/test_utilities/BUILD.bazel index 58a99af821e9..18cd1b7cbf3d 100644 --- a/systems/analysis/test_utilities/BUILD.bazel +++ b/systems/analysis/test_utilities/BUILD.bazel @@ -1,11 +1,16 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "test_utilities", testonly = 1, deps = [ diff --git a/systems/controllers/plan_eval/BUILD.bazel b/systems/controllers/plan_eval/BUILD.bazel index 93b94288e055..7a94a650bc06 100644 --- a/systems/controllers/plan_eval/BUILD.bazel +++ b/systems/controllers/plan_eval/BUILD.bazel @@ -23,7 +23,7 @@ drake_cc_library( "//manipulation/util:trajectory_utils", "//multibody:rigid_body_tree_alias_groups", "//systems/controllers:setpoint", - "//systems/controllers/qp_inverse_dynamics", + "//systems/controllers/qp_inverse_dynamics:control", "//systems/controllers/qp_inverse_dynamics:param_parser", "//systems/framework:value", ], diff --git a/systems/controllers/qp_inverse_dynamics/BUILD.bazel b/systems/controllers/qp_inverse_dynamics/BUILD.bazel index 871966043ac5..3ed6f5a5fe72 100644 --- a/systems/controllers/qp_inverse_dynamics/BUILD.bazel +++ b/systems/controllers/qp_inverse_dynamics/BUILD.bazel @@ -34,8 +34,8 @@ drake_cc_library( "param_parser.h", ], deps = [ + ":control", ":id_controller_config", - ":qp_inverse_dynamics", "//common/proto:protobuf", "//multibody:rigid_body_tree_alias_groups", ], @@ -55,7 +55,12 @@ drake_cc_library( ) drake_cc_library( - name = "qp_inverse_dynamics", + # We use the label name ":control" (even though the files are not named + # that way) because the name ":qp_inverse_dynamics" is reserved for the + # drake_cc_package_library that provides all of the code from this package + # in a single library. Since our package has more code than just this + # controller, we need to use a name that implies a smaller scope. + name = "control", srcs = [ "qp_inverse_dynamics.cc", "qp_inverse_dynamics_common.cc", @@ -78,9 +83,9 @@ drake_cc_library( srcs = ["qp_inverse_dynamics_system.cc"], hdrs = ["qp_inverse_dynamics_system.h"], deps = [ + ":control", "//lcmtypes:inverse_dynamics_debug_info", "//multibody:rigid_body_tree", - "//systems/controllers/qp_inverse_dynamics", "//systems/framework:leaf_system", ], ) @@ -90,7 +95,7 @@ drake_cc_library( srcs = ["lcm_utils.cc"], hdrs = ["lcm_utils.h"], deps = [ - ":qp_inverse_dynamics", + ":control", "//lcmtypes:body_acceleration", "//lcmtypes:constrained_values", "//lcmtypes:contact_information", @@ -108,8 +113,8 @@ drake_cc_library( srcs = ["qp_output_translator_system.cc"], hdrs = ["qp_output_translator_system.h"], deps = [ + ":control", "//multibody:rigid_body_tree", - "//systems/controllers/qp_inverse_dynamics", "//systems/framework:leaf_system", "@lcmtypes_bot2_core", ], @@ -168,8 +173,8 @@ drake_cc_googletest( ], tags = gurobi_test_tags(), deps = [ + ":control", ":param_parser", - ":qp_inverse_dynamics", "//common:find_resource", "//common/test_utilities:eigen_matrix_compare", "//multibody/parsers", @@ -225,13 +230,13 @@ drake_cc_googletest( ], tags = gurobi_test_tags(), deps = [ + ":control", + ":param_parser", "//common:find_resource", "//common/test_utilities:eigen_matrix_compare", "//examples/valkyrie:valkyrie_constants", "//multibody/parsers", "//systems/controllers:setpoint", - "//systems/controllers/qp_inverse_dynamics", - "//systems/controllers/qp_inverse_dynamics:param_parser", ], ) diff --git a/systems/framework/BUILD.bazel b/systems/framework/BUILD.bazel index 1c4c87eb9fab..cdac208ad5f8 100644 --- a/systems/framework/BUILD.bazel +++ b/systems/framework/BUILD.bazel @@ -1,17 +1,21 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -# This should encompass every non-testonly cc_library in this package. -drake_cc_library( +drake_cc_package_library( name = "framework", - srcs = [], deps = [ ":abstract_values", ":cache_and_dependency_tracker", + ":cache_entry", ":context", ":context_base", ":continuous_state", @@ -40,6 +44,7 @@ drake_cc_library( ":system_scalar_converter", ":system_symbolic_inspector", ":value", + ":value_checker", ":vector", ":vector_system", ], diff --git a/systems/framework/test_utilities/BUILD.bazel b/systems/framework/test_utilities/BUILD.bazel index 984cd33b8e1b..e2133e4aed0a 100644 --- a/systems/framework/test_utilities/BUILD.bazel +++ b/systems/framework/test_utilities/BUILD.bazel @@ -1,15 +1,18 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "test_utilities", testonly = 1, - srcs = [], - hdrs = [], deps = [ ":my_vector", ":pack_value", diff --git a/systems/lcm/BUILD.bazel b/systems/lcm/BUILD.bazel index b9ea781ae90b..fe7afdb31a62 100644 --- a/systems/lcm/BUILD.bazel +++ b/systems/lcm/BUILD.bazel @@ -171,4 +171,8 @@ drake_cc_googletest( ], ) -add_lint_tests() +add_lint_tests( + # We need to more seriously refactor and deprecate some names here before + # we can start following package_library conventions. + enable_library_lint = False, +) diff --git a/systems/plants/spring_mass_system/BUILD.bazel b/systems/plants/spring_mass_system/BUILD.bazel index ff6a8909d4e2..b752deec8c43 100644 --- a/systems/plants/spring_mass_system/BUILD.bazel +++ b/systems/plants/spring_mass_system/BUILD.bazel @@ -1,14 +1,27 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "spring_mass_system", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = ["spring_mass_system.cc"], hdrs = ["spring_mass_system.h"], + visibility = ["//visibility:private"], deps = [ "//systems/framework", ], diff --git a/systems/primitives/BUILD.bazel b/systems/primitives/BUILD.bazel index 34f84eb8a85b..2020c0cf0307 100644 --- a/systems/primitives/BUILD.bazel +++ b/systems/primitives/BUILD.bazel @@ -1,17 +1,21 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "primitives", - srcs = [], - hdrs = [], deps = [ ":adder", ":affine_system", + ":barycentric_system", ":constant_value_source", ":constant_vector_source", ":demultiplexer", @@ -28,8 +32,10 @@ drake_cc_library( ":saturation", ":signal_log", ":signal_logger", + ":sine", ":time_varying_data", ":trajectory_source", + ":wrap_to_system", ":zero_order_hold", ], ) diff --git a/systems/sensors/BUILD.bazel b/systems/sensors/BUILD.bazel index 0e4bab94de73..a6d821a4ac37 100644 --- a/systems/sensors/BUILD.bazel +++ b/systems/sensors/BUILD.bazel @@ -1,10 +1,11 @@ # -*- python -*- load( - "//tools:drake.bzl", + "@drake//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest", "drake_cc_library", + "drake_cc_package_library", ) load( "@drake//tools/vector_gen:vector_gen.bzl", @@ -14,18 +15,24 @@ load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "sensors", - srcs = [], - hdrs = [], deps = [ ":accelerometer", ":beam_model", + ":beam_model_params", ":camera_info", ":depth_sensor", + ":depth_sensor_to_lcm_point_cloud_message", + ":gyroscope", + ":image", + ":image_to_lcm_image_array_t", + ":optitrack_encoder", + ":optitrack_sender", ":rgbd_camera", ":rgbd_renderer", ":rotary_encoders", + ":vtk_util", ], ) diff --git a/tools/install/libdrake/build_components.bzl b/tools/install/libdrake/build_components.bzl index 2e8f300c2a27..038b8f9b6554 100644 --- a/tools/install/libdrake/build_components.bzl +++ b/tools/install/libdrake/build_components.bzl @@ -155,6 +155,7 @@ LIBDRAKE_COMPONENTS = [ "//multibody/collision:collision_api", "//multibody/collision:model", "//multibody/constraint:constraint", + "//multibody/constraint:constraint_problem_data", "//multibody/constraint:constraint_solver", "//multibody/joints:joints", "//multibody/multibody_tree/math:spatial_acceleration", @@ -253,10 +254,10 @@ LIBDRAKE_COMPONENTS = [ "//systems/analysis:simulator", "//systems/controllers/plan_eval:generic_plan", "//systems/controllers/plan_eval:plan_eval_base_system", + "//systems/controllers/qp_inverse_dynamics:control", "//systems/controllers/qp_inverse_dynamics:id_controller_config", "//systems/controllers/qp_inverse_dynamics:lcm_utils", "//systems/controllers/qp_inverse_dynamics:param_parser", - "//systems/controllers/qp_inverse_dynamics:qp_inverse_dynamics", "//systems/controllers/qp_inverse_dynamics:qp_inverse_dynamics_system", "//systems/controllers/qp_inverse_dynamics:qp_output_translator_system", "//systems/controllers/qp_inverse_dynamics:robot_kinematic_state", diff --git a/tools/lint/BUILD.bazel b/tools/lint/BUILD.bazel index 6ca0b43b6e83..5f32bfa40285 100644 --- a/tools/lint/BUILD.bazel +++ b/tools/lint/BUILD.bazel @@ -76,6 +76,12 @@ drake_py_binary( deps = [":formatter"], ) +drake_py_binary( + name = "library_lint_reporter", + testonly = 1, + srcs = ["library_lint_reporter.py"], +) + # === test === drake_py_unittest( diff --git a/tools/lint/library_lint.bzl b/tools/lint/library_lint.bzl new file mode 100644 index 000000000000..5b3d276ceca4 --- /dev/null +++ b/tools/lint/library_lint.bzl @@ -0,0 +1,129 @@ +# -*- python -*- + +load("//tools/skylark:drake_py.bzl", "py_test_isolated") + +# Keep this constant in sync with library_lint_reporter.py. +_TAG_EXCLUDE_FROM_PACKAGE = "exclude_from_package" + +def library_lint( + existing_rules = None): + """Within the current package, checks that drake_cc_package_library has been + used correctly, reports a lint (test) error if not. (To understand proper + use of drake_cc_package_library, consult its API documentation.) + + Note that //examples/... packages are excluded from some checks, because + they should generally not use drake_cc_package_library. + """ + if existing_rules == None: + existing_rules = native.existing_rules().values() + package_name = "//" + native.package_name() # e.g., "//systems/framework" + short_package_name = package_name.split("/")[-1] # e.g., "framework" + + # We use a python helper script to report lint errors. As we find possible + # lint problems, we will append arguments here that will be passed along to + # the helper. + library_lint_reporter_args = [ + "--package-name", package_name, + ] + library_lint_reporter_data = [] + + # Within the current package, find all cc_library rules, and the (at most) + # one package_library rule. + cc_library_rules = [] + package_library_rule = None + for one_rule in existing_rules: + # We only want cc_library. + if one_rule["kind"] != "cc_library": + continue + # Ignore magic private libraries. + if one_rule["name"].startswith("_"): + continue + # Found a cc_library. + cc_library_rules.append(one_rule) + # Is it a package_library? + if "drake_cc_package_library" in one_rule["tags"]: + not package_library_rule or fail("Two package libraries?") + package_library_rule = one_rule + elif one_rule["name"] == short_package_name: + # Failure to use drake_cc_package_library is a lint error, except + # in examples folders because their libraryes are never installed. + if not package_name.startswith("//examples"): + library_lint_reporter_args += ["--untagged-package-library"] + + # If there is no C++ code in this package, then we're done. + if not cc_library_rules: + return + + # Sanity check the package_library_rule name. + if package_library_rule and ( + package_library_rule["name"] != short_package_name): + fail("drake_cc_package_library should not allow wrong-names?!") + + # Unless the package_library rule exists and is testonly, then we should + # exclude testonly cc_library targets from the scope we're going to insist + # that it covers. + exclude_testonly = not (package_library_rule or {}).get("testonly", False) + + # We are going to run genquery over all of this package's cc_library rules. + scope = [ + package_name + ":" + one_rule["name"] + for one_rule in cc_library_rules + ] + all_libraries = " + ".join(scope) + + # This expression computes the exact result for what we want the deps of + # the drake_cc_package_library to be. + correct_deps_expression = " ".join([ + # Start with all this package's cc_library rules. + "({})".format(all_libraries), + # Remove items that have opted-out of the package_library. + "except attr(tags, '{}', {})".format( + _TAG_EXCLUDE_FROM_PACKAGE, all_libraries), + # Maybe remove libraries tagged testonly = 1. + "except attr(testonly, 1, {})".format( + all_libraries) if exclude_testonly else "", + ]) + + # Find libraries that are deps of the package_library but shouldn't be. + extra_deps_expression = "deps({}, 1) except ({})".format( + package_name, correct_deps_expression) + # Find libraries that should be deps of the package_library but aren't. + # Note that our library_lint_reporter.py tool filters out some false + # positives from this report. + missing_deps_expression = "({}) except deps({}, 1) ".format( + correct_deps_expression, package_name) + + # If there was a package_library rule, ensure its deps are comprehensive. + if package_library_rule: + native.genquery( + name = "library_lint_missing_deps", + expression = missing_deps_expression, + scope = scope, + testonly = 1, + visibility = ["//visibility:private"], + ) + native.genquery( + name = "library_lint_extra_deps", + expression = extra_deps_expression, + scope = scope, + testonly = 1, + visibility = ["//visibility:private"], + ) + library_lint_reporter_data += [ + ":library_lint_missing_deps", + ":library_lint_extra_deps", + ] + library_lint_reporter_args += [ + "--missing-deps", "$(location :library_lint_missing_deps)", + "--extra-deps", "$(location :library_lint_extra_deps)", + ] + + # Report all of the library_lint results. + py_test_isolated( + name = "library_lint", + srcs = ["@drake//tools/lint:library_lint_reporter.py"], + main = "@drake//tools/lint:library_lint_reporter.py", + args = library_lint_reporter_args, + data = library_lint_reporter_data, + tags = ["lint", "library_lint"], + ) diff --git a/tools/lint/library_lint_reporter.py b/tools/lint/library_lint_reporter.py new file mode 100644 index 000000000000..3702656185c6 --- /dev/null +++ b/tools/lint/library_lint_reporter.py @@ -0,0 +1,82 @@ +"""A helper program for library_lint.bzl that reports lint errors in a +human-readable way, with suggested fixes, etc. +""" + +import argparse +import sys + +# Keep this constant in sync with library_lint.bzl. +_TAG_EXCLUDE_FROM_PACKAGE = "exclude_from_package" + + +def main(): + parser = argparse.ArgumentParser( + prog='library_lint', + description=__doc__) + parser.add_argument( + '--package-name', metavar='LABEL', type=str, required=True, + help='Required name of package, e.g., //common/test_utilities') + parser.add_argument( + '--missing-deps', metavar='FILE', type=str, + help=('Filename containing list of labels that are missing. ' + 'It is a lint error when this file is non-empty.')) + parser.add_argument( + '--extra-deps', metavar='FILE', type=str, + help=('Filename containing list of labels that are extra. ' + 'It is a lint error when this file is non-empty.')) + parser.add_argument( + '--untagged-package-library', action='store_true', default=False, + help=('A cc_library exists with the short_package_name but it ' + 'does not use drake_cc_package_library')) + args = parser.parse_args() + build_file_name = args.package_name[2:] + "/BUILD.bazel" + short_package_name = args.package_name.split("/")[-1] + + return_code = 0 + + if args.untagged_package_library: + print(("error: The package {} has a cc_library(name = \":{}\") " + "declared without using drake_cc_package_library().").format( + args.package_name, + short_package_name)) + return_code = 1 + + with open(args.missing_deps or '/dev/null') as opened: + missing_deps = opened.readlines() + if missing_deps: + print(("error: Missing deps in {}'s drake_cc_package_library.").format( + args.package_name)) + print(("note: In the {} rule for drake_cc_package_library, " + "add the following lines to the deps:").format( + build_file_name)) + for dep in sorted(missing_deps): + print(" \":{}\",".format(dep.strip().split(":")[-1])) + print(("note: Alternatively, if some of these libraries should not be " + "added to the drake_cc_package_library, you may tag them with " + "\"{}\" in order to explicitly exclude them.").format( + _TAG_EXCLUDE_FROM_PACKAGE)) + return_code = 1 + + with open(args.extra_deps or '/dev/null') as opened: + extra_deps = opened.readlines() + extra_deps = [ + # Filter out false positives. All C++ code is OK to depend on these. + item for item in extra_deps + if not (item.startswith("//tools/cc_toolchain:") or + item.startswith("@bazel_tools//")) + ] + if extra_deps: + print(("error: Extra deps in {}'s drake_cc_package_library.").format( + args.package_name)) + print(("note: In the {} rule for drake_cc_package_library, " + "remove the following lines from the deps:").format( + build_file_name)) + for dep in sorted(extra_deps): + print(" \"{}\",".format(dep.strip())) + return_code = 1 + + return return_code + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/lint/lint.bzl b/tools/lint/lint.bzl index 0444e24c2ae2..82555c3cdb7f 100644 --- a/tools/lint/lint.bzl +++ b/tools/lint/lint.bzl @@ -2,6 +2,7 @@ load("//tools/lint:bazel_lint.bzl", "bazel_lint") load("//tools/lint:cpplint.bzl", "cpplint") +load("//tools/lint:library_lint.bzl", "library_lint") load("//tools/lint:python_lint.bzl", "python_lint") def add_lint_tests( @@ -11,7 +12,8 @@ def add_lint_tests( python_lint_exclude = None, python_lint_extra_srcs = None, bazel_lint_ignore = None, - bazel_lint_extra_srcs = None): + bazel_lint_extra_srcs = None, + enable_library_lint = True): """For every rule in the BUILD file so far, and for all Bazel files in this directory, adds test rules that run Drake's standard lint suite over the sources. Thus, BUILD file authors should call this function at the *end* @@ -36,3 +38,6 @@ def add_lint_tests( bazel_lint( ignore = bazel_lint_ignore, extra_srcs = bazel_lint_extra_srcs) + if enable_library_lint: + library_lint( + existing_rules = existing_rules) diff --git a/tools/skylark/drake_cc.bzl b/tools/skylark/drake_cc.bzl index 84e8864bb463..1afa3febe153 100644 --- a/tools/skylark/drake_cc.bzl +++ b/tools/skylark/drake_cc.bzl @@ -342,6 +342,42 @@ def drake_cc_library( install_hdrs_exclude = install_hdrs_exclude, **kwargs) +def _check_package_library_name(name): + # Assert that :name is the default library for native.package_name(). + expected_name = native.package_name().split("/")[-1] + if name != expected_name: + fail(("The drake_cc_package_library(name = \"{}\", ...) " + + "should be named \"{}\"").format(name, expected_name)) + +def drake_cc_package_library( + name, + deps = [], + testonly = 0, + visibility = ["//visibility:public"]): + """Creates a rule to declare a C++ "package" library -- a library whose + target name matches the current package and whose dependencies are + (usually) all of the other drake_cc_library targets in the current package. + In short, a library named //foo/bar (short for //foo/bar:bar) that + conveniently provides all of the C++ code from the //foo/bar package in one + place. + + Using this macro documents the intent that the library is a summation of + everything in the current package and enables Drake's linter rules to + confirm that all of the drake_cc_library targets have been listed as deps. + + The name must be the same as the final element of the current package. + This rule does not accept srcs, hdrs, etc. -- only deps. + The testonly argument has the same meaning as the native cc_library. + By default, this target has public visibility, but that may be overridden. + """ + _check_package_library_name(name) + drake_cc_library( + name = name, + testonly = testonly, + tags = ["drake_cc_package_library"], + visibility = visibility, + deps = deps) + def drake_cc_binary( name, srcs = [], diff --git a/util/BUILD.bazel b/util/BUILD.bazel index ec198da49c14..c5e5f246d028 100644 --- a/util/BUILD.bazel +++ b/util/BUILD.bazel @@ -1,20 +1,32 @@ # -*- python -*- -load("//tools:drake.bzl", "drake_cc_googletest", "drake_cc_library") +load( + "@drake//tools/skylark:drake_cc.bzl", + "drake_cc_googletest", + "drake_cc_library", + "drake_cc_package_library", +) load("//tools/lint:lint.bzl", "add_lint_tests") package(default_visibility = ["//visibility:public"]) -drake_cc_library( +drake_cc_package_library( name = "util", + deps = [ + ":everything", + ], +) + +drake_cc_library( + name = "everything", srcs = [ "drakeGeometryUtil.cpp", ], hdrs = [ "drakeGeometryUtil.h", "drakeUtil.h", - "lcmUtil.h", ], + visibility = ["//visibility:private"], deps = [ "//common:essential", "//math:geometric_transform", @@ -26,6 +38,11 @@ drake_cc_library( drake_cc_library( name = "lcm_util", hdrs = ["lcmUtil.h"], + tags = [ + # Don't add this library into the ":util" package library, since its a + # deprecated forwarding convenience and should not be used in batch. + "exclude_from_package", + ], deps = [ "//manipulation/util:bot_core_lcm_encode_decode", ], From 234db4f3775ca14e11757987590d011e0b130b48 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Tue, 24 Apr 2018 14:47:08 -0400 Subject: [PATCH 2/2] Re-apply "Only run library_lint tests when snopt is enabled" This reapplies ab9732b9af5f8dad4ee9c0be47d7f51b1a90e37d which was reverted by a57e772d8a4a7979354227fe8380335eb49fbb37. --- .../multibody_plant/BUILD.bazel | 5 +-- tools/lint/library_lint.bzl | 36 ++++++++++++++++--- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/multibody/multibody_tree/multibody_plant/BUILD.bazel b/multibody/multibody_tree/multibody_plant/BUILD.bazel index 4ccfd032793f..acd4ca711b0e 100644 --- a/multibody/multibody_tree/multibody_plant/BUILD.bazel +++ b/multibody/multibody_tree/multibody_plant/BUILD.bazel @@ -16,12 +16,13 @@ package( drake_cc_package_library( name = "multibody_plant", deps = [ - ":everything", + ":coulomb_friction", + ":multibody_plant_core", ], ) drake_cc_library( - name = "everything", + name = "multibody_plant_core", srcs = [ "multibody_plant.cc", ], diff --git a/tools/lint/library_lint.bzl b/tools/lint/library_lint.bzl index 5b3d276ceca4..7056c78613af 100644 --- a/tools/lint/library_lint.bzl +++ b/tools/lint/library_lint.bzl @@ -95,17 +95,45 @@ def library_lint( # If there was a package_library rule, ensure its deps are comprehensive. if package_library_rule: + # If snopt is disabled, we have to use a dummy scope and expression. + # + # Details: When snopt is enabled, we obtain the @snopt source code via + # git. When snopt is disabled, there is no guarantee that git is still + # going to work (e.g., the repository is authenticated). Normally this + # is not a problem. However, during a `bazel query` or `genquery()` + # computation, Bazel still builds a graph where the conditional + # dependency on snopt is reified, and so its repository_rule gets run + # (and fails). Thus, we shouldn't do any genquery() operations unless + # snopt is enabled. Instead, we'll scope the genquery to a dummy + # label, and nerf the expression to be definitionally empty. + # + # TODO(jwnimmer-tri) Figure out how make linting work even when snopt + # is disabled. + dummy = "@bazel_tools//tools/cpp:empty" + dummy_expression = "{} except {}".format(dummy, dummy) native.genquery( name = "library_lint_missing_deps", - expression = missing_deps_expression, - scope = scope, + expression = select({ + "//tools:with_snopt": missing_deps_expression, + "//conditions:default": dummy_expression, + }), + scope = select({ + "//tools:with_snopt": scope, + "//conditions:default": [dummy], + }), testonly = 1, visibility = ["//visibility:private"], ) native.genquery( name = "library_lint_extra_deps", - expression = extra_deps_expression, - scope = scope, + expression = select({ + "//tools:with_snopt": extra_deps_expression, + "//conditions:default": dummy_expression, + }), + scope = select({ + "//tools:with_snopt": scope, + "//conditions:default": [dummy], + }), testonly = 1, visibility = ["//visibility:private"], )