* [PATCH] meson.build: Don't look for libudev for static builds @ 2020-10-02 10:52 Peter Maydell 2020-10-02 11:15 ` 罗勇刚(Yonggang Luo) 2020-10-02 12:02 ` Paolo Bonzini 0 siblings, 2 replies; 22+ messages in thread From: Peter Maydell @ 2020-10-02 10:52 UTC (permalink / raw) To: qemu-devel; +Cc: Paolo Bonzini commit f01496a314d916 moved the logic for detecting libudev from configure to meson.build, but in the process it dropped the condition that meant we only ask pkg-config about libudev for a non-static build. This breaks static builds of the system emulators on at least Ubuntu 18.04.4, because on that host there is no static libudev but pkg-config still claims it exists. Reinstate the logic that we had in the configure check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- We could certainly do something cleverer here, but basic "convert from configure to meson" should in general not also be changing the detection logic IMHO. We can make the logic smarter as a follow-on patch if desired. --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 3161c1f037a..07da66e1d81 100644 --- a/meson.build +++ b/meson.build @@ -271,7 +271,7 @@ if 'CONFIG_CURL' in config_host link_args: config_host['CURL_LIBS'].split()) endif libudev = not_found -if targetos == 'linux' and (have_system or have_tools) +if targetos == 'linux' and (have_system or have_tools) and not enable_static libudev = dependency('libudev', required: get_option('mpath').enabled(), static: enable_static) -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 10:52 [PATCH] meson.build: Don't look for libudev for static builds Peter Maydell @ 2020-10-02 11:15 ` 罗勇刚(Yonggang Luo) 2020-10-02 12:03 ` Paolo Bonzini 2020-10-02 12:02 ` Paolo Bonzini 1 sibling, 1 reply; 22+ messages in thread From: 罗勇刚(Yonggang Luo) @ 2020-10-02 11:15 UTC (permalink / raw) To: Peter Maydell; +Cc: Paolo Bonzini, qemu-level [-- Attachment #1: Type: text/plain, Size: 1675 bytes --] So the better way is pkg-config handling sttaic properly? On Fri, Oct 2, 2020 at 6:53 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > commit f01496a314d916 moved the logic for detecting libudev from > configure to meson.build, but in the process it dropped the condition > that meant we only ask pkg-config about libudev for a non-static > build. > > This breaks static builds of the system emulators on at least Ubuntu > 18.04.4, because on that host there is no static libudev but > pkg-config still claims it exists. > > Reinstate the logic that we had in the configure check. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > We could certainly do something cleverer here, but basic "convert > from configure to meson" should in general not also be changing the > detection logic IMHO. We can make the logic smarter as a follow-on > patch if desired. > --- > meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index 3161c1f037a..07da66e1d81 100644 > --- a/meson.build > +++ b/meson.build > @@ -271,7 +271,7 @@ if 'CONFIG_CURL' in config_host > link_args: config_host['CURL_LIBS'].split()) > endif > libudev = not_found > -if targetos == 'linux' and (have_system or have_tools) > +if targetos == 'linux' and (have_system or have_tools) and not enable_static > libudev = dependency('libudev', > required: get_option('mpath').enabled(), > static: enable_static) > -- > 2.20.1 > > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo [-- Attachment #2: Type: text/html, Size: 2128 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 11:15 ` 罗勇刚(Yonggang Luo) @ 2020-10-02 12:03 ` Paolo Bonzini 0 siblings, 0 replies; 22+ messages in thread From: Paolo Bonzini @ 2020-10-02 12:03 UTC (permalink / raw) To: luoyonggang, Peter Maydell; +Cc: qemu-level On 02/10/20 13:15, 罗勇刚(Yonggang Luo) wrote: > So the better way is pkg-config handling sttaic properly? The problem is that you cannot handle it properly. Consider for example libmultipath, which requires the program to define a couple functions for static linking to work. A compile-time check that the library works would fail, and therefore Meson (and configure) are just punting and declaring it a user configuration issue. Meson will warn about it, but it will proceed anyway. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 10:52 [PATCH] meson.build: Don't look for libudev for static builds Peter Maydell 2020-10-02 11:15 ` 罗勇刚(Yonggang Luo) @ 2020-10-02 12:02 ` Paolo Bonzini 2020-10-02 12:35 ` Peter Maydell 1 sibling, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2020-10-02 12:02 UTC (permalink / raw) To: Peter Maydell, qemu-devel On 02/10/20 12:52, Peter Maydell wrote: > commit f01496a314d916 moved the logic for detecting libudev from > configure to meson.build, but in the process it dropped the condition > that meant we only ask pkg-config about libudev for a non-static > build. > > This breaks static builds of the system emulators on at least Ubuntu > 18.04.4, because on that host there is no static libudev but > pkg-config still claims it exists. > > Reinstate the logic that we had in the configure check. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> I don't think this is a good idea. Having a completely new build system also lets us notice all these weird one-off tests, decide whether they are worth being kept (like the SDL -Wno-undef workaround) or not, and possibly come up with a policy that avoids having to make one-off decisions. If "../configure --static" worked and now doesn't then it would be a clear regression, but a one off check should have a bigger justification than "39 --disable switches have worked so far and 39 < 40". Here are three alternatives to the patch: - the workaround: just leave things as they are and add --disable-libudev to your script. We have just added a month ago a completely new dependency that would have the same issue (libu2f); we didn't add "test $static" just because you don't have libu2f installed and therefore you didn't notice. The day you updated your system ended up with libu2f-dev installed, you would probably just add --disable-libu2f instead of adding a test for $static. So why should libudev be treated differently? - the cheap fix: deprecate static building of emulators in the configure script (in the meanwhile you add --disable-libudev); if people complain, we figure out the dependencies that they care about and we try to understand if it's actually possible to build a statically linked emulator for their usecase. - the complicated fix: check which statically linked libraries are available in e.g. Debian and disable _all_ other dependencies with --static (most of them will be the ones that you already have to disable in your script, and most of them will be in configure). Based on the outcome, decide whether it's actually possible to build a statically linked emulator that makes sense. I wouldn't be surprised if people trying to statically build emulators are getting worse performance just due to a missing static libaio or libio_uring, for example, and these people might even be building a statically-linked QEMU for use with KVM (like Firecracker), i.e. they would care about performance. Finally, no matter how we proceed, static building of system emulators is not covered by either CI or tests/docker (only user-mode emulators are). Even if we deprecate it, we should make sure that your configuration is reproduced in either Travis or GitLab, otherwise people will keep on breaking it. That would also document that building a statically-linked system emulator is messy and requires a few dozen --disable options. Thanks, Paolo > We could certainly do something cleverer here, but basic "convert > from configure to meson" should in general not also be changing the > detection logic IMHO. We can make the logic smarter as a follow-on > patch if desired. > --- > meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index 3161c1f037a..07da66e1d81 100644 > --- a/meson.build > +++ b/meson.build > @@ -271,7 +271,7 @@ if 'CONFIG_CURL' in config_host > link_args: config_host['CURL_LIBS'].split()) > endif > libudev = not_found > -if targetos == 'linux' and (have_system or have_tools) > +if targetos == 'linux' and (have_system or have_tools) and not enable_static > libudev = dependency('libudev', > required: get_option('mpath').enabled(), > static: enable_static) > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 12:02 ` Paolo Bonzini @ 2020-10-02 12:35 ` Peter Maydell 2020-10-02 12:36 ` Peter Maydell 2020-10-02 13:05 ` Paolo Bonzini 0 siblings, 2 replies; 22+ messages in thread From: Peter Maydell @ 2020-10-02 12:35 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers On Fri, 2 Oct 2020 at 13:02, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 02/10/20 12:52, Peter Maydell wrote: > > commit f01496a314d916 moved the logic for detecting libudev from > > configure to meson.build, but in the process it dropped the condition > > that meant we only ask pkg-config about libudev for a non-static > > build. > > > > This breaks static builds of the system emulators on at least Ubuntu > > 18.04.4, because on that host there is no static libudev but > > pkg-config still claims it exists. > > > > Reinstate the logic that we had in the configure check. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > I don't think this is a good idea. Having a completely new build system > also lets us notice all these weird one-off tests, decide whether they > are worth being kept (like the SDL -Wno-undef workaround) or not, and > possibly come up with a policy that avoids having to make one-off decisions. > > If "../configure --static" worked and now doesn't then it would be a > clear regression, but a one off check should have a bigger justification > than "39 --disable switches have worked so far and 39 < 40". My configure command line used to work and now it doesn't. There is no workaround that I'm aware of that I can use by tweaking the configure options. That's a clear regression. > Here are three alternatives to the patch: > > - the workaround: just leave things as they are and add > --disable-libudev to your script. There is no --disable-udev ! If there was I'd just have used it. e104462:bionic:static-sys$ '../../configure' '--target-list=arm-softmmu' '--enable-debug' '--static' '--disable-tools' '--disable-sdl' '--disable-gtk' '--disable-vnc' '--disable-virtfs' '--disable-attr' '--disable-libiscsi' '--disable-libnfs' '--disable-libusb' '--disable-opengl' '--disable-numa' '--disable-usb-redir' '--disable-bzip2' '--audio-drv-list=' '--disable-guest-agent' '--disable-vte' --disable-udev ERROR: unknown option --disable-udev Try '../../configure --help' for more information > We have just added a month ago a > completely new dependency that would have the same issue (libu2f); we > didn't add "test $static" just because you don't have libu2f installed > and therefore you didn't notice. The day you updated your system ended > up with libu2f-dev installed, you would probably just add > --disable-libu2f instead of adding a test for $static. So why should > libudev be treated differently? The last time this came up it was in an all-in-configure test, so I took the approach of making the test a bit smarter to sanity check that what it was getting from pkg-config really worked: https://patchew.org/QEMU/20200928160402.7961-1-peter.maydell@linaro.org/ I don't know enough meson to do that in meson, so this patch is the simple change that fixes the regression by reinstating the logic configure had. > - the complicated fix: check which statically linked libraries are > available in e.g. Debian and disable _all_ other dependencies with > --static (most of them will be the ones that you already have to disable > in your script, and most of them will be in configure). Based on the > outcome, decide whether it's actually possible to build a statically > linked emulator that makes sense. I don't think we want to try to say "look at what statically linked libraries are in Debian". The point of configure style logic is to look at what is present when we try to compile and, when something is an optional feature, only compile it in if it's going to work. It would be better to do the "see if a static library is present" test. This isn't too hard to do in configure (compare that six line fix to the detection of libgio). Hopefully it is not too hard to do in meson ? > Finally, no matter how we proceed, static building of system emulators > is not covered by either CI or tests/docker (only user-mode emulators > are). Even if we deprecate it, we should make sure that your > configuration is reproduced in either Travis or GitLab, otherwise people > will keep on breaking it. That would also document that building a > statically-linked system emulator is messy and requires a few dozen > --disable options. I agree that we don't really very solidly support this use case, and it's pretty much "if it doesn't work we accept point-fixes to the build machinery and/or recommend more use of --disable-foo". But it is still useful sometimes to have. thanks -- PMM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 12:35 ` Peter Maydell @ 2020-10-02 12:36 ` Peter Maydell 2020-10-02 12:43 ` Paolo Bonzini 2020-10-02 13:05 ` Paolo Bonzini 1 sibling, 1 reply; 22+ messages in thread From: Peter Maydell @ 2020-10-02 12:36 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers On Fri, 2 Oct 2020 at 13:35, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 2 Oct 2020 at 13:02, Paolo Bonzini <pbonzini@redhat.com> wrote: > > - the workaround: just leave things as they are and add > > --disable-libudev to your script. > > There is no --disable-udev ! ...and there's no --disable-libudev either :-) -- PMM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 12:36 ` Peter Maydell @ 2020-10-02 12:43 ` Paolo Bonzini 2020-10-02 13:01 ` Peter Maydell 0 siblings, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2020-10-02 12:43 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 02/10/20 14:36, Peter Maydell wrote: > On Fri, 2 Oct 2020 at 13:35, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Fri, 2 Oct 2020 at 13:02, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> - the workaround: just leave things as they are and add >>> --disable-libudev to your script. >> >> There is no --disable-udev ! > > ...and there's no --disable-libudev either :-) You're right---I was sure I used some get_option but it's not --disable-libudev; it's --disable-mpath. That made sense when mpath was the only thing that used libudev, but probably now we should add a separate --disable-libudev. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 12:43 ` Paolo Bonzini @ 2020-10-02 13:01 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2020-10-02 13:01 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers On Fri, 2 Oct 2020 at 13:44, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 02/10/20 14:36, Peter Maydell wrote: > > On Fri, 2 Oct 2020 at 13:35, Peter Maydell <peter.maydell@linaro.org> wrote: > >> > >> On Fri, 2 Oct 2020 at 13:02, Paolo Bonzini <pbonzini@redhat.com> wrote: > >>> - the workaround: just leave things as they are and add > >>> --disable-libudev to your script. > >> > >> There is no --disable-udev ! > > > > ...and there's no --disable-libudev either :-) > > You're right---I was sure I used some get_option but it's not > --disable-libudev; it's --disable-mpath. That made sense when mpath was > the only thing that used libudev, but probably now we should add a > separate --disable-libudev. Well, this gets into the question of what --disable-foo options should be doing. They're user facing, so really they ought to be for user-facing features. So for instance: seccomp seccomp support libssh ssh block device support dmg dmg image format support gtk gtk UI are all enabling or disabling features the user might plausibly have an opinion about whether they want or not. These are bit more dubious -- they are enabling/disabling a specific library, and so the help text has had to do the translation of that for the user into "what actual feature in QEMU am I losing here if I disable this?"; libxml2 for Parallels image format libusb libusb (for usb passthrough) and then these give the user no hints at all: libpmem libpmem support libdaxctl libdaxctl support so it's not clear what they're intended for (unless it's solely for the niche use case of "work around the build machinery not correctly detecting whether the library is available"...) Having a --disable-libudev seems to me strictly worse than either (a) the build system actually being able to correctly identify that the host has a libudev available to link to (b) having an option with a name and help text that tells the user what they're actually gaining or losing by the feature being enabled/disabled thanks -- PMM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 12:35 ` Peter Maydell 2020-10-02 12:36 ` Peter Maydell @ 2020-10-02 13:05 ` Paolo Bonzini 2020-10-02 13:09 ` Peter Maydell 1 sibling, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2020-10-02 13:05 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 02/10/20 14:35, Peter Maydell wrote: > > It would be better to do the "see if a static library is present" > test. This isn't too hard to do in configure (compare that > six line fix to the detection of libgio). Hopefully it is > not too hard to do in meson ? Yes, something like: if enable_static skeleton = 'int main(void) { return 0; }' if not cc.links(skeleton, dependencies: libudev) if get_option('mpath').enabled() error('Cannot link with libudev') else warning('Cannot link with libudev, disabling') libudev = not_found endif endif endif endif It can be done in meson.build also for dependencies that are still detected in configure, and we can place it in a loop if we want to do many tests like these: if enable_static dependencies = {'libudev': 'mpath', ...} skeleton = 'int main(void) { return 0; }' foreach var, option: dependencies if not cc.links(skeleton, dependencies: get_variable(var)) if get_option(option).enabled() error('Cannot link with @0@'.format(var)) else warning('Cannot link with @0@, disabling'.format(skeleton)) set_variable(var, not_found) endif endif endif endforeach endif This way we don't have 500 lines of this kind of test. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 13:05 ` Paolo Bonzini @ 2020-10-02 13:09 ` Peter Maydell 2020-10-02 14:08 ` Paolo Bonzini 2020-10-03 7:24 ` 罗勇刚(Yonggang Luo) 0 siblings, 2 replies; 22+ messages in thread From: Peter Maydell @ 2020-10-02 13:09 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 02/10/20 14:35, Peter Maydell wrote: > > > > It would be better to do the "see if a static library is present" > > test. This isn't too hard to do in configure (compare that > > six line fix to the detection of libgio). Hopefully it is > > not too hard to do in meson ? > > Yes, something like: > > if enable_static > skeleton = 'int main(void) { return 0; }' > if not cc.links(skeleton, dependencies: libudev) > if get_option('mpath').enabled() > error('Cannot link with libudev') > else > warning('Cannot link with libudev, disabling') > libudev = not_found > endif > endif > endif > endif This duplicates the information that the thing that depends on libudev is mpath. Can we put this in a wrapper around dependency() so that we could just say something like libudev = compile_checked_dependency('libudev', required: get_option('mpath').enabled(), static: enable_static) for those dependencies that want to do the "does this compile" check ? thanks -- PMM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 13:09 ` Peter Maydell @ 2020-10-02 14:08 ` Paolo Bonzini 2020-10-02 14:18 ` Peter Maydell 2020-10-03 7:24 ` 罗勇刚(Yonggang Luo) 1 sibling, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2020-10-02 14:08 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 02/10/20 15:09, Peter Maydell wrote: > This duplicates the information that the thing that depends > on libudev is mpath. Can we put this in a wrapper around > dependency() so that we could just say something like > libudev = compile_checked_dependency('libudev', > required: get_option('mpath').enabled(), > static: enable_static) > > for those dependencies that want to do the "does this compile" > check ? No, there's no functions at all. You can of course put the detection and test in a single loop: dependencies = {} ... if targetos == 'linux' and (have_system or have_tools) dependencies += {'libudev': 'mpath'} endif ... skeleton = 'int main(void) { return 0; }' foreach var, option: dependencies dep = dependency(var, required: get_option(option).enabled(), static: enable_static) if dep.found() and enable_static and not cc.links(skeleton, dependencies: get_variable(var)) if get_option(option).enabled() error('Cannot link with @0@'.format(var)) else warning('Cannot link with @0@, disabling'.format(skeleton)) set_variable(var, not_found) endif endif endif endforeach Doing this check for all libraries is certainly a good idea. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 14:08 ` Paolo Bonzini @ 2020-10-02 14:18 ` Peter Maydell 2020-10-02 15:14 ` Paolo Bonzini 0 siblings, 1 reply; 22+ messages in thread From: Peter Maydell @ 2020-10-02 14:18 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers On Fri, 2 Oct 2020 at 15:08, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 02/10/20 15:09, Peter Maydell wrote: > > This duplicates the information that the thing that depends > > on libudev is mpath. Can we put this in a wrapper around > > dependency() so that we could just say something like > > libudev = compile_checked_dependency('libudev', > > required: get_option('mpath').enabled(), > > static: enable_static) > > > > for those dependencies that want to do the "does this compile" > > check ? > > No, there's no functions at all. You can of course put the detection and > test in a single loop: > > dependencies = {} > ... > if targetos == 'linux' and (have_system or have_tools) > dependencies += {'libudev': 'mpath'} > endif > ... > skeleton = 'int main(void) { return 0; }' > foreach var, option: dependencies > dep = dependency(var, > required: get_option(option).enabled(), > static: enable_static) > if dep.found() and enable_static and not cc.links(skeleton, dependencies: get_variable(var)) > if get_option(option).enabled() > error('Cannot link with @0@'.format(var)) > else > warning('Cannot link with @0@, disabling'.format(skeleton)) > set_variable(var, not_found) > endif > endif > endif > endforeach That is a lot uglier. I'm really getting fed up with Meson's persistent "no, you can't do that" attitude :-( thanks -- PMM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 14:18 ` Peter Maydell @ 2020-10-02 15:14 ` Paolo Bonzini 2020-10-02 15:28 ` Peter Maydell 0 siblings, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2020-10-02 15:14 UTC (permalink / raw) To: Peter Maydell; +Cc: QEMU Developers On 02/10/20 16:18, Peter Maydell wrote: >> No, there's no functions at all. You can of course put the detection and >> test in a single loop: >> >> dependencies = {} >> ... >> if targetos == 'linux' and (have_system or have_tools) >> dependencies += {'libudev': 'mpath'} >> endif >> ... >> skeleton = 'int main(void) { return 0; }' >> foreach var, option: dependencies >> dep = dependency(var, >> required: get_option(option).enabled(), >> static: enable_static) >> if dep.found() and enable_static and not cc.links(skeleton, dependencies: get_variable(var)) >> if get_option(option).enabled() >> error('Cannot link with @0@'.format(var)) >> else >> warning('Cannot link with @0@, disabling'.format(skeleton)) >> set_variable(var, not_found) >> endif >> endif >> endif >> endforeach > That is a lot uglier. The code above is ugly but it is also botched; it should be more like: dependencies = {} ... if targetos == 'linux' and (have_system or have_tools) dependencies += {'libudev': get_option('mpath')} else libudev = not_found endif ... skeleton = 'int main(void) { return 0; }' foreach var, opt: dependencies dep = dependency(var, required: opt.enabled(), static: enable_static) if dep.found() and enable_static and not cc.links(skeleton, dependencies: dep) # Meson should have already warned about the lack of a static library if opt.enabled() error('Cannot link with @0@'.format(var)) else dep = not_found endif set_variable(var, dep) endforeach which both shorter and more readable. Or is it loops vs. functions that you find ugly? Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 15:14 ` Paolo Bonzini @ 2020-10-02 15:28 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2020-10-02 15:28 UTC (permalink / raw) To: Paolo Bonzini; +Cc: QEMU Developers On Fri, 2 Oct 2020 at 16:15, Paolo Bonzini <pbonzini@redhat.com> wrote: > Or is it loops vs. functions that you find ugly? Lack of functions. Being able to abstract out what you're doing into something with a comprehensible name is a pretty basic software engineering requirement. Yes, you *can* rearrange everything so that you ensure that all the code that wants to do something sets up some variables and then you loop over them, but then you're contorting things out of the most readable way to express them to work around limitations of the build tool. thanks -- PMM ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-02 13:09 ` Peter Maydell 2020-10-02 14:08 ` Paolo Bonzini @ 2020-10-03 7:24 ` 罗勇刚(Yonggang Luo) 2020-10-03 7:50 ` Paolo Bonzini 1 sibling, 1 reply; 22+ messages in thread From: 罗勇刚(Yonggang Luo) @ 2020-10-03 7:24 UTC (permalink / raw) To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 1497 bytes --] On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 02/10/20 14:35, Peter Maydell wrote: > > > > > > It would be better to do the "see if a static library is present" > > > test. This isn't too hard to do in configure (compare that > > > six line fix to the detection of libgio). Hopefully it is > > > not too hard to do in meson ? > > > > Yes, something like: > > > > if enable_static > > skeleton = 'int main(void) { return 0; }' > > if not cc.links(skeleton, dependencies: libudev) > > if get_option('mpath').enabled() > > error('Cannot link with libudev') > > else > > warning('Cannot link with libudev, disabling') > > libudev = not_found > > endif > > endif > > endif > > endif > > This duplicates the information that the thing that depends > on libudev is mpath. Can we put this in a wrapper around > dependency() so that we could just say something like > libudev = compile_checked_dependency('libudev', > required: get_option('mpath').enabled(), > static: enable_static) > Hi Bonzini, This looks like a frequently used function, can we upstrem to meson? > for those dependencies that want to do the "does this compile" > check ? > > thanks > -- PMM > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo [-- Attachment #2: Type: text/html, Size: 2084 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-03 7:24 ` 罗勇刚(Yonggang Luo) @ 2020-10-03 7:50 ` Paolo Bonzini 2020-10-03 8:28 ` 罗勇刚(Yonggang Luo) 2020-10-03 8:29 ` 罗勇刚(Yonggang Luo) 0 siblings, 2 replies; 22+ messages in thread From: Paolo Bonzini @ 2020-10-03 7:50 UTC (permalink / raw) To: luoyonggang, Peter Maydell; +Cc: QEMU Developers On 03/10/20 09:24, 罗勇刚(Yonggang Luo) wrote: > > > On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell <peter.maydell@linaro.org > <mailto:peter.maydell@linaro.org>> wrote: >> >> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini <pbonzini@redhat.com > <mailto:pbonzini@redhat.com>> wrote: >> > >> > On 02/10/20 14:35, Peter Maydell wrote: >> > > >> > > It would be better to do the "see if a static library is present" >> > > test. This isn't too hard to do in configure (compare that >> > > six line fix to the detection of libgio). Hopefully it is >> > > not too hard to do in meson ? >> > >> > Yes, something like: >> > >> > if enable_static >> > skeleton = 'int main(void) { return 0; }' >> > if not cc.links(skeleton, dependencies: libudev) >> > if get_option('mpath').enabled() >> > error('Cannot link with libudev') >> > else >> > warning('Cannot link with libudev, disabling') >> > libudev = not_found >> > endif >> > endif >> > endif >> > endif >> >> This duplicates the information that the thing that depends >> on libudev is mpath. Can we put this in a wrapper around >> dependency() so that we could just say something like >> libudev = compile_checked_dependency('libudev', >> required: get_option('mpath').enabled(), >> static: enable_static) >> > Hi Bonzini, > This looks like a frequently used function, can we upstrem to meson? Yes, I think adding a "links" argument to dependency (similar to find_library's has_headers argument) makes sense. That would be written dependency('libudev', required: get_option('mpath').enabled(), static: enable_static, links: skeleton) But anyway that shouldn't be a blocker for more improvements to qemu's meson.build. Now that we have 5-10 dependencies converted we have a clearer idea of how to abstract the tests. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-03 7:50 ` Paolo Bonzini @ 2020-10-03 8:28 ` 罗勇刚(Yonggang Luo) 2020-10-03 8:42 ` Paolo Bonzini 2020-10-03 8:29 ` 罗勇刚(Yonggang Luo) 1 sibling, 1 reply; 22+ messages in thread From: 罗勇刚(Yonggang Luo) @ 2020-10-03 8:28 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Maydell, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 2291 bytes --] On Sat, Oct 3, 2020 at 3:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 03/10/20 09:24, 罗勇刚(Yonggang Luo) wrote: > > > > > > On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell <peter.maydell@linaro.org > > <mailto:peter.maydell@linaro.org>> wrote: > >> > >> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini <pbonzini@redhat.com > > <mailto:pbonzini@redhat.com>> wrote: > >> > > >> > On 02/10/20 14:35, Peter Maydell wrote: > >> > > > >> > > It would be better to do the "see if a static library is present" > >> > > test. This isn't too hard to do in configure (compare that > >> > > six line fix to the detection of libgio). Hopefully it is > >> > > not too hard to do in meson ? > >> > > >> > Yes, something like: > >> > > >> > if enable_static > >> > skeleton = 'int main(void) { return 0; }' > >> > if not cc.links(skeleton, dependencies: libudev) > >> > if get_option('mpath').enabled() > >> > error('Cannot link with libudev') > >> > else > >> > warning('Cannot link with libudev, disabling') > >> > libudev = not_found > >> > endif > >> > endif > >> > endif > >> > endif > >> > >> This duplicates the information that the thing that depends > >> on libudev is mpath. Can we put this in a wrapper around > >> dependency() so that we could just say something like > >> libudev = compile_checked_dependency('libudev', > >> required: get_option('mpath').enabled(), > >> static: enable_static) > >> > > Hi Bonzini, > > This looks like a frequently used function, can we upstrem to meson? > > Yes, I think adding a "links" argument to dependency (similar to > find_library's has_headers argument) makes sense. That would be written > > dependency('libudev', > required: get_option('mpath').enabled(), > static: enable_static, > links: skeleton) make sense, may also need extra cflags and link flags. > > But anyway that shouldn't be a blocker for more improvements to qemu's > meson.build. Now that we have 5-10 dependencies converted we have a > clearer idea of how to abstract the tests. > > Paolo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo [-- Attachment #2: Type: text/html, Size: 3351 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-03 8:28 ` 罗勇刚(Yonggang Luo) @ 2020-10-03 8:42 ` Paolo Bonzini 0 siblings, 0 replies; 22+ messages in thread From: Paolo Bonzini @ 2020-10-03 8:42 UTC (permalink / raw) To: luoyonggang; +Cc: Peter Maydell, QEMU Developers On 03/10/20 10:28, 罗勇刚(Yonggang Luo) wrote: >> Yes, I think adding a "links" argument to dependency (similar to >> find_library's has_headers argument) makes sense. That would be written >> >> dependency('libudev', >> required: get_option('mpath').enabled(), >> static: enable_static, >> links: skeleton) > make sense, may also need extra cflags and link flags. They would be provided by the dependency, in general. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-03 7:50 ` Paolo Bonzini 2020-10-03 8:28 ` 罗勇刚(Yonggang Luo) @ 2020-10-03 8:29 ` 罗勇刚(Yonggang Luo) 2020-10-03 8:43 ` Paolo Bonzini 1 sibling, 1 reply; 22+ messages in thread From: 罗勇刚(Yonggang Luo) @ 2020-10-03 8:29 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Maydell, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 3713 bytes --] On Sat, Oct 3, 2020 at 3:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 03/10/20 09:24, 罗勇刚(Yonggang Luo) wrote: > > > > > > On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell <peter.maydell@linaro.org > > <mailto:peter.maydell@linaro.org>> wrote: > >> > >> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini <pbonzini@redhat.com > > <mailto:pbonzini@redhat.com>> wrote: > >> > > >> > On 02/10/20 14:35, Peter Maydell wrote: > >> > > > >> > > It would be better to do the "see if a static library is present" > >> > > test. This isn't too hard to do in configure (compare that > >> > > six line fix to the detection of libgio). Hopefully it is > >> > > not too hard to do in meson ? > >> > > >> > Yes, something like: > >> > > >> > if enable_static > >> > skeleton = 'int main(void) { return 0; }' > >> > if not cc.links(skeleton, dependencies: libudev) > >> > if get_option('mpath').enabled() > >> > error('Cannot link with libudev') > >> > else > >> > warning('Cannot link with libudev, disabling') > >> > libudev = not_found > >> > endif > >> > endif > >> > endif > >> > endif > >> > >> This duplicates the information that the thing that depends > >> on libudev is mpath. Can we put this in a wrapper around > >> dependency() so that we could just say something like > >> libudev = compile_checked_dependency('libudev', > >> required: get_option('mpath').enabled(), > >> static: enable_static) > >> > > Hi Bonzini, > > This looks like a frequently used function, can we upstrem to meson? > > Yes, I think adding a "links" argument to dependency (similar to > find_library's has_headers argument) makes sense. That would be written > > dependency('libudev', > required: get_option('mpath').enabled(), > static: enable_static, > links: skeleton) > For some meson script like this: curses = not_found if iconv.found() and not get_option('curses').disabled() curses_libname_list = ['ncursesw', 'ncurses', 'cursesw', 'pdcurses'] curses_test = ''' #include <locale.h> #include <curses.h> #include <wchar.h> int main(void) { wchar_t wch = L'w'; setlocale(LC_ALL, ""); resize_term(0, 0); addwstr(L"wide chars\n"); addnwstr(&wch, 1); add_wch(WACS_DEGREE); return 0; }''' foreach curses_libname : curses_libname_list libcurses = dependency(curses_libname, required: false, method: 'pkg-config', static: enable_static) if not libcurses.found() dirs = ['/usr/include/ncursesw'] if targetos == 'windows' dirs = [] endif libcurses = cc.find_library(curses_libname, required: false, dirs: dirs, static: enable_static) endif if libcurses.found() if cc.links(curses_test, dependencies: [libcurses]) curses = declare_dependency(compile_args: '-DNCURSES_WIDECHAR', dependencies: [libcurses]) break endif endif endforeach endif We also need to define extra compile_args '-DNCURSES_WIDECHAR' as the part of dependencies. > But anyway that shouldn't be a blocker for more improvements to qemu's > meson.build. Now that we have 5-10 dependencies converted we have a > clearer idea of how to abstract the tests. > > Paolo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo [-- Attachment #2: Type: text/html, Size: 5243 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-03 8:29 ` 罗勇刚(Yonggang Luo) @ 2020-10-03 8:43 ` Paolo Bonzini 2020-10-03 9:32 ` 罗勇刚(Yonggang Luo) 0 siblings, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2020-10-03 8:43 UTC (permalink / raw) To: luoyonggang; +Cc: Peter Maydell, QEMU Developers On 03/10/20 10:29, 罗勇刚(Yonggang Luo) wrote: > For some meson script like this: > curses = not_found > if iconv.found() and not get_option('curses').disabled() > curses_libname_list = ['ncursesw', 'ncurses', 'cursesw', 'pdcurses'] > curses_test = ''' > #include <locale.h> > #include <curses.h> > #include <wchar.h> > int main(void) { > wchar_t wch = L'w'; > setlocale(LC_ALL, ""); > resize_term(0, 0); > addwstr(L"wide chars\n"); > addnwstr(&wch, 1); > add_wch(WACS_DEGREE); > return 0; > }''' > foreach curses_libname : curses_libname_list > libcurses = dependency(curses_libname, > required: false, > method: 'pkg-config', > static: enable_static) > > if not libcurses.found() > dirs = ['/usr/include/ncursesw'] > if targetos == 'windows' > dirs = [] > endif > libcurses = cc.find_library(curses_libname, > required: false, > dirs: dirs, > static: enable_static) > endif > if libcurses.found() > if cc.links(curses_test, dependencies: [libcurses]) > curses = declare_dependency(compile_args: > '-DNCURSES_WIDECHAR', dependencies: [libcurses]) > break > endif > endif > endforeach > endif > > We also need to define extra compile_args '-DNCURSES_WIDECHAR' as the > part of dependencies. You can do that with #define before including <curses.h>. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] meson.build: Don't look for libudev for static builds 2020-10-03 8:43 ` Paolo Bonzini @ 2020-10-03 9:32 ` 罗勇刚(Yonggang Luo) 0 siblings, 0 replies; 22+ messages in thread From: 罗勇刚(Yonggang Luo) @ 2020-10-03 9:32 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Peter Maydell, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 2071 bytes --] On Sat, Oct 3, 2020 at 4:43 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 03/10/20 10:29, 罗勇刚(Yonggang Luo) wrote: > > For some meson script like this: > > curses = not_found > > if iconv.found() and not get_option('curses').disabled() > > curses_libname_list = ['ncursesw', 'ncurses', 'cursesw', 'pdcurses'] > > curses_test = ''' > > #include <locale.h> > > #include <curses.h> > > #include <wchar.h> > > int main(void) { > > wchar_t wch = L'w'; > > setlocale(LC_ALL, ""); > > resize_term(0, 0); > > addwstr(L"wide chars\n"); > > addnwstr(&wch, 1); > > add_wch(WACS_DEGREE); > > return 0; > > }''' > > foreach curses_libname : curses_libname_list > > libcurses = dependency(curses_libname, > > required: false, > > method: 'pkg-config', > > static: enable_static) > > > > if not libcurses.found() > > dirs = ['/usr/include/ncursesw'] > > if targetos == 'windows' > > dirs = [] > > endif > > libcurses = cc.find_library(curses_libname, > > required: false, > > dirs: dirs, > > static: enable_static) > > endif > > if libcurses.found() > > if cc.links(curses_test, dependencies: [libcurses]) > > curses = declare_dependency(compile_args: > > '-DNCURSES_WIDECHAR', dependencies: [libcurses]) > > break > > endif > > endif > > endforeach > > endif > > > > We also need to define extra compile_args '-DNCURSES_WIDECHAR' as the > > part of dependencies. > > You can do that with #define before including <curses.h>. Yeap, but the -DNCURSES_WIDECHAR not only used for testing compile of #include<curses.h> but also as a dependencies of qemu. > > Paolo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo [-- Attachment #2: Type: text/html, Size: 2944 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 00/10] target/arm: Various v8.1M minor features @ 2020-10-12 15:33 Peter Maydell 2020-10-12 15:33 ` [PATCH] meson.build: Don't look for libudev for static builds Peter Maydell 0 siblings, 1 reply; 22+ messages in thread From: Peter Maydell @ 2020-10-12 15:33 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Richard Henderson This patchseries implements various minor v8.1M new features, notably the branch-future and low-overhead-loop extensions. (None of this will get enabled until we have enough to implement a CPU model which has v8.1M, which will be the Cortex-M55, but as usual we can get stuff into the tree gradually.) Patch 1 is a decodetree fix suggested by Richard that is necessary to avoid wrong-decode of the changes to t32.decode by later patches. thanks -- PMM Peter Maydell (10): decodetree: Fix codegen for non-overlapping group inside overlapping group target/arm: Implement v8.1M NOCP handling target/arm: Implement v8.1M conditional-select insns target/arm: Make the t32 insn[25:23]=111 group non-overlapping target/arm: Don't allow BLX imm for M-profile target/arm: Implement v8.1M branch-future insns (as NOPs) target/arm: Implement v8.1M low-overhead-loop instructions target/arm: Fix has_vfp/has_neon ID reg squashing for M-profile target/arm: Implement FPSCR.LTPSIZE for M-profile LOB extension target/arm: Fix writing to FPSCR.FZ16 on M-profile target/arm/cpu.h | 7 ++ target/arm/m-nocp.decode | 10 ++- target/arm/t32.decode | 50 +++++++---- target/arm/cpu.c | 34 ++++--- target/arm/translate.c | 157 +++++++++++++++++++++++++++++++++ target/arm/vfp_helper.c | 30 +++++-- scripts/decodetree.py | 2 +- target/arm/translate-vfp.c.inc | 17 +++- 8 files changed, 268 insertions(+), 39 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] meson.build: Don't look for libudev for static builds 2020-10-12 15:33 [PATCH 00/10] target/arm: Various v8.1M minor features Peter Maydell @ 2020-10-12 15:33 ` Peter Maydell 0 siblings, 0 replies; 22+ messages in thread From: Peter Maydell @ 2020-10-12 15:33 UTC (permalink / raw) To: qemu-arm, qemu-devel; +Cc: Richard Henderson commit f01496a314d916 moved the logic for detecting libudev from configure to meson.build, but in the process it dropped the condition that meant we only ask pkg-config about libudev for a non-static build. This breaks static builds of the system emulators on at least Ubuntu 18.04.4, because on that host there is no static libudev but pkg-config still claims it exists. Reinstate the logic that we had in the configure check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- We could certainly do something cleverer here, but basic "convert from configure to meson" should in general not also be changing the detection logic IMHO. We can make the logic smarter as a follow-on patch if desired. --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 3161c1f037a..07da66e1d81 100644 --- a/meson.build +++ b/meson.build @@ -271,7 +271,7 @@ if 'CONFIG_CURL' in config_host link_args: config_host['CURL_LIBS'].split()) endif libudev = not_found -if targetos == 'linux' and (have_system or have_tools) +if targetos == 'linux' and (have_system or have_tools) and not enable_static libudev = dependency('libudev', required: get_option('mpath').enabled(), static: enable_static) -- 2.20.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-10-12 15:50 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-02 10:52 [PATCH] meson.build: Don't look for libudev for static builds Peter Maydell 2020-10-02 11:15 ` 罗勇刚(Yonggang Luo) 2020-10-02 12:03 ` Paolo Bonzini 2020-10-02 12:02 ` Paolo Bonzini 2020-10-02 12:35 ` Peter Maydell 2020-10-02 12:36 ` Peter Maydell 2020-10-02 12:43 ` Paolo Bonzini 2020-10-02 13:01 ` Peter Maydell 2020-10-02 13:05 ` Paolo Bonzini 2020-10-02 13:09 ` Peter Maydell 2020-10-02 14:08 ` Paolo Bonzini 2020-10-02 14:18 ` Peter Maydell 2020-10-02 15:14 ` Paolo Bonzini 2020-10-02 15:28 ` Peter Maydell 2020-10-03 7:24 ` 罗勇刚(Yonggang Luo) 2020-10-03 7:50 ` Paolo Bonzini 2020-10-03 8:28 ` 罗勇刚(Yonggang Luo) 2020-10-03 8:42 ` Paolo Bonzini 2020-10-03 8:29 ` 罗勇刚(Yonggang Luo) 2020-10-03 8:43 ` Paolo Bonzini 2020-10-03 9:32 ` 罗勇刚(Yonggang Luo) 2020-10-12 15:33 [PATCH 00/10] target/arm: Various v8.1M minor features Peter Maydell 2020-10-12 15:33 ` [PATCH] meson.build: Don't look for libudev for static builds Peter Maydell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.