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

add liburing-2.0 wrap #127

Merged
merged 1 commit into from
Aug 26, 2021
Merged

Conversation

fischerling
Copy link
Contributor

@fischerling fischerling commented Aug 5, 2021

The meson build files for liburing-2.0 are taken from https://github.com/fischerling/liburing/tree/2.0-meson
and are based upon axboe/liburing#297.

I tested the wrap using the method described in the README and meson version 0.59:

$meson setup builddir -Dwraps=liburing

The Meson build system
Version: 0.59.0
Source dir: /home/muhq/code/wrapdb
Build dir: /home/muhq/code/wrapdb/builddir
Build type: native build
Project name: wrapdb
Project version: undefined
Host machine cpu family: x86_64
Host machine cpu: x86_64

Executing subproject liburing

liburing| Project name: liburing
liburing| Project version: 0.7
liburing| C compiler for the host machine: ccache cc (gcc 11.1.0 "cc (GCC) 11.1.0")
liburing| C linker for the host machine: cc ld.bfd 2.36.1
liburing| C++ compiler for the host machine: ccache c++ (gcc 11.1.0 "c++ (GCC) 11.1.0")
liburing| C++ linker for the host machine: c++ ld.bfd 2.36.1
liburing| Run-time dependency threads found: YES
liburing| Compiler for C supports arguments -Wstringop-overflow: YES
liburing| Compiler for C supports arguments -Warray-bounds: YES
liburing| Checking for type "__kernel_rwf_t" : YES
liburing| Checking whether type "struct __kernel_timespec" has members "tv_sec", "tv_nsec" : YES
liburing| Checking if "open_how" compiles: NO
liburing| Checking if "statx" compiles: YES
liburing| Checking if "C++" compiles: YES
liburing| Checking for type "ucontext_t" : YES
liburing| Configuring config-host.h using configuration
liburing| Configuring compat.h using configuration
liburing| Build targets in project: 2
liburing| Subproject liburing finished.

Program meson found: YES (/sbin/meson)
Build targets in project: 3

liburing 0.7

  Directories
    bindir  : bin
    libdir  : lib
    datadir : share

  Configuration
    examples: NO
    tests   : NO

wrapdb undefined

  Subprojects
    liburing: YES

Found ninja-1.10.2 at /sbin/ninja

$ meson compile -C builddir
ninja: Entering directory `builddir'
[6/6] Linking target subprojects/liburing-liburing-2.0/src/liburing.so.2.0.0

Closes #126 .

@eli-schwartz
Copy link
Member

@xclaesse the CI should NOT pass here, because releases.json never got updated. I think maybe this is because comparing tags to releases.json doesn't actually check when new wraps are added without the latter?

@eli-schwartz
Copy link
Member

ping?

@fischerling
Copy link
Contributor Author

ping?

Upstream has not answered my question yet.

I would merge it as it is using 0.7 as project version and 2.0 as the library
version.
As far as I know this allows resolving dependencies as expected in meson because the
library version overrides the project version in the declared dependency object.

@jpakkane
Copy link
Member

Which version number do they end up writing in the .pc file? I could not immediately tell all their configure code is custom and handwritten.

@fischerling
Copy link
Contributor Author

Which version number do they end up writing in the .pc file? I could not immediately tell all their configure code is custom and handwritten.

Using the liburing-2.0 git tag they will write 0.7 into the pkgconfig file which is wrong and was fixed by 1815337f7dbb.
After seeing this commit I think it is correct to use 2.0 everywhere and drop the project and library version separation

@fischerling fischerling force-pushed the add-liburing-wrap branch 3 times, most recently from bf0dfdb to 7964f17 Compare August 23, 2021 09:28
@eli-schwartz
Copy link
Member

Needs to be rebased on master, sorry. :D

@fischerling fischerling force-pushed the add-liburing-wrap branch 2 times, most recently from dfca1c0 to e2bcbd9 Compare August 23, 2021 18:16
@fischerling
Copy link
Contributor Author

Needs to be rebased on master, sorry. :D

No problem ;)

@fischerling
Copy link
Contributor Author

I expect the CI to still fail because the C++ code included in the meson.build file to check certain compiler features include tabs.
Which is detected by sanity_checks.py and apparently not allowed in meson code.
Should I replace the tabs with spaces? In the C++ code?
This is probably way easier than modifying sanity_checks.py to allow tabs in strings.

@fischerling
Copy link
Contributor Author

fischerling commented Aug 23, 2021

I expect the CI to still fail because the C++ code included in the meson.build file to check certain compiler features include tabs.
Which is detected by sanity_checks.py and apparently not allowed in meson code.
Should I replace the tabs with spaces? In the C++ code?
This is probably way easier than modifying sanity_checks.py to allow tabs in strings.

I have removed the tabs from the C code strings.

@eli-schwartz
Copy link
Member

The alternative might involve adding an AST parser, so yeah, that's a lot easier no matter how meh the error is in this highly specific case.

@fischerling
Copy link
Contributor Author

fischerling commented Aug 23, 2021

Seems like I missed some tabs. Sorry for all those incremental fixes and force pushes

@eli-schwartz
Copy link
Member

eli-schwartz commented Aug 23, 2021

And as I've merged another PR ahead of yours, releases.json is clashing again... sorry to make you rebase yet again. :D

EDIT: I'll take responsibility for that rebase.

@eli-schwartz
Copy link
Member

subprojects/packagefiles/liburing/src/include/liburing/compat.h.in seems unfortunate, actually pretty curious why upstream doesn't use a template with a simple sed to replace e.g. #if @HAS__KERNEL_TIMESPEC@ with #if 1

@jpakkane
Copy link
Member

The "typical" way of doing these is that instead of an foo.h.in, you have a foo.h.meson. This way it won't clash with the existing configuration template file and you can add any needed changes there. Maybe we should loosen the check and also permit .h.in files?

@fischerling
Copy link
Contributor Author

The "typical" way of doing these is that instead of an foo.h.in, you have a foo.h.meson. This way it won't clash with the existing configuration template file and you can add any needed changes there. Maybe we should loosen the check and also permit .h.in files?

Upstream has no template file they generate the header directly in their custom configure script.
But I have no problem with changing it from compat.h.in to compat.h.meson if that is the typical naming scheme.
Just let me know ;)

@jpakkane
Copy link
Member

That is the current scheme, yes. Changing that should make this pass CI.

The meson build files for liburing-2.0 are taken from
https://github.com/fischerling/liburing/tree/2.0-meson
and are based upon
axboe/liburing#297
@fischerling
Copy link
Contributor Author

fischerling commented Aug 25, 2021

That is the current scheme, yes. Changing that should make this pass CI.

Done :)

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.

new wrap: liburing
3 participants