Add Meson support
This is an initial attempt at adding Meson support to pmdk.
Signed-off-by: Tristan Partin [email protected]
I'd like to gather thoughts as to including Meson support in pmdk or ripping out the Makefiles/Visual Studio project files entirely for a Meson replacement. Assuming Meson support is well-received, I would like to discuss various design decisions that I have made up to this point. I have formalized 3 internal libraries: libpmemcommon.a, libpmemcore.a, and libpmemshared.a. In the Make-based system, certain files are compiled numerous times which can obviously lead to slower builds. On the other hand I seem to have blown up the amount of symbols in libpmem.so by quite a bit. I didn't see any symbol visibility modifiers either.
If this PR is declined, then I will be submitting my finished product to https://github.com/mesonbuild/wrapdb regardless for community-based Meson support of pmdk.
My reason for wanting Meson support is for our upcoming pmem-based work in https://github.com/hse-project/hse.
My plan is to just go library by library, then eventually tools and tests.
We've been contemplating the switch to an entirely different build system for years now (https://github.com/pmem/pmdk/issues/4120, https://github.com/pmem/pmdk/issues/4164). But the build system we have right now is simply massive and encompasses a lot of legacy crust, and is difficult to get rid of. We've been slowly (very slowly) refactoring our codebase (libpmem2 and libpmemset are essentially two side-effects of that refactoring work), intending to eventually split up the PMDK repo into many smaller repos with individual libraries.
I'm not sure how I feel about including an additional set of new build scripts. We are already maintaining two separate systems (makefiles and MSVC). As you suggest, I'd much rather we rip out what we have and replace it. On the other hand, we've been planning on doing just that for a very long time with not much to show for it.
For now, at the very least, we'd need to create new CI checks that use meson to make sure we don't break it.
As for the extraneous symbols, link files are not used in your scripts: https://github.com/pmem/pmdk/blob/e7b740f2e22d5f21bca3ff9e3d297ceb8e97c747/src/Makefile.inc#L199
Codecov Report
Merging #5369 (d77c32f) into master (174df77) will increase coverage by
0.10%. The diff coverage isn/a.
@@ Coverage Diff @@
## master #5369 +/- ##
==========================================
+ Coverage 72.13% 72.23% +0.10%
==========================================
Files 190 190
Lines 29665 29726 +61
==========================================
+ Hits 21400 21474 +74
+ Misses 8265 8252 -13
But the build system we have right now is simply massive and encompasses a lot of legacy crust, and is difficult to get rid of
Definitely understand that. I am willing to identify cruft that could be cleaned up with my PR should this idea take off. I have some ideas.
I'm not sure how I feel about including an additional set of new build scripts.
I don't want to add any unwanted maintenance burden to pmdk, so should this be rejected no hard feelings.
On the other hand, we've been planning on doing just that for a very long time with not much to show for it.
I am willing to put in the leg work for a Meson conversion, but should CMake become tapped, I cannot help the project since I have no intention of ever writing CMake.
This current PR encompasses about 2 hours worth of work. I was happy to at least get libpmem to compile. Nothing is impossible as long as I have some support of people who know the current build system well-enough to critique missing features. I find reading Makefiles to be a tedious task.
I don't think we have a strong preference for CMake. The only potential issue for Meson is that it's a fairly "new" thing and isn't included in some older distros (like Red Hat 7, where we see a lot of people using PMDK). I like the way it looks based on what you've done so far.
I've given it some thought over the weekend, and I think it would be a good idea to finally pull the trigger on this. Looking over what you've done so far, the big pieces missing are:
- CI integration - if this new build system isn't checked automatically, it will definitely break the first time anyone adds a new thing
- windows (hopefully, this would just work for the most part?)
- make cstyle/make check-license and similar code sanity checks
- integration with the test system(s) (make test/make check targets)
- builds for tools (this will likely be a PITA since some of the tools use individual extracted symbols from unstripped object files...)
- removal of the "debug" build (so essentially #4164).
We'd definitely do this in stages and spread the work evenly. But, for now, to merge this patch, I think the most important bits we'd need would be the first two.
The only potential issue for Meson is that it's a fairly "new" thing and isn't included in some older distros (like Red Hat 7, where we see a lot of people using PMDK).
Meson is distributed through PyPI, but Meson really requires platforms with Python>= 3.6. What version of Python is currently available for rhel 7? Does the EPEL distribute Meson? I think EPEL distributes cmake updates at least (not sure).
Does pmdk have a mailing list or irc room where I can communicate?
Since it is Thanksgiving week, maybe I can make more headway. I have gotten about 5-ish of the libraries building so far although I know I am missing some features for each of them. Stopped where one of the libraries made a mention of private symbols due to what seemed like extraction of an object and haven't returned.
Is the idea to keep the top-level makefile?
Yes, I just checked and Meson is indeed distributed in EPEL. Thanks for thinking of that.
Our mailing list is: https://groups.google.com/group/pmem
We have an irc channel #pmem on OFTC and a public slack channel. I'm most often found on the slack channel.
We would probably want to keep the top-level makefile as either a one-click solution to just get the thing working for people that just can't be bothered or as a way to point people at docs that explain how to use the new build system. I think I prefer the latter.
Nov 27 update: All libraries except for libpmempool build. Building is a pretty low bar though. Onto tools next probably.
Looks like #pmem isn't bridged to Matrix which is unfortunate since at the moment Micron blocks Slack and IRC :). Corporate firewalls are just so much fun!
Thanks for all the work!
I feel your pain ;-) Out of curiosity, I tried joining #_oftc_#pmem:matrix.org, and it seems to be working just fine.
12/7 update: Got daxio and rpmemd compiling. libpmempool still not compiling due to object file missing (pmdk is so fun :)). pmempool is not currently being compiled but it has the same BS as libpmempool. Tools functionality seems to be half complete from my POV. Left to do (that I can currently think of): python files, pmempool, install pmempool bash completion file.
What should be done with the python files in pmemreorder?
I think we've had ambitions to package pmreorder as an independent tool, but we haven't done that (yet :)). Looking at the pmreorder's makefile, it seems all it does is style checking with flake (https://github.com/pmem/pmdk/blob/master/src/tools/pmreorder/Makefile) - so just doing the same in Meson scripts would be fine.
And yes, our existing method of building pmempool/libpmempool is a horrid abomination. I'm sorry you had to see it ;) This is what happens when trying to get clever instead of solving the underlying problem.
Ok. It was just unclear whether those Python files should be installed or not.
12/31 update: Ok, so I got all the tools and libs building now! Onto tests I guess... I think one thing that would really help if we could enable -fvisibility=hidden and export only the public symbols from the libraries. @pbalcer what are your thoughts on that? Right now the visibility kind of makes things difficult in my opinion. Don't really know what object files are necessary or not.
@tristan957 sorry for not looking at this earlier. I haven't noticed the update after I got back from vacation.
Setting -fvisibility=hidden should be fine for release builds. But isn't that almost the same thing as using the version scripts as we do right now? Or are you asking if it wouldn't be better to get rid of those scripts and use this flag instead?
Version scripts and visibility attributes serve very different roles, and at the same time very overlapping roles.
Both can limit what is publicly available in the symbol table.
Version scripts can do lots of neat tricks with the symbol table, such as editing the exported names and providing versions symbols. It also lets you have a visible tracked description of how your ABI changes over time. And for this use case, it lets you mark the non-public symbols as hidden.
Visibility attributes let you mark symbols at the codegen level as not exported, which means that the compiler can optimize them as it sees fit and leading to more efficient code, since it doesn't have to generate symbol code to begin with.
@pbalcer no worries. Obviously not in a rush to merge this. If you talk to my manager so that I can work on this while at work, then maybe that could change :wink:.
1/25/2022 update: Started working on tests...mainly trying to scaffold out the meson.build file. Looking at the number of tests, I think I will hate my life for a while :). Might have some questions once the tests are actually ran since the README seems to make it seem like some prerequisites are required. I also have the issue of not owning any persistent memory lol. Micron, help me out :+1:. First step will be to just get each test compiling one-by-one.
@pbalcer do you mind if I submit a PR to get that miniasync library building with Meson? Would help me out quite a bit because then I could get it working as a Meson subproject instead of the in-tree copied files.
Isn't Meson compatible with CMake? Anyhow, we will eventually transition off of the in-tree copy - it's temporary solution. So yes, any changes should go to the main repo. And I genuinely have no preference on CMake vs Meson, I think we just went with CMake because it was easier for us to get started with. But I'll let @lplewa chime in on what's the preferred solution.
By the way, we haven't completely forgotten about this patch. You gave us motivation to finally purge the old build system, and we are actively working on this right now. So hopefully we will have this PR merged soon(ish). Are you still working on the tests? If so, I think @blazej-smorawski can take that over, so you no longer have to hate your life ;)
It can. I didn't realize there was a CMake build for it. Been a few weeks since I originally asked this question on IRC.
As far as CMake vs Meson, I think you guys should use what you are most comfortable with and it should be consistent across the many repos in this org. I only know Intel has a core contributor (also a Mesa contributor) to Meson, so there is at least a lot of institutional knowledge there. The Meson build in this PR is currently broken. I have to go back and fix it, but I can make do with the miniasync CMake build.
I am also fine if this PR gets closed in favor of a CMake build you guys would put together.
We are going to use Meson for the pmdk repo. All hope for any sort of uniformity across the org is long gone. Please hold off on making any further changes so that we don't work on the same changes.
Hi @tristan957, we have already rebased your patch onto current master and added a few new features. Is it alright with you if we start contributing into your fork of pmdk?
@blazej-smorawski i think it will be easier to just work on your fork while preserving @tristan957's commit history.
Isn't Meson compatible with CMake?
Sure, it can use cmake subprojects with reasonable success. Most things tend to work. We do periodically get bug reports from people with CMake subprojects that use less common cmake features that meson cannot handle; those usually do get fixed, so if you end up running into any problems, please try the latest version of Meson and then submit a bug report if it still doesn't work.
Note that that does mean:
- you require an additional build tool dependency (meson actually runs cmake to generate a build plan)
- if you want to support a prebuilt system copy, the cmake support module doesn't handle dependency fallbacks for you
@blazej-smorawski I can add you as a contributor or you can just take my commits and use them in your branch. Whatever you prefer. Please let me know and I can add you as a contributor to my fork if you decide to go that route. Please feel free to close this PR as you see fit.
@tristan957 I think that we will be working on our own branch with your commits from now, so we won't bother you with it anymore. Thanks for helping us out with it, we really appreciate this!
Very cool. Excited to see what y'all come up with. Please ping if you would me to review your final PR.
Hey guys, I was wondering if there was a timeline for this work. Would love to get it for the next major release of the software I work on.
@tristan957 As you can imagine, we are in the process of reprioritizing our current work. But I'm hoping we can finish the build system changes soon. We are 95% done ;)