Skip to content

pycross_wheel_library: enable implicit namespace packages#216

Open
tobyh-canva wants to merge 1 commit intojvolkman:mainfrom
tobyh-canva:enable-implicit-namespace-pkgs
Open

pycross_wheel_library: enable implicit namespace packages#216
tobyh-canva wants to merge 1 commit intojvolkman:mainfrom
tobyh-canva:enable-implicit-namespace-pkgs

Conversation

@tobyh-canva
Copy link

Context

Currently, pycross_wheel_library installs wheels and unconditionally converts any implicit namespace packages into legacy pkgutil-style namespace packages.

This pkgutil-style conversion can be problematic and unexpected in a lot of cases, and should be unnecessary beyond Python 3.3 when the --incompatible_default_to_explicit_init_py=true Bazel flag is set.

Looking at the pycross_wheel_library rule implementation, and the source code of wheel_installer.py, it appears as though there was an intention to allow users to disable this pkgutil-style conversion, but the option isn't exposed via the Bazel interfaces, and it's also not used inside the script itself.

Intent

Allow users to disable the automatic conversion of PEP 420 implicit namespace packages to pkgutil-style namespace packages, whilst maintaining backwards compatibility.

Changes

  • Introduce a boolean bazelrc flag, --@rules_pycross//config_settings:default_enable_implicit_namespace_pkgs, to configure the default behaviour of enabling implicit namespace packages in pycross_wheel_library.
  • Update the pycross_wheel_library rule and wheel_installer.py script to respect the flag.
  • Document the flag.

## Context
Currently, `pycross_wheel_library` installs wheels and unconditionally converts any implicit namespace packages into legacy `pkgutil`-style namespace packages.

This `pkgutil`-style conversion can be problematic and unexpected in a lot of cases, and should be unnecessary beyond Python 3.3 when the `--incompatible_default_to_explicit_init_py=true` Bazel flag is set.

Looking at the `pycross_wheel_library` rule implementation, and the source code of `wheel_installer.py`, it appears as though there was an intention to allow users to disable this `pkgutil`-style conversion, but the option isn't exposed via the Bazel interfaces, and it's also not used inside the script itself.

## Intent
Allow users to disable the automatic conversion of PEP 420 implicit namespace packages to `pkgutil`-style namespace packages, whilst maintaining backwards compatibility.

## Changes
- Introduce a boolean bazelrc flag, `--@rules_pycross//config_settings:default_enable_implicit_namespace_pkgs`, to configure the default behaviour of enabling implicit namespace packages in `pycross_wheel_library`.
- Update the `pycross_wheel_library` rule and ``wheel_installer.py` script to respect the flag.
- Document the flag.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, I wrote this doc by hand, it's not generated 😅 I couldn't figure out a way to use stardoc to generate docs for a Bazel flag. LMK if there's a preferred way to generate this doc.

Comment on lines +118 to +119
"enable_implicit_namespace_pkgs": attr.int(
default = -1,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no tests for this option. I'd be happy to add some, but I'm not sure where they should go - in //e2e or //pycross/tests? And if so, should I just check in a dummy wheel file containing a namespace package? Please LMK what you would prefer 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant