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

Extract metadata from cmake #172

Open
LecrisUT opened this issue Jan 15, 2023 · 14 comments
Open

Extract metadata from cmake #172

LecrisUT opened this issue Jan 15, 2023 · 14 comments

Comments

@LecrisUT
Copy link
Collaborator

Currently this project considers the Python build to be the main controller, but many projects, especially when the python api is just a wrapper, have the cmake as the main build workflow. Therefore we should add more functionalities to have the main information in cmake. This can be achieved by using dynamic in the pyproject.toml and have the builder scikit-build-core handle the logic of how to import them. Maybe this implementation is a good starting point to implement this.

Some metadata that can be set from the project() cmake function:

  • version
  • name
  • description
  • url
@henryiii
Copy link
Collaborator

You cannot set name dynamically, this is disallowed by PEP 621. I'm not sure URL would be helpful either, as you have a dict of them and there's no "default" one. Description is supposed to be one line, so duplicating that isn't that big of a deal - this only leaves version. That would be very nice to have - see #116. You need CMake to properly compute this, but I think we have get_requires_for_build_* in the various places we'd need to produce a version, and we already have parsing tools for the FileAPI of CMake, so we could implement this (and description, and maybe URL if we have something reasonable to map it to).

Meson-Python has a non-PEP 621 mode where you can read all of this info from Meson. I'm not in favor of this, it complicates the backend a lot and makes adding the other metadata much harder (since you can't use PEP 621 fields in that case) - realistic projects need these other fields, like requires-python and such.

@LecrisUT
Copy link
Collaborator Author

Would be helpful to also make it expandable. E.g. the project only defines version 1.4.2, while -rc1 are only defined in the git via tags. Then we could make it conditional, if there is a release tag, use that as the field, otherwise use the cmake defined. That way we would have feature parity with github repo and pypi uploaded version and also other pip installs do not complain of unsupported versions when we develop locally without git tags.

@LecrisUT
Copy link
Collaborator Author

With the #197 merged, even if we do not have a cmake hook, at least it is possible to unify the version configuration to be determined by git tags. I have a small project to dynamically set the cmake version according to .git_archival.txt so that it is compatible with setuptools_scm. Would it make sense to link to that documentation as a supported design?

@henryiii
Copy link
Collaborator

Sure, links are fine (and encouraged). I think we should be able to add a cmake hook soon as well, I plan to try to write one before 0.3.0.

@JeanChristopheMorinPerso

Hello, I will have a need for this in MaterialX. More particularly, the pyproject.toml version should be set based on the value in the main CMakeList.txt file.

I took a quick look and it seems like this is not actually possible, even with a custom plugin? It would require the "configuration" step to be run before the plugins, which I'm not sure is the case right now. Unless I misunderstand something.

@JeanChristopheMorinPerso

Or is the plan to regex our way out and simply read the CMake file and get the version using a regex?

@JeanChristopheMorinPerso

I ended up writing a plugin:

"""
This is a custom scikit-build-core plugin that will
fetch the MaterialX version from the CMake project.
"""
import os
import tempfile
import subprocess
from pathlib import Path
from typing import FrozenSet, Dict, Optional, Union, List

from scikit_build_core.file_api.query import stateless_query
from scikit_build_core.file_api.reply import load_reply_dir


