ueber-devel / ueberzug

Continuation of ueberzug
GNU General Public License v3.0
103 stars 6 forks source link

C extension building #14

Open eylles opened 1 year ago

eylles commented 1 year ago

@jstkdng to be honest i do feel tempted to just be lazy and take https://github.com/gokberkgunes/ueberzug-tabbed/pull/5 into this repo, adjust what needs to be adjusted and then try the many cmake to meson converters to see if what those spit out can be a good starting point for building the c extension... or just say screw it and use cmake for the time being but i want some feedback before i take the easy route.

jstkdng commented 1 year ago

I don't know if meson-python is be available (or supported) on all distros ueberzug is currently packaged or would be packaged in the future. Cmake on the contrary is available almost everywhere.

eylles commented 1 year ago

welp, that is good enough for me at the time, perhaps moving the c extension build to regular meson in the future since that is also a widely available tool and finally in another future move to meson-python if it somehow provides some packaging ergonomics sounds like a roadmap to me but i will give it some more time for anyone to chime in.

eli-schwartz commented 1 year ago

Which distros are you concerned about? meson-python is fairly widely available, and it's also trivial to package using stock templates for building and packaging wheels.

eylles commented 1 year ago

i will be honest, i'm clueless about the availability of meson-python but since a prototype for cmake exists i feel tempted to just use that let the build system be a secondary thing and give priority to have this and U++ on a comparable feature set, maybe even have the facility for application developers to know that if they support ueberzug x.x.x then U++ will just work™

eli-schwartz commented 1 year ago

The prototype in question needs some work in order to produce correct results anyway, and that probably means using scikit-build-core instead, which wraps cmake for you instead of using setup.py. Incidentally, scikit-build-core is less widely packaged than meson-python. ;)

If it was a straight comparison to cmake, you could run meson in setup.py with a custom MesonExtension class instead of CMakeExtension. But it's very easy to do without setup.py altogether:

project('ueberzug', 'c')

py = import('python').find_installation()

x_dep = [
  dependency('x11'),
  dependency('xext'),
  dependency('xres'),
]

py.extension_module(
    'X',
    'ueberzug/X/X.c',
    'ueberzug/X/Xshm.c',
    'ueberzug/X/display.c',
    'ueberzug/X/window.c',
    dependencies: x_dep,
    install: true,
    subdir: 'ueberzug',
)

install_subdir('ueberzug', install_dir: py.get_install_dir(), exclude_directories: 'X')

i will be honest, i'm clueless about the availability of meson-python

Due to naming differences, it ends up sorted into two groups, but: https://repology.org/project/python:meson-python/versions https://repology.org/project/meson-python/versions

Alpine, Arch, Debian, Fedora, Gentoo, OpenSUSE, Void...

jstkdng commented 1 year ago

I see it is fairly available, my bad. In the end, the CMakeLists.txt file I wrote can also be used to generate a compile_commands.json file which can be/was used to aid in the c-extension development (thanks to clangd), without installing. Either way, adding another dependency will inconvenience current maintainers so you'll have to choose. Can the compile_commands.json file be generate with meson-python?

The prototype in question needs some work in order to produce correct results anyway

I remember it producing correct results when I used it, it was a long time ago so maybe things changed?

eli-schwartz commented 1 year ago

Can the compile_commands.json file be generate with meson-python?

Both meson and cmake produce one, yes.

I remember it producing correct results when I used it, it was a long time ago so maybe things changed?

The compiled extension can be loaded by the Python interpreter -- but it lacks the "cpython-311-x86_64-linux-gnu" etc. part of the shared library extension, instead using a python2-style unversioned name. Normally, modern python offers a safeguard against accidentally importing extensions built for the wrong version of python into a different interpreter.

It's also assuming that the platform filename for a shared library is correct in the first place -- it is not correct e.g. on msys2, which follows the lead of "regular" Microsoft Windows and uses .pyd; it needs to get the value of EXT_SUFFIX.

jstkdng commented 1 year ago

Both meson and cmake produce one, yes.

I know meson can, but in this case in particular I mean, isn't meson-python for packaging? How did seebye even develop this thing without an lsp. No wonder the code is such a mess.

e.g. on msys2,

I don't think msys2 is even supported but I understand.

alright then, it's up to you now @eylles seeing as you'd be in charge of further developing ueberzug

eli-schwartz commented 1 year ago

I know meson can, but in this case in particular I mean, isn't meson-python for packaging?

Well yes, but then meson-python runs meson under the hood, which produces compile_commands.json -- no different from how with the cmake prototype, setup.py is used for packaging and runs cmake under the hood, and cmake produces compile_commands.json.

I suppose in this case you'd use pip install -e to install the project in editable mode, and then just point your editor at the build directory which meson-python creates. Hack on the code, let the editable hook automatically rebuild the extension, etc.

ionenwks commented 1 year ago

Just to +1 I'd like to see this use meson-python as I briefly suggested before in issue #9

Major packages are already using it (like scipy), and distros that don't provide it (yet) will eventually add it either way for these. So personally wouldn't feel too concerned about that.

eylles commented 1 year ago

well, reworking the c extension building is a goal for 18.3.0, building extensions and integrating build systems into setup.py is completely outside my field of knowledge thus snippets are very appreciated.