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

There should be a cmake option to use pre-installed cmake #511

Open
yurivict opened this issue Jun 22, 2024 · 4 comments
Open

There should be a cmake option to use pre-installed cmake #511

yurivict opened this issue Jun 22, 2024 · 4 comments

Comments

@yurivict
Copy link

We already have cmake on FreeBSD.
Installing cmake againhas these problems:

  1. It causes conflicts withe cmake package
  2. It wastes CPU to build cmake when it is already present.

Please add a cmake option, for example USE_EXTERNAL_CMAKE, that would just launch 'cmake' assuming that it already installed.

@henryiii
Copy link
Contributor

Could you guarantee that the CMake version exactly matched the package version? How would you ensure that someone wouldn't try to distribute this "empty" wheel? Or that they versions would continue to be in sync, what if you update the system cmake but not this "empty" package?

Opt-in isn't as bad, but it's still problematic. And it's the wrong solution.

The correct solution is simple: don't request the cmake PyPI package if you already have provided CMake via a different mechanism. You wouldn't ask for conda-forge metadata to be included in FreeBSD, so why ask for PyPI metadata?

If it's because people are putting "cmake" in build-system.requires. then just disable verification. And request that they use a proper system like scikit-build-core or a wrapper around get_requries_for_build_wheel that checks for a system cmake before requesting PyPI cmake.

And this is a duplicate of #80.

@LecrisUT
Copy link

Other Fedora packagers would still want to investigate packaging python-cmake proper, so I've investigated this approach. Here is what I've tried to implement:

  • Download the python-cmake source from whatever is the latest version of this repo
  • Patch out the CMAKE_DATA logic and replace with hard-coded CMAKE_DATA = "/usr"
    cmake_executable_path = None
    cmake_files = distribution("cmake").files
    assert cmake_files is not None, "This is the cmake package so it must be installed and have files"
    for script in cmake_files:
    if str(script).startswith("cmake/data/bin/cmake"):
    resolved_script = Path(script.locate()).resolve(strict=True)
    cmake_executable_path = resolved_script.parents[1]
    break
    CMAKE_DATA = str(cmake_executable_path) if cmake_executable_path else None
  • Modify the project.version and tests/test_cmake.py to use the CMake version from the system (%{_cmake_version})
  • Delete the projects.scripts so that /usr/bin/* take precedence
  • Pass --config-settings=wheel.cmake=false to the pip install command so that it only installs the python files

I am not convinced if this is a good approach, so I will keep it open for review. Technically speaking, this still works on Fedora side because the python3dist(*) is populated from the pyproject.toml metadata that is patched to be in sync with the installed CMake

$ rpm -q --provides python3-cmake-3.30.2-1.fc42.noarch.rpm 
python-cmake = 3.30.2-1.fc42
python3-cmake = 3.30.2-1.fc42
python3.13-cmake = 3.30.2-1.fc42
python3.13dist(cmake) = 3.28.3
python3dist(cmake) = 3.28.3

and this resolves rebuild burden when python files are changed and the discrepancy in the version of cmake and python-cmake. But still I am not sure that we are not overlooking something in this approach, so feel free to comment on this.

You can check the spec and srpm for the patch and steps above. I am not sure how readily the srpm can be decompressed on non-linux systems, but I am able to simply open it with Ark (KDE archive manager) and I can navigate the files within, and similary for the final binary rpm.

@henryiii
Copy link
Contributor

As a maintainer of this package, I strongly advise against this. This has come up numerous times, and the answer has always been redistributing a redistribution is wrong. See the extensive discussions on the ninja redistribution at scikit-build/ninja-python-distributions#127 for more context. A few quick points:

  • This can't be done with some redistributions; like conda and homebrew. It creates links between the cmake package and the various Python packages those provide. The "cmake" or "python-cmake" package should not pull in python3.9, python3.10, etc. And you don't want to or can't have a package for each Python.
  • This should not be done for every package that has a PyPI redistribution - and there are a lot of them! cmake, ninja, pkg-info, gcc, clang-format, zig, clang-tidy, node-js, just to name a few.
  • This is manipulating and lying about this package. If we made a change for cmake 3.21.3, then a user of this package should be able to rely on that change happening in 3.21.3. But the approach above disconnects the package version from the CMake version.
  • Downstream packages aren't relying on this package already! They are mostly available on conda, and conda doesn't redistribute this package, so they (correctly) use the "cmake" command if the cmake import isn't available. In fact, I don't really like the cmake import wrapper, but it's the simplest and most reliable way to make sure you get the correct CMake if it's been pip installed into the same environment.
  • The main issue is metadata claims this is needed when people put it in their requirements, but that's incorrect, and there's both an upstream fix (use something like we use to add it if missing), and a simple downstream fix - don't validate build-system.requires! In pip, it's that way by default, and in in build, you pass -nx instead of -n.
  • This will hurt efforts to standardize declaring system dependencies in Python, namely PEP 725. This is trying to add a way to describe these sorts of dependencies.

@LecrisUT
Copy link

I'm all up for recommending downstream distros against packaging this, and I try to link the conversations back-and-forth on this topic. This part came up on the matrix channel w.r.t. to the topic of "But wouldn't packaging the python files be a good enough solution", so I've done it in a way that would take into consideration as many nuances as possible so that most topics can be explored. Try to bear with it a little longer.

It creates links between the cmake package and the various Python packages those provide.

Indeed I believe that is generally a problem. In Fedora there is only one python version for packages, so technically that is not a concern, but I also don't like it because it triggers unnecessary rebuildsdown the chain. How I've tried to address this is to keep cmake and python-cmake as completely independent packages, except for the python-cmake being dependent on cmake and it masking the python metadata version with the imported cmake ones (the python3dist in the snippet of my last comment).

This should not be done for every package that has a PyPI redistribution - and there are a lot of them!

Agreed, and in most cases it is even not possible to do so. We do agree that creating the metadata package was unnecessary, and it will likely not happen for other such packages.

If we made a change for cmake 3.21.3, then a user of this package should be able to rely on that change happening in 3.21.3.

Understandable, I mostly wanted to show the workflow and how the versioning works. If it were in production the versioning dependency would be inverted, i.e. python-cmake == cmake (i.e. we download the source of python-cmake that matches current cmake version), except for the additional patch versions in python-cmake, and if rc cmake lands in Fedora, than python-cmake will pretend to be that higher version since pypi version does not distribute it.

don't validate build-system.requires!

That we can't do, because it we use it to dynamically import the distro package dependencies. Instead what should be done is to just patch out the requirement from pyproject.toml.

This will hurt efforts to standardize declaring system dependencies in Python, namely PEP 725. This is trying to add a way to describe these sorts of dependencies.

Perfect point, hopefully this would be a convincing argument for removing the current fake package. Also I believe there are no voices from Fedora packagers on that thread? I will try to get some people to chime in on that one.

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

3 participants