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

openexr_skbuild_plugin.py adds an unnecessary cmake build requirement #1957

Open
mgorny opened this issue Jan 16, 2025 · 0 comments · May be fixed by #1958
Open

openexr_skbuild_plugin.py adds an unnecessary cmake build requirement #1957

mgorny opened this issue Jan 16, 2025 · 0 comments · May be fixed by #1958

Comments

@mgorny
Copy link

mgorny commented Jan 16, 2025

def get_requires_for_dynamic_metadata(
_settings: Optional[Dict[str, object]] = None,
) -> List[str]:
return ["cmake"]

This code is unconditionally adding a dependency on cmake PyPI project. This is inconsistent with how scikit-build-core operates, as it prefers using system CMake and adds the dependency only if it can't find a working version. However, the plugin effectively forces it to use the version from PyPI which is less portable, as it lacks downstream patching.

Given that scikit-build-core requires CMake by its purpose, I don't really understand why the plugin would need to add a dependency in the first place. And if you really to do it, I think it would be better to use the same approach as scikit-build-core does, i.e.:

https://github.com/scikit-build/scikit-build-core/blob/4fbd3b702db5157fa8c742b6c799a60adfb038e5/src/scikit_build_core/build/__init__.py#L154-L164

mgorny added a commit to mgorny/openexr that referenced this issue Jan 20, 2025
Remove the duplicate `cmake` dependency that was added by
`openexr_skbuild_plugin.py`.  Scikit-build-core is adding a dependency
on CMake if necessary itself, and adding one unconditionally has a side
effect of installing a local PyPI version of CMake that overrides
the system CMake that scikit-build-core would be using instead.  This
can be particularly problematic when downstream patching of CMake
is required on the system in question.

Fixes AcademySoftwareFoundation#1957

Signed-off-by: Michał Górny <[email protected]>
@mgorny mgorny linked a pull request Jan 20, 2025 that will close this issue
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 a pull request may close this issue.

1 participant