-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
meson: build systemd using meson #5704
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
Conversation
|
@keszybz , could you elaborate what the benefits of using |
|
Arrgh, I forgot that I didn't write the rule to create The reasons:
and partials builds are near-instantaneous: That's all using ccache, but for development, it's a fair comparison. We're compiling the same stuff over and over, and being able to do a modification-compilation-test rounding in a second instead of a minute is very pleasant. |
meson.build
Outdated
| '--param=ssp-buffer-size=4', | ||
| ] | ||
| if compiler.has_argument(arg) | ||
| add_global_arguments(arg, language : 'c') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_project_arguments() is a bit better because it would not propagate to subprojects (though you don't have any)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RIght. I read about the distinction between global and project, but it didn't really sink in. Sounds reasonable to move it though.
meson.build
Outdated
| @1@ | ||
| ''' | ||
| gperf_snippet_format = 'echo foo,bar | @0@ -L ANSI-C' | ||
| gperf_snippet = run_command('bash', '-c', gperf_snippet_format.format(gperf.path())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh, why not just sh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
|
|
||
| ##################################################################### | ||
|
|
||
| intltool_merge = find_program('intltool-merge') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not to use here i18n.merge_file() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work:
+i18n.merge_file(
+ 'org.freedesktop.systemd1.policy',
+ po_dir : po_dir,
+ input : policy_in,
+ output : 'org.freedesktop.systemd1.policy',
+ install : true,
+ install_dir : polkitpolicydir)
FAILED: src/core/org.freedesktop.systemd1.policy
'msgfmt' '--xml' '--template' 'src/core/org.freedesktop.systemd1.policy.in' '-d' '/home/zbyszek/src/systemd/po' '-o' 'src/core/org.freedesktop.systemd1.policy'
msgfmt: cannot locate ITS rules for src/core/org.freedesktop.systemd1.policy.in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds like a bug, can you report it please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shell-completion/bash/meson.build
Outdated
| bashcompletiondir = get_option('bashcompletiondir') | ||
| if bashcompletiondir == '' | ||
| out = run_command( | ||
| pkg_config, '--variable=completionsdir', 'bash-completion') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_pkgconfig_variable(varname) (Added 0.36.0) will get the pkg-config variable specified, or, if invoked on a non pkg-config dependency, error out
probably you want something like
bashcomp = dependency('bash-completion', required : false)
if bashcomp.found()
bashcom[pletiondir = bashcomp.get_pkgconfig_variable('completionsdir')
else
...
endifThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right.
|
Was not able to review all 5k lines, but looks good ;) |
|
apart from tens of awk scripts ;) isn't it possible to replace them with something saner? like python scripts? (you have py3 anyway since meson runs on top of it) |
|
Python scripts are relatively more verbose than awk scripts. Probably something like ~3-4 times for such simple cases. I'd even prefer to have the awk scripts inline, but cannot because of mesonbuild/meson#1564. And it's less error-prone to take the awk scripts from the Makefile and just copy them over then to write stuff from scratch. But maybe in the future, if autotools support is dropped. this could be done. Thanks for your comments, much appreciated. |
|
For comparison, how long does an Autotools build take on the same machine. Note that by default Autotools builds with -O2 -g whereas the default for Meson is -O0 -g. For a fair comparison either tell Autotools to build with -O0 -g or set Meson buildtype to Yes, it's not really an apples-to-apples comparison because of some missing functionality but this is useful information as a rough estimate. |
In Debian, m4 is installed as /usr/bin/m4 and we don't have a /bin/ → /usr/bin symlink yet |
|
Changing meson.build to I get m4 is a directory though. Sounds like a bug in find_program() |
|
@mbiebl it's even written in commit message ;) |
Indeed, missed that |
|
I merged #5706, could you rebase this one? |
|
on Debian, PATH for a regular uses doesn't include /sbin and /usr/sbin. Even with the quota package installed, I get Two issues:
|
|
|
Looks like 'src' is missing in |
That check is unnecessary. When using dependency(), a check for pkg-config is already included with an error message like this: |
|
|
|
I've done a few test builds on my X220 (Core i7, 4 cores) The split-usr support in the meson build is broken atm, so I used a non-split config for both. The meson build is currently missing:
An interesting observation is that the meson build produces bigger binaries (for the 02 build its 24M vs 31M stripped). I diffed the file lists (attached as txt). It seems we are missing a lot of symlinks and empty directories. |
|
Ok, the missing importd build is easy to explain: The meson build uses pkg-config to detect bzip2, but the bzip2 package in Debian doesn't ship a .pc file. As a result, bzip2 support is not detected and thus importd disabled. |
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=853803 |
|
@keszybz , thanks for the explanation.
If you know Python :-) Some results: I tried to fix Has moving to |
|
$ ./systemctl --version
systemd
+PAM -AUDIT -SELINUX -IMA -APPARMOR -SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP -GCRYPT +GNUTLS +ACL +XZ +LZ4 +SECCOMP +BLKID +ELFUTILS +KMOD +IDN default-hierarchy=hybrid
$ grep -r PACKAGE_V
Binary file meson-private/build.dat matches
config.h:#define PACKAGE_VERSION "233" |
That was a typo in meson.build (
Hm, yeah. I don't see a nice way to do this. You can do
That was a bug in the way the headers were included. Fixed in a separate patch.
Indeed. That was a left-over from before when I called pkg-config by hand, and @ignatenkobrain pointed out that this is not necessary. Removed.
Yeah, there was no rule for that. It's been added now.
The meson DSL is not Python, it's its own special thing. I'd argue that it is simpler than autoconf or m4 or make, and has better error handling. But even if you aren't convinced, it wins by the virtue of doing all three jobs ;)
Not much. It's one of those cases where it's hard to discuss without a POC. This is the POC. I'll send a message to the mailing list if nobody beats me to it.
Looks like a bug in either meson or the libraries. Maybe even both.
Yeah, I'll work on that.
This probably is a result of linking being done in a slightly different way. I probably a result of the way that libraries are built: libtool uses
I messed up PACKAGE_STRING. Fixed. So the missing part are the rules for tests and sd-boot. |
|
Yet another update:
This builds properly in Fedora koji on 6 arches: https://koji.fedoraproject.org/koji/taskinfo?taskID=19191039.
Me too, I think Similarly, the distinction between "enable" and "with" was often unclear. We were rather inconsistent. And different options can move from one category to another when dependency on some library changes between optional and required. @mbiebl requested the change from |
|
I did another local autopkgtest run with meson against the current code here, all good. 👍 Thanks @keszybz ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be part of a separate PR (cgtop: check cgroups after parsing options)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be part of a separate PR (nspawn: check cgroups after parsing options)
meson.build
Outdated
| @@ -380,7 +380,7 @@ endforeach | |||
| sed = find_program('sed') | |||
| grep = find_program('grep') | |||
| awk = find_program('awk') | |||
| m4 = find_program('m4') | |||
| m4 = find_program('/usr/bin/m4') | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that 0.40.0 has been released, I would probably revert this commit and bump the minimum required version of meson to 0.40.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Previous checks did nothing, because cc.has_argument only does compilation, without any linking. Unfortunately cc.links() cannot be used, because it does not accept any options. Providing the test file as a static source is easiest, even if not every elegant. mesonbuild/meson#1676
This reverts 78b68dc.
|
|
|
I reverted the m4 patch and split out the two patches into a separate PR. |
|
There's a new "meson" label. Please use it to tag stuff specifically related to meson. I expect a bunch of those once this is merged ;) |
|
@evverx Thanks! Celebratory note on https://in.waw.pl/~zbyszek/blog/systemd-meson.html. |
|
Seems a bit risky to merge this without waiting for at least one integration test, but there's a more than good chance that it will work :-) Many thanks @keszybz, awesome work! |
|
@keszybz , it is a little bit early to celebrate :-) There is a clash with #5752, which was merged 3 hours ago ../src/basic/missing.h:762:6: warning: "HAVE_DECL_IFLA_GENEVE_LABEL" is not defined [-Wundef]
#if !HAVE_DECL_IFLA_GENEVE_LABEL
^~~~~~~~~~~~~~~~~~~~~~~~~~~Sorry about that. I should have rebased on |
|
FWIW the fix has just been merged. #5804 |
|
@keszybz thanks a million for working on this, this is great work! |
|
FWIW there is a discussion of moving
|
|
@evverx I guess that becomes relevant only once systemd starts requiring a newer meson version? Related to that, with my Debian maintainer hat on: In the past we did provide backports of newer systemd versions for current Debian stable releases. Once we drop the autotools build system, it means we will have to build using meson. Our current minimum required version is 0.40, the latest version in stretch is 0.37. |
It's crucial that we can build systemd using VS2010!
... er, wait, no, that's not the official reason. We need to shed old systems
by requring python 3! Oh, no, it's something else. Maybe we need to throw out
345 years of knowlege accumulated in autotools? Whatever, this new thing is
cool and shiny, let's use it.
This is not complete, I'm throwing it out here for your amusement and critique.
rules for sd-boot are missing. Those might be quite complicated.
rules for tests are missing too. Those are probably quite simple and
repetitive, but there's lots of them.
it's likely that I didn't get all the conditions right, I only tested "full"
compilation where most deps are provided and nothing is disabled.
busname.target and all .busname units are skipped on purpose.
Otherwise, installation into $DESTDIR has the same list of files and the
autoconf install, except for .la files.
It'd be great if people had a careful look at all the library linking options.
I added stuff until things compiled, and in the end there's much less linking
then in the old system. But it seems that there's still a lot of unnecessary
deps.
meson has a
shared_modulestatement, which sounds like something appropriatefor our nss and pam modules. Unfortunately, I couldn't get it to work. For the
nss modules, we need an .so version of '2', but
shared_moduledisallows theversion argument. For the pam module, it also didn't work, I forgot the reason.
The handling of .m4 and .in and .m4.in files is rather awkward. It's likely
that this could be simplified. If make support is ever dropped, I think it'd
make sense to switch to a different templating system so that two different
languages and not required, which would make everything simpler yet.
--
Feel free to push stuff on top (including fixup commits to be squashed).