fix: various fixes for the deps library - #4106
Conversation
Correctly wire args around and disable precompiling for the deps nodes. Work towards bazel-contrib#2948
| ), | ||
| # do not precomple `pyc` because this does not have any sources. The sources | ||
| # will be pre-compiled in the srcs folder once and for all. | ||
| precompile = "disabled", |
There was a problem hiding this comment.
This sounds like a bug in precompiling.
Precompile should be OK with empty sources.
Is it actually for directory sources?
There was a problem hiding this comment.
Ohh, is the idea here to pass a py_library as sources? And then precompile has to be disabled because, if the py_library is generating pyc, then those end up in srcs, and then the precompiler gives an error?
In any case, I have a WIP to make precompiling work with directory inputs and handle existing pyc in sources.
rickeylev
left a comment
There was a problem hiding this comment.
this is titled fix, but there's no news entry. is it fixing a released user visible behavior? If so, please add news
…pectation Fix CI analysis and test failures caused by an undefined variable in whl_library_targets and missing precompile attribute expectation in analysis tests. Change deps_template to dep_template in whl_library_targets and add "precompile": "disabled" to the expected py_library dictionary in whl_library_deps_targets tests.
Address code review feedback on PR bazel-contrib#4106 by wrapping docstrings and improving comment clarity. Wrap whl_library_deps_targets docstrings to 80 columns, clarify the rationale for setting precompile = "disabled" on the wrapper target, and remove a redundant comment in whl_library_targets.
The APIs have never been said to be public and available to be used. Though, I can add a news item if needed. |
Correctly wire args around and disable precompiling
for the deps nodes.
Work towards #2948