Skip to content

fix(pypi): correctly handle custom names in pipstar platforms #3054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 7, 2025

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jul 4, 2025

Before it seems that we were relying on particular names in the pipstar
platforms. This ensures that we rely on this less. Whilst at it fix a
few typos and improve the formatting of the code.

Work towards #2949
Work towards #2747

Before it seems that we were relying on particular names in the pipstar
platforms. This ensures that we rely on this less. Whilst at it fix a
few typos and improve the formatting of the code.

Work towards bazel-contrib#2949
Work towards bazel-contrib#2747
@aignas aignas force-pushed the fix/platform_name branch from 2f64607 to ead6071 Compare July 6, 2025 13:21
@aignas
Copy link
Collaborator Author

aignas commented Jul 6, 2025

The requirement_files_by_platform is still a little broken as we can see in #3063.

@@ -383,6 +387,12 @@ def _configure(config, *, platform, os_name, arch_name, config_settings, env = {
if key not in _SUPPORTED_PEP508_KEYS:
fail("Unsupported key in the PEP508 environment: {}".format(key))

if not os_name:
fail("'os_name' is required")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Were os_name and arch_name effectively required before, or is this new? Though, I think this pip.defaults API is still experimental, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is new. It is experimental, but I've been dragging some of the hidden constants from various parts of the PyPI code into the internally defined pip.default instances. This has been really useful in that sense. I think maybe I want to revert this requirement. It is not going to be compatible with the builder pattern.

@aignas aignas enabled auto-merge July 7, 2025 02:08
@aignas aignas added this pull request to the merge queue Jul 7, 2025
Merged via the queue into bazel-contrib:main with commit 3d93274 Jul 7, 2025
3 checks passed
@aignas aignas deleted the fix/platform_name branch July 7, 2025 04:28
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