def dynamic_metadata(
    fields: FrozenSet[str],
    settings: Optional[Dict[str, object]] = None,
) -> Dict[str, Union[str, Dict[str, Optional[str]]]]:
    print("mtx_skbuild_plugin: Computing MaterialX version from CMake...")

    if fields != {"version"}:
        msg = "Only the 'version' field is supported"
        raise ValueError(msg)

    if settings:
        msg = "No inline configuration is supported"
        raise ValueError(msg)

    current_dir = os.path.dirname(__file__)

    with tempfile.TemporaryDirectory() as tmpdir:
        # We will use CMake's file API to get the version
        # instead of parsing the CMakeLists files.

        # First generate the query folder so that CMake can generate replies.
        reply_dir = stateless_query(Path(tmpdir))

        # Run cmake (configure). CMake will generate a reply automatically.
        try:
            subprocess.run(
                [
                    "cmake",
                    "-S",
                    current_dir,
                    "-B",
                    tmpdir,
                    "-DMATERIALX_BUILD_SHARED_LIBS=OFF",
                    "-DMATERIALX_BUILD_PYTHON=OFF",
                    "-DMATERIALX_TEST_RENDER=OFF",
                    "-DMATERIALX_BUILD_TESTS=OFF",
                    "-DMATERIALX_INSTALL_PYTHON=OFF",
                    "-DMATERIALX_BUILD_RENDER=OFF",
                ],
                stdout=subprocess.PIPE,
                stderr=subprocess.STDOUT,
                check=True,
                text=True,
            )
        except subprocess.CalledProcessError as exc:
            print(exc.stdout)
            raise RuntimeError(
                "Failed to configure project to get the version"
            ) from exc

        # Get the generated replies.
        index = load_reply_dir(reply_dir)

        # Get the version from the CMAKE_PROJECT_VERSION variable.
        entries = [
            entry
            for entry in index.reply.cache_v2.entries
            if entry.name == "CMAKE_PROJECT_VERSION"
        ]

        if not entries:
            raise ValueError("Could not find MaterialX version from CMake project")

        if len(entries) > 1:
            raise ValueError("More than one entry for CMAKE_PROJECT_VERSION found...")

    version = entries[0].value
    print("mtx_skbuild_plugin: Computed version: {0}".format(version))

    return {"version": version}


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

@Freed-Wu
Copy link
Contributor

If we can set dynamic = ["version"], then get version from CMakeLists.txt like meson-python, it will be great.

@henryiii
Copy link
Collaborator

There's a regex plugin built in for this and it's much faster than setting up a CMake run would be.

@Freed-Wu
Copy link
Contributor

Freed-Wu commented Nov 30, 2024

Another method is write version to CMakeLists.txt, then genereate __init__.py to contains __version__ = "XXX". Then

[project]
dynamic = ["version"]

# LookupError: pyproject.toml does not contain a tool.setuptools_scm section
[tool.setuptools_scm]

[tool.setuptools_scm] just make pyproject.toml happy.

@eli-schwartz
Copy link

There's a regex plugin built in for this and it's much faster than setting up a CMake run would be.

If you're doing a cmake run either way then I can't see how it matters in terms of speed. But the advantage of doing a cmake run is precisely that you don't have to deal with regexes.

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.

Regular expressions are almost always a tradeoff of some sort. If it turns out that you can anyways extract the info a better way through something that already exists (a previous cmake run) then really, whyever not?

Another method is write version to CMakeLists.txt, then genereate __init__.py to contains __version__ = "XXX". Then

@Freed-Wu,

I don't really understand this though. setuptools-scm reads version information from git or PyPA metadata, and writes to an __init__.py, and you need a bit more to get scikit-build-core to use setuptools-scm than just adding tool.setuptools_scm?

Furthermore, if it requires running cmake to generate your __init__.py then we are back to needing to run cmake, at which point it might as well be done properly via the FileAPI with explicit support for only configuring the project once and using it for both metadata and compilation.

@Freed-Wu
Copy link
Contributor

Freed-Wu commented Dec 1, 2024

Regular Expression

I heard cmake will generate some json data in build/ after cmake -Bbuild. Maybe read the json can avoid RE.

Another method

Refer astyle-wheel.

@henryiii
Copy link
Collaborator

henryiii commented Dec 1, 2024

The version-collection CMake run has to be a separate run, since it's collecting metadata information before running CMake properly (which has access to the version!). After you configure is too late (currently). If you are just collecting metadata via the prepare metadata hooks, CMake does not need to run at all if you use the regex.

Long term, it might be possible to cleverly integrate it, and also collect other information too, but for now, the simplest and fastest way is to use the regex plugin. And you can implement it as a custom plugin.

@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Dec 2, 2024

Can CMake File-api run on the first configure? We could probably combine those.

Another option is that we could make the version undefined from scikit-build-core, but then populate it after the build and before the wheel creation. Is the version needed for any other steps before that?

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

5 participants