paths to Python packages may not contain '-'. WHY?

[emranbm]

Would you please tell why it's illegal for a py_binary rule to have - in its paths?
And is there any way to bypass that?

bazel/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java

Lines 933 to 940 in 2f42d17

	/**
	* Checks that the package name of this Python rule does not contain a '-'.
	*/
	void validatePackageName() {
	if (Util.containsHyphen(ruleContext.getLabel().getPackageFragment())) {
	ruleContext.ruleError("paths to Python packages may not contain '-'");
	}
	}

[ccate]

ditto for py_library...

even a dir without dashes (src/mymodule/BUILD) cannot use srcs from a filegroup group rule in a path with dashes (src/python/my-python-module)

[ccate]

Will #8010 solve this?

[emranbm]

@ccate No. Suppose we have only one py_binary rule at all:

root
|_ WORKSPACE
|_ path-to-py
    |_ BUILD  << Contains the py_binary rule.
    |_ main.py

Run bazel run path-to-py:main.
It errors:

paths to Python packages may not contain '-'

[Sagre]

@emranbm Did you find any workaround or even an explanation why this restriction exists?

[ccate]

I have worked around this in one case with a custom rule that takes python sources from a dir with '-''s in their path. The rule takes the sources as inputs, and copies them as outputs in an action.

The consumer (py_library) of the rule still has to be in a dir without '-', but it enabled me to use bazel without changing the existing src directories.

[emranbm]

@emranbm Did you find any workaround or even an explanation why this restriction exists?

Workaround

A workaround may be referencing the python sources from an outer BUILD file.
In my above example, just move out that BUILD file near path-to-py and update corresponding fields of the rule inside. (i.e change "main.py" to "path-to-py/main.py"). It will work!

Why this restriction exists?!

But, about your second question, why this restriction exists?.
The answer (or an excuse!) is: Python environment and modules convention, actually restricted the module names having - in their names.
However, I'm still not convinced! Since a project may contain several techs (i.e. c++, java, ... and python-based sub-projects) and - unfortunately! - somewhere near the root of the project tree, the directory name may include - in its name:

root
|_ top-level-project  <<< this is a general project that just *contains*
    |                                  a python sub-project among all others.
    |_ cpp
    |   |_ BUILD
    |_ java
    |   |_ BUILD
    |_ python
        |_ BUILD <<<

Now running bazel build //top-level-project/python:something will encounter the error again! But obviously, the design of the whole project structure should not be affected by an inner sub-project.

[ashi009]

Now running bazel build //top-level-project/python:something will encounter the error again! But obviously, the design of whole project structure should not be affected from an inner sub-project.

How about renaming them? To bazel build multiple languages altogether, this is the price to pay ;)

[Sagre]

Sometimes you have a rather large project ( with '-' in its name) going on and only in a small subportion you want to make use of a python tool. Renaming the whole project only because you want to use a python tool and forcing all participants to follow the rename is really not a solution...

I now went around and build a shell script as a wrapper for the python script. This is really ugly, but at least it works without having to move a BUILD file at the top of the project (where it would be out of place).

Thanks for your explanation @emranbm !

[or-shachar]

Hey! Just wondering - is there any work / plans around being a little more flexible with this restriction?

We are considering moving Python builds to Bazel (after successful migration of Scala) and this is one of the main blockers.

We have several big repositories that refuse to pay the price of renaming folders in order to migrate to bazel, same reasons as @emranbm presented, also cross language proto packages are not always named without hyphens and other languages don't care about it.

[alexeagle]

@brandjon I have this problem too - I'm writing a rule to integrate Poetry with bazel (in the "external" style of rules_jvm_external) and want to use the directory layout written by poetry, not move things around. I also have a big monorepo I'm migrating and many top-level projects have hyphenated names which are referenced in configs scattered all over, renaming is probably not an option.

It seems like the thread was dropped when @beasleyr-vmw abandoned #8010 (comment)
should that be re-opened?

Alternatively if we make a starlark implementation of py_binary/py_library et al, then I suppose we are free of the google3 constraint. Maybe something to propose to rules_python maintainers?

(Hi Or!)

[ulfjack]

Google3 can enforce the constraint in other ways. I'm not sure that's a reason to keep it around if it's more harmful than helpful. Alternatively, add a flag to disable the check?

[or-shachar]

Can we open up this discussion again? Wix is very interested in adding Python builds to the Bazel build stack and this is one of the blockers for migration.

proto_library came up with a neat solution of adding the strip_import_prefix which can allow us to discard any prefix that has hyphen in it and live in one big mono-repo. Can we do something similar here as well?

[alandonovan]

@brandjon

IMHO Blaze should allow any printable character except '/' and ':' in a package directory name. That's all the file system demands, after all, and Blaze shouldn't need to make any stronger internal assumptions. Of course there will be compilers that impose their own restrictions, but it should not be Blaze's responsibility to anticipate all their needs. That's the job for a depot-wide presubmit check such as we have on Google's code base.

[alexeagle]

Hey all, I'm working actively in rules_python now and need this as well. Could we get any indication from someone at Google if you intend to make this change and what timeframe we could expect? Otherwise I think we'll need to start the Starlark implementation of python_* rules in the next couple weeks.

[ashi009]

Using bazel allows us to import any first-party code in the codebase in a consistent and discoverable way. I concern relaxing this will encourage inconsistent import practices.

[alexeagle]

@ashi009 as Ulf says above, there are lots of ways for you to enforce invariants for your codebase without enforcing them for everyone in this way that blocks adopting Bazel in an existing codebase.

[alexeagle]

@brandjon ping again for an update - should we just start on a starlark implementation of the python rules?

[nacl]

Linking in #6841, which seems to be asking for the same thing.

[alexeagle]

@hicksjoseph FYI, the issue we discussed

[alandonovan]

Paging @brandjon.

[alexeagle]

Okay I really need this now, so I'm going to write a new starlark py_library in rules_python.

[hicksjoseph]

Does this https://github.com/dropbox/dbx_build_tools help?

[brandjon]

Given the overwhelming demand and the good point that the benefits of this restriction can be achieved in other ways (presubmit checks), we'll remove the restriction from the native Bazel Python rules. Compared with the discussion in #8010, which proposed keeping the check but adding loopholes, it just sounds simpler and less controversial to not do the check at all.

[gpshead]

The reason we prohibit this internally is that you cannot write unittests for Python code living within something that cannot be imported as a python package. Allowing people to write untestable Python code is an antipattern destined to lead to pain. If Bazel doesn't enforce the repo pathname as a package name in Python it may not make sense for bazel to care, just let people shoot their own feet. :)

[alexeagle]

Thanks for saving the day @brandjon !

@gpshead to be clear, you can absolutely write unittests for Python code in a directory with a hyphen so long as it's in the PYTHONPATH https://docs.bazel.build/versions/master/be/python.html#py_library.imports
which is how any legacy codebase would use this feature as they migrate to Bazel.