fix(pip): Pass --abi flag when downloading wheel for freethreaded python#3762
fix(pip): Pass --abi flag when downloading wheel for freethreaded python#3762meteorcloudy wants to merge 1 commit intobazel-contrib:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces logic to automatically append the correct --abi flag for freethreaded Python platforms when download_only is enabled. This is achieved by passing the download_only and python_version attributes through the hub builder to the requirement parser. A review comment suggests optimizing the implementation by avoiding redundant list copies and refining the check for existing --abi flags.
| plat_args = list(pip_args) | ||
| if download_only and plat.endswith("freethreaded") and python_version: | ||
| if not any([a.startswith("--abi") for a in plat_args]): | ||
| major, _, tail = python_version.partition(".") | ||
| minor, _, _ = tail.partition(".") | ||
| if major and minor: | ||
| plat_args.append("--abi=cp{}{}t".format(major, minor)) | ||
| options[plat] = plat_args |
There was a problem hiding this comment.
The current implementation creates a new list copy for every platform, even when no modifications are needed. This can be optimized by only copying the list if the conditions for adding the --abi flag are met. Additionally, the check for existing --abi flags can be made more precise to avoid matching other flags that might start with the same prefix (though unlikely in this context).
| plat_args = list(pip_args) | |
| if download_only and plat.endswith("freethreaded") and python_version: | |
| if not any([a.startswith("--abi") for a in plat_args]): | |
| major, _, tail = python_version.partition(".") | |
| minor, _, _ = tail.partition(".") | |
| if major and minor: | |
| plat_args.append("--abi=cp{}{}t".format(major, minor)) | |
| options[plat] = plat_args | |
| plat_args = pip_args | |
| if download_only and plat.endswith("freethreaded") and python_version: | |
| if not any([a == "--abi" or a.startswith("--abi=") for a in plat_args]): | |
| major, _, tail = python_version.partition(".") | |
| minor, _, _ = tail.partition(".") | |
| if major and minor: | |
| plat_args = list(pip_args) | |
| plat_args.append("--abi=cp{}{}t".format(major, minor)) | |
| options[plat] = plat_args |
When
pip downloadis used for fetching pip dependency, we need to pass--abiflag to fetch the correct wheel file for freethreaded python.Essentially making https://github.com/google-ml-infra/rules_ml_toolchain/blob/1c2352bde159ee14cc5050a871c437622fe25e1f/py/python_init_pip.bzl#L69-L71 unnecessary.