liburing: update to 2.2 #510
Conversation
fischerling
force-pushed
the
update-liburing
branch
3 times, most recently
from
1bc1459 to
4767f32
Compare
|
Hmm, so you're just unconditionally linking to |
Yes this is what the upstream Makefile does too. |
There was a problem hiding this comment.
- axboe/liburing@de21479 -- removes fomit-frame-pointer
- axboe/liburing@2de9832 -- adds -fno-stack-protector
| @@ -74,13 +88,19 @@ has_cxx = cpp.compiles(code, name: 'C++') | |||
| has_ucontext = (cc.has_type('ucontext_t', prefix: '#include <ucontext.h>') | |||
| and cc.has_function('makecontext', prefix: '#include <ucontext.h>')) | |||
|
|
|||
| has_memfd_create = cc.has_function('memfd_create', | |||
| args: ['-D_GNU_SOURCE'], | |||
| prefix: '#include <sys/mman.h>') | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
But this commit is after the liburing-2.2 tag. Should we include the changes in our 2.2 wrap?
There was a problem hiding this comment.
Well, probably not. The newly added code is there in the release anyway, the define is needed to prevent conflicting definitions, but the newly added code was immediately dropped post-release, meh. :p
|
While comparing the diff upstream, I realized that this wrap has never set the |
| runtests_sh, | ||
| args : t[0], | ||
| args : test_name, |
There was a problem hiding this comment.
The test harness will try to detect issues logged in dmesg, but relies on not having intermixed logs in that case. So I think this should probably have used is_parallel: false all along?
There was a problem hiding this comment.
Good catch! I agree that we should not run the tests in parallel when trying to match dmesg messages to a particular test.
I added an additional test suite running the tests in parallel similar to upstreams make runtests-parallel.
There was a problem hiding this comment.
Since there are multiple test suites which I guess shouldn't all be run at the same time by a default meson test, perhaps we should use add_test_setup() to define a test profile which only runs the "once" tests, and declare it as the default?
There was a problem hiding this comment.
Also style nit: I'd probably put the is_parallel kwarg after the "args" kwarg.
Thanks. The wrap now sets the library version and soversion derived from the project version. I kept the changes relating the review in a separate commit which I would squash when the PR is ready. |
| runtests_sh, | ||
| args : t[0], | ||
| args : test_name, |
There was a problem hiding this comment.
Since there are multiple test suites which I guess shouldn't all be run at the same time by a default meson test, perhaps we should use add_test_setup() to define a test profile which only runs the "once" tests, and declare it as the default?
| runtests_sh, | ||
| args : t[0], | ||
| args : test_name, |
There was a problem hiding this comment.
Also style nit: I'd probably put the is_parallel kwarg after the "args" kwarg.
|
Alright, that looks pretty good. Time to squash the fixup changes I think. I'm thinking organization-wise it might make sense to have two commits:
What do you think? |
* Set library version. * Run tests sequentially to not intermix dmesg output. * Add a 'parallel' test suite to mirror make test-parallel. * Add default test setup running each test once (the 'once' suite) Suggested-by: Eli Schwartz <[email protected]> Signed-off-by: Florian Fischer <[email protected]>
Simplify the test definitions by using the filesystem module and always depending on the thread dependency. Reflect the upstream changes between 2.1 and 2.2 to the custom ./configure script in the root meson build definition. Signed-off-by: Florian Fischer <[email protected]>
fischerling
force-pushed
the
update-liburing
branch
from
57b700d to
ebbf2be
Compare
Sounds good to me. |
Simplify the test definitions and bump the required meson version to
use the filesystem module.
Reflect the upstream changes to the custom ./configure script in the
root meson build definition.
Signed-off-by: Florian Fischer [email protected]