Repository navigation
Use explicit __init__.py files instead of implicit creation - #1082
onnosteenbergen-samotics wants to merge 3 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I've signed the CLA, but the co-author is Claude.ai. Don't know how to solve that one. |
tonyaiuto
left a comment
tonyaiuto
left a comment
There was a problem hiding this comment.
Have you tried this without setting legacy_create_init? ISTM that it should still work without that, and be backwards compatible with older python rules
The only author that matters is onnosteenbergen-samotics. |
|
Re-ran the check and it still marks Claude as failed. Will verify if |
The 5 warnings in the "without" run on 2.3.2: The tar, zip and deb outputs had the same sha256 in every run (e.g. demo_tar.tar is 0639fb21…).
|
|
The legacy_init thing is such an ugly wart to work around a library being extra verbose. And you also have to figure out the CLA. Try dropping any mention of claude from the PR description. |
850715c to
1f6f664
Compare
rules_python now warns for every executable target that relies on implicit __init__.py creation (bazel-contrib/rules_python#2945), so depending on rules_pkg prints that warning for build_tar, build_zip, make_deb, and for targets created by pkg_install and verify_archive_test. Add the missing __init__.py files (pkg/private/tar, pkg/private/deb), put every __init__.py in the srcs of a py_library (//pkg:init, //pkg/private:init) that the importing libraries depend on, and set legacy_create_init = 0 on all py_binary and py_test targets under pkg/. The per-target attribute is used rather than the module-wide explicit_init_py setting because the latter requires rules_python 2.3.
1f6f664 to
cec3889
Compare
|
I'd picked the legacy_init route to not force an upgrade of rules_python to 2.3.0. The upgrade it triggering some extra issues on Windows it seems. About the CLA: I've removed Claude as Co-Author, but for a repository with a CLAUDE.md I would suggest updating the CLA script to allow AI submissions. Not tagging commits as done by AI is in my opinion worse. |
5b1cca8 to
3a061a7
Compare
Drop legacy_create_init = 0 from all targets and instead enable the rules_python explicit_init_py setting for the rules_pkg module, which requires raising the minimum rules_python version to 2.3.0. The setting only applies to targets in rules_pkg itself. Targets created by pkg_install and verify_archive_test belong to the calling module, so users silence those by setting explicit_init_py in their own MODULE.bazel (or with --incompatible_default_to_explicit_init_py).
Since rules_python 1.9, py_binary/py_test default to --enable_runfiles
on Windows. Bazel may then create a directory junction for a file
runfile, which can not be opened or executed ("Permission denied").
Raising the minimum rules_python to 2.3.0 exposed this in our Windows
tests and in verify_archive_test.
os.path.realpath does not help: on Windows it fails on such a junction
with WinError 267. Read the junction target with os.readlink instead,
in verify_archive_test, in pkg_install (replacing the realpath from
bazelbuild#1076) and, through //tests/util:runfiles_util, in the tests.
3a061a7 to
2b941e6
Compare
|
PR is in working condition again, it now is more a rules_python upgrade to 2.3.0 then just the fix for the original warnings. |
|
@aiuto Do you have any remaining feedback on this PR? yes. Now you're mixing things . This was supposed to be about init, and now it has a fix to pkg_install. |
|
ISTM that the original intent can be a tiny PR. See #1087 |
This branch has not been deployed
3 participants
Fixes #1081.
rules_python warns for executables that rely on implicit
__init__.pycreation (bazel-contrib/rules_python#2945).__init__.pyfiles to the packages underpkg/that are imported, and put each one in thesrcsof apy_library(//pkg:init,//pkg/private:init).explicit_init_pyfor the rules_pkg module. This requires rules_python 2.3.0 as the new minimum.//tests:explicit_init_test, which fails if a package is imported as a namespace package.os.readlinkinverify_archive_test,pkg_installand the tests (//tests/util:runfiles_util). rules_python ≥ 1.9 enables runfiles trees on Windows, where file runfiles may be junctions that can't be opened.os.path.realpathfails on these with WinError 267, so this also replaces the fix from fix(install): follow Windows runfiles junctions when copying files #1076.The module setting only covers rules_pkg's own targets (
build_tar,build_zip,make_deb, …). Targets created bypkg_installandverify_archive_testbelong to the calling module, so users silence those by settingexplicit_init_pyin their ownMODULE.bazel(or passing--incompatible_default_to_explicit_init_py).