From ab821213a2f004ef31c1abde585e33528c2a3417 Mon Sep 17 00:00:00 2001 From: AutomatedTester Date: Thu, 9 Jul 2020 22:12:37 +0100 Subject: [PATCH 1/5] Check if input_file is directory and ignore If in a dependency tree a directory is used as out this is passed as input_file to wheelmaker which then generates an unhandled error as it tries to write the directory as a file. --- experimental/examples/wheel/BUILD | 27 +++++++++++++++++++++++++++ experimental/tools/wheelmaker.py | 17 +++++++++-------- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/experimental/examples/wheel/BUILD b/experimental/examples/wheel/BUILD index 0613e6a74c..e57adcc3a8 100644 --- a/experimental/examples/wheel/BUILD +++ b/experimental/examples/wheel/BUILD @@ -31,6 +31,14 @@ py_library( ], ) +py_library( + name = "main_with_gen_data", + srcs = ["main.py"], + data = [ + ":gen_dir", + ], +) + # Package just a specific py_libraries, without their dependencies py_wheel( name = "minimal_with_py_library", @@ -53,6 +61,12 @@ py_package( deps = [":main"], ) +py_package( + name = "example_pkg_with_data", + packages = ["experimental.examples.wheel"], + deps = [":main_with_gen_data"] +) + py_wheel( name = "minimal_with_py_package", # Package data. We're building "example_minimal_package-0.0.1-py3-none-any.whl" @@ -144,6 +158,19 @@ py_wheel( deps = [ ":example_pkg", ], + name = "use_genrule_with_dir_in_outs", + distribution = "use_genrule_with_dir_in_outs", + python_tag = "py3", + version = "0.0.1", + deps = [ + ":example_pkg_with_data" + ] +) + +genrule( + name = "gen_dir", + outs = ["someDir"], + cmd = "mkdir -p $@", ) py_wheel( diff --git a/experimental/tools/wheelmaker.py b/experimental/tools/wheelmaker.py index 18e63573e9..0ee0d39b39 100644 --- a/experimental/tools/wheelmaker.py +++ b/experimental/tools/wheelmaker.py @@ -108,14 +108,15 @@ def arcname_from(name): # Find the hash and length hash = hashlib.sha256() size = 0 - with open(real_filename, 'rb') as f: - while True: - block = f.read(2 ** 20) - if not block: - break - hash.update(block) - size += len(block) - self._add_to_record(arcname, self._serialize_digest(hash), size) + if not os.path.isdir(real_filename): + with open(real_filename, 'rb') as f: + while True: + block = f.read(2 ** 20) + if not block: + break + hash.update(block) + size += len(block) + self._add_to_record(arcname, self._serialize_digest(hash), size) def add_wheelfile(self): """Write WHEEL file to the distribution""" From 25a565c76bc7d67a17746e184f98a534829d0458 Mon Sep 17 00:00:00 2001 From: AutomatedTester Date: Fri, 10 Jul 2020 09:03:03 +0100 Subject: [PATCH 2/5] Updating after comments. --- experimental/examples/wheel/BUILD | 3 +++ experimental/tools/wheelmaker.py | 22 +++++++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/experimental/examples/wheel/BUILD b/experimental/examples/wheel/BUILD index e57adcc3a8..60d7b73acc 100644 --- a/experimental/examples/wheel/BUILD +++ b/experimental/examples/wheel/BUILD @@ -158,6 +158,9 @@ py_wheel( deps = [ ":example_pkg", ], +) + +py_wheel( name = "use_genrule_with_dir_in_outs", distribution = "use_genrule_with_dir_in_outs", python_tag = "py3", diff --git a/experimental/tools/wheelmaker.py b/experimental/tools/wheelmaker.py index 0ee0d39b39..6eed1a468e 100644 --- a/experimental/tools/wheelmaker.py +++ b/experimental/tools/wheelmaker.py @@ -102,21 +102,25 @@ def arcname_from(name): return normalized_arcname + # If anywhere in the dependency tree a directory is set + # it will be passed in as an input file and we need to + # ignore. + if os.path.isdir(real_filename): + return arcname = arcname_from(package_filename) self._zipfile.write(real_filename, arcname=arcname) # Find the hash and length hash = hashlib.sha256() size = 0 - if not os.path.isdir(real_filename): - with open(real_filename, 'rb') as f: - while True: - block = f.read(2 ** 20) - if not block: - break - hash.update(block) - size += len(block) - self._add_to_record(arcname, self._serialize_digest(hash), size) + with open(real_filename, 'rb') as f: + while True: + block = f.read(2 ** 20) + if not block: + break + hash.update(block) + size += len(block) + self._add_to_record(arcname, self._serialize_digest(hash), size) def add_wheelfile(self): """Write WHEEL file to the distribution""" From 196b1187353fea5ee76dbb628259f8d5ad5730b3 Mon Sep 17 00:00:00 2001 From: AutomatedTester Date: Wed, 16 Dec 2020 11:49:04 +0000 Subject: [PATCH 3/5] Updating after comments. --- experimental/examples/wheel/BUILD | 13 +++++++------ experimental/examples/wheel/wheel_test.py | 15 +++++++++++++++ experimental/tools/wheelmaker.py | 8 +++++--- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/experimental/examples/wheel/BUILD b/experimental/examples/wheel/BUILD index 60d7b73acc..052c74c9bd 100644 --- a/experimental/examples/wheel/BUILD +++ b/experimental/examples/wheel/BUILD @@ -39,6 +39,12 @@ py_library( ], ) +genrule( + name="gen_dir", + outs=["someDir"], + cmd="mkdir -p $@ && touch $@/foo.py", +) + # Package just a specific py_libraries, without their dependencies py_wheel( name = "minimal_with_py_library", @@ -170,12 +176,6 @@ py_wheel( ] ) -genrule( - name = "gen_dir", - outs = ["someDir"], - cmd = "mkdir -p $@", -) - py_wheel( name = "python_abi3_binary_wheel", abi = "abi3", @@ -198,5 +198,6 @@ py_test( ":minimal_with_py_package", ":python_abi3_binary_wheel", ":python_requires_in_a_package", + ":use_genrule_with_dir_in_outs", ], ) diff --git a/experimental/examples/wheel/wheel_test.py b/experimental/examples/wheel/wheel_test.py index e461fa9dde..aa33d53e8d 100644 --- a/experimental/examples/wheel/wheel_test.py +++ b/experimental/examples/wheel/wheel_test.py @@ -219,6 +219,21 @@ def test_python_abi3_binary_wheel(self): """, ) + def test_genrule_creates_directory_and_is_included_in_wheel(self): + filename = os.path.join(os.environ['TEST_SRCDIR'], + 'rules_python', 'experimental', + 'examples', 'wheel', + 'use_genrule_with_dir_in_outs-0.0.1-py3-none-any.whl') + + with zipfile.ZipFile(filename) as zf: + self.assertEquals( + zf.namelist(), + ['experimental/examples/wheel/main.py', + 'experimental/examples/wheel/someDir/foo.py', + 'use_genrule_with_dir_in_outs-0.0.1.dist-info/WHEEL', + 'use_genrule_with_dir_in_outs-0.0.1.dist-info/METADATA', + 'use_genrule_with_dir_in_outs-0.0.1.dist-info/RECORD']) + if __name__ == '__main__': unittest.main() diff --git a/experimental/tools/wheelmaker.py b/experimental/tools/wheelmaker.py index 6eed1a468e..a6d683012a 100644 --- a/experimental/tools/wheelmaker.py +++ b/experimental/tools/wheelmaker.py @@ -102,11 +102,13 @@ def arcname_from(name): return normalized_arcname - # If anywhere in the dependency tree a directory is set - # it will be passed in as an input file and we need to - # ignore. if os.path.isdir(real_filename): + directory_contents = os.listdir(real_filename) + for file_ in directory_contents: + self.add_file(f"{package_filename}/{file_}", + f"{real_filename}/{file_}") return + arcname = arcname_from(package_filename) self._zipfile.write(real_filename, arcname=arcname) From aba500220280e4816e3e43f06bb011fa94dace8f Mon Sep 17 00:00:00 2001 From: AutomatedTester Date: Wed, 16 Dec 2020 14:05:00 +0000 Subject: [PATCH 4/5] fix error created from merge --- experimental/examples/wheel/BUILD | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/experimental/examples/wheel/BUILD b/experimental/examples/wheel/BUILD index 052c74c9bd..64f1d66047 100644 --- a/experimental/examples/wheel/BUILD +++ b/experimental/examples/wheel/BUILD @@ -40,9 +40,9 @@ py_library( ) genrule( - name="gen_dir", - outs=["someDir"], - cmd="mkdir -p $@ && touch $@/foo.py", + name = "gen_dir", + outs = ["someDir"], + cmd = "mkdir -p $@ && touch $@/foo.py", ) # Package just a specific py_libraries, without their dependencies From e7b32cf4f16ea456aad14abf15a08d200cc0d100 Mon Sep 17 00:00:00 2001 From: AutomatedTester Date: Wed, 16 Dec 2020 14:10:40 +0000 Subject: [PATCH 5/5] make formatting py2 compatible --- experimental/tools/wheelmaker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/tools/wheelmaker.py b/experimental/tools/wheelmaker.py index a6d683012a..418dfdbb1a 100644 --- a/experimental/tools/wheelmaker.py +++ b/experimental/tools/wheelmaker.py @@ -105,8 +105,8 @@ def arcname_from(name): if os.path.isdir(real_filename): directory_contents = os.listdir(real_filename) for file_ in directory_contents: - self.add_file(f"{package_filename}/{file_}", - f"{real_filename}/{file_}") + self.add_file("{}/{}".format(package_filename, file_), + "{}/{}".format(real_filename, file_)) return arcname = arcname_from(package_filename)