--build_python_zip fails when runfiles include file containing '=' character (affects pyspark==2.4.6)

[thundergolfer]

trafficstars

Description of the problem / feature request:

A Bazel Python 'zipapp' cannot be built using --build_python_zip when the underlying py_binary target depends on pyspark==2.4.6. I think this is because pyspark contains files that include the "=" character in their filename, which breaks some logic in the --build_python_zip action.

Example Error:

INFO: Analyzed 2 targets (22 packages loaded, 708 targets configured).
INFO: Found 2 targets...
ERROR: /Users/jonathon/work/reproduce_zipapp_bug/spark_hello_world/BUILD:4:10: PythonZipper spark_hello_world/main.zip failed (Exit 255): zipper failed: error executing command external/bazel_tools/tools/zip/zipper/zipper cC bazel-out/darwin-fastbuild/bin/spark_hello_world/main.zip @bazel-out/darwin-fastbuild/bin/spark_hello_world/main.zip-0.params

Use --sandbox_debug to see verbose messages from the sandbox zipper failed: error executing command external/bazel_tools/tools/zip/zipper/zipper cC bazel-out/darwin-fastbuild/bin/spark_hello_world/main.zip @bazel-out/darwin-fastbuild/bin/spark_hello_world/main.zip-0.params

Use --sandbox_debug to see verbose messages from the sandbox
File kittens/date=2018-01/not-image.txt=external/pypi/pypi__pyspark/pyspark/data/mllib/images/partitioned/cls=kittens/date=2018-01/not-image.txt does not seem to exist.
INFO: Elapsed time: 2.814s, Critical Path: 0.35s
INFO: 2 processes: 2 internal.
FAILED: Build did NOT complete successfully

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I have a full public reproduction over in this repo: https://github.com/thundergolfer/bazel-build_python_zip-bug-reproduction (instructions in the README)

What operating system are you running Bazel on?

MacOS Catalina 10.15.7

What's the output of bazel info release?

release 3.7.2

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

Replace this line with your answer.

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

git remote get-url origin ; git rev-parse main ; git rev-parse HEAD                                                                                                                                                                                             
[email protected]:thundergolfer/bazel-build_python_zip-bug-reproduction.git
ff5d23b14ade117b74494ecd3a0ed5666b8f224e
ff5d23b14ade117b74494ecd3a0ed5666b8f224e

Have you found anything relevant by searching the web?

• GitHub issues: https://github.com/bazelbuild/rules_docker/issues/1254 seems relevant.

---

👋 I can look further into this and submit a fix + test, when time permits.

[brody-cormier-techlabs]

I'm also able to reproduce this issue with pyspark==3.0.2 so this issue has not gone away on their side.

[thundergolfer]

Looking into this just now, we may be able to get away with just changing off of ParameterFileType.UNQUOTED. https://github.com/bazelbuild/bazel/blob/0b1e6126b78dd356a88e2ea86ffb372a961c8a91/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java#L403

    /**
     * A parameter file with every parameter on a separate line. This format
     * cannot handle newlines in parameters. It is currently used for most
     * tools, but may not be interpreted correctly if parameters contain
     * white space or other special characters. It should be avoided for new
     * development.
     */
    UNQUOTED,

Update: This suggestion does not work.

[thundergolfer]

After directly engaging with the 'zipper' tool:

./bazel-bazel-build_python_zip-bug-reproduction/external/bazel_tools/tools/zip/zipper/zipper c x.zip "foo=bar.py=foo=bar.py"

I can see that the tool just doesn't seem to support having a destination path in the zip include the = character. Quoting the arguments or escaping the =, eg. "foo\=bar.py=foo=bar.py" does not work.

[thundergolfer]

Looking through the source, I'm fairly confident the zipper is third_party/ijar/zip_main.cc. Seems quite possible that the original use case of zipping IJars never intended to support = characters in filenames.

I'm not sure whether Bazel maintainers would support a patch to that binary just to support the Python use-case, so maybe Python zipping should use a different tool?

cc @brandjon

[xhuoroku]

ran into this issue with pyspark_2.4.3

[thundergolfer]

Same kind of issue in pyarrow==3.0.0. It includes a file with path: pyarrow/tests/data/feather/v0.17.0.version=2-compression=lz4.feather.

[comius]

I'm not sure whether Bazel maintainers would support a patch to that binary just to support the Python use-case

The patch would be welcome. I don't see any problems fixing zipper to support = in the filenames.

[benjaminRomano]

Running into this as well for pyarrow. Here is example of what inputs look like:

runfiles/py_deps_pypi__pyarrow/pyarrow/tensor.pxi=external/py_deps_pypi__pyarrow/pyarrow/tensor.pxi
runfiles/py_deps_pypi__pyarrow/pyarrow/tensorflow/plasma_op.cc=external/py_deps_pypi__pyarrow/pyarrow/tensorflow/plasma_op.cc
runfiles/py_deps_pypi__pyarrow/pyarrow/tests/bound_function_visit_strings.pyx=external/py_deps_pypi__pyarrow/pyarrow/tests/bound_function_visit_strings.pyx
runfiles/py_deps_pypi__pyarrow/pyarrow/tests/data/feather/v0.17.0.version=2-compression=lz4.feather=external/py_deps_pypi__pyarrow/pyarrow/tests/data/feather/v0.17.0.version=2-compression=lz4.feather
runfiles/py_deps_pypi__pyarrow/pyarrow/tests/data/orc/README.md=external/py_deps_pypi__pyarrow/pyarrow/tests/data/orc/README.md

The compression=lz4.feather is the offending line.

[benjaminRomano]

Potential fix here: https://github.com/bazelbuild/bazel/pull/13850

[danieljanes]

Is there any progress on this? This issue breaks our build since we introduced a new dependency that itself has a sub-dependency on pyarrow (as described by @thundergolfer and @benjaminRomano).

[benjaminRomano]

cc @comius @brandjon

[mbkroese]

I'm also affected by this problem when I use pyspark as a dependency.

[thundergolfer]

I'll jump back in to note how we avoid this. We no longer zip up our apps, using --build_python_zip, Pex, or google/subpar. We either execute py_binary, or ship the py_binary into a rules_docker py_image-like thing which isn't a zip.

Zips may be used for AWS Lambda, but you don't want --build_python_zip in that case because it builds an executable, whereas for Lambda you want a .zip 'importable'.

[benjaminRomano]

To add onto that note, the executable zip generated by --build_python_zip after I applied my patch has terrible cold start performance due to Bazel's implementation not caching the extracted files after first run.

I ended up abandoning this approach for distributing my python code.

[rahul-theorem]

Sounds like --build_python_zip is implicitly unsupported/unmaintained going forward? We also encountered similar cold-start times with subpar & are going to try to adapt it into something that supports building standalone binaries with Pex.

[github-actions[bot]]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.