rules_python icon indicating copy to clipboard operation
rules_python copied to clipboard

fix: handle whitespace in the python file names

Open vors opened this issue 6 months ago • 4 comments

This is not a breaking change because previously we just failed at the runfiles with

link or target filename contains space on line 9877: '_main/external/rules_python~override~pip~pypi_311_imgaug/site-packages/imgaug/augmenters/arithmetic (copy).py /root/.cache/bazel/_bazel_root/00e0182df830644af7af00c92693c660/external/rules_python~override~pip~pypi_311_imgaug/site-packages/imgaug/augmenters/arithmetic (copy).py'

the only sane way to handle names in whitespaces in .py files from whl is to ignore it, so we can as well do this in the core rule itself.

vors avatar Feb 02 '24 21:02 vors

I get where this change is coming from but I don't think this is the right solution. A Python wheel which lays down files that contain spaces needs to be patched to work under Bazel anyway. Allowing the install to succeed and produce a partially viable state isn't IMO desirable behavior. This isn't just an issue of .py files with ah invalid module names, but also data files. For instance https://github.com/jaraco/jaraco.text/pull/12.

arrdem avatar Feb 02 '24 23:02 arrdem

Thank you, just want to point out that data files already have exactly this exclusion https://github.com/bazelbuild/rules_python/blob/e7f8f0f786aa615bd60731ce1d151b19bf8dd18f/python/pip_install/private/generate_whl_library_build_bazel.bzl#L294

If anything then data is more important not to change: the python src file that uses space doesn't have any way to be used in the import statement.

Anyway, if you feel strongly it's fine.

I also want to ask about the right way to do the annotation exclusion for this package

    annotations = {
        "imgaug": '{"srcs_exclude_glob": ["**/* *"]}',
    },

tried this, but it doesn't seem to have any affect.

vors avatar Feb 03 '24 23:02 vors

For now I resorted to patching rules_python on the fly with

single_version_override(module_name = "rules_python", version = "0.29.0", patches=["//lib/bazel:rules_python.patch"])

since I cannot figure out how to use annotations even after reading the code 😅

vors avatar Feb 03 '24 23:02 vors

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

github-actions[bot] avatar Aug 02 '24 22:08 github-actions[bot]