Allow hyphen in package path

[brandjon]

We prohibit - in the package path of python code (though not in source filenames). The reasons seem historic and google-specific, so we should probably eliminate this restriction.

[brandjon]

Modules with hyphens in their name or package path can't be imported using the normal import syntax, but according to SO you can still use other means like __import__(string).

[brandjon]

The opinion of our internal Python team seems to be that, at least for Google's monorepo, hyphens are unnatural enough in Python that we should not allow them. So the question is whether there's a compelling case for permitting them for other Bazel users, even though to use them you'd have to resort to weirdness like __import__().

[ghost]

The opinion of our internal Python team seems to be that, at least for Google's monorepo, hyphens are unnatural enough in Python that we should not allow them. So the question is whether there's a compelling case for permitting them for other Bazel users, even though to use them you'd have to resort to weirdness like __import__().

My codebase has a lot of packages with hyphenated names. These packages typically contain IDL sources from which we generate bindings for multiple languages, including Python. Under our pre-Bazel build, the Python sources would either be aggregated or output directories appended to the Python search path. As I understand it, Bazel helps with the latter via py_library's imports attribute, so I'd expect the following to work just fine:

# //src/foo-idl:BUILD
emitter_rule(
    name = "foo-srcs",
    srcs = ["foo.idl"],
    outs = [
        "python/com_example_foo/__init__.py",
        "python/com_example_foo/Foo.py,
    ]
)

py_library(
    name = "foo-idl-py",
    srcs = [":foo-srcs"],
    imports = ["python"],
)

# //src/bar:BUILD
py_binary(
    name = "bar",
    srcs = ["bar.py"],
    deps = ["//src/foo-idl:foo-idl-py"],
)

# //src/bar:bar.py
from com_example_foo import Foo
...

[brandjon]

We don't generally allow hyphens in Google's monorepo. This is important for ensuring that Python code can live in any package, even when the package was not necessarily originally designed with Python in mind (e.g. a C++ project that later had auto-generated Python bindings added to it). As a member of our Python team put it,

Being conservative with the lowest common denominator on allowed directory names is a good thing. For anything that absolutely requires - in directory names, coming up with a way to make specific exceptions for those is better to the health of a codebase than blindly allowing them everywhere.

In Bazel we don't have this restriction, so (as you've discovered) users already have the problem that they can't put Python code in arbitrary packages. I don't think we're going to clamp down on prohibiting hyphens everywhere just for the sake of Python, so the opposite proposal (your PR #8010) is to relax this restriction for Bazel and make it the user's responsibility to provide imports declarations if their package names are not addressable within Python due to hyphens. I think I'm ok with this, I just want to see if there are further objections on our side.

[brandjon]

Please see this comment for the precise wording of the proposed change.

[gpshead]

For a py_binary, allowing those to exist in a package with - hyphens in their name seems reasonable. We've got use cases in our internal monorepo for that: Starlark macros generating genrule exec_tools that may happen to be Python based for use by other things anywhere in the code base they happen to be needed, hyphens included.

For a py_library it does not. Modules cannot be imported from a package with illegal characters in its name. Using hacks to get around that such as importlib are a bad code smell and should not be encouraged.

If there are Bazel users living in a world where they don't fully qualify their imports and rely on Python putting the dirname of the main .py file into Python's sys.path, they might want this. For code health reasons we do not recommend that practice so we'll keep the existing py_library restriction internally.

[daicoden]

I'm running into problems using py_proto_library where protos were defined for other languages using hyphen packages. Is there a work around?

protocolbuffers/protobuf#7061 looks like it could help solve the issue for protos.

[github-actions]

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 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[github-actions]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!