Skip to content

fix(bzlmod pip): ensure that sub-modules do not have invalid repos#1549

Merged
rickeylev merged 2 commits intobazel-contrib:mainfrom
aignas:fix/1548/default_version_render_aliases
Nov 16, 2023
Merged

fix(bzlmod pip): ensure that sub-modules do not have invalid repos#1549
rickeylev merged 2 commits intobazel-contrib:mainfrom
aignas:fix/1548/default_version_render_aliases

Conversation

@aignas
Copy link
Copy Markdown
Collaborator

@aignas aignas commented Nov 9, 2023

This fixes the cases where the 'default_version' is passed to the
'render_pkg_aliases' utility but the 'default_version' is not present
for the wheels. This usually happens when a sub-module is using the
'pip.parse' extension and the default_version can only be set by the
root module.

Previously, such a case would generate a select() expression that mapped
the default condition to a non-existent target (because the sub-module didn't
call pip.parse() with that version). This would either result in errors
due the target not existing, or silently using a target intended for a
different Python version (which may work, but isn't correct to so).

Now, it results in a error via select.no_match_error.

Fixes #1548.

…repos

This fixes the cases where the 'default_version' is passed to the
'render_pkg_aliases' utility but the 'default_version' is not present
for the wheels. This usually happens when a sub-module is using the
'pip.parse' extension and the default_version can only be set by the
root module.

Fixes bazel-contrib#1548.
@rickeylev
Copy link
Copy Markdown
Collaborator

An edge case I don't think is being handled: when the default python version is the sub-module's version. Recall that the version-unaware rules (//python:py_binary.bzl) won't match the :is_py_X.Y config conditions. So given something like:

# root/MODULE.bazel
python.toolchain(python_version="3.9", is_default=True)
# submodule/MODULE.bazel
pip.parse(python_version="3.9", ...)
# submodule/BUILD.bazel
load("@rules_python//python:py_binary.bzl", "py_binary")
py_binary(name="subbin", ...) 

# generate foo/BUILD.bzl

alias(name = "foo", actual = select({
  ":is_py_3.9": "pip_39_foo//:pkg",
  no_match = "error message"
}))

Then it'll result in an error when it should work.

So the logic needs to be something like...

if default in python_version_to_pip_target:
  actuals["//conditions:default"] = python_version_to_pip_target[default]

@aignas
Copy link
Copy Markdown
Collaborator Author

aignas commented Nov 10, 2023

@rickeylev, I think the scenario you are thinking about is covered by https://github.com/bazelbuild/rules_python/pull/1549/files#diff-6b599a1257ef3562418f2372fe871a2a18bbd09517414d6085c87f7f9d994327L75 and it is as you would expect.

@rickeylev rickeylev changed the title fix(render_pkg_aliases): ensure that sub-modules do not have invalid repos fix(bzlmod pip): ensure that sub-modules do not have invalid repos Nov 16, 2023
@rickeylev rickeylev added this pull request to the merge queue Nov 16, 2023
Merged via the queue into bazel-contrib:main with commit 530fd85 Nov 16, 2023
@aignas aignas deleted the fix/1548/default_version_render_aliases branch November 16, 2023 22:47
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.

bzlmod pip.parse default is leaking across to the other_module

2 participants