Skip to content

Enable support for source distribution if whl is compatible #2354

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Enable support for source distribution if whl is compatible #2354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c17ac20
Enable support for source distribution if whl is compatible
WeishiZ Mar 17, 2025
b6aaf31
add comment; run pre-commit
WeishiZ Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion metaflow/plugins/pypi/conda_environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ def _path(url, local_path):
unquote(base), prefix, file
)
else:
return urlparse(url).netloc + urlparse(url).path
parsed_url = urlparse(url)
base = os.path.dirname(parsed_url.path)
# Using file extension from local_path instead of url
file = os.path.basename(local_path)
return parsed_url.netloc + os.path.join(base, file)

local_packages = {
url: {
Expand Down
41 changes: 34 additions & 7 deletions metaflow/plugins/pypi/pip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def solve(self, id_, packages, python, platform):
cmd = [
"install",
"--dry-run",
"--only-binary=:all:", # only wheels
"--prefer-binary",
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you sure that this works -

command '/Users/savin/.metaflowconfig/micromamba/bin/micromamba run --prefix /Users/savin/micromamba/envs/metaflow/osx-arm64/c9a70c6534aed5f pip3 --disable-pip-version-check --no-color --no-input --isolated install --dry-run --prefer-binary --upgrade-strategy=only-if-needed --target=/var/folders/jt/vd16shsx4ts47bvv4fnwscmc0000gn/T/tmp2vb9d3fx --report=/var/folders/jt/vd16shsx4ts47bvv4fnwscmc0000gn/T/tmp2vb9d3fx/report.json --progress-bar=off --quiet --abi cp313 --abi abi3 --abi none --platform macosx_15_0_arm64 --platform macosx_15_0_universal2 --platform macosx_14_0_arm64 --platform macosx_14_0_universal2 --platform macosx_13_0_arm64 --platform macosx_13_0_universal2 --platform macosx_12_0_arm64 --platform macosx_12_0_universal2 --platform macosx_11_0_arm64 --platform macosx_11_0_universal2 --platform macosx_10_16_universal2 --platform macosx_10_15_universal2 --platform macosx_10_14_universal2 --platform macosx_10_13_universal2 --platform macosx_10_12_universal2 --platform macosx_10_11_universal2 --platform macosx_10_10_universal2 --platform macosx_10_9_universal2 --platform macosx_10_8_universal2 --platform macosx_10_7_universal2 --platform macosx_10_6_universal2 --platform macosx_10_5_universal2 --platform macosx_10_4_universal2 --platform any requests>=2.21.0 boto3>=1.14.0 fire==0.7.0' returned error (1)
    ERROR: When restricting platform and interpreter constraints using --python-version, --platform, --abi, or --implementation, either --no-deps must be set, or --only-binary=:all: must be set and --no-binary must not be set (or must be set to :none:).

Copy link
Author

Choose a reason for hiding this comment

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

I see the issue now. This is indeed a problem.
In our internal fork we have --no-deps added that made this work. We have --no-deps since we use poetry to resolve dependencies first and then copy the resolved ones to @pypi. (will send you more details on the internal plugin in a separate thread.)

This error suggests that pip install --platform xxx --abi xxx has to use --no-deps if want to support non-binary.

ERROR: When restricting platform and interpreter constraints using --python-version, --platform, --abi, or --implementation, either --no-deps must be set, or --only-binary=:all: must be set and --no-binary must not be set (or must be set to :none:).

I'll need to explore more to understand why pip is restrictive about building transitive deps from source.

Possible direction could be to use a separate standalone resolver, or remove the pip platform options (--platform xxx --abi xxx) if it's a sdist. As long as it can ensure the built wheels are correctly tagged, then pip.py will decide if it's compatible with the target platform.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, neither of those approaches will work while solving for the environment. There is a possibility to make it work when the local_platform is the same as the target_platform - but outside of that - for cross-platform or cross-python solves, we can't really solve it by depending on pip alone - it is tricky because one needs to build the source to generate downstream dependency graph - which may not be always possible. I am hopeful that there is some approach that allows us to include well-behaved source distributions, given that git dependencies work well already.

"--upgrade-strategy=only-if-needed",
"--target=%s" % tmp_dir,
"--report=%s" % report,
Expand All @@ -87,11 +87,18 @@ def solve(self, id_, packages, python, platform):
*(["--index-url", custom_index_url] if custom_index_url else []),
*(
chain.from_iterable(
product(["--extra-index-url"], set(extra_index_urls))
product(
["--extra-index-url"], iter(dict.fromkeys(extra_index_urls))
)
)
),
# Ordered dedup so pip command sees the `--platform abc` options in deterministic order every time.
*(chain.from_iterable(product(["--abi"], iter(dict.fromkeys(abis))))),
*(
chain.from_iterable(
product(["--platform"], iter(dict.fromkeys(platforms)))
)
),
*(chain.from_iterable(product(["--abi"], set(abis)))),
*(chain.from_iterable(product(["--platform"], set(platforms)))),
# *(chain.from_iterable(product(["--implementations"], set(implementations)))),
]
for package, version in packages.items():
Expand Down Expand Up @@ -131,6 +138,19 @@ def _format(dl_info):
# used to deduplicate the storage location in case wheel does not
# build with enough unique identifiers.
res["hash"] = vcs_info["commit_id"]
# Handle non-VCS sources
else:
archive_info = dl_info.get("archive_info")
# Extracting hash from archive_info or generating from URL
if archive_info:
hash_value = archive_info.get("hash")
if hash_value:
res["hash"] = hash_value.split("=", 1)[-1]
else:
import hashlib

hash_obj = hashlib.sha256(res["url"].encode())
res["hash"] = hash_obj.hexdigest()
return res

with open(report, mode="r", encoding="utf-8") as f:
Expand Down Expand Up @@ -223,11 +243,18 @@ def _build(key, package):
*(["--index-url", custom_index_url] if custom_index_url else []),
*(
chain.from_iterable(
product(["--extra-index-url"], set(extra_index_urls))
product(
["--extra-index-url"], iter(dict.fromkeys(extra_index_urls))
)
)
),
# Ordered dedup so pip command sees the `--platform abc` options in deterministic order every time.
*(chain.from_iterable(product(["--abi"], iter(dict.fromkeys(abis))))),
*(
chain.from_iterable(
product(["--platform"], iter(dict.fromkeys(platforms)))
)
),
*(chain.from_iterable(product(["--abi"], set(abis)))),
*(chain.from_iterable(product(["--platform"], set(platforms)))),
# *(chain.from_iterable(product(["--implementations"], set(implementations)))),
]
packages = [package for package in packages if not package["require_build"]]
Expand Down
Loading