Skip to content
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

remove_avoided_requirements() should check for the full package name instead of a substring #100

Open
tttapa opened this issue Feb 17, 2025 · 3 comments

Comments

@tttapa
Copy link

tttapa commented Feb 17, 2025

In the current implementation, the remove_avoided_requirements function removes any packages that contain the substring cmake or patchelf in their name.
As a result, building packages using the py-build-cmake backend is impossible, since pyodide-build refuses to install it (because of the cmake in the package name).

def remove_avoided_requirements(
requires: set[str], avoided_requirements: set[str] | list[str]
) -> set[str]:
for reqstr in list(requires):
req = Requirement(reqstr)
for avoid_name in set(avoided_requirements):
if avoid_name in req.name.lower():
requires.remove(reqstr)
return requires

I suggest using avoid_name == req.name.lower() instead of avoid_name in req.name.lower().

@hoodmane
Copy link
Member

We tried to fix this before:
#60

Turns out it breaks. It requires some further investigation but probably the correct code here wouldn't be that much more complicated.

@tttapa
Copy link
Author

tttapa commented Feb 18, 2025

My bad, I only searched the issues, not the PRs.

The CI failure you mentioned seems unrelated to this specific change, though.
The circleci workflow fails because of ValidationError: 1 validation error for MetaConfig: requirements.constraint. Extra inputs are not permitted, which appears to be caused by the fact that your pyodide-build fork does not yet include this commit: b8bd8d8

Could you rebase your PR and try again?
Thanks!

@hoodmane
Copy link
Member

Well it's good to have an issue for this too. Thanks for the report!

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

No branches or pull requests

2 participants