Skip to content

fix: use the python micro version to parse whl metadata in bzlmod #2793

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 3 commits into from
Apr 24, 2025

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Apr 20, 2025

Add <micro> version to the target platform. Instead of cpxy_os_cpu
the target platform string format becomes cpxy.z_os_cpu. This is a
temporary measure until we get a better API for defining target
platforms.

Summary:

  • test select_whls function needs to be tested to ensure that the
    whl selection is not impacted when we have the full version in the
    target platform.
  • download_only legacy whl code path in bzlmod needs further
    testing.
  • test whl_config_setting handling and config setting creation.
    The config settings in the hub repo should not use the full version,
    because from the outside, the whl is compatible with all micro
    versions of a given 3.<minor_version> of the Python interpreter.
    This means that the already documented config setting do not need to
    be changed.
  • pep508_deps tests for handling the full_python_version
    correctly.
  • pep508_deps tests for ensuring the default_abi is being
    handled correctly.

Fixes #2319

@aignas aignas force-pushed the fix/micro-version branch 2 times, most recently from fd89eca to 669dab4 Compare April 23, 2025 23:57
@aignas aignas mentioned this pull request Apr 24, 2025
@aignas aignas force-pushed the fix/micro-version branch 2 times, most recently from c86673f to d5f76c4 Compare April 24, 2025 00:17
Add `<micro>` version to the target platform. Instead of `cpxy_os_cpu`
the target platform string format becomes `cpxy.z_os_cpu`. This is a
temporary measure until we get a better API for defining target
platforms.

Summary:
- [x] test `select_whls` function needs to be tested to ensure that the
  whl selection is not impacted when we have the full version in the
  target platform.
- [ ] `download_only` legacy whl code path in `bzlmod` needs further
  testing.
- [x] Extra testing to ensure that the default version selection works
  correctly.
- [x] test `whl_config_setting` handling and config setting creation.
  The config settings in the hub repo should not use the full version,
  because from the outside, the whl is compatible with all `micro`
  versions of a given `3.<minor_version>` of the Python interpreter.
  This means that the already documented config setting do not need to
  be changed.
- [x] changelog.
- [x] `pep508_deps` tests for handling the `full_python_version`
  correctly.

Fixes bazel-contrib#2319
@aignas aignas force-pushed the fix/micro-version branch from d5f76c4 to dbc046e Compare April 24, 2025 00:21
@aignas aignas marked this pull request as ready for review April 24, 2025 00:21
@aignas aignas requested review from rickeylev and groodt as code owners April 24, 2025 00:21
@aignas
Copy link
Collaborator Author

aignas commented Apr 24, 2025

I will cherry pick this to the 1.4 release branch to fix #2815.

@aignas aignas enabled auto-merge April 24, 2025 12:59
@aignas aignas added this pull request to the merge queue Apr 24, 2025
Merged via the queue into bazel-contrib:main with commit 1e21dbd Apr 24, 2025
3 checks passed
@aignas aignas mentioned this pull request Apr 24, 2025
3 tasks
rickeylev pushed a commit that referenced this pull request Apr 24, 2025
)

Add `<micro>` version to the target platform. Instead of `cpxy_os_cpu`
the target platform string format becomes `cpxy.z_os_cpu`. This is a
temporary measure until we get a better API for defining target
platforms.

Summary:
- [x] test `select_whls` function needs to be tested to ensure that the
  whl selection is not impacted when we have the full version in the
  target platform.
- [ ] `download_only` legacy whl code path in `bzlmod` needs further
  testing.
- [x] test `whl_config_setting` handling and config setting creation.
  The config settings in the hub repo should not use the full version,
  because from the outside, the whl is compatible with all `micro`
  versions of a given `3.<minor_version>` of the Python interpreter.
  This means that the already documented config setting do not need to
  be changed.
- [x] `pep508_deps` tests for handling the `full_python_version`
  correctly.
- [x] `pep508_deps` tests for ensuring the `default_abi` is being
  handled correctly.

Fixes #2319

(cherry picked from commit 1e21dbd)
aignas added a commit to aignas/rules_python that referenced this pull request Apr 27, 2025
This has been fixed in the Starlark implementation in bazel-contrib#2793 and in this
PR I am backporting the changes to handle the full python version
target platform strings so that we can have the same behaviour for now.

At the same time I have simplified and got rid of the specialization
handling in the Python algorithm just like I did in the starlark, which
simplifies the tests and makes the algorithm more correct.

Work towards bazel-contrib#2830
github-merge-queue bot pushed a commit that referenced this pull request Apr 28, 2025
Handling of `python_full_version` correctly has been fixed in the
Starlark
implementation in #2793 and in this PR I am backporting the changes to
handle
the full python version target platform strings so that we can have the
same
behaviour for now.

At the same time I have simplified and got rid of the specialization
handling
in the Python algorithm just like I did in the starlark, which
simplifies the
tests and makes the algorithm more correct.

Summary:
* Handle `cp3x.y_os_arch` strings in the `platform.py`
* Produce correct strings when the `micro_version` is unset. Note, that
we use version `0` in evaluating but we use the default version in the
config setting. This is to keep compatibility with the current behaviour
when the target platform is not fully specified (which would be the case
for WORKSPACE users).
* Adjust the tests and the code to be more similar to the starlark impl.

Work towards #2830
aignas added a commit that referenced this pull request Apr 29, 2025
Handling of `python_full_version` correctly has been fixed in the
Starlark
implementation in #2793 and in this PR I am backporting the changes to
handle
the full python version target platform strings so that we can have the
same
behaviour for now.

At the same time I have simplified and got rid of the specialization
handling
in the Python algorithm just like I did in the starlark, which
simplifies the
tests and makes the algorithm more correct.

Summary:
* Handle `cp3x.y_os_arch` strings in the `platform.py`
* Produce correct strings when the `micro_version` is unset. Note, that
we use version `0` in evaluating but we use the default version in the
config setting. This is to keep compatibility with the current behaviour
when the target platform is not fully specified (which would be the case
for WORKSPACE users).
* Adjust the tests and the code to be more similar to the starlark impl.

Work towards #2830

(cherry picked from commit 9e613d5)
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.

[pypi] Support python micro version in env marker based dependency evaluation
2 participants