Skip to content

perf(pypi): extract wheels once and reuse - #3856

Draft
aignas wants to merge 1 commit into
bazel-contrib:mainfrom
aignas:aignas.refactor.single_dep_whl_library
Draft

perf(pypi): extract wheels once and reuse#3856
aignas wants to merge 1 commit into
bazel-contrib:mainfrom
aignas:aignas.refactor.single_dep_whl_library

Conversation

@aignas

@aignas aignas commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

Summary:

  • Extract wheels in a repository A.
  • Then parse METADATA in a repository B and C and reuse A everywhere.
  • We also alias the targets for each entry point that is created in whl_library.
  • The wheel repository names will be shorter in general that should help on systems
    like Windows, where path length is sometimes an issue.

Limitations:

  • If users use the whl_mods API we cannot reuse the extracted whls.
  • If users patch wheels, those are not reused.
  • Reused only within the context of a single hub repo, we could do better with
    reuse across all of the hubs, but then we need to solve the "how to tell users
    that we need to have a single index per package? how to customize it?" problem.
  • If we find the same wheel downloaded from different indexes in the same hub repo
    then we stop reuse for that particular wheel file. This should be rare?

Extra thoughts on the design:

  • This is a step towards automatic cycle resolution.
  • With uv.lock we can point directly to the extracted sources because
    the dep graph can be passed to the hub repo.

Once #3791 is resolved this should bring reasonable speedups, especially if there the same wheel used in multiple places.

Fixes #2948
Work towards #2530

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request implements an optimized mode for whl_library (gated by the RULES_PYTHON_WHL_LIBRARY_OPTIMIZED environment variable) to allow wheel reuse across different Python versions by omitting the Python version from spoke repository names, generating per-extra targets, and creating explicit aliases in the hub repository. The code review identified several critical issues with this implementation: static aliases in render_pkg_aliases.bzl break multi-version/multi-platform setups, and iterating over extras_info causes aliases to overwrite each other. Additionally, a naming mismatch in extension.bzl between the unified hub alias and the hub repository package name will result in broken aliases, and skipping pkg__extra target generation when there are no extra dependencies leads to build failures.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +90 to +106
def _render_extra_alias(*, name, repo, target):
return """\
package(default_visibility = ["//visibility:public"])

alias(
name = "pkg",
actual = "@{repo}//:{target}",
)

alias(
name = "whl",
actual = "@{repo}//:{target}",
)
""".format(
repo = repo,
target = target,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

The _render_extra_alias function generates a static alias pointing to a single spoke repository. However, in a multi-version or multi-platform setup, there are multiple spoke repositories. We should replace this with a multiplatform alias generator that uses select to choose the correct spoke repository based on the active configuration.

def _render_extra_alias_multiplatform(*, name, repo_mapping, target_suffix):
    if type(repo_mapping) == type(""):
        actual_expr = repr("@{}//:{}".format(repo_mapping, target_suffix))
        load_statement = ""
    else:
        actual_dict = {}
        for config_setting, repo_name in repo_mapping.items():
            key = _repr_config_setting(config_setting)
            actual_dict[key] = repr("@{}//:{}".format(repo_name, target_suffix))

        if len(actual_dict) == 1 and list(actual_dict.keys())[0] == repr("//conditions:default"):
            actual_expr = list(actual_dict.values())[0]
        else:
            sorted_pairs = sorted(actual_dict.items())
            actual_expr = "select({\n"
            for k, v in sorted_pairs:
                actual_expr += "        {}: {},\n".format(k, v)
            actual_expr += "    })"

        needs_load = any(["whl_config_setting" in k for k in actual_dict.keys()])
        load_statement = ""
        if needs_load:
            load_statement = 'load("@rules_python//python/private/pypi:whl_config_setting.bzl", "whl_config_setting")\n'

    return """\
{load_statement}package(default_visibility = ["//visibility:public"])

alias(
    name = "pkg",
    actual = {actual_expr},
)

alias(
    name = "whl",
    actual = {actual_expr},
)
""".format(
        load_statement = load_statement,
        actual_expr = actual_expr,
    )

Comment on lines +158 to +176
# Generate extra alias directories for optimized mode extras.
for name, extras_info in whl_extras.items():
normalized = normalize_name(name)
for repo_name, extra_names in extras_info.items():
for extra in extra_names:
extra_pkg = "{}_{}".format(normalized, extra)
files["{}/BUILD.bazel".format(extra_pkg)] = _render_extra_alias(
name = extra_pkg,
repo = repo_name,
target = "{}__{}".format(normalized, extra),
)

# Also create pkg__ alias (no extras)
no_extras_pkg = "{}__".format(normalized)
files["{}/BUILD.bazel".format(no_extras_pkg)] = _render_extra_alias(
name = no_extras_pkg,
repo = repo_name,
target = normalized,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

In a multi-version or multi-platform setup, whl_extras contains multiple spoke repositories for the same wheel. Iterating over extras_info.items() and writing directly to files causes the alias for one spoke to overwrite the others, breaking multi-version/multi-platform support for extras. We should use the multiplatform alias generator to create a single select-based alias that correctly routes to the appropriate spoke repository.

    # Generate extra alias directories for optimized mode extras.
    for name, extras_info in whl_extras.items():
        normalized = normalize_name(name)
        pkg_aliases = aliases.get(normalized)
        if not pkg_aliases:
            continue

        # Find all unique extras across all repos for this wheel
        unique_extras = {}
        for extra_names in extras_info.values():
            for extra in extra_names:
                unique_extras[extra] = True

        for extra in unique_extras:
            extra_pkg = "{}_{}".format(normalized, extra)
            files["{}/BUILD.bazel".format(extra_pkg)] = _render_extra_alias_multiplatform(
                name = extra_pkg,
                repo_mapping = pkg_aliases,
                target_suffix = "{}__{}".format(normalized, extra),
            )

        # Also create pkg__ alias (no extras)
        no_extras_pkg = "{}__".format(normalized)
        files["{}/BUILD.bazel".format(no_extras_pkg)] = _render_extra_alias_multiplatform(
            name = no_extras_pkg,
            repo_mapping = pkg_aliases,
            target_suffix = normalized,
        )

Comment thread python/private/pypi/extension.bzl Outdated
norm_pkg = normalize_name(whl_name)
for extra_names in extras_info.values():
for extra in extra_names:
alias_name = "%s__%s" % (norm_pkg, extra)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

There is a naming mismatch between the unified hub alias name and the hub repository package name. The unified hub alias is generated with double underscores __ (e.g., requests__security), whereas render_pkg_aliases.bzl generates the hub repository package with a single underscore _ (e.g., requests_security). This mismatch will cause the unified hub aliases to point to non-existent packages. Update this to use a single underscore to match the hub repository package naming.

Suggested change
alias_name = "%s__%s" % (norm_pkg, extra)
alias_name = "%s_%s" % (norm_pkg, extra)

Comment on lines +182 to +183
if not extra_only_deps and not extra_only_deps_select:
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Skipping the generation of the pkg__extra target when there are no extra dependencies (or when all extra dependencies are already present in the base dependencies) will cause the hub repository's alias to point to a non-existent target, leading to build failures. We should always generate the pkg__extra target, even if it only depends on ":pkg", to ensure that the hub aliases remain valid.

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

saw this was draft, so didn't look too thoroughly, just for things that looked like the bot was obviously wrong

Comment thread .agents/plans/single_dep_whl_library.md Outdated
Comment thread .agents/plans/single_dep_whl_library.md Outdated
Comment thread .agents/plans/single_dep_whl_library.md Outdated
Comment thread python/private/pypi/render_pkg_aliases.bzl Outdated
@aignas

aignas commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator Author

Again, ran out of free tokens, so will postpone for another day.

@aignas
aignas force-pushed the aignas.refactor.single_dep_whl_library branch from 453b0b3 to 23c95ab Compare June 28, 2026 14:06
pull Bot pushed a commit to garymm/rules_python that referenced this pull request Jun 29, 2026
This is no longer used starting when we enabled pipstar by default and
did a code cleanup where Python is no longer used to extract the wheels.

Split from bazel-contrib#3856
@aignas

aignas commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author
  • 2 types of whl_library repos - one with extracted files, one wiring the deps.

@aignas aignas closed this Jul 2, 2026
@aignas
aignas force-pushed the aignas.refactor.single_dep_whl_library branch from 23c95ab to e433644 Compare July 2, 2026 13:22
@aignas

aignas commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

So it seems that it is much better to just separate the repositories - one with extracted whl sources, one with the parsed METADATA. This way the change is surgical and very easy to reason about. This was spiked by hand and then vibed and debugged. The build should work but there are many things missing.

TODO:

  • split the new repo into a separate file.
  • Create a separate function for the BUILD.bazel file generation and add unit tests.
  • Update existing hub_builder.bzl unit tests.
  • Remove the confabulated "fixes" to make the code easier to read.

@aignas aignas reopened this Jul 2, 2026
Comment thread python/private/pypi/extension.bzl Outdated
Comment thread python/private/pypi/hub_builder.bzl Outdated
Comment thread python/private/pypi/hub_builder.bzl Outdated
Comment thread python/private/pypi/hub_builder.bzl Outdated
@aignas aignas changed the title refactor(pypi): reuse the same whl_library instances perf(pypi): extract wheels once and reuse Jul 2, 2026
@aignas
aignas force-pushed the aignas.refactor.single_dep_whl_library branch from db1494b to c3a21b3 Compare July 4, 2026 05:20
@rickeylev

Copy link
Copy Markdown
Collaborator

/review

Comment thread python/private/pypi/extension.bzl
@aignas

aignas commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

This should now work.

EDIT: just realized that merging main into this branch should not have affected it.

@aignas

aignas commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

/review

rickeylev added a commit to rickeylev/rules_python that referenced this pull request Jul 24, 2026
…ive and pip_archive (bazel-contrib#3948)

Before this PR the `whl_library` would be a do-all repository rule.
Whilst it
is convenient to reuse the code, it is actually really difficult to
maintain
and make it more performant. Side effect here is that the python
dependencies
(like `setuptools`, etc) will no longer be downloaded for whl-only
extracts,
it makes it a tiny bit faster.

With this split we can drop certain dependencies from the whl extraction
and
optimize the common path - whl extraction where the URL for downloading
the
wheel is known. This also allows us to start handling the sdists in an
entirely
different way.

In a followup PR I plan to split the part which just extracts the wheel
to
lay a more surgical foundation to bazel-contrib#3856.

Foundation work for bazel-contrib#2410.
Split out of bazel-contrib#3856.
Work towards bazel-contrib#2948.

---------

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
aignas added a commit to aignas/rules_python that referenced this pull request Jul 25, 2026
With this we are starting to separate some of the targets.

Split out of bazel-contrib#3856
Work towards bazel-contrib#2948
pull Bot pushed a commit to garymm/rules_python that referenced this pull request Jul 26, 2026
)

With this we are starting to separate whl_library_targets
into 2 parts - one for sources only (without deps) and
another one is just the deps parts.

Next PR I'll create a way to create 2 separate instances.

Split out of bazel-contrib#3856
Work towards bazel-contrib#2948
rickeylev added a commit to rickeylev/rules_python that referenced this pull request Aug 1, 2026
Summary:
- Add a new repo rule to just read metadata.json
- Add integration tests for the repository rules in `whl_library.bzl`
  file.
- Make some of the arguments optional in the BUILD.bazel code
  generation.

No changelog, because the rule is not yet exposed to the user in any
way.

Split out of bazel-contrib#3856
Work towards bazel-contrib#2948
Fixes bazel-contrib#3071

---------

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
@aignas
aignas force-pushed the aignas.refactor.single_dep_whl_library branch 2 times, most recently from 80a0a90 to ffbe7c1 Compare August 1, 2026 12:37
@aignas
aignas marked this pull request as ready for review August 1, 2026 12:42
@aignas

aignas commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

OK, I personally like the fact that everything is done as part of whl_library macro, which allows us to change less code.

@aignas
aignas marked this pull request as draft August 1, 2026 12:43
@aignas

aignas commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

I'll add some unit tests for piece of mind.

rickeylev added a commit to rickeylev/rules_python that referenced this pull request Aug 1, 2026
…util_whls

Update site-packages repository path prefixes to match the shared wheel
extraction repos introduced by PR bazel-contrib#3856.
@aignas

aignas commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Hmmmm. This is an interesting one:

    common: {
        "requirement": "alabaster==0.7.13",
        "index_url": "https://pypi.org/simple/alabaster",
        "urls": ["https://files.pythonhosted.org/packages/64/88/c7083fc61120ab661c5d0b82cb77079fc1429d3f913a456c1c82cf4658f7/alabaster-0.7.13-py3-none-any.whl"],
        "integrity": "sha256-HuGayoAburtbo/XyWORCLfqG+C8+nO+whZsoPN1/YqM=",
        "filename": "alabaster-0.7.13-py3-none-any.whl",
    }
    missing: {
        "envsubst": ["PIP_INDEX_URL"],
    }

I think in this particular case, we could just add the envsubst to the ignore_keys part. However, if the index_url and the urls differ before interpolation with envsubst, but not after, then maybe we should handle this in some particular way... Some ideas:

  • Add the extra URL for each wheel.
  • For each URL, we try the URL synchronously.
  • For each URL we also add the index_url to ensure that we explicitly state which is which.
  • We change the URLs to a dict, where the index_url is the key and we use the setdefault to populate it. For any index URL the value should then be the same and we avoid the error.

@aignas

aignas commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Extra things that we should do:

  • parse the wheel name and do not use the full wheel name but only use: distribution, py_tag, abi_tag, platform_tag, and a little bit of sha256? Since we start using sub-resource shas, maybe we should not use the sha256 at all?

This is the common denominator and for now no warnings are printed, but
there are opportunities to do this.

This approach is way more surgical than the previous one.

Fixes bazel-contrib#2948
@aignas
aignas force-pushed the aignas.refactor.single_dep_whl_library branch from 1441ff7 to 8a10382 Compare August 23, 2026 13:59
@aignas

aignas commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator Author

Given the recent refactors, I've did it again in a more surgical way.

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.

Setup fewer wheel repos in pip.parse

2 participants