Merge pull request RobotLocomotion#8724 from EricCousineau-TRI/issue/…

…8557

drake_py: Remove alias from isolated targets; rename prefix from `_isolated/` to `py/`

tools/skylark/drake_py.bzl

@@ -12,6 +12,34 @@ def drake_py_library(
         deps = deps,
         **kwargs)
 
+def _disable_test_impl(ctx):
+    info = dict(
+        bad_target = ctx.attr.bad_target,
+        good_target = ctx.attr.good_target,
+    )
+    content = """#!/bin/bash
+echo "ERROR: Please use '{good_target}'; the label '{bad_target}'" \
+     "has been removed." >&2
+exit 1
+""".format(**info)
+    ctx.actions.write(
+        output = ctx.outputs.executable,
+        content = content,
+    )
+    return [DefaultInfo()]
+
+# Defines a test which will fail when run via `bazel run` or `bazel test`,
+# pointing the user to the correct binary to use. This should typically have
+# a "manual" tag.
+_disable_test = rule(
+    attrs = {
+        "bad_target": attr.string(mandatory = True),
+        "good_target": attr.string(mandatory = True),
+    },
+    test = True,
+    implementation = _disable_test_impl,
+)
+
 def _py_target_isolated(
         name,
         py_target = None,
@@ -26,8 +54,8 @@ def _py_target_isolated(
     # Do not isolate targets that are already isolated. This generally happens
     # when linting tests (which are isolated) are invoked for isolated Python
     # targets. Without this check, the actual test turns into
-    # `_isolated/_isolated/{name}`.
-    prefix = "_isolated/"
+    # `py/py/{name}`.
+    prefix = "py/"
     if isolate and not name.startswith(prefix):
         actual = prefix + name
         # Preserve original functionality.
@@ -39,11 +67,18 @@ def _py_target_isolated(
             name = actual,
             srcs = srcs,
             main = main,
-            visibility = ["//visibility:private"],
+            visibility = visibility,
             **kwargs)
-        native.alias(
+        # Disable and redirect original name.
+        package_prefix = "//" + native.package_name() + ":"
+        # N.B. We make the disabled rule a test, even if the original was not.
+        # This ensures that developers will see the redirect using both
+        # `bazel run` or `bazel test`.
+        _disable_test(
             name = name,
-            actual = actual,
+            good_target = package_prefix + actual,
+            bad_target = package_prefix + name,
+            tags = ["manual"],
             visibility = visibility,
         )
     else: