-
Notifications
You must be signed in to change notification settings - Fork 22
Allow overriding the list of avoided build requirements #187
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
Conversation
Argh, there's a problem with a circular import here because of the
returns
|
Thanks for working on this!
I think we can bypass it by importing |
BTW, I think the "avoided" is very poor name (I named it 🙄). How about renaming it to something like "skipped" or "ignored" or "filtered"? |
Thanks, @ryanking13!
I agree with you. I've changed it to "ignored" for now. Though, I was thinking we could use "excluded"? |
4d9c398
to
356a420
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks agriya! Maybe we can think about the better structure to handle circular dependencies later, for now it looks good to me.
"default_cross_build_env_url": "", | ||
"xbuildenv_path": "", | ||
# A list of PEP508 build-time requirements to be ignored when building a wheel | ||
"ignored_build_requirements": " ".join(BASE_IGNORED_REQUIREMENTS), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(no action requird for this PR)
It would be great if we could support List types here. The question is how to support the list type in an env variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree – I faced this problem when implementing it. I feel there is no neat option to do it in this case, as cibuildwheel
also largely suffers from the same limitation.
Co-authored-by: Gyeongjae Choi <[email protected]>
fed25c3
to
d3e5c34
Compare
for more information, see https://pre-commit.ci
Thanks for the review, @ryanking13! |
This PR adds a new configuration variable in
pyodide config
calledignored_build_requirements
, which accepts whitespace-separated normalised dependency names, which will be ignored during the wheel build procedure. The default value is"patchelf oldest-supported-numpy"
.Users may now utilise the
IGNORED_BUILD_REQUIREMENTS
environment variable or theignored_build_requirements
value in a[tool.pyodide.build]
TOML table inpyproject.toml
to set to a value of their choice, and thepyodide config get ignored_build_requirements
CLI invocation to retrieve them.As a by-product of this PR, the
ConfigManager
is now loaded more lazily in order to avoid circular imports across our build pipeline.Closes #184