Evaluate PEP 508 environment markers for package dependencies (bazel-contrib#50)

nikhaldi · mattmoor · commit f2e01f91c365 · 2018-01-09T08:35:36.000-08:00
* Evaluate PEP 508 environment markers for package dependencies Previously any wheel dependencies that had an environment marker (such as 'python_version>3.3') were simply ignored, leading to missing packages in the Python environment constructed by bazel. Fixes bazel-contrib#49 * Regenerate the piptool.par Required after making changes to whl.py * Pin the version of setuptools in piptool & extract whltool Some common operators in version markers (e.g., <=) are only supported in setuptools>=17.1. Rather than risk failing because the environment has an old setuptools version it's better to include it. Pinning to an exact version (currently the latest) to make things as predictable as possible. In addition, whl.py used during workspace setup also now depends on setuptools. We package this in a separate whltool.par to make this predictable as well.
diff --git a/python/requirements.txt b/python/requirements.txt
@@ -1,2 +1,6 @@
 pip==9.0.1
+setuptools==38.2.4
 wheel==0.30.0a0
+
+# For tests
+mock==2.0.0
diff --git a/python/whl.bzl b/python/whl.bzl
@@ -44,7 +44,7 @@ whl_library = repository_rule(
         "extras": attr.string_list(),
         "_script": attr.label(
             executable = True,
-            default = Label("//rules_python:whl.py"),
+            default = Label("//tools:whltool.par"),
             cfg = "host",
         ),
     },
diff --git a/rules_python/BUILD b/rules_python/BUILD
@@ -16,10 +16,14 @@ package(default_visibility = ["//visibility:public"])
 licenses(["notice"])  # Apache 2.0
 
 load("//python:python.bzl", "py_binary", "py_library", "py_test")
+load("@piptool_deps//:requirements.bzl", "requirement")
 
 py_library(
     name = "whl",
     srcs = ["whl.py"],
+    deps = [
+        requirement("setuptools"),
+    ],
 )
 
 py_test(
@@ -32,18 +36,29 @@ py_test(
         "@grpc_whl//file",
         "@mock_whl//file",
     ],
-    deps = [":whl"],
+    deps = [
+        ":whl",
+        requirement("mock"),
+    ],
 )
 
 load("@subpar//:subpar.bzl", "par_binary")
-load("@piptool_deps//:requirements.bzl", "all_requirements")
 
-# TODO(mattmoor): Bundle this tool as a PAR without any
-# system-installed pre-requisites.  See TODOs in piptool.py.
 par_binary(
     name = "piptool",
     srcs = ["piptool.py"],
     deps = [
         ":whl",
-    ] + all_requirements,
+        requirement("pip"),
+        requirement("wheel"),
+    ],
+)
+
+par_binary(
+    name = "whltool",
+    srcs = ["whl.py"],
+    main = "whl.py",
+    deps = [
+        ":whl",
+    ],
 )
diff --git a/rules_python/whl.py b/rules_python/whl.py
@@ -16,6 +16,7 @@
 import argparse
 import json
 import os
+import pkg_resources
 import re
 import zipfile
 
@@ -86,10 +87,10 @@ def dependencies(self, extra=None):
       if requirement.get('extra') != extra:
         # Match the requirements for the extra we're looking for.
         continue
-      if 'environment' in requirement:
-        # TODO(mattmoor): What's the best way to support "environment"?
-        # This typically communicates things like python version (look at
-        # "wheel" for a good example)
+      marker = requirement.get('environment')
+      if marker and not pkg_resources.evaluate_marker(marker):
+        # The current environment does not match the provided PEP 508 marker,
+        # so ignore this requirement.
         continue
       requires = requirement.get('requires', [])
       for entry in requires:
diff --git a/rules_python/whl_test.py b/rules_python/whl_test.py
@@ -15,6 +15,8 @@
 import os
 import unittest
 
+from mock import patch
+
 from rules_python import whl
 
 
@@ -54,32 +56,65 @@ def test_whl_with_METADATA_file(self):
     self.assertEqual(set(wheel.dependencies()), set())
     self.assertEqual('pypi__futures_2_2_0', wheel.repository_name())
 
-  def test_mock_whl(self):
+  @patch('platform.python_version', return_value='2.7.13')
+  def test_mock_whl(self, *args):
     td = TestData('mock_whl/file/mock-2.0.0-py2.py3-none-any.whl')
     wheel = whl.Wheel(td)
     self.assertEqual(wheel.name(), 'mock')
     self.assertEqual(wheel.distribution(), 'mock')
     self.assertEqual(wheel.version(), '2.0.0')
     self.assertEqual(set(wheel.dependencies()),
-                     set(['pbr', 'six']))
+                     set(['funcsigs', 'pbr', 'six']))
     self.assertEqual('pypi__mock_2_0_0', wheel.repository_name())
+
+  @patch('platform.python_version', return_value='3.3.0')
+  def test_mock_whl_3_3(self, *args):
+    td = TestData('mock_whl/file/mock-2.0.0-py2.py3-none-any.whl')
+    wheel = whl.Wheel(td)
+    self.assertEqual(set(wheel.dependencies()),
+                     set(['pbr', 'six']))
+
+  @patch('platform.python_version', return_value='2.7.13')
+  def test_mock_whl_extras(self, *args):
+    td = TestData('mock_whl/file/mock-2.0.0-py2.py3-none-any.whl')
+    wheel = whl.Wheel(td)
     self.assertEqual(['docs', 'test'], wheel.extras())
-    self.assertEqual(set(wheel.dependencies(extra='docs')), set())
+    self.assertEqual(set(wheel.dependencies(extra='docs')), set(['sphinx']))
     self.assertEqual(set(wheel.dependencies(extra='test')), set(['unittest2']))
 
-  def test_google_cloud_language_whl(self):
+  @patch('platform.python_version', return_value='3.0.0')
+  def test_mock_whl_extras_3_0(self, *args):
+    td = TestData('mock_whl/file/mock-2.0.0-py2.py3-none-any.whl')
+    wheel = whl.Wheel(td)
+    self.assertEqual(['docs', 'test'], wheel.extras())
+    self.assertEqual(set(wheel.dependencies(extra='docs')), set(['sphinx', 'Pygments', 'jinja2']))
+    self.assertEqual(set(wheel.dependencies(extra='test')), set(['unittest2']))
+
+  @patch('platform.python_version', return_value='2.7.13')
+  def test_google_cloud_language_whl(self, *args):
     td = TestData('google_cloud_language_whl/file/' +
                   'google_cloud_language-0.29.0-py2.py3-none-any.whl')
     wheel = whl.Wheel(td)
     self.assertEqual(wheel.name(), 'google-cloud-language')
     self.assertEqual(wheel.distribution(), 'google_cloud_language')
     self.assertEqual(wheel.version(), '0.29.0')
+    expected_deps = ['google-gax', 'google-cloud-core',
+                     'googleapis-common-protos[grpc]', 'enum34']
     self.assertEqual(set(wheel.dependencies()),
-                     set(['google-gax', 'google-cloud-core',
-                          'googleapis-common-protos[grpc]']))
+                     set(expected_deps))
     self.assertEqual('pypi__google_cloud_language_0_29_0',
                      wheel.repository_name())
     self.assertEqual([], wheel.extras())
 
+  @patch('platform.python_version', return_value='3.4.0')
+  def test_google_cloud_language_whl_3_4(self, *args):
+    td = TestData('google_cloud_language_whl/file/' +
+                  'google_cloud_language-0.29.0-py2.py3-none-any.whl')
+    wheel = whl.Wheel(td)
+    expected_deps = ['google-gax', 'google-cloud-core',
+                     'googleapis-common-protos[grpc]']
+    self.assertEqual(set(wheel.dependencies()),
+                     set(expected_deps))
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/tools/BUILD b/tools/BUILD
@@ -16,4 +16,4 @@ package(default_visibility = ["//visibility:public"])
 licenses(["notice"])  # Apache 2.0
 
 # This is generated and updated by ./update_tools.sh
-exports_files(["piptool.par"])
+exports_files(["piptool.par", "whltool.par"])
diff --git a/tools/piptool.par b/tools/piptool.par
diff --git a/tools/whltool.par b/tools/whltool.par
diff --git a/update_tools.sh b/update_tools.sh
@@ -16,5 +16,6 @@
 
 set -euo pipefail
 
-bazel build //rules_python:piptool.par
+bazel build //rules_python:piptool.par //rules_python:whltool.par
 cp bazel-bin/rules_python/piptool.par tools/piptool.par
+cp bazel-bin/rules_python/whltool.par tools/whltool.par
