Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bazel/py] Omit google/__init__.py from the Protobuf runtime. #7908

Merged
merged 1 commit into from Sep 23, 2020

Conversation

dlj-NaN
Copy link
Contributor

@dlj-NaN dlj-NaN commented Sep 23, 2020

Since google is a Python namespace package, the google/__init__.py
file should be omitted to avoid collisions. For example, its presence
may cause other Pip packages in the google namespace not to be found.

This change narrows which __init__.py files are included in the
//:protobuf_runtime target to include only those under
google/protobuf. It also moves their canonical inclusion to the
internal //:python_srcs rule, eliminating the dual specification in
python_protobuf.extra_srcs.

This is the difference in files, lexicographically ordered:

$ bazel cquery 'labels(srcs, kind(py_library, deps(:protobuf_python)))' | sort > /tmp/deps-{before,after}.txt
$ diff /tmp/deps-{before,after}.txt
1,5d0
< //:python/compatibility_tests/v2.5.0/tests/__init__.py (null)
< //:python/compatibility_tests/v2.5.0/tests/google/__init__.py (null)
< //:python/compatibility_tests/v2.5.0/tests/google/protobuf/__init__.py (null)
< //:python/compatibility_tests/v2.5.0/tests/google/protobuf/internal/__init__.py (null)
< //:python/google/__init__.py (null)
51d45
< //:python/protobuf_distutils/protobuf_distutils/__init__.py (null)

This seems like a desirable change, since it avoids polluting other
packages, too.

This is conceptually similar to #7877, but for Bazel.

Since `google` is a Python namespace package, the google/__init__.py
file should be omitted to avoid collisions. For example, its presence
may cause other Pip packages in the google namespace not to be found.

This change narrows which __init__.py files are included in the
`//:protobuf_runtime` target to include only those under
`google/protobuf`. It also moves their canonical inclusion to the
internal `//:python_srcs` rule, eliminating the dual specification in
`python_protobuf.extra_srcs`.

This is the difference in files, lexicographically ordered:

```
$ bazel cquery 'labels(srcs, kind(py_library, deps(:protobuf_python)))' | sort > /tmp/deps-{before,after}.txt
$ diff /tmp/deps-{before,after}.txt
1,5d0
< //:python/compatibility_tests/v2.5.0/tests/__init__.py (null)
< //:python/compatibility_tests/v2.5.0/tests/google/__init__.py (null)
< //:python/compatibility_tests/v2.5.0/tests/google/protobuf/__init__.py (null)
< //:python/compatibility_tests/v2.5.0/tests/google/protobuf/internal/__init__.py (null)
< //:python/google/__init__.py (null)
51d45
< //:python/protobuf_distutils/protobuf_distutils/__init__.py (null)
```

This seems like a desirable change, since it avoids polluting other
packages, too.
@dlj-NaN
Copy link
Contributor Author

dlj-NaN commented Sep 23, 2020

I'm not sure if this actually warrants release notes, but we could say something like "we don't break the google package namespace anymore in Python builds under Bazel."

@dlj-NaN dlj-NaN merged commit 0295562 into protocolbuffers:master Sep 23, 2020
@dlj-NaN dlj-NaN deleted the py-build branch September 23, 2020 22:20
vam-google added a commit to vam-google/gapic-generator-python that referenced this pull request Jan 27, 2021
…kaged as native namespace package)

More details about Python namespace packaging here: https://packaging.python.org/guides/packaging-namespace-packages/#native-namespace-packages

The protobuf changes, which made this fix necessary are here:
protocolbuffers/protobuf#7902
protocolbuffers/protobuf#7908

The tracking bug for this issue (probably not the only one) googleapis/gapic-generator#3334

This is only part of the fix, for the proper fix other google-namespaced python packages must be changed as well.
vam-google added a commit to googleapis/gapic-generator-python that referenced this pull request Jan 28, 2021
…kaged as native namespace package) (#753)

More details about Python namespace packaging here: https://packaging.python.org/guides/packaging-namespace-packages/#native-namespace-packages

The protobuf changes, which made this fix necessary are here:
protocolbuffers/protobuf#7902
protocolbuffers/protobuf#7908

The tracking bug for this issue (probably not the only one) googleapis/gapic-generator#3334

This is only part of the fix, for the proper fix other google-namespaced python packages must be migrated to [pkgutil-style-namespace-packages](https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages) and make sure they **do not** have `namespace_packages` in their `setup.py` file (an artifact from the legacy `pkg_resources-style`packages)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants