Skip to content

Conversation

agriyakhetarpal
Copy link
Member

@agriyakhetarpal agriyakhetarpal commented Apr 10, 2025

This PR adds a new configuration variable in pyodide config called ignored_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 the ignored_build_requirements value in a [tool.pyodide.build] TOML table in pyproject.toml to set to a value of their choice, and the pyodide 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

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Apr 10, 2025

Argh, there's a problem with a circular import here because of the get_host_build_flag("AVOIDED_BUILD_REQUIREMENTS") calculation that is performed:

pyodide config get avoided_build_requirements

returns

Traceback (most recent call last):
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/venv/bin/pyodide", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/venv/lib/python3.12/site-packages/pyodide_cli/app.py", line 111, in main
    register_plugins()
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/venv/lib/python3.12/site-packages/pyodide_cli/app.py", line 81, in register_plugins
    plugins = {ep.name: (ep.load(), ep) for ep in eps}
                         ^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/lib/python3.12/importlib/metadata/__init__.py", line 205, in load
    module = import_module(match.group('module'))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/lib/python3.12/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 995, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/pyodide_build/cli/build.py", line 13, in <module>
    from pyodide_build.build_env import (
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/pyodide_build/build_env.py", line 18, in <module>
    from pyodide_build.config import ConfigManager, CrossBuildEnvConfigManager
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/pyodide_build/config.py", line 14, in <module>
    from pyodide_build.pypabuild import AVOIDED_REQUIREMENTS
  File "/Users/agriyakhetarpal/Desktop/pyodide-build/pyodide_build/pypabuild.py", line 18, in <module>
    from pyodide_build.build_env import (
ImportError: cannot import name 'get_build_flag' from partially initialized module 'pyodide_build.build_env' (most likely due to a circular import) (/Users/agriyakhetarpal/Desktop/pyodide-build/pyodide_build/build_env.py)

@ryanking13
Copy link
Member

Thanks for working on this!

Argh, there's a problem with a circular import here because of the get_host_build_flag("AVOIDED_BUILD_REQUIREMENTS") calculation that is performed:

I think we can bypass it by importing AVOIDED_BUILD_REQUIREMENTS inside install_reqs or in other function.

@ryanking13
Copy link
Member

ryanking13 commented Apr 10, 2025

BTW, I think the "avoided" is very poor name (I named it 🙄). How about renaming it to something like "skipped" or "ignored" or "filtered"?

agriyakhetarpal added a commit that referenced this pull request Apr 10, 2025
@agriyakhetarpal
Copy link
Member Author

Thanks, @ryanking13!

BTW, I think the "avoided" is very poor name (I named it 🙄). How about renaming it to something like "skipped" or "ignored" or "filtered"?

I agree with you. I've changed it to "ignored" for now. Though, I was thinking we could use "excluded"?

Copy link
Member

@ryanking13 ryanking13 left a 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),
Copy link
Member

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.

Copy link
Member Author

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.

@agriyakhetarpal agriyakhetarpal force-pushed the override-avoided-requirements branch from fed25c3 to d3e5c34 Compare April 12, 2025 06:23
@agriyakhetarpal
Copy link
Member Author

Thanks for the review, @ryanking13!

@agriyakhetarpal agriyakhetarpal merged commit da2978a into main Apr 12, 2025
18 checks passed
@agriyakhetarpal agriyakhetarpal deleted the override-avoided-requirements branch April 12, 2025 06:51
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.

Allow overriding avoided build requirements for package builds

2 participants