Escape characters in the wheel filename. (#518)

Fixes #517.

Author: pstradomski

examples/wheel/BUILD

@@ -189,6 +189,17 @@ py_wheel(
     version = "0.0.1",
 )
 
+py_wheel(
+    name = "filename_escaping",
+    # Per https://www.python.org/dev/peps/pep-0427/#escaping-and-unicode
+    # runs of non-alphanumeric, non-digit symbols should be replaced with a single underscore.
+    # Unicode non-ascii letters should *not* be replaced with underscore.
+    distribution = "file~~name-escaping",
+    python_tag = "py3",
+    version = "0.0.1-r7",
+    deps = [":example_pkg"],
+)
+
 py_test(
     name = "wheel_test",
     srcs = ["wheel_test.py"],
@@ -197,6 +208,7 @@ py_test(
         ":custom_package_root_multi_prefix",
         ":custom_package_root_multi_prefix_reverse_order",
         ":customized",
+        ":filename_escaping",
         ":minimal_with_py_library",
         ":minimal_with_py_package",
         ":python_abi3_binary_wheel",

examples/wheel/wheel_test.py

@@ -112,6 +112,34 @@ def test_customized_wheel(self):
 first = first.main:f
 second = second.main:s""")
 
+    def test_filename_escaping(self):
+        filename = os.path.join(os.environ['TEST_SRCDIR'],
+                                'rules_python',
+                                'examples', 'wheel',
+                                'file_name_escaping-0.0.1_r7-py3-none-any.whl')
+        with zipfile.ZipFile(filename) as zf:
+            self.assertEquals(
+                zf.namelist(),
+                ['examples/wheel/lib/data.txt',
+                 'examples/wheel/lib/module_with_data.py',
+                 'examples/wheel/lib/simple_module.py',
+                 'examples/wheel/main.py',
+                 # PEP calls for replacing only in the archive filename.
+                 # Alas setuptools also escapes in the dist-info directory
+                 # name, so let's be compatible.
+                 'file_name_escaping-0.0.1_r7.dist-info/WHEEL',
+                 'file_name_escaping-0.0.1_r7.dist-info/METADATA',
+                 'file_name_escaping-0.0.1_r7.dist-info/RECORD'])
+            metadata_contents = zf.read(
+                'file_name_escaping-0.0.1_r7.dist-info/METADATA')
+            self.assertEquals(metadata_contents, b"""\
+Metadata-Version: 2.1
+Name: file~~name-escaping
+Version: 0.0.1-r7
+
+UNKNOWN
+""")
+
     def test_custom_package_root_wheel(self):
         filename = os.path.join(os.environ['TEST_SRCDIR'],
                                 'rules_python',

python/packaging.bzl

@@ -83,13 +83,31 @@ Sub-packages are automatically included.
     },
 )
 
+def _escape_filename_segment(segment):
+    """Escape a segment of the wheel filename.
+
+    See https://www.python.org/dev/peps/pep-0427/#escaping-and-unicode
+    """
+
+    # TODO: this is wrong, isalnum replaces non-ascii letters, while we should
+    # not replace them.
+    # TODO: replace this with a regexp once starlark supports them.
+    escaped = ""
+    for character in segment.elems():
+        # isalnum doesn't handle unicode characters properly.
+        if character.isalnum() or character == ".":
+            escaped += character
+        elif not escaped.endswith("_"):
+            escaped += "_"
+    return escaped
+
 def _py_wheel_impl(ctx):
     outfile = ctx.actions.declare_file("-".join([
-        ctx.attr.distribution,
-        ctx.attr.version,
-        ctx.attr.python_tag,
-        ctx.attr.abi,
-        ctx.attr.platform,
+        _escape_filename_segment(ctx.attr.distribution),
+        _escape_filename_segment(ctx.attr.version),
+        _escape_filename_segment(ctx.attr.python_tag),
+        _escape_filename_segment(ctx.attr.abi),
+        _escape_filename_segment(ctx.attr.platform),
     ]) + ".whl")
 
     inputs_to_package = depset(

tools/wheelmaker.py

@@ -18,6 +18,7 @@
 import hashlib
 import os
 import os.path
+import re
 import sys
 import zipfile
 
@@ -31,6 +32,11 @@ def commonpath(path1, path2):
     return os.path.sep.join(ret)
 
 
+def escape_filename_segment(segment):
+    """Escapes a filename segment per https://www.python.org/dev/peps/pep-0427/#escaping-and-unicode"""
+    return re.sub(r"[^\w\d.]+", "_", segment, re.UNICODE)
+
+
 class WheelMaker(object):
     def __init__(self, name, version, build_tag, python_tag, abi, platform,
                  outfile=None, strip_path_prefixes=None):
@@ -43,6 +49,9 @@ def __init__(self, name, version, build_tag, python_tag, abi, platform,
         self._outfile = outfile
         self._strip_path_prefixes = strip_path_prefixes if strip_path_prefixes is not None else []
 
+        self._distinfo_dir = (escape_filename_segment(self._name) + '-' +
+                              escape_filename_segment(self._version) +
+                              '.dist-info/')
         self._zipfile = None
         self._record = []
 
@@ -64,14 +73,11 @@ def filename(self):
         components += [self._python_tag, self._abi, self._platform]
         return '-'.join(components) + '.whl'
 
-    def distname(self):
-        return self._name + '-' + self._version
-
     def disttags(self):
         return ['-'.join([self._python_tag, self._abi, self._platform])]
 
     def distinfo_path(self, basename):
-        return self.distname() + '.dist-info/' + basename
+        return self._distinfo_dir + basename
 
     def _serialize_digest(self, hash):
         # https://www.python.org/dev/peps/pep-0376/#record