From c43d7660eca6f187a114c679a6546bd8f686bf9e Mon Sep 17 00:00:00 2001
From: Misha Seltzer <misha@armis.com>
Date: Mon, 29 Jun 2020 17:47:17 -0400
Subject: [PATCH] Only set public visibility to direct dependencies

This resolves issue #38.
---
 src/bin/generate_pip_repositories.py  | 20 +++++++++++++-------
 test/BUILD                            | 26 ++++++++++++++++++++++++++
 test/visibility_test.py               |  6 ++++++
 thirdparty/pip/requirements-lock.json |  8 ++++----
 thirdparty/pip/requirements.in        |  1 +
 5 files changed, 50 insertions(+), 11 deletions(-)
 create mode 100644 test/visibility_test.py

diff --git a/src/bin/generate_pip_repositories.py b/src/bin/generate_pip_repositories.py
index 084b63f..dd38710 100755
--- a/src/bin/generate_pip_repositories.py
+++ b/src/bin/generate_pip_repositories.py
@@ -179,17 +179,17 @@ def _generate_package_for_requirement(
         write_file(repos_file_path, repos_file_content)
 
     def _generate_build_file_content(self, build_rules):
-        return textwrap.dedent("""
-            package(default_visibility = ["//visibility:public"])
-
-            {rules}
-        """).strip().format(
-            rules="\n".join(build_rules),
-        )
+        return textwrap.dedent("\n".join(build_rules))
 
     def _generate_rules_for_requirement(self, requirement_name, python_subtree):
         top_alias = SelectAlias(requirement_name)
 
+        # We want to make the top alias "visible" only if the package is specified
+        # in the requirements.txt file (i.e. has `is_direct=True` field in the lock
+        # file). Since there's an `is_direct` key for each python_version/platform,
+        # we're going to set the visibility  to public if any of those is True.
+        visibility = "//:__subpackages__"
+
         for python_version, platform_subtree in python_subtree.items():
             for platform, requirement_details in platform_subtree.items():
                 yield self._generate_py_library(
@@ -197,6 +197,8 @@ def _generate_rules_for_requirement(self, requirement_name, python_subtree):
                     python_version,
                     platform,
                 )
+                if requirement_details["is_direct"]:
+                    visibility = "//visibility:public"
 
             version_alias = self._generate_python_version_alias(
                 python_version,
@@ -208,6 +210,7 @@ def _generate_rules_for_requirement(self, requirement_name, python_subtree):
             python_version_label = _make_python_version_label(python_version)
             top_alias.actual[python_version_label] = version_alias.name
 
+        top_alias.visibility = visibility
         yield str(top_alias)
 
     def _generate_py_library(self, requirement_details, python_version, platform):
@@ -331,16 +334,19 @@ class SelectAlias(object):
     def __init__(self, name):
         self.name = name
         self.actual = {}
+        self.visibility = "//visibility:private"
 
     def __str__(self):
         return textwrap.dedent("""
             alias(
                 name = "{name}",
                 actual = select({actual}),
+                visibility = ["{visibility}"],
             )
         """).strip().format(
             name=self.name,
             actual=self.actual,
+            visibility=self.visibility,
         )
 
 
diff --git a/test/BUILD b/test/BUILD
index 63f91a7..4deb274 100644
--- a/test/BUILD
+++ b/test/BUILD
@@ -59,6 +59,32 @@ py_test(
     tags = ["new"],
 )
 
+py_test(
+    name = "visibility_test",
+    srcs = ["visibility_test.py"],
+    data = [
+        ":visibility_test_pytest_six",
+        ":visibility_test_lib_six",
+    ],
+)
+
+genquery(
+    name = "visibility_test_pytest_six",
+    expression = "visible('@pip//pytest', '@pip//six')",
+    scope = ["@pip//pytest"],
+)
+
+py_library(
+    name = "visibility_test_library",
+    deps = ["@pip//pytest"],
+)
+
+genquery(
+    name = "visibility_test_lib_six",
+    expression = "visible('//test:visibility_test_library', '@pip//six')",
+    scope = [":visibility_test_library"],
+)
+
 genrule(
     name = "pyang-main",
     srcs = [pyang_repo + "//scripts:pyang"],
diff --git a/test/visibility_test.py b/test/visibility_test.py
new file mode 100644
index 0000000..daf455d
--- /dev/null
+++ b/test/visibility_test.py
@@ -0,0 +1,6 @@
+with open("./test/visibility_test_pytest_six") as f:
+    assert f.read().strip() == "@pip//six:six"
+
+with open("./test/visibility_test_lib_six") as f:
+    assert not f.read().strip()
+
diff --git a/thirdparty/pip/requirements-lock.json b/thirdparty/pip/requirements-lock.json
index a3bd416..9b1f5dc 100644
--- a/thirdparty/pip/requirements-lock.json
+++ b/thirdparty/pip/requirements-lock.json
@@ -44,7 +44,7 @@
         },
         "lxml": {
           "dependencies": [],
-          "is_direct": false,
+          "is_direct": true,
           "source": "lxml_4_3_3_cp27_cp27m_macosx_10_6_intel_macosx_10_9_intel_macosx_10_9_x86_64_macosx_10_10_intel_macosx_10_10_x86_64",
           "version": "4.3.3"
         },
@@ -176,7 +176,7 @@
         },
         "lxml": {
           "dependencies": [],
-          "is_direct": false,
+          "is_direct": true,
           "source": "lxml_4_3_3_cp36_cp36m_macosx_10_6_intel_macosx_10_9_intel_macosx_10_9_x86_64_macosx_10_10_intel_macosx_10_10_x86_64",
           "version": "4.3.3"
         },
@@ -300,7 +300,7 @@
         },
         "lxml": {
           "dependencies": [],
-          "is_direct": false,
+          "is_direct": true,
           "source": "lxml_4_3_3_cp27_cp27mu_manylinux1_x86_64",
           "version": "4.3.3"
         },
@@ -432,7 +432,7 @@
         },
         "lxml": {
           "dependencies": [],
-          "is_direct": false,
+          "is_direct": true,
           "source": "lxml_4_3_3_cp36_cp36m_manylinux1_x86_64",
           "version": "4.3.3"
         },
diff --git a/thirdparty/pip/requirements.in b/thirdparty/pip/requirements.in
index 8ba0c5d..523e026 100644
--- a/thirdparty/pip/requirements.in
+++ b/thirdparty/pip/requirements.in
@@ -1,4 +1,5 @@
 isort
+lxml
 pathlib2 ; python_version < "3.0"
 pyang
 pytest