Skip to content

fix: various fixes for the deps library - #4106

Open
aignas wants to merge 3 commits into
bazel-contrib:mainfrom
aignas:aignas.fix.deps_targets_fixes
Open

fix: various fixes for the deps library#4106
aignas wants to merge 3 commits into
bazel-contrib:mainfrom
aignas:aignas.fix.deps_targets_fixes

Conversation

@aignas

@aignas aignas commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Correctly wire args around and disable precompiling
for the deps nodes.

Work towards #2948

Correctly wire args around and disable precompiling
for the deps nodes.

Work towards bazel-contrib#2948
@aignas
aignas requested a review from rickeylev as a code owner August 23, 2026 14:05
),
# 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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This sounds like a bug in precompiling.

Precompile should be OK with empty sources.

Is it actually for directory sources?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment thread python/private/pypi/whl_library_deps_targets.bzl Outdated
Comment thread python/private/pypi/whl_library_targets.bzl Outdated

@rickeylev rickeylev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.
@aignas

aignas commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

this is titled fix, but there's no news entry. is it fixing a released user visible behavior? If so, please add news

The APIs have never been said to be public and available to be used. Though, I can add a news item if needed.

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.

2 participants