-
Notifications
You must be signed in to change notification settings - Fork 411
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 meson based buildsystem v2 #622
base: master
Are you sure you want to change the base?
Conversation
d52fd0c
to
144a971
Compare
Commit 8ec4e25 ("add Meson build system") is missing a Signed-off-by tag from the committer. If you pick someone's else patch, you have to put your Signed-off-by tag as well. |
Should we test the meson build system with GitHub bot too? |
Two things from just a quick look:
|
Ah that was already in the TODO, excellent. |
Most certainly. |
I am thinking about the minimum required meson version. meson >= 0.60 would allow implementing the current Currently we require meson >= 0.57 to ensure support for Debian Bullseye for example ships meson 0.56. Should we consider meson versions included by distributions at all? |
It's possible to degrade this on older versions of Meson, by guarding the On the plus side, this means that you can still configure the project on 0.56.0 using Debian Bullseye's distro-provided Meson. On the negative side, this means that if you try running A hybrid approach could work, if backwards-compatibility is needed. Only with new Meson, define the loop/parallel tests and register the default test setup. Anyone who not only wants to run the tests, but also wants to run them in a loop... will simply be required to run a slightly more modern version of Meson (it's compatible with every single version of python that isn't end of life). |
After pondering this some more, I do believe we should keep the original as well. The point of adding meson should be easier integration for folks. Building liburing is already mostly as trivial as it can be. That does have the downside of then having two build systems, but I don't think this will be too bad as it's actually pretty simple. It does come with the cost of making sure we stay in sync, we'll just have to see how that goes over time. But if we retain both, perhaps that can simplify the meson side? We don't really need runtests support, for example, building and installing the core + man pages would really be all that's required. No? |
From my point of view, it's a good strategy for a while to look at how Meson will work and how folks will adopt it before completely dropping Makefile.
not really, it's not so complicated and I believe both build systems should be full functionally. |
do we have an issue on the meson tracker? I really want to avoid any shell script run from Meson. |
A shell script is needed in order to implement custom functionality above and beyond merely executing the compiled test executables. Particularly, grepping dmesg for possible errors. So I think shell scripts are unavoidable (and honestly not a real problem IMO). |
If a test should fail if there's something in the dmesg then perhaps that should be incorporated into the test or runner itself so that it returns a negative return value then? That way you can just run |
144a971
to
b71c445
Compare
The runtests-loop make target executes all tests in an infinite loop exiting only if a test failed. I see two possible solutions:
I think I prefer the second variant. |
This is already the case, that's what the shell script wrapper does.
Hmm, good point, this may already work okay like that. Really high numbers is not quite the same as infinite, but for all intents and purposes... |
we should reduce the strictness of checking or changing API (as I understand we are not interesting in) |
Yes, that is deliberately left unused for now, it's so we can avoid adding Yet Another identical function when we do add flags for this. |
And then apply it in src/register.c:io_uring_register_buf_ring() as we don't currently use the flags argument. Link: #622 Signed-off-by: Jens Axboe <[email protected]>
I'd say keep the unused variables, but add annotations so we can do |
Done |
@fischerling can you rebase PR or merge with master to pull updates. |
b71c445
to
b0dd07d
Compare
still failed |
9212535
to
9d93f5d
Compare
Sorry about that. The unnecessary explicit setting of the FLAGS in the workflow overrode the |
From https://github.com/fischerling/liburing/commit/111dc30d980abe0126d513f9839f56aab16c258e.patch:
No, please keep the original build script and Makefile. Jens has said I think we should make meson an optional thing rather than a |
Regardless of any other factor, it's usually prudent to implement build system migration as a multi-stage process. First implement the new build system, then let people try it out and decide whether it suits all their needs, potentially wait a release cycle to give people a chance to shake out any bugs. And only then decide to either:
It's surprisingly common for projects to support two build systems, anyway. |
Is it possible to updated/rediff that PR against latest version + /usr/bin/patch -p1 -s --fuzz=0 --no-backup-if-mismatch -f
1 out of 1 hunk FAILED -- saving rejects to file configure.rej
1 out of 1 hunk FAILED -- saving rejects to file src/Makefile.rej
1 out of 1 hunk FAILED -- saving rejects to file test/Makefile.rej |
9d93f5d
to
555d2d4
Compare
Just in case .. I'm not compalinig. |
28ac7e5
to
2427a59
Compare
ok, what next? |
Any update on this TODO? |
I have no strong opinion on this. Since some distros still ship meson 0.53 we could try to make all features But since getting from |
@eli-schwartz any comment? |
Debian packages Meson 0.56.2, and Ubuntu 20.04 packages Meson 0.53.2 Sticking to 0.53 here, means the build is compatible with both. Sticking to 0.54 here, means that the build is compatible with Debian out of the box (but not with Ubuntu). We could avoid rewriting all the test definitions by sticking with 0.54 but not going all the way to 0.57 ... Any system that has 0.53 already, can install 0.57 using Of course, it's very convenient to be able to build with the distro version. On the other hand, the latest LTS of Ubuntu has a perfectly good enough version of Meson -- it's only old versions of Ubuntu that are stuck on 0.53, and if you're going to let that stand in the way of progress you cannot use new things for the better part of a decade... |
2427a59
to
8383101
Compare
So I decided on meson I dropped the use of the meson filesystem module from The |
@ammarfaizi2 I have slightly changed the github bot config (Install meson from ubuntu repos, do not pass compilers to nolibc build via environment). |
Looks fine to me.
Thanks! |
77452d5
to
714c149
Compare
Meson is a fast and simple build system. Adoption started mainly in the desktop space (Gnome, X11, Mesa) to replace autotools, but since then, some low level projects (systemd, qemu) have switched to it too. Since liburing is a rather small codebase, the difference in speed and simplicity are not as huge as with other projects. Nonetheless, there are still some advantages: * It comes with some nice features (e.g. out-of-source builds), that one would need to implement in shell otherwise. * Other projects that use Meson could integrate liburing easily into their build system by using Meson's subproject() functionality. * Meson provides some useful editor/IDE integration, e.g. by generating compile_commands.json for clangd. * Distribution packagers are provided with a "standard" build system, with well known options/features/behavior, that is actively developed. Co-developed-by: Florian Fischer <[email protected]> Signed-off-by: Peter Eszlari <[email protected]> Signed-off-by: Florian Fischer <[email protected]>
* meson has builtin methods to check if the used compiler supports certain types and their expected members. Therefore we don't need to check if code using those types compiles. This makes the build file more readable. Suggested-By: Nils Tonnätt <[email protected]> * do not use -Wpedantic like the custom build system * check if ucontext functions are available. See: b5f2347 * add explicit run_command check kwarg The default will change in future meson versions causing possible unexpected behavior. And the awk command should not fail in the first place. * set -DLIBURING_INTERNAL introduced in 8be8af4 * include linux/openat2.h for struct open_how. See: 326ed97 * check if glibc provides struct statx. See: 44b12f5 * use -O3 as default. See: 7d1cce2 * update project CFLAGS. Remove -fomit-frame-pointer (de21479) and add -fno-stack-protector (2de9832). Reported-by: Eli Schwartz <[email protected]> Signed-off-by: Florian Fischer <[email protected]>
* Update the available tests. * Check for -Warray-bound and -Wstringop-overflow and use them if available. Include test/helper.c when building the test executables. * Bump required meson version from 0.53 to 0.54 to use fs.stem. * Simplify the meson test definition code by using a plain list of source files instead of the complex list of lists. Obtain the test name by stripping the file suffix from the test source using the meson fs module. * Link each test with the thread dependency similar to: 664bf78. * Run tests sequentially to prevent dmesg log intermixing expected by the test tooling. Suggested-by: Eli Schwartz <[email protected]> * Add a 'parallel' test suite to mirror make test-parallel. Signed-off-by: Florian Fischer <[email protected]>
Signed-off-by: Florian Fischer <[email protected]>
With this patch running `meson test` in the build directory behaves like `make -C test runtests`. To execute the other test suites (running the tests in a loop or in parallel) run: `meson test --suite=loop` or `meson test --suite=parallel`. Suggested-by: Eli Schwartz <[email protected]> Signed-off-by: Florian Fischer <[email protected]>
Introduce a new meson option 'nolibc'. Include the headers in src/arch when building liburing. Build the src/syscall.c as seperate library for the tests when building without libc. Signed-off-by: Florian Fischer <[email protected]>
The 'raw' test suite runs each compiled test without the runtest*.sh wrapper scripts. This is useful to debug tests using meson's gdb support. To debug a test in gdb run `meson test -C build --suite=raw <testname>_raw` Signed-off-by: Florian Fischer <[email protected]>
714c149
to
b2e1313
Compare
* Use meson CPU family names in matrix * Install meson and ninja * Create a cross compilation file * Build with meson * Build nolibc variant with meson * Test installation with meson Acked-by: Ammar Faizi <[email protected]> Signed-off-by: Florian Fischer <[email protected]>
Use an array of sources instead of declaring each example individually. Add examples introduced by Pavel and Dylan in 8200139, c1d15e7 and 61d472b. Signed-off-by: Florian Fischer <[email protected]>
b2e1313
to
e29fafb
Compare
Cannot cleanly apply this PR on top of https://github.com/axboe/liburing//archive/liburing-2.2.tar.gz + /usr/bin/cat /home/tkloczko/rpmbuild/SOURCES/liburing-add-meson-based-buildsystem-v2.patch
+ /usr/bin/patch -p1 -s --fuzz=0 --no-backup-if-mismatch -f
2 out of 5 hunks FAILED -- saving rejects to file .github/workflows/build.yml.rej |
After manually remove patching .github/workflows/build.yml meson fails with + /usr/bin/meson --buildtype=plain --prefix=/usr --libdir=/usr/lib64 --libexecdir=/usr/libexec --bindir=/usr/bin --sbindir=/usr/sbin --includedir=/usr/include --datadir=/usr/share --mandir=/usr/share/man --infodir=/usr/share/info --localedir=/usr/share/locale --sysconfdir=/etc --localstatedir=/var --sharedstatedir=/var/lib --wrap-mode=nodownload --auto-features=enabled . x86_64-redhat-linux-gnu
The Meson build system
Version: 0.63.2
Source dir: /home/tkloczko/rpmbuild/BUILD/liburing-liburing-2.2
Build dir: /home/tkloczko/rpmbuild/BUILD/liburing-liburing-2.2/x86_64-redhat-linux-gnu
Build type: native build
Project name: liburing
Project version: 2.2
C compiler for the host machine: /usr/bin/gcc (gcc 12.2.1 "gcc (GCC) 12.2.1 20220819 (Red Hat 12.2.1-1)")
C linker for the host machine: /usr/bin/gcc ld.bfd 2.39-3
C++ compiler for the host machine: /usr/bin/g++ (gcc 12.2.1 "g++ (GCC) 12.2.1 20220819 (Red Hat 12.2.1-1)")
C++ linker for the host machine: /usr/bin/g++ ld.bfd 2.39-3
Host machine cpu family: x86_64
Host machine cpu: x86_64
Run-time dependency threads found: YES
Compiler for C supports arguments -Wstringop-overflow=0: YES
Compiler for C supports arguments -Warray-bounds=0: YES
Checking for type "__kernel_rwf_t" : YES
Checking whether type "struct __kernel_timespec" has members "tv_sec", "tv_nsec" : YES
Checking whether type "struct open_how" has members "flags", "mode", "resolve" : YES
Checking if "statx" compiles: YES
Checking if "glibc_statx" compiles: YES
Checking for type "ucontext_t" : YES
Checking for function "makecontext" : YES
Configuring config-host.h using configuration
Configuring compat.h using configuration
man/meson.build:1:0: ERROR: File io_uring_prep_recv_multishot.3 does not exist. |
Patch fixing prev issue: --- a/man/meson.build
+++ b/man/meson.build
@@ -19,6 +19,7 @@
'io_uring_prep_accept.3',
'io_uring_prep_accept_direct.3',
'io_uring_prep_cancel.3',
+ 'io_uring_prep_cancel64.3',
'io_uring_prep_close.3',
'io_uring_prep_close_direct.3',
'io_uring_prep_connect.3',
@@ -48,9 +49,7 @@
'io_uring_prep_readv2.3',
'io_uring_prep_readv.3',
'io_uring_prep_recv.3',
- 'io_uring_prep_recv_multishot.3',
'io_uring_prep_recvmsg.3',
- 'io_uring_prep_recvmsg_multishot.3',
'io_uring_prep_remove_buffers.3',
'io_uring_prep_rename.3',
'io_uring_prep_renameat.3',
@@ -77,13 +76,6 @@
'io_uring_queue_exit.3',
'io_uring_queue_init.3',
'io_uring_queue_init_params.3',
- 'io_uring_recvmsg_cmsg_firsthdr.3',
- 'io_uring_recvmsg_cmsg_nexthdr.3',
- 'io_uring_recvmsg_name.3',
- 'io_uring_recvmsg_out.3',
- 'io_uring_recvmsg_payload.3',
- 'io_uring_recvmsg_payload_length.3',
- 'io_uring_recvmsg_validate.3',
'io_uring_register.2',
'io_uring_register_buffers.3',
'io_uring_register_buf_ring.3', More issues:
test/meson.build:169:4: ERROR: File fd-pass.c does not exist.
examples/meson.build:16:4: ERROR: File io_uring-udp.c does not exist. |
On July 5 you said the same thing, and several people pointed out that this PR is based on the development version and isn't supposed to apply on top of a stable release. For example, #622 (comment)
To be quite honest, I'm highly surprised you're asking the same question again, in the same PR, for the same old version. Did you completely forget you asked it? Why do you even think that people submitting PRs or patch series' to a project would be submitting it for the frozen release tag, which cannot ever be accepted ever under any circumstances ever... ... rather than doing like literally every software developer, or external contributor, for any project anywhere, and targeting the development version? Why do we need 3 different comments describing how something that isn't supposed to work, doesn't work? What's your goal here? If you're not here to waste everyone's time, why not just use the version you were linked to a couple months ago? |
Because this PR still is in in opened state. |
add a meson based build system
This is based on and supersedes the meson PR #297.
I am willing to maintain the meson build system and provide help for all those
not familiar with meson (@tp-m and @stalkerg are also willing to help).
Anybody can reach out to me using [email protected].
Additionally, I am already following the io_uring mailing list and will also respond
there.
@axboe Should I post this also to the io_uring mailing list?
TODOS
decide the minimum required meson versionI settled with meson>=0.53
git request-pull output:
Click to show/hide pull request guidelines
Pull Request Guidelines
notification, use
[GIT PULL]
as a prefix in your PR title.Commit message format rules:
Signed-off-by
tag with your real name and email. For example:The description should be word-wrapped at 72 chars. Some things should
not be word-wrapped. They may be some kind of quoted text - long
compiler error messages, oops reports, Link, etc. (things that have a
certain specific format).
Note that all of this goes in the commit message, not in the pull
request text. The pull request text should introduce what this pull
request does, and each commit message should explain the rationale for
why that particular change was made. The git tree is canonical source
of truth, not github.
Each patch should do one thing, and one thing only. If you find yourself
writing an explanation for why a patch is fixing multiple issues, that's
a good indication that the change should be split into separate patches.
If the commit is a fix for an issue, add a
Fixes
tag with the issueURL.
Don't use GitHub anonymous email like this as the commit author:
Use a real email address!
Commit message example:
By submitting this pull request, I acknowledge that: