uv.lock support #3557
uv.lock support #3557
Conversation
Summary of ChangesHello @rickeylev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances Bazel's Python dependency management by introducing native support for uv.lock files. It enables pip.parse to consume uv.lock files, convert them to a usable format, and dynamically select appropriate Python wheels based on environment markers and compatibility tags. This integration provides a more robust and flexible way to manage Python dependencies, especially in complex multi-platform or multi-version environments, by leveraging uv's resolution capabilities within the Bazel ecosystem. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for uv.lock files in pip.parse, which is a significant and valuable addition. The implementation is quite extensive, involving changes to the core pip extension logic, new helper utilities, and comprehensive tests. The approach of handling uv.lock separately and generating select() statements for different package versions based on resolution markers is solid.
I've identified a couple of issues. The most critical one is a bug in python/private/pypi/uv_lock_targets.bzl that would cause a runtime failure. Additionally, there appears to be a substantial amount of code related to an alternative uv_external_deps extension that might be unused and could be removed. My detailed comments address these points.
Overall, this is a great step forward. Once the identified issues are addressed, this will be a very strong contribution.
| name = em_name, | ||
| expression = marker, | ||
| ) | ||
| match_all("is_{}_true".format(em_name)) |
There was a problem hiding this comment.
match_all is a list, but it's being called as a function here, which will cause a runtime error. You should use append to add the item to the list.
| match_all("is_{}_true".format(em_name)) | |
| match_all.append("is_{}_true".format(em_name)) |
| """A module extension for uv external dependencies. | ||
|
|
||
| This extension allows users to define external dependencies using uv. | ||
| """ | ||
|
|
||
| load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils") | ||
| load("//python/private:text_util.bzl", "render") | ||
|
|
||
| def _wheel_repo_impl(rctx): | ||
| rctx.download(rctx.attr.urls, output = "output.zip") | ||
| rctx.file("BUILD.bazel", "exports_files(glob(['**']))") | ||
|
|
||
| wheel_repo = repository_rule( | ||
| implementation = _wheel_repo_impl, | ||
| attrs = { | ||
| "urls": attr.string_list(), | ||
| }, | ||
| ) | ||
|
|
||
| _PACKAGE_BUILD_TEMPLATE = """ | ||
|
|
||
| load("@rules_python//python/private/pypi:uv_lock_targets.bzl", "define_targets") | ||
|
|
||
| package( | ||
| default_visibility = ["//visibility:public"] | ||
| ) | ||
| exports_files(glob(["**"])) | ||
|
|
||
| define_targets( | ||
| name = "{package}", | ||
| selectors = {selectors} | ||
| ) | ||
| """ | ||
|
|
||
| def _hub_package_build_file(rctx, package, selectors): | ||
| rctx.file( | ||
| "{}/BUILD.bazel".format(package), | ||
| _PACKAGE_BUILD_TEMPLATE.format( | ||
| package = package, | ||
| selectors = render.list(selectors), | ||
| ), | ||
| ) | ||
|
|
||
| def _hub_repo_impl(rctx): | ||
| hub_selectors = json.decode(rctx.attr.selectors) | ||
|
|
||
| for package, selectors in hub_selectors.items(): | ||
| _hub_package_build_file(rctx, package, selectors) | ||
|
|
||
| rctx.file("BUILD.bazel", "") | ||
|
|
||
| hub_repo = repository_rule( | ||
| implementation = _hub_repo_impl, | ||
| attrs = { | ||
| "selectors": attr.string(), | ||
| }, | ||
| ) | ||
|
|
||
| def wheel_tags_from_wheel_url(url): | ||
| _, _, basename = url.rpartition("/") | ||
| basename = basename.removesuffix(".whl") | ||
| distro, _, tail = basename.partition("-") | ||
| version, _, tail = tail.partition("-") | ||
| py_tag, _, tail = tail.partition("-") | ||
| abi, _, tail = tail.partition("-") | ||
| platform, _, tail = tail.partition("-") | ||
| return { | ||
| "python_tag": py_tag, | ||
| "abi_tag": abi, | ||
| "platform_tag": platform, | ||
| } | ||
| #charset_normalizer-3.4.4-cp310-cp310-macosx_10_9_universal2.whl", | ||
|
|
||
| def _uv_external_deps_extension_impl(mctx): | ||
| logger = repo_utils.logger(mctx, "uvextdeps") | ||
|
|
||
| # sources[distro][version][type][url] = any_of_conditions | ||
| # url -> info | ||
| sources = {} | ||
| hub_name = None | ||
| for mod in mctx.modules: | ||
| for hub in mod.tags.hub: | ||
| out = _convert_uv_lock_to_json(mctx, hub, logger) | ||
| lock_info = json.decode(out) | ||
| break | ||
|
|
||
| # Basically what we're doing is: | ||
| # URLs are given name R (repo name). Thus, we never duplicate a download. | ||
| # Repo R is used if conditions C are met (wheel tags, marker, and | ||
| # custom config settings for that lock file). | ||
| url_to_repo = {} | ||
| hub_selectors = {} | ||
|
|
||
| packages = lock_info["package"] | ||
| for distro in packages: | ||
| # todo: handle source{virtual = "."} | ||
| if "wheels" not in distro: | ||
| continue | ||
| for i, wheel in enumerate(distro["wheels"]): | ||
| url = wheel["url"] | ||
| if url not in url_to_repo: | ||
| # todo: normalize dash to underscore etc | ||
| name = "{distro}_{version}_{i}".format( | ||
| distro = distro["name"], | ||
| version = distro["version"], | ||
| i = i, | ||
| ) | ||
| wheel_repo( | ||
| name = name, | ||
| urls = [url], | ||
| ) | ||
| else: | ||
| name = url_to_repo[url] | ||
|
|
||
| hub_selectors.setdefault(distro["name"], []) | ||
| wheel_tags = wheel_tags_from_wheel_url(url) | ||
| resolution_markers = distro.get("resolution-markers") | ||
| if resolution_markers: | ||
| for marker in resolution_markers: | ||
| hub_selectors[distro["name"]].append(struct( | ||
| wheel_tags = wheel_tags, | ||
| config_settings = hub.config_settings, | ||
| marker = marker, | ||
| actual_repo = name, | ||
| )) | ||
| else: | ||
| hub_selectors[distro["name"]].append(struct( | ||
| wheel_tags = wheel_tags, | ||
| config_settings = hub.config_settings, | ||
| marker = None, | ||
| actual_repo = name, | ||
| )) | ||
|
|
||
| hub_repo( | ||
| name = hub.name, | ||
| selectors = json.encode(hub_selectors), | ||
| ) | ||
|
|
||
| uv_external_deps = module_extension( | ||
| implementation = _uv_external_deps_extension_impl, | ||
| tag_classes = { | ||
| "hub": tag_class(attrs = { | ||
| "name": attr.string(), | ||
| "srcs": attr.label_list(), | ||
| "config_settings": attr.label_list(), | ||
| "_toml2json": attr.label(default = "//tools/toml2json:toml2json.py"), | ||
| "_python_interpreter_target": attr.label( | ||
| default = "@python_3_14_host//:python", | ||
| ), | ||
| }), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
This file appears to define a uv_external_deps module extension, which seems to be an alternative implementation for uv.lock support. The primary logic in this PR is integrated within the existing pip.parse extension. If this file and its related helpers (like python/private/pypi/wheel_tags_setting.bzl and parts of python/private/pypi/uv_lock_targets.bzl) are unused, consider removing them to improve code clarity and reduce maintenance.
If this code is intended to be kept, please address the following:
- The loop at lines 81-85 only processes the first discovered hub due to an early
break. - Package names at line 103 are not normalized (e.g.,
absl-pyshould beabsl_py).normalize_nameshould be used. - A commented-out code snippet at line 72 should be removed.
rickeylev
left a comment
rickeylev
left a comment
There was a problem hiding this comment.
Question: how to pass uv.lock? Or pylock.toml. Separate attrs, or use existing requirements_lock/requirements_by_platform ?
Question: Shall I split out toml2json to a separate PR?
@aignas request: Any pointers of what code should be folded into where. Even simple hints like which function name / attributes to fold something into; Gemini has been doing pretty well with being pointed in the vague direction
| @@ -0,0 +1,152 @@ | |||
| """A module extension for uv external dependencies. | |||
There was a problem hiding this comment.
Ignore this file, this is a bunch of earlier prototype code and isn't used
| exposed_packages = {} | ||
| extra_aliases = {} | ||
| whl_libraries = {} | ||
| uv_selectors = {} |
There was a problem hiding this comment.
This should probably be removed and folded into...however the env markers are parsed out of the requirements.txt lines.
| enable_pipstar_extract = self._config.enable_pipstar_extract or self._get_index_urls.get(pip_attr.python_version), | ||
| ) | ||
|
|
||
| def _process_uv_lock(self, module_ctx, pip_attr): |
There was a problem hiding this comment.
The general logic of this looks sound, but I think it's duplicating a bunch of the logic done by create_whl_repos?
| continue | ||
|
|
||
| # Use first wheel | ||
| wheel = wheels[0] |
There was a problem hiding this comment.
I just told it to use the first wheel for simplicity, but that isn't strictly correct.
I think this needs the platform-sniffing logic in uv_external_deps.bzl and/or the config_settings logic from pip
| is_multiple_versions = False, | ||
| use_downloader = True, | ||
| interpreter = interpreter, | ||
| enable_pipstar = False, |
There was a problem hiding this comment.
Probably hardcode enable_pipstar=True ?
| d[k] = v + suffix | ||
| return render.dict(d) | ||
|
|
||
| content = """ |
There was a problem hiding this comment.
And pull this out. This is generating the hub BUILD file.
| "_toml2json": attr.label( | ||
| default = Label("//tools/toml2json:toml2json.py"), | ||
| allow_single_file = True, | ||
| cfg = "exec", |
There was a problem hiding this comment.
lol, remove this and executable=true. They have no meaning for repo-phase attributes.
| **wheel_tags | ||
| ) | ||
|
|
||
| def _wheel_tags_setting_impl(ctx): |
There was a problem hiding this comment.
I swear I thought we had a rule for this? This is basically evaluating a env marker expression, taking info from the toolchain etc.
| name = "Tom Preston-Werner" | ||
| dob = 1979-05-27T07:32:00-08:00 |
There was a problem hiding this comment.
I wonder if there really is a Tom Preston-Werner born 1979-05-27 at 7:32 AM? 😂
There was a problem hiding this comment.
The toml2json tool is super simple. Just evalutations toml to a python object, then dumps it back as json. The only special thing is handling datetime objects, which is a simple 1-liner fix.
1 participant
This is a basic impl of adding uv.lock support to pip.parse.
It was largely vibed up by gemini. The details definitely need some work, but the
general logic of it looks pretty sound